-
Notifications
You must be signed in to change notification settings - Fork 130
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
[JENKINS-48466] Provide JUnit 5 support for JenkinsRule
#438
Merged
+215
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
11 tasks
timja
approved these changes
Jun 17, 2022
daniel-beck
reviewed
Jun 17, 2022
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!
src/main/java/org/jvnet/hudson/test/junit/jupiter/EnableJenkins.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jvnet/hudson/test/junit/jupiter/EnableJenkins.java
Outdated
Show resolved
Hide resolved
offa
approved these changes
Jun 24, 2022
timja
approved these changes
Jun 24, 2022
Thank you @basil! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
See JENKINS-48466. Continues work done in #303 and #375. Upstream of jenkinsci/jenkins#6657 and jenkinsci/plugin-pom#560.
Problem
JUnit 4 rules are not supported by JUnit 5. In order to use
JenkinsRule
in a test one cannot switch to JUnit 5 for these tests. You still need to have a dependency to JUnit 4. This inhibits migration to JUnit 5.Solution
Implement a JUnit 5 extension providing
JenkinsRule
integration. Many thanks to @offa for originally implementing this class in #303.Implementation
As recommended in https://junit.org/junit5/docs/current/user-guide/#running-tests-build-maven and https://github.com/junit-team/junit5-samples/blob/7c6536901a4446edf7d1f3e42a413ce61c676fc2/junit5-migration-maven/pom.xml we updated consumers of this test library in Jenkins core and the plugin parent POM to include
junit-jupiter-engine
andjunit-vintage-engine
intest
scope. This avoids SUREFIRE-1911. Many thanks to @reda-alaoui for observing the need forjunit-vintage-engine
in #375. We plan to mention the new test classpath requirements for consumers as a breaking change in the Jenkins Test Harness release notes.https://maven.apache.org/surefire/maven-surefire-plugin/examples/junit-platform.html describes an alternative "Smart Resolution of Jupiter Engine and Vintage Engine for JUnit 4", but we judged this as perhaps too clever and avoided it.
Testing done
Functional testing
We migrated two existing tests to the new
@EnableJenkins
JUnit extension: one in core and one in a plugin. With the changes in this PR, jenkinsci/plugin-pom#560, and jenkinsci/jenkins#6657 we ran core's test suite and the plugin's test suite, both of which were now mixed between JUnit 4 tests and the new JUnit 5 test. We observed that the existing JUnit 4 tests still ran and the new JUnit 5 tests also ran. We also verified that "Using auto detected providerorg.apache.maven.surefire.junitplatform.JUnitPlatformProvider
" was printed instead of "Using auto detected providerorg.apache.maven.surefire.junit4.JUnit4Provider
" in both cases.Regression testing
A previous attempt at integration ran into SUREFIRE-1911 and had to be reverted. The functional testing done above proved this would not be an issue this time around. The reason this is not an issue this time around is that we are including
junit-vintage-engine
on the classpath in any environment where these tests are run as mentioned above in the "implementation" section.Another previous attempt at integration destabilized the Jenkins core test suite with impact to releases and a subsequent revert. We performed eight (8) Jenkins core PR builds, each of which ran 4 parallel test branches for a total of 32 individual Maven test runs. 23 out of 32 test runs passed, for a 71% pass rate, which is about the same as the test run pass rate for trunk Jenkins core PR builds without these changes. We then examined the console logs of the remaining 9 failed test runs and observed only agent disconnections (also observed on trunk test runs) and Artifactory failures (also observed on trunk test runs). In other words, we observed no new pathologies that are not already present on trunk test runs. Furthermore, we recalled that during the previous integration attempt the success rate was well below 30% (the entire page of open PRs was red), giving us confidence we are not seeing the same issues we saw during the last integration attempt. Finally, we speculated as to the possible cause of the disappearance of these issues: in recent months we have rationalized the memory configuration of the Maven process during core builds, upgraded Surefire to the latest version, and increased the JVM heap size for Maven for core builds, all of which could have contributed to chasing this problem away.