Skip to content
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

test: upgrade sniper unit tests to JUnit 5 #4781

Merged
merged 1 commit into from
Jul 7, 2022

Conversation

gtoison
Copy link
Contributor

@gtoison gtoison commented Jul 5, 2022

Following the discussion on #4780 the new unit test I've added was copied from other existing JUnit 4 tests.
This PR upgrades these similar tests to JUnit 5

@MartinWitt
Copy link
Collaborator

LGTM, will merge. Thank you @gtoison

@MartinWitt MartinWitt changed the title review: test: upgraded some sniper unit tests to JUnit 5 test: upgrade sniper unit tests to JUnit 5 Jul 7, 2022
@MartinWitt MartinWitt merged commit 76a427d into INRIA:master Jul 7, 2022
@slarse
Copy link
Collaborator

slarse commented Jul 8, 2022

Wait. Wait. @MartinWitt how are we still running JUnit4 tests? Did you not remove those dependencies?

@MartinWitt
Copy link
Collaborator

It seems like some dependency is providing JUnit 4 as transitive dependency. And the really strange package import were not matched by my analysis. I never thought anyone creates static package imports.

@gtoison
Copy link
Contributor Author

gtoison commented Jul 8, 2022

I swear I'm innocent!
I looked into it because https://github.com/INRIA/spoon/blob/master/CONTRIBUTING.md says that PR migrating to JUnit 5 are welcome, so maybe this should be amended too?

@slarse
Copy link
Collaborator

slarse commented Jul 8, 2022

@MartinWitt From mvn dependency:tree, the venerable junit4.11-beta-1 is pulled in by com.github.stefanbirnker:system-rules.

[INFO] \- com.github.stefanbirkner:system-rules:jar:1.19.0:test
[INFO]    \- junit:junit:jar:4.11-beta-1:test
[INFO]       \- org.hamcrest:hamcrest-core:jar:1.3:test

I don't think any of those rules are used anywhere, so I will attempt to just remove it and try to remove any remaining uses of JUnit4.

@gtoison No worries, it's a good thing we caught on to the fact that we weren't quite as free of JUnit4 as we thought :)

@slarse
Copy link
Collaborator

slarse commented Jul 8, 2022

See #4792

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants