-
Notifications
You must be signed in to change notification settings - Fork 267
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
Use JUnit 5 instead of JUnit 4 #473
Use JUnit 5 instead of JUnit 4 #473
Conversation
31d4e3c
to
25040fd
Compare
what is the point if you do not migrate code as well? |
Version 64 of the parent POM will provide the dependency information for JUnit 5. Therefore the entry in the dependency management section can be removed if version 64 will be released.
25040fd
to
da37339
Compare
I will do it in the one of the next commits. I prefer to do smaller contributions as I can not guarantee when I will be able to continue my work. I think even having the possibility to use JUni 5 will help new contributors. WDYT? |
Sounds good to me! @olamy I wouldn't be too strict here. Besides, I really think this is a valid and really, really good point. |
@bmarwell @obfischer I just wonder if we add this dependencies with still old junit dependencies available, contributors may start contributing code with junit5. |
@olamy It depends on the version of Surefire. The one currently used runs them via JUnit 5. JUnit 5 itself provides the support via a dedicated test engine to run also JUni 4 tests. I checked this and Surefire runs the same number of tests after the change as before it. |
Yeah, the Junit5 vintage engine is a drop in replacement. Just make sure we do not downgrade surefire. 😉 I've been using this setup for ages. |
I will not oppose to this change but well frankly I just feel it weird (even no sense) to make a change to add a dependency which is not used... that's my point. |
Which one is unused? It is replacing junit4. If you remove it, no test will run. |
Why it is not used? It runs the tests, allows users to use write tests in a modern way and modernizes the project, as every dependency update it does. It it elementes this moment "WTF, JUnit 4" 😨 |
As already I mean what is the point of this if you are not replacing |
Well, I do. Future tests can be written with junit-jupiter. The vintage engine dependency contains I am going to merge it now. |
@olamy I understand your point and as I wrote in one of my comments, I plan to migrate all currently existing test cases. But I decided to split them up in small, but IMHO useful parts, as I can't give any guarantee when I will be able to do it. I don't wan't simply to add to another project a new stale issue, hanging around for some time, which causes only load on the reviewers and maintainers. I hope I was able to illustrate my point. ;-) |
Works for me! :) Quick question: Is there any hurry to convert tests? Maybe we could keep the existing ones for a while unless they need conversions for particular reasons ("don't touch working code")? |
I am not in a hurry, but I would like to convert the tests, so that new users will automatically start to write tests using JUnit 5. |
That is not too important: New users would need to create a PR which we could review. ;-) |
Updated the dependencies to use JUnit 5 instead of JUnit 4. The usage of the JUnit Vintage plattform ensures, that the existing test can be executed without any changes.
Fixed #472