Migrate tests to JUnit5

[strangelookingnerd]

This PR aims to migrate all tests to JUnit5. Changes include:

• Migrate annotations and imports
• Migrate assertions
• Remove public visibility for test classes and methods
• Minor clean up

I am well aware that this is a quite large changeset however I hope that there is still interest in this PR and it will be reviewed.
If there are any questions, please do not hesitate to ping me.

Submitter checklist

• Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
• Ensure that the pull request title represents the desired changelog entry
• Please describe what you did
• Link to relevant issues in GitHub or Jira
• Link to relevant pull requests, esp. upstream and downstream changes
• Ensure you have provided tests that demonstrate the feature works or the issue is fixed

[daniel-beck]

Could you provide a motivation for this change? What is the benefit of this, especially given it's not even complete and needs amending in the future?

[strangelookingnerd]

Could you provide a motivation for this change? What is the benefit of this, especially given it's not even complete and needs amending in the future?

JUnit4 is no longer actively maintained. JUnit5 has taken over as its successor and is now widely considered the modern standard. JUnit6 is expected to be released later this year.

Although JUnit4 still functions well, remains compatible with most tools and frameworks, and currently has no known vulnerabilities, there's no guarantee this will remain the case in the future.

If JUnit were a typical runtime dependency, upgrading to a newer version would seem natural over time. In my opinion, this logic should apply equally to test dependencies. Yet there seems to be a common perception among developers that once a test is written, it should never need to be touched again. I believe that's a misconception. Test code is just as important to maintain as production code. That's why I'm doing my part to help projects modernize their test suites and ensure they're maintainable in the long run by adopting up-to-date tools and best practices.

As you noted, the migration to JUnit5 isn’t complete - and that’s perfectly fine. It’s a starting point. It can serve as a blueprint for writing new tests and help lower the barrier of entry for contributors, both to this project and similar ones. The migrated tests are fully functional, as are those still using JUnit4.

From a technical standpoint, there isn’t a major immediate benefit in this project’s specific context. While JUnit5 offers improvements beyond just aesthetic changes, they may not be critical here. Usage, syntax, and performance are roughly equivalent. However, I do anticipate that over time, more tooling and frameworks will drop support or stop adding features for JUnit4 - making it increasingly harder to justify sticking with it.

[daniel-beck]

Thanks for this explanation and your perspective.

Any ETA for when the migration could be completed? I'd rather not run the risk of making it more work when having to migrate another test to/from RJR because of some random BOM change not involving this plugin (#136 / jenkinsci/bom#1911 (comment)) (even thought admittedly miniscule).

[strangelookingnerd]

Thanks for this explanation and your perspective.

Any ETA for when the migration could be completed? I'd rather not run the risk of making it more work when having to migrate another test to/from RJR because of some random BOM change not involving this plugin (#136 / jenkinsci/bom#1911 (comment)) (even thought admittedly miniscule).

Understandable. I requested a review on jenkinsci/jenkins-test-harness#988 just now - let's see what comes out of it.

[daniel-beck]

@strangelookingnerd WDYT about this? (Not yet requesting its inclusion, just generally.) Would prevent accidental regressions of this change. Might this be something for the parent pom, enabled via property perhaps?

diff --git a/pom.xml b/pom.xml
index 8ce7fdd..abece2d 100644
--- a/pom.xml
+++ b/pom.xml
@@ -114,4 +114,38 @@
       <url>https://repo.jenkins-ci.org/public/</url>
     </pluginRepository>
   </pluginRepositories>
+
+  <build>
+    <plugins>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-enforcer-plugin</artifactId>
+        <dependencies>
+          <dependency>
+            <groupId>de.skuzzle.enforcer</groupId>
+            <artifactId>restrict-imports-enforcer-rule</artifactId>
+            <version>2.6.1</version>
+          </dependency>
+        </dependencies>
+        <executions>
+          <execution>
+            <id>check-junit-imports</id>
+            <goals>
+              <goal>enforce</goal>
+            </goals>
+            <phase>process-sources</phase>
+            <configuration>
+              <rules>
+                <RestrictImports>
+                  <reason>Use JUnit 5 (org.junit.jupiter.*)</reason>
+                  <bannedImport>org.junit.**</bannedImport>
+                  <allowedImport>org.junit.jupiter.**</allowedImport>
+                </RestrictImports>
+              </rules>
+            </configuration>
+          </execution>
+        </executions>
+      </plugin>
+    </plugins>
+  </build>
 </project>

[strangelookingnerd]

@strangelookingnerd WDYT about this? (Not yet requesting its inclusion, just generally.) Would prevent accidental regressions of this change. Might this be something for the parent pom, enabled via property perhaps?

For projects that have already migrated (fully) this would be a great addition to prevent regressions.
Integrating it into the parent would need to be opt-in like spotless.check.skip.

[daniel-beck]

Integrating it into the parent would need to be opt-in like spotless.check.skip.

Agree, that's what I meant by

enabled via property perhaps

[daniel-beck]

🤦