Optionally ban JUnit 4 imports

[daniel-beck]

This PR adds a new property ban-junit4-imports.skip. Similar to spotless.check.skip before it, it is true by default. If overridden, an enforcer rule banning org.junit.* imports, but allowing org.junit.jupiter.* imports, is run.

It's motivated by PRs like jenkinsci/matrix-auth-plugin#175 or jenkinsci/csp-plugin#48 that look far too easy to regress. Going via parent POM seems like the logical choice.

Feedback welcome. If this looks acceptable in principle, I'll look into where best to document this, as well as adding tests.

Testing done

Installed a snapshot locally, then:

• Fails on matrix-auth-plugin:master if skip = false.
• Passes on matrix-auth-plugin:master if skip = true (default)
• Passes on Migrate tests to JUnit5 matrix-auth-plugin#175 with either skip value.

Failure error message output

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-enforcer-plugin:3.6.1:enforce (check-junit-imports) on project matrix-auth: 
[ERROR] Rule 0: org.apache.maven.plugins.enforcer.RestrictImports failed with message:
[ERROR] 
[ERROR] Banned imports detected in TEST code:
[ERROR] 
[ERROR] Reason: Use JUnit 5 (org.junit.jupiter.*)
[ERROR]         in file:///Users/danielbeck/Repositories/github.com/daniel-beck/matrix-auth-plugin/src/test/java/com/cloudbees/hudson/plugins/folder/properties/AuthorizationMatrixPropertyTest.java
[ERROR]                 static org.junit.Assert.*               (Line: 27, Matched by: org.junit.**)
[ERROR]                 org.junit.Assert                        (Line: 47, Matched by: org.junit.**)
[ERROR]                 org.junit.Rule                          (Line: 48, Matched by: org.junit.**)
[ERROR]                 org.junit.Test                          (Line: 49, Matched by: org.junit.**)
[ERROR]         in file:///Users/danielbeck/Repositories/github.com/daniel-beck/matrix-auth-plugin/src/test/java/com/cloudbees/hudson/plugins/folder/properties/IdStrategyTest.java
[ERROR]                 static org.junit.Assert.assertEquals    (Line: 3, Matched by: org.junit.**)
[ERROR]                 static org.junit.Assert.fail            (Line: 4, Matched by: org.junit.**)
[ERROR]                 org.junit.Rule                          (Line: 13, Matched by: org.junit.**)
[ERROR]                 org.junit.Test                          (Line: 14, Matched by: org.junit.**)
[ERROR]         in file:///Users/danielbeck/Repositories/github.com/daniel-beck/matrix-auth-plugin/src/test/java/hudson/security/AuthorizationMatrixPropertyTest.java
[ERROR]                 org.junit.Assert                        (Line: 16, Matched by: org.junit.**)
[ERROR]                 org.junit.Rule                          (Line: 17, Matched by: org.junit.**)
[ERROR]                 org.junit.Test                          (Line: 18, Matched by: org.junit.**)
[ERROR]         in file:///Users/danielbeck/Repositories/github.com/daniel-beck/matrix-auth-plugin/src/test/java/hudson/security/ProjectMatrixAuthorizationStrategyTest.java
[ERROR]                 org.junit.Assert                        (Line: 25, Matched by: org.junit.**)
[ERROR]                 org.junit.Rule                          (Line: 26, Matched by: org.junit.**)
[ERROR]                 org.junit.Test                          (Line: 27, Matched by: org.junit.**)
[ERROR]         in file:///Users/danielbeck/Repositories/github.com/daniel-beck/matrix-auth-plugin/src/test/java/hudson/security/Security410Test.java
[ERROR]                 static org.junit.Assert.assertFalse     (Line: 3, Matched by: org.junit.**)
[ERROR]                 org.junit.Rule                          (Line: 7, Matched by: org.junit.**)
[ERROR]                 org.junit.Test                          (Line: 8, Matched by: org.junit.**)
[ERROR]         in file:///Users/danielbeck/Repositories/github.com/daniel-beck/matrix-auth-plugin/src/test/java/org/jenkinsci/plugins/matrixauth/AmbiguityTest.java
[ERROR]                 static org.junit.Assert.assertEquals    (Line: 3, Matched by: org.junit.**)
[ERROR]                 static org.junit.Assert.assertFalse     (Line: 4, Matched by: org.junit.**)
[ERROR]                 static org.junit.Assert.assertNotNull   (Line: 5, Matched by: org.junit.**)
[ERROR]                 static org.junit.Assert.assertThrows    (Line: 6, Matched by: org.junit.**)
[ERROR]                 static org.junit.Assert.assertTrue      (Line: 7, Matched by: org.junit.**)
[ERROR]                 org.junit.Rule                          (Line: 29, Matched by: org.junit.**)
[ERROR]                 org.junit.Test                          (Line: 30, Matched by: org.junit.**)
[ERROR]         in file:///Users/danielbeck/Repositories/github.com/daniel-beck/matrix-auth-plugin/src/test/java/org/jenkinsci/plugins/matrixauth/AuthorizationContainerDescriptorTest.java
[ERROR]                 org.junit.Assert                        (Line: 9, Matched by: org.junit.**)
[ERROR]                 org.junit.Test                          (Line: 10, Matched by: org.junit.**)
[ERROR]         in file:///Users/danielbeck/Repositories/github.com/daniel-beck/matrix-auth-plugin/src/test/java/org/jenkinsci/plugins/matrixauth/AuthorizationMatrixNodePropertyTest.java
[ERROR]                 org.junit.Assert                        (Line: 37, Matched by: org.junit.**)
[ERROR]                 org.junit.Rule                          (Line: 38, Matched by: org.junit.**)
[ERROR]                 org.junit.Test                          (Line: 39, Matched by: org.junit.**)
[ERROR]         in file:///Users/danielbeck/Repositories/github.com/daniel-beck/matrix-auth-plugin/src/test/java/org/jenkinsci/plugins/matrixauth/Jenkins57313Test.java
[ERROR]                 org.junit.Assert                        (Line: 6, Matched by: org.junit.**)
[ERROR]                 org.junit.Rule                          (Line: 7, Matched by: org.junit.**)
[ERROR]                 org.junit.Test                          (Line: 8, Matched by: org.junit.**)
[ERROR]         in file:///Users/danielbeck/Repositories/github.com/daniel-beck/matrix-auth-plugin/src/test/java/org/jenkinsci/plugins/matrixauth/PermissionAdderTest.java
[ERROR]                 org.junit.Assert                        (Line: 11, Matched by: org.junit.**)
[ERROR]                 org.junit.Rule                          (Line: 12, Matched by: org.junit.**)
[ERROR]                 org.junit.Test                          (Line: 13, Matched by: org.junit.**)
[ERROR]         in file:///Users/danielbeck/Repositories/github.com/daniel-beck/matrix-auth-plugin/src/test/java/org/jenkinsci/plugins/matrixauth/ReadOnlyTest.java
[ERROR]                 org.junit.Assert                        (Line: 16, Matched by: org.junit.**)
[ERROR]                 org.junit.Before                        (Line: 17, Matched by: org.junit.**)
[ERROR]                 org.junit.Rule                          (Line: 18, Matched by: org.junit.**)
[ERROR]                 org.junit.Test                          (Line: 19, Matched by: org.junit.**)
[ERROR]         in file:///Users/danielbeck/Repositories/github.com/daniel-beck/matrix-auth-plugin/src/test/java/org/jenkinsci/plugins/matrixauth/Security2180Test.java
[ERROR]                 static org.junit.Assert.assertFalse     (Line: 6, Matched by: org.junit.**)
[ERROR]                 static org.junit.Assert.assertNotNull   (Line: 7, Matched by: org.junit.**)
[ERROR]                 static org.junit.Assert.assertTrue      (Line: 8, Matched by: org.junit.**)
[ERROR]                 org.junit.After                         (Line: 36, Matched by: org.junit.**)
[ERROR]                 org.junit.Assert                        (Line: 37, Matched by: org.junit.**)
[ERROR]                 org.junit.ClassRule                     (Line: 38, Matched by: org.junit.**)
[ERROR]                 org.junit.Rule                          (Line: 39, Matched by: org.junit.**)
[ERROR]                 org.junit.Test                          (Line: 40, Matched by: org.junit.**)
[ERROR]         in file:///Users/danielbeck/Repositories/github.com/daniel-beck/matrix-auth-plugin/src/test/java/org/jenkinsci/plugins/matrixauth/inheritance/InheritanceMigrationTest.java
[ERROR]                 org.junit.Assert                        (Line: 14, Matched by: org.junit.**)
[ERROR]                 org.junit.Rule                          (Line: 15, Matched by: org.junit.**)
[ERROR]                 org.junit.Test                          (Line: 16, Matched by: org.junit.**)
[ERROR]         in file:///Users/danielbeck/Repositories/github.com/daniel-beck/matrix-auth-plugin/src/test/java/org/jenkinsci/plugins/matrixauth/integrations/casc/ExportTest.java
[ERROR]                 static org.junit.Assert.assertEquals    (Line: 5, Matched by: org.junit.**)
[ERROR]                 static org.junit.Assert.assertNotNull   (Line: 6, Matched by: org.junit.**)
[ERROR]                 org.junit.Rule                          (Line: 16, Matched by: org.junit.**)
[ERROR]                 org.junit.Test                          (Line: 17, Matched by: org.junit.**)
[ERROR]         in file:///Users/danielbeck/Repositories/github.com/daniel-beck/matrix-auth-plugin/src/test/java/org/jenkinsci/plugins/matrixauth/integrations/casc/ImportTest.java
[ERROR]                 static org.junit.Assert.assertEquals    (Line: 5, Matched by: org.junit.**)
[ERROR]                 static org.junit.Assert.assertFalse     (Line: 6, Matched by: org.junit.**)
[ERROR]                 static org.junit.Assert.assertNotNull   (Line: 7, Matched by: org.junit.**)
[ERROR]                 static org.junit.Assert.assertTrue      (Line: 8, Matched by: org.junit.**)
[ERROR]                 org.junit.Rule                          (Line: 29, Matched by: org.junit.**)
[ERROR]                 org.junit.Test                          (Line: 30, Matched by: org.junit.**)
[ERROR] 
[ERROR] Analysis of 47 files took less than 1 second

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]

Perhaps something similar could be done for commons-lang? That would have identified jenkinsci/git-plugin#1789 (comment).

[timja]

makes sense

[daniel-beck]

@timja Any suggestions or preferences for naming before we lock this in forever?

[timja]

Name looks good

[daniel-beck]

FTR I have no intention of doing any sort of larger-scale migration or other followup for this. This is just intended as an additional safeguard for changes like the ones I pointed out in the PR comment (and IMO is almost a prerequisite to accepting them).

[timja]

fyi @strangelookingnerd

[strangelookingnerd]

FTR I have no intention of doing any sort of larger-scale migration or other followup for this. This is just intended as an additional safeguard for changes like the ones I pointed out in the PR comment (and IMO is almost a prerequisite to accepting them).

Thanks for doing this 👍🏼
I'll look into adding the flag to plugins I have migrated in the past and also include it in future migrations.

[strangelookingnerd]

@daniel-beck FYI, causes: https://github.com/jenkinsci/generic-webhook-trigger-plugin/pull/369/checks?check_run_id=48162665919

Banned imports detected in TEST code:
 
 Reason: Use JUnit 5 (org.junit.jupiter.*)
	in file:///home/jenkins/agent/workspace/ic-webhook-trigger-plugin_PR-369/src/test/java/org/jenkinsci/plugins/gwt/bdd/RunTest.java
		org.junit.platform.suite.api.IncludeEngines          	(Line: 3, Matched by: org.junit.**)
 		org.junit.platform.suite.api.SelectClasspathResource 	(Line: 4, Matched by: org.junit.**)
 		org.junit.platform.suite.api.Suite                   	(Line: 5, Matched by: org.junit.**)

I'll create a follow-up to fix it if you don't mind.