Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

chore: add java21 support #2640

Merged
merged 2 commits into from
Sep 25, 2024
Merged

chore: add java21 support #2640

merged 2 commits into from
Sep 25, 2024

Conversation

Vovchyk
Copy link
Contributor

@Vovchyk Vovchyk commented Jul 24, 2024

Description

Motivation and Context

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • Tests for the changes have been added (for bug fixes / features)
  • Requires Activation Code (Hard Fork)

fed:vovchyk/java21-support

@Vovchyk Vovchyk force-pushed the vovchyk/java21-support branch 5 times, most recently from 482470b to 026ce0b Compare July 24, 2024 14:34
- name: Setup Java JDK
uses: actions/setup-java@v3
- name: Setup Java & Gradle
uses: actions/setup-java@v4
with:
java-version: '17'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should not be updated to 21 aswell?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting question.. I guess we should be targeting java17 as more stable one. But perhaps it'd make sense to run unit tests against both 17 and 21. Added that in this 1ddcfd7

asoto-iov
asoto-iov previously approved these changes Jul 25, 2024

class MaxSizeHashMapTest {

@Test
void maxSizeMap_Test() {
int maxSize = 50_000;
void maxSizeMapTest() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was this test updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are couple of reasons: first, this test tries to "test" the feature not as a "blackbox", but an internal state of the instance of a system class. Secondly and mosts importantly, this test uses reflection to obtain an internal field of the system class (via field.setAccessible(true)), which is prohibited for non-open java packages in recent versions of junit. There's a workaround by opening system package (via --add-opens java.base/java.util=ALL-UNNAMED), but I'm not sure that worth it

Copy link

@Vovchyk Vovchyk force-pushed the vovchyk/min-java-ver-bump branch from e86e8f3 to a808164 Compare September 25, 2024 08:40
@Vovchyk Vovchyk force-pushed the vovchyk/java21-support branch from 1ddcfd7 to 0a5f8a2 Compare September 25, 2024 08:54
@Vovchyk Vovchyk changed the base branch from vovchyk/min-java-ver-bump to master September 25, 2024 11:56
@Vovchyk Vovchyk dismissed asoto-iov’s stale review September 25, 2024 11:56

The base branch was changed.

@Vovchyk Vovchyk force-pushed the vovchyk/java21-support branch from 0a5f8a2 to 2cc5bef Compare September 25, 2024 12:12
Copy link

@Vovchyk Vovchyk merged commit 2a0d333 into master Sep 25, 2024
11 checks passed
@Vovchyk Vovchyk deleted the vovchyk/java21-support branch September 25, 2024 13:04
@aeidelman aeidelman added this to the Arrowhead 6.4.0 milestone Oct 30, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants