-
Notifications
You must be signed in to change notification settings - Fork 78
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
Add recipes for migrating Arquillian from JUnit 4 to JUnit 5. #654
Conversation
Signed-off-by: James R. Perkins <[email protected]>
Thanks already @jamezp ! I've moved your recipes out into a separate file and added a test class for the dependency issue you had reported. Hope that helps you there. Let me know when this is ready for review by unmarking it as draft. |
Excellent, thank you @timtebeek! I'll run some more conversions on some other projects to look for any additional possible issues. |
src/main/java/org/openrewrite/java/testing/junit5/ReplaceArquillianInSequenceAnnotation.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/ReplaceArquillianInSequenceAnnotation.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/ReplaceArquillianInSequenceAnnotation.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit5/ReplaceArquillianInSequenceAnnotation.java
Outdated
Show resolved
Hide resolved
Signed-off-by: James R. Perkins <[email protected]>
@timtebeek I added one more change to ensure a import was done. I also ran it against a much larger test suite, https://github.com/wildfly/wildfly/tree/main/testsuite/integration/basic, and ran into some issues. These might just be issues with the tests themselves though. A lot of removal of exceptions from the test methods causing compile issues and changing try/catch/fail to assertThrows with non-effective final variables. Again, I don't think that's necessarily a bug in any of the recipes. I also noticed it doesn't seem to handle multiple source directories well, but I also know that is an odd setup so not something I'm too concerned about |
...main/java/org/openrewrite/java/testing/arquillian/ReplaceArquillianInSequenceAnnotation.java
Outdated
Show resolved
Hide resolved
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.
Thanks a lot for kicking this off @jamezp ! I've changed the recipe you added to reuse JavaTemplate and ChangeType, which are more convenient than the explicit LST element constructors seen before; I've also added a test and now include your recipes in the JUnit 4 to 5 migration, such that folks get these changes applied by default. 🥳
Thank you @timtebeek This is awesome! |
What's changed?
Add recipes for migrating Arquillian from JUnit 4 to JUnit 5
What's your motivation?
Migrating Jakarta EE test suites from JUnit 4 to JUnit 5 when using Arquillian.
Anything in particular you'd like reviewers to focus on?
Please ensure I'm using the correct API's and best practices for the project :)
Have you considered any alternatives or workarounds?
The only workarounds I'm aware of would be a manual find/replace which is what I've currently been doing.
Any additional context
Checklist
I've added unit tests to cover both positive and negative cases
I've read and applied the recipe conventions and best practices
I've used the IntelliJ IDEA auto-formatter on affected files
resolves Add recipe for migrating Arquillian tests from JUnit 4 to JUnit 5. #652