-
Notifications
You must be signed in to change notification settings - Fork 49
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
Upgrade tests to use JUnit5 #691
Upgrade tests to use JUnit5 #691
Conversation
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.
I put some print statements in the evaluate()
methods and didn't see them getting executed, so I don't think multiple Jenkins sessions (with the same Jenkins home directory) are working in JUnit 5 yet, at least not in the manner demonstrated in this PR. My preference would be for us to get that working first before we start to recommend JUnit 5 project-wide. Otherwise we would be recommending a replacement that has fewer features than the original.
@basil Thanks for checking. I will look into it and try to find a proper solution. I was a little put off by the assertion stating |
FileUtils.deleteQuietly(jenkins.getInstance().getRootDir()); | ||
} | ||
|
||
private static JenkinsRule restartJenkins(JenkinsRule currentJenkinsRule) throws Throwable { |
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.
I do not feel comfortable yet recommending this as a general pattern in the archetype because it involves copying and pasting this code into every plugin that wants to use this feature with JUnit 5. The existing JUnit 4 functionality is generalized and easy-to-use, and I think its JUnit 5 equivalent should be equally generalized and easy-to-use without copying and pasting code like this into every Jenkins plugin.
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.
100% agree on that, should have converted to draft. I was just looking for a solution that works at all. There are quite some roadblocks ahead to make JUnit5 work nicely with the current JenkinsRule
implementation.
It should be considered best practice to start plugin development using JUnit5 over JUnit4.
org.junit.*
imports being updated to JUnit5 equivalents@WithJenkins
in combination withJenkinsRule
I'm not 100% convinced that I migrated
SampleConfigurationTest
correctly since I was not able to find a proper equivalent forJenkinsSessionRule
- maybe @basil could confirm if the test still covers the intended functionality.Testing done
Ran
mvn clean verify
without errors.Submitter checklist