Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@ You can set up `mvn spotless:apply` to run automatically (in `validate` phase) f
</settings>
```

## Additional Checks

You can set the `ban-junit4-imports.skip` to `false` if your tests are written with JUnit 5 (Jupiter) and you want to ensure that no JUnit 4 imports are used in your plugin.
This will prevent imports of JUnit 4 classes in your code to ensure newly added or improved tests do not reintroduce JUnit 4 (e.g., out of habit or from IDE import autocompletion).
Note that this will only address imports of JUnit 4 in your code, and not uses from dependencies.

## Running Benchmarks

To run JMH benchmarks from JUnit tests, activate the `benchmark`
Expand Down
25 changes: 25 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,9 @@
<!-- Set to false to enable Spotless -->
<spotless.check.skip>true</spotless.check.skip>

<!-- Set to false to enable enforcer banning org.junit.* imports -->
<ban-junit4-imports.skip>true</ban-junit4-imports.skip>

<!-- Whether to skip tests during release phase (they are executed in the prepare phase). -->
<release.skipTests>true</release.skipTests>
<!-- By default we do not create *-tests.jar. Set to false to allow other plugins to depend on test utilities in yours, using <classifier>tests</classifier>. -->
Expand Down Expand Up @@ -533,6 +536,11 @@
<artifactId>extra-enforcer-rules</artifactId>
<version>1.9.0</version>
</dependency>
<dependency>
<groupId>de.skuzzle.enforcer</groupId>
<artifactId>restrict-imports-enforcer-rule</artifactId>
<version>2.6.1</version>
</dependency>
</dependencies>
<executions>
<execution>
Expand Down Expand Up @@ -658,6 +666,23 @@
</rules>
</configuration>
</execution>
<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>
<skip>${ban-junit4-imports.skip}</skip>
Comment on lines +676 to +683
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we doing this at the source level and not banning the dep?

This means that IDEs will happily suggest junit4 etc in auto completion and tests in IDEs will pass (but the build will fail from the CLI).
Banning the dependencies directly in a profile activated by the property would seem to solve the issue by having compilation fail would it not?
@daniel-beck was there a reason that banning the dependency was not done rather than just banning the import?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we doing this at the source level and not banning the dep?

This means that IDEs will happily suggest junit4 etc in auto completion and tests in IDEs will pass (but the build will fail from the CLI). Banning the dependencies directly in a profile activated by the property would seem to solve the issue by having compilation fail would it not? @daniel-beck was there a reason that banning the dependency was not done rather than just banning the import?

Yes it is harder to apply as this (IIRC) need to go in the .mvn/maven.config rather than just a pom property, but we do that for CD releases so its nothing new per say

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was there a reason that banning the dependency was not done rather than just banning the import?

I did not consider that option.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we doing this at the source level and not banning the dep?

would that work with dependencies that depend on junit 4 ?

I am thinking of this long standing testcontainers issue as an example testcontainers/testcontainers-java#970

Copy link
Member

@jtnord jtnord Aug 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you would need to add exclusions on the dependency that brough junit4 in.

This would be almost every plugin due to https://github.com/jenkinsci/jenkins-test-harness/blob/0c38b4e6bcfa401dc675f39ce8112b839150cc75/pom.xml#L134-L137 but then that would serve as a reminder that perhaps some other things still need to happen first.

But I am not fully sure what you are asking - maven's invocation via surefire of things is platform aware so anything pure junit5 should work without exception (how gradle handles test engines is unknown to me).
And if it is not pure junit-5 then you would not opt in to the ban.

</configuration>
</execution>
</executions>
</plugin>
<plugin>
Expand Down
2 changes: 2 additions & 0 deletions src/it/ban-juni4-fail/invoker.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
invoker.goals=-Dstyle.color=always -ntp verify
invoker.buildResult=failure
30 changes: 30 additions & 0 deletions src/it/ban-juni4-fail/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>@project.version@</version>
<relativePath />
</parent>
<groupId>org.jenkins-ci.plugins.its</groupId>
<artifactId>ban-junit4-fail-it</artifactId>
<version>1.0-SNAPSHOT</version>
<packaging>hpi</packaging>
<properties>
<jenkins.version>2.479.3</jenkins.version>
<ban-junit4-imports.skip>false</ban-junit4-imports.skip>
</properties>
<repositories>
<repository>
<id>repo.jenkins-ci.org</id>
<url>https://repo.jenkins-ci.org/public/</url>
</repository>
</repositories>
<pluginRepositories>
<pluginRepository>
<id>repo.jenkins-ci.org</id>
<url>https://repo.jenkins-ci.org/public/</url>
</pluginRepository>
</pluginRepositories>
</project>
12 changes: 12 additions & 0 deletions src/it/ban-juni4-fail/postbuild.groovy
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Build failed
def hpi = new File(basedir, 'target/ban-junit4-fail-it.jar')
assert !hpi.exists()

// Because of banned imports
def log = new File(basedir, 'build.log')
assert log.text.contains('Reason: Use JUnit 5 (org.junit.jupiter.*)')
assert log.text.contains('org.junit.Test')
assert log.text.contains('(Line: 3, Matched by: org.junit.**)')
assert log.text.contains('static org.junit.Assert.assertTrue')
assert log.text.contains('(Line: 5, Matched by: org.junit.**)')
return true
2 changes: 2 additions & 0 deletions src/it/ban-juni4-fail/src/main/resources/index.jelly
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<?jelly escape-by-default='true'?>
<div/>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package banjunit4;

import org.junit.Test;

import static org.junit.Assert.assertTrue;

public class BanJUnit4FailTest {
@Test
public void thisFails() {
assertTrue(true);
}
}
1 change: 1 addition & 0 deletions src/it/ban-junit4-pass/invoker.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
invoker.goals=-Dstyle.color=always -ntp verify
30 changes: 30 additions & 0 deletions src/it/ban-junit4-pass/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>@project.version@</version>
<relativePath />
</parent>
<groupId>org.jenkins-ci.plugins.its</groupId>
<artifactId>ban-junit4-pass-it</artifactId>
<version>1.0-SNAPSHOT</version>
<packaging>hpi</packaging>
<properties>
<jenkins.version>2.479.3</jenkins.version>
<ban-junit4-imports.skip>false</ban-junit4-imports.skip>
</properties>
<repositories>
<repository>
<id>repo.jenkins-ci.org</id>
<url>https://repo.jenkins-ci.org/public/</url>
</repository>
</repositories>
<pluginRepositories>
<pluginRepository>
<id>repo.jenkins-ci.org</id>
<url>https://repo.jenkins-ci.org/public/</url>
</pluginRepository>
</pluginRepositories>
</project>
10 changes: 10 additions & 0 deletions src/it/ban-junit4-pass/postbuild.groovy
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Build passes
def hpi = new File(basedir, 'target/ban-junit4-pass-it.jar')
assert hpi.exists()

def log = new File(basedir, 'build.log')
assert !log.text.contains('org.junit.Test')
assert !log.text.contains('(Line: 3, Matched by: org.junit.**)')
assert !log.text.contains('static org.junit.Assert.assertTrue')
assert !log.text.contains('(Line: 5, Matched by: org.junit.**)')
return true
2 changes: 2 additions & 0 deletions src/it/ban-junit4-pass/src/main/resources/index.jelly
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<?jelly escape-by-default='true'?>
<div/>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package banjunit4;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertTrue;

public class BanJUnit4PassTest {
@Test
public void thisFails() {
assertTrue(true);
}
}