jfx icon indicating copy to clipboard operation
jfx copied to clipboard

8369049: [TestBug] Update JUnit to Version 6.0.1

Open Maran23 opened this issue 2 months ago • 16 comments

JUnit 6.0.1 is out and we can consider updating to it.

Release Notes: https://docs.junit.org/current/release-notes/index.html#release-notes-6.0.0 https://docs.junit.org/current/release-notes/index.html#release-notes-6.0.1

Notes:

  • Java 17 is the baseline (so not a problem for us, since we have a higher baseline)
  • Deprecation were removed (not a problem, as we don't rely on any)
  • JUnit Vintage Engine is deprecated (not a problem, as we dropped support for it some months ago)

JUnit 6 now uses a single version number for all dependencies, that is platform, jupiter and vintage (which we do not use anymore). That makes updating it easier.

It also uses JSpecify, which is an annotation library for @NonNull and @Nullable. All version numbers for JUnit now use a single variable.


Progress

  • [x] Change must not contain extraneous whitespace
  • [x] Commit message must refer to an issue
  • [ ] Change must be properly reviewed (3 reviews required, with at least 1 Reviewer, 2 Authors)

Issue

  • JDK-8369049: [TestBug] Update JUnit to Version 6.0.1 (Enhancement - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1925/head:pull/1925
$ git checkout pull/1925

Update a local copy of the PR:
$ git checkout pull/1925
$ git pull https://git.openjdk.org/jfx.git pull/1925/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1925

View PR using the GUI difftool:
$ git pr show -t 1925

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1925.diff

Using Webrev

Link to Webrev Comment

Maran23 avatar Oct 02 '25 15:10 Maran23

:wave: Welcome back mhanl! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

bridgekeeper[bot] avatar Oct 02 '25 15:10 bridgekeeper[bot]

❗ This change is not yet ready to be integrated. See the Progress checklist in the description for automated requirements.

openjdk[bot] avatar Oct 02 '25 15:10 openjdk[bot]

Webrevs

mlbridge[bot] avatar Oct 02 '25 15:10 mlbridge[bot]

@Maran23 This requires a new third-party library, which will need approval. Also, I'll need to review the PR.

Since you didn't make any changes to the test source code, I presume that JUnit 6 is compatible with 5 (leaving aside the removed deprecations, which don't affect us), unlike the transition from 4 to 5 (version 5 was effectively an entirely new API). I'll take a look at the release notes, but can you summarize the changes you are aware of?

Reviewers: @kevinrushforth @arapte @tiainen or @johanvos

/reviewers 3

kevinrushforth avatar Oct 02 '25 16:10 kevinrushforth

@kevinrushforth The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 3 (with at least 1 Reviewer, 2 Authors).

openjdk[bot] avatar Oct 02 '25 16:10 openjdk[bot]

This PR changes gradle build only. What about Eclipse and other IDEs? Are they compatible? Are we going to lose the ability to run junit tests from within the IDE?

andy-goryachev-oracle avatar Oct 02 '25 16:10 andy-goryachev-oracle

Since you didn't make any changes to the test source code, I presume that JUnit 6 is compatible with 5 (leaving aside the removed deprecations, which don't affect us), unlike the transition from 4 to 5 (version 5 was effectively an entirely new API).

Yes, it is completely compatible and I could not spot any problems. Locally, everything works and also the GHA work (did a test run before I made the PR).

I'll take a look at the release notes, but can you summarize the changes you are aware of?

Sure! Most of the changes are either removal of deprecated functions or internal changes / dependency changes which we do not use. There is a lot of new functionality for launching test classes / using a custom test engine (we use junit-jupiter-engine). We don't do any custom stuff, so we are good here. But for reference, here is more information: https://docs.junit.org/current/user-guide/index.html#launcher-api-launcher-cancellation.

One interesting note and addition is: Stack traces are now pruned up to the test method or lifecycle method. The result is that the stack trace is slightly shorter, as it does not contain anything before the test method. But for some reason I could not observe that in JavaFX, only another project I did.

.\gradlew test

Before:

...
at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145)
at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:531)
at de.abc.def.domain.ExportTriggerTest.testExportTriggerPositive(ExportTriggerTest.java:26)
at java.base/java.lang.reflect.Method.invoke(Method.java:565)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)

After:

...
at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:158)
at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:139)
at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:201)
at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:152)
at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:147)
at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:558)
at de.abc.def.domain.ExportTriggerTest.testExportTriggerPositive(ExportTriggerTest.java:26)

This PR changes gradle build only. What about Eclipse and other IDEs? Are they compatible? Are we going to lose the ability to run junit tests from within the IDE?

Yes they all are compatible, as nothing really changed when writing tests. Imports are the same, the methods are the same, everything is still compatible. I can also verify that it works in IntelliJ.

Maran23 avatar Oct 02 '25 16:10 Maran23

I would suggest we postpone the migration to junit6 until after Eclipse provides support for it (screenshot taken with Eclipse Version: 2025-06 (4.36) Build id: I20250528-1830):

Screenshot 2025-10-02 at 10 46 25

andy-goryachev-oracle avatar Oct 02 '25 17:10 andy-goryachev-oracle

I would suggest we postpone the migration to junit6

Can you check out this branch and run the tests with the JUnit 5 Option? I'm sure it will work, as again, there is no breaking change here.

Maran23 avatar Oct 02 '25 18:10 Maran23

Can you check out this branch and run the tests with the JUnit 5 Option?

I can, but what is the point of migration then? The moment someone uses a junit6 feature, the Eclipse project will break.

andy-goryachev-oracle avatar Oct 02 '25 18:10 andy-goryachev-oracle

I also don't see the hurry. Absent a compelling reason to update, we can wait for a while.

kevinrushforth avatar Oct 02 '25 18:10 kevinrushforth

I can, but what is the point of migration then? The moment someone uses a junit6 feature, the Eclipse project will break.

Sure, but I can't think of any new features we might want to use. This should really be treated as a normal dependency update.

Not that I think we need to merge this anytime soon (that's why I wrote "consider"), but if Eclipse doesn't have "support" for it in 6 months, I would stop waiting at one point.

Maran23 avatar Oct 02 '25 18:10 Maran23

Since you didn't make any changes to the test source code, I presume that JUnit 6 is compatible with 5 (leaving aside the removed deprecations, which don't affect us), unlike the transition from 4 to 5 (version 5 was effectively an entirely new API). I'll take a look at the release notes, but can you summarize the changes you are aware of?

See https://github.com/junit-team/junit-framework/wiki/Upgrading-to-JUnit-6.0.

nlisker avatar Oct 05 '25 01:10 nlisker

I would suggest we postpone the migration to junit6 until after Eclipse provides support for it (screenshot taken with Eclipse Version: 2025-06 (4.36) Build id: I20250528-1830):

Screenshot 2025-10-02 at 10 46 25

The JUnit 5 runner works with JUnit 6. I don't know if they'll add another item to that list or just rename it to "JUnit 5+" for example.

nlisker avatar Oct 06 '25 12:10 nlisker

@Maran23 This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Nov 03 '25 12:11 bridgekeeper[bot]

@Maran23 This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Dec 04 '25 22:12 bridgekeeper[bot]