-
Notifications
You must be signed in to change notification settings - Fork 501
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8094 java17 #9291
8094 java17 #9291
Conversation
For a lot of places where files are read from filesystem or other places, both authority and identifier of the dataset are required to build a proper path. When either of those is missing (=null), all sorts of NPEs will occur, as paths cannot be built with null values. In reality, this will hardly ever occur, as a missing identifier let alone authority is completely unrealistic. Yet, some functions are not validating this data very well. Adding this mock here means covering potential NPEs, but again: these are unlikely to happen outside unit tests.
- Migrate to JUnit5 - Enable using @SystemProperty to avoid a messy global state after test ran
During unit testing it occured that sometimes parts of paths are null (there was a missing mock for authority). Changing the code structure here to catch this anytime. This hard to track cause of trouble was revealed when running tests with JDK 17, because since JDK 16 all java.nio.file.Path may not contain null elements for the first component. See also: https://stackoverflow.com/questions/68791709
When building with Java 17, it became obvious v2.4 is not compatible with JDK 17. Upgrading required.
…sured Since RestAssured 3.0 the groupID changed from "com.jayway.restassured" to simply "io.restassured". Migrating the namespace in all tests here.
e711f08
to
56b794d
Compare
A few (6) API tests are broken:
|
- Install Java 17 instead of Java 11 - Install the "ps" util, necessary to create the Solr index
Grooming: This issue was in the New or No Status columns on the Dataverse Global Backlog Board. This is NOT a reflection of the priority of these issues. To get this item onto the Dataverse Global Backlog Board, please reach out to one of the Stewards on the board. |
We aren't sizing this yet. @poikilotherm is going to hack some more. Thanks! 🎉 |
Grooming
|
Sizing:
|
Specifically I would say that this cannot be MERGED until 5.14 is released. We can certainly resolve merge conflicts ahead of time and generally keep this PR in a state that it will be ready to be reviewed, tested, and merged. Also, the point about Payara is that we need to upgrade Payara for security reasons (Payara 5 is end of life per #8305). This Java upgrade can wait if necessary. We just thought we'd try to bundle a few platform upgrades together if there's time to get them all in. |
|
Sizing: Recommended as 33 |
I saw some merge conflicts and wanted to step through the upgrade myself anyway. I'm closing this is favor of a new PR I just made: That said, I'll definitely refer back to this PR as I work away. Here's where I learned we need to upgrade REST Assured. |
What this PR does / why we need it:
Enable usage of Java 17. Lots of good new stuff, better GC etc.
Which issue(s) this PR closes:
Closes #8094
Special notes for your reviewer:
Remember to use Java 17. Maybe https://sdkman.io is of interest for you?
Suggestions on how to test this:
TODO
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Nope.
Is there a release notes update needed for this change?:
Yes. Not yet included.
Additional documentation:
TODO