Skip to content
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

Closed
wants to merge 7 commits into from
Closed

8094 java17 #9291

wants to merge 7 commits into from

Conversation

poikilotherm
Copy link
Contributor

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

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.
@poikilotherm poikilotherm added Component: Code Infrastructure formerly "Feature: Code Infrastructure" Feature: Installer Feature: Performance & Stability Feature: Installation Guide User Role: Sysadmin Installs, upgrades, and configures the system, connects via ssh Component: Containers Anything related to cloudy Dataverse, shipped in containers. labels Jan 16, 2023
@poikilotherm
Copy link
Contributor Author

A few (6) API tests are broken:

[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   DatasetsIT.testExcludeEmail:738 expected:<[]> but was:<null>
[ERROR]   DatasetsIT.testExport:590 1 expectation failed.
JSON path data.latestVersion.metadataBlocks.citation.fields[2].value[0].datasetContactEmail.value doesn't match.
Expected: sammi@sample.com
  Actual: null

[ERROR]   DatasetsIT.testReCreateDataset:2509 1 expectation failed.
Expected status code <201> but was <403>.

[ERROR]   DatasetsIT.testSemanticMetadataAPIs:2407 https://dataverse.org/schema/citation/datasetContact
Expected: https://dataverse.org/schema/citation/datasetContactEmail
     but none found

[ERROR]   HarvestingServerIT.testMultiRecordOaiSet:606 Wrong number of items on the first ListIdentifiers page expected:<2> but was:<5>
[ERROR] Errors: 
[ERROR]   DatasetsIT.testPrivateUrl:964 » Connect Verbindungsaufbau abgelehnt (Connectio...
[INFO] 
[ERROR] Tests run: 193, Failures: 5, Errors: 1, Skipped: 8
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  11:10 min
[INFO] Finished at: 2023-01-16T23:22:45+01:00
[INFO] ------------------------------------------------------------------------

- Install Java 17 instead of Java 11
- Install the "ps" util, necessary to create the Solr index
@mreekie
Copy link

mreekie commented Jan 23, 2023

Grooming:

This issue was in the New or No Status columns on the Dataverse Global Backlog Board.
Those columns have been removed.
This issue has been removed from the board

This is NOT a reflection of the priority of these issues.
However, as we have worked on the grooming process, it became clear that these issues which are in columns that are not stewarded will likely not get prioritized or sized.

To get this item onto the Dataverse Global Backlog Board, please reach out to one of the Stewards on the board.
You can also reach out to @mreekie and I can connect you with the steward of the appropriate queue

@pdurbin
Copy link
Member

pdurbin commented Feb 7, 2023

We aren't sizing this yet. @poikilotherm is going to hack some more. Thanks! 🎉

@mreekie
Copy link

mreekie commented Feb 28, 2023

Grooming

  • Including as part of Payara 6 upgrade deliverable

@mreekie mreekie added the D: Payara 6 Upgrade Issues and PRs about the move to Jakarta EE 10 + Payara 6 label Feb 28, 2023
@poikilotherm poikilotherm mentioned this pull request Mar 16, 2023
@mreekie
Copy link

mreekie commented Apr 19, 2023

Sizing:

  • This cannot be completed until 5.14 is released.
  • The next small step that can be done here is resolving file conflicts.
  • This will be put off until we work through the Payara6 upgrades first.

@pdurbin
Copy link
Member

pdurbin commented Apr 19, 2023

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.

@scolapasta scolapasta added this to the 6.0 milestone May 15, 2023
@cmbz
Copy link

cmbz commented May 15, 2023

  • Prioritizing Payara 6 release
  • Sizing needed

@cmbz
Copy link

cmbz commented May 17, 2023

Sizing: Recommended as 33

@cmbz cmbz added the Size: 30 A percentage of a sprint. 21 hours. (formerly size:33) label May 17, 2023
@mreekie mreekie added this to Ready for Review ⏩ in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) via automation Jun 21, 2023
@mreekie mreekie moved this from Ready for Review ⏩ to ▶ SPRINT READY in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Jun 21, 2023
@pdurbin pdurbin moved this from ▶ SPRINT READY to This Sprint 🏃‍♀️ 🏃 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Aug 2, 2023
@pdurbin pdurbin moved this from This Sprint 🏃‍♀️ 🏃 to IQSS Team - In Progress 💻 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Aug 7, 2023
@pdurbin pdurbin self-assigned this Aug 7, 2023
@pdurbin
Copy link
Member

pdurbin commented Aug 7, 2023

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.

@pdurbin pdurbin closed this Aug 7, 2023
@pdurbin pdurbin removed this from IQSS Team - In Progress 💻 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Aug 7, 2023
@pdurbin pdurbin removed their assignment Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Code Infrastructure formerly "Feature: Code Infrastructure" Component: Containers Anything related to cloudy Dataverse, shipped in containers. D: Payara 6 Upgrade Issues and PRs about the move to Jakarta EE 10 + Payara 6 Feature: Installation Guide Feature: Installer Feature: Performance & Stability Size: 30 A percentage of a sprint. 21 hours. (formerly size:33) User Role: Sysadmin Installs, upgrades, and configures the system, connects via ssh
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Migrate codebase to Java 17 LTS
5 participants