-
Notifications
You must be signed in to change notification settings - Fork 3k
Upgrade to JUnit 6 #51412
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
base: main
Are you sure you want to change the base?
Upgrade to JUnit 6 #51412
Conversation
|
@gsmet this is what I came up with my investigation back then: Note that you also need to bump |
Also adjust QuarkusTestNestedTestCase as the order in which nested tests are executed have changed and is now a lot more logical.
And put relocations in place. Update recipes will also be provided.
|
@gastaldi ah sorry, I totally missed you had started this work. |
|
/cc @cescoffier @geoand @Sanne @aloubyansky @gastaldi please check the approach described in the description as the renaming is a rather massive change and I would like to be sure we're all on the same page. |
|
Also /cc @holly-cummins @mkouba as you were involved in some of the testing infra. |
|
That's the plan I was +1 with :) |
|
🎊 PR Preview 5b8ebed has been successfully built and deployed to https://quarkus-pr-main-51412-preview.surge.sh/version/main/guides/
|
|
I'm ok with this approach - but I wonder if you considered the alternative of introducing a new set of quarkus-junit6 artifacts rather than renaming? This could also be useful if we had some semantic changes on our wishlist which we didn't dare do because of user update hassle. (But I fully agree this is all unnecessary if it's a very simple drop-in change) |
|
I understand that JUnit6 is more like an evolution with minimal breaking changes and as such a good candidate for drop-in replacement. And the user has an option to go back to JUnit5 and it should work, more or less. But I wonder if we should clarify the strategy for future releases. Also I don't have any numbers but I think that most users do not care about platform/jupiter/vintage, so Another question is - what if JUnit7 will be a "revolution", i.e. not a drop-in replacement? The artifactId remains the same, but Quarkus will silently drop support for older versions of JUnit? |
|
|
||
| <properties> | ||
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
| <version.junit>5.13.4</version.junit> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this property be defined in the parent POM?
| @@ -178,9 +178,9 @@ void beforeEach() { | |||
| @Test | |||
| void testOne() { | |||
| assertEquals(1, COUNT_BEFORE_ALL.get(), "COUNT_BEFORE_ALL"); | |||
| assertEquals(3, COUNT_BEFORE_EACH.get(), "COUNT_BEFORE_EACH"); | |||
| assertEquals(1, COUNT_TEST.getAndIncrement(), "COUNT_TEST"); | |||
| assertEquals(1, COUNT_AFTER_EACH.get(), "COUNT_AFTER_EACH"); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know what causes these changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, yes, that looks like it might need investigation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's expected, the order in which nested tests are executed have been changed and is now more natural.
That's true. OTOH, the same could be said about Quarkus extensions, such as
|
|
We could go with Now I'm not sold with the names, we can adjust but it's not really easy to find something that is future proof given we have no idea what the future will be. |
|
As for |
I couldn't agree more. We should only support one |
Like a replacement for Jupiter? 🤔 That would mean a new programming model/extension model and I don't think we will be able to support multiple extension models easily...
💯 |
Well at some point we had a JUnit 4 artifact IIRC. If they push a brand new version of JUnit that breaks everything, we will probably need new artifacts and yes a brand new implementation. And we might want to support both models for a while. Now, we could decide to rename the old ones to a disctinctive names and keep I'm fine with moving to |
Fine with me as well |
|
No objections, but thinking ahead how about |
That seems too generic to me; it doesn't give people enough information about what's inside. In particular, it's not obvious from the dependency name that you wouldn't need JUnit on your classpath, because this is giving you JUnit. It would also rule out some future in which we decide we don't have enough work and want to support both JUnit and a cool new challenger testing framework. It would kind of work if we switched from JUnit to CoolNewChallengerTest, but I'm not sure we'd want the name to stay the same if we did such a massive breaking change. |
|
We also could create something on our own and name it |
|
Obviously, the only constant in technology is change, but given what we've seen in the past few years, I don't think that many people are asking for / and no one I know of is working on a new unit test framework in Java - JUnit has won, and rightly so, as it is awesome. I don't see the JUnit 4 vs TestNG vs Spock situation repeating itself any time soon (and I'd be willing to put my money where my mouth is on this :)). Calling the extension |
|
Yeah, I'm -1 for If nobody objects by Monday evening, I will go do that on Tuesday. |

Note that the assumption in this work is that
junit-jupiterwill mostly stay compatible and that they won't break everything. But I think it's a relatively fair assumption given how they handled the JUnit 4 -> JUnit 5 and the JUnit 5 -> JUnit 6 move.