Migrate tests to JUnit5

* Migrate annotations and imports
* Migrate assertions
* Remove public visibility for test classes and methods
* Minor code cleanup

Author: strangelookingnerd

pom.xml

@@ -28,7 +28,7 @@
     <parent>
         <groupId>org.jenkins-ci.plugins</groupId>
         <artifactId>plugin</artifactId>
-        <version>5.24</version>
+        <version>5.26</version>
         <relativePath />
     </parent>
     <groupId>org.jenkins-ci.plugins.workflow</groupId>
@@ -68,6 +68,7 @@
         <jenkins.baseline>2.479</jenkins.baseline>
         <jenkins.version>${jenkins.baseline}.1</jenkins.version>
         <no-test-jar>false</no-test-jar>
+        <ban-junit4-imports.skip>false</ban-junit4-imports.skip>
         <hpi.strictBundledArtifacts>true</hpi.strictBundledArtifacts>
     </properties>
     <dependencyManagement>
@@ -82,6 +83,12 @@
         </dependencies>
     </dependencyManagement>
     <dependencies>
+        <dependency>
+            <groupId>org.awaitility</groupId>
+            <artifactId>awaitility</artifactId>
+            <version>4.3.0</version>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.mockito</groupId>
             <artifactId>mockito-core</artifactId>

src/test/java/org/jenkinsci/plugins/workflow/steps/AbstractStepImplTest.java

@@ -3,11 +3,12 @@
 import hudson.Extension;
 import hudson.model.Node;
 import jenkins.model.Jenkins;
-import static org.junit.Assert.*;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
+import static org.junit.jupiter.api.Assertions.*;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.junit.jupiter.WithJenkins;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.DataBoundSetter;
 
@@ -21,32 +22,33 @@
  * @author Kohsuke Kawaguchi
  */
 @SuppressWarnings("deprecation") // it is all deprecated
-public class AbstractStepImplTest {
-    @Rule
-    public JenkinsRule j = new JenkinsRule();
+@WithJenkins
+class AbstractStepImplTest {
+    
+    private JenkinsRule j;
 
     @Inject
-    BogusStep.DescriptorImpl d;
+    private BogusStep.DescriptorImpl d;
 
-    @Before
-    public void setUp() {
+    @BeforeEach
+    void setUp(JenkinsRule rule) {
+        j = rule;
         j.getInstance().getInjector().injectMembers(this);
     }
 
     @Test
-    public void inject() throws Exception {
-
+    void inject() throws Exception {
         Map<String, Object> r = new HashMap<>();
         r.put("a",3);
         r.put("b","bbb");
         r.put("c",null);
         r.put("d","ddd");
         BogusStep step = (BogusStep) d.newInstance(r);
 
-        assertEquals(step.a,3);
-        assertEquals(step.b,"bbb");
+        assertEquals(3, step.a);
+        assertEquals("bbb", step.b);
         assertNull(step.c);
-        assertEquals(step.d,"ddd");
+        assertEquals("ddd", step.d);
 
         StepContext c = mock(StepContext.class);
         when(c.get(Node.class)).thenReturn(j.getInstance());
@@ -56,10 +58,12 @@ public void inject() throws Exception {
     }
 
     public static class BogusStep extends AbstractStepImpl {
-        int a;
-        String b;
-        @DataBoundSetter String c;
-        Object d;
+
+        private int a;
+        private String b;
+        @DataBoundSetter
+        private String c;
+        private Object d;
 
         @DataBoundConstructor
         public BogusStep(int a, String b) {
@@ -92,11 +96,12 @@ public String getDisplayName() {
     }
 
     public static class BogusStepExecution extends AbstractSynchronousStepExecution<Void> {
+
         @Inject
-        Jenkins jenkins;
+        private Jenkins jenkins;
 
         @StepContextParameter
-        Node n;
+        private Node n;
 
         @Override
         protected Void run() {

src/test/java/org/jenkinsci/plugins/workflow/steps/DynamicContextTest.java

@@ -25,12 +25,15 @@
 package org.jenkinsci.plugins.workflow.steps;
 
 import java.io.IOException;
-import org.junit.Test;
-import static org.junit.Assert.*;
+import org.junit.jupiter.api.Test;
 
-public class DynamicContextTest {
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 
-    @Test public void subclassing() throws Exception {
+class DynamicContextTest {
+
+    @Test
+    void subclassing() throws Exception {
         class Super {}
         class Sub extends Super {}
         class Sub2 extends Super {}
@@ -48,10 +51,9 @@ class Dyn extends DynamicContext.Typed<Super> {
             }
         }
         DynamicContext ctx = new Dyn();
-        assertNotNull("can look up via supertype", ctx.get(Super.class, nullContext));
-        assertNotNull("can look up via subtype", ctx.get(Sub.class, nullContext));
-        assertNull("but not via a mismatched subtype", ctx.get(Sub2.class, nullContext));
-        assertNull("nor via an unrelated supertype", ctx.get(Runnable.class, nullContext));
+        assertNotNull(ctx.get(Super.class, nullContext), "can look up via supertype");
+        assertNotNull(ctx.get(Sub.class, nullContext), "can look up via subtype");
+        assertNull(ctx.get(Sub2.class, nullContext), "but not via a mismatched subtype");
+        assertNull(ctx.get(Runnable.class, nullContext), "nor via an unrelated supertype");
     }
-
 }

src/test/java/org/jenkinsci/plugins/workflow/steps/GeneralNonBlockingStepExecutionTest.java

@@ -24,10 +24,11 @@
 
 package org.jenkinsci.plugins.workflow.steps;
 
+import static org.awaitility.Awaitility.await;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.empty;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import hudson.model.Result;
 import hudson.model.TaskListener;
@@ -41,26 +42,41 @@
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
 import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep;
-import org.junit.Before;
-import org.junit.ClassRule;
-import org.junit.Rule;
-import org.junit.Test;
-import org.jvnet.hudson.test.BuildWatcher;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
-import org.jvnet.hudson.test.LoggerRule;
+import org.jvnet.hudson.test.LogRecorder;
 import org.jvnet.hudson.test.TestExtension;
+import org.jvnet.hudson.test.junit.jupiter.BuildWatcherExtension;
+import org.jvnet.hudson.test.junit.jupiter.WithJenkins;
 import org.kohsuke.stapler.DataBoundConstructor;
 
-public class GeneralNonBlockingStepExecutionTest {
+@WithJenkins
+class GeneralNonBlockingStepExecutionTest {
 
-    @ClassRule public static BuildWatcher buildWatcher = new BuildWatcher();
+    @SuppressWarnings("unused")
+    @RegisterExtension
+    private static final BuildWatcherExtension BUILD_WATCHER = new BuildWatcherExtension();
 
-    @Rule public JenkinsRule r = new JenkinsRule();
+    private static Semaphore startEnter, startExit, endEnter, endExit;
+
+    private final LogRecorder logging = new LogRecorder();
 
-    @Rule public LoggerRule logging = new LoggerRule();
+    private JenkinsRule r;
+
+    @BeforeEach
+    void setUp(JenkinsRule rule) {
+        r = rule;
+        startEnter = new Semaphore(0);
+        startExit = new Semaphore(0);
+        endEnter = new Semaphore(0);
+        endExit = new Semaphore(0);
+    }
 
-    @Test public void getStatus() throws Exception {
+    @Test
+    void getStatus() throws Exception {
         WorkflowJob p = r.createProject(WorkflowJob.class, "p");
         p.setDefinition(new CpsFlowDefinition("slowBlock {semaphore 'wait'}", true));
         WorkflowRun b = p.scheduleBuild2(0).waitForStart();
@@ -71,9 +87,7 @@ public class GeneralNonBlockingStepExecutionTest {
         startExit.release();
         SemaphoreStep.waitForStart("wait/1", b);
         assertThat(((CpsFlowExecution) b.getExecution()).getThreadDump().toString(), containsString("at DSL.slowBlock(not currently scheduled, or running blocks)"));
-        while (b.getExecutor().getAsynchronousExecution().blocksRestart()) {
-            Thread.sleep(100); // as above
-        }
+        await().until(() -> !b.getExecutor().getAsynchronousExecution().blocksRestart());
         SemaphoreStep.success("wait/1", null);
         endEnter.acquire();
         assertThat(((CpsFlowExecution) b.getExecution()).getThreadDump().toString(), containsString("at DSL.slowBlock(running in thread: org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution [#"));
@@ -83,7 +97,8 @@ public class GeneralNonBlockingStepExecutionTest {
         r.assertBuildStatusSuccess(r.waitForCompletion(b));
     }
 
-    @Test public void stop() throws Exception {
+    @Test
+    void stop() throws Exception {
         WorkflowJob p = r.createProject(WorkflowJob.class, "p");
         p.setDefinition(new CpsFlowDefinition("slowBlock {semaphore 'wait'}", true));
         logging.record(CpsStepContext.class, Level.WARNING).capture(100);
@@ -120,7 +135,8 @@ public class GeneralNonBlockingStepExecutionTest {
     }
 
     @Issue("JENKINS-58878")
-    @Test public void shouldNotHang() throws Exception {
+    @Test
+    void shouldNotHang() throws Exception {
         int iterations = 50;
         startExit.release(iterations); // Prevents the semaphores from blocking inside of the slowBlock step.
         endExit.release(iterations);
@@ -134,61 +150,72 @@ public class GeneralNonBlockingStepExecutionTest {
         r.buildAndAssertSuccess(p);
     }
 
-    private static Semaphore startEnter, startExit, endEnter, endExit;
+    @SuppressWarnings("unused")
+    public static final class SlowBlockStep extends Step {
 
-    @Before public void semaphores() {
-        startEnter = new Semaphore(0);
-        startExit = new Semaphore(0);
-        endEnter = new Semaphore(0);
-        endExit = new Semaphore(0);
-    }
+        @DataBoundConstructor
+        public SlowBlockStep() {}
 
-    public static final class SlowBlockStep extends Step {
-        @DataBoundConstructor public SlowBlockStep() {}
-        @Override public StepExecution start(StepContext context) {
+        @Override
+        public StepExecution start(StepContext context) {
             return new Execution(context, this);
         }
+
         private static final class Execution extends GeneralNonBlockingStepExecution {
             private final transient SlowBlockStep step;
+
             Execution(StepContext context, SlowBlockStep step) {
                 super(context);
                 this.step = step;
             }
+
             private void println(String msg) throws Exception {
                 getContext().get(TaskListener.class).getLogger().println(msg);
             }
-            @Override public boolean start() throws Exception {
+
+            @Override
+            public boolean start() throws Exception {
                 println("starting step");
                 run(this::doStart);
                 return false;
             }
+
             private void doStart() throws Exception {
                 println("starting background part of step");
                 startEnter.release();
                 startExit.acquire();
                 println("starting body");
                 getContext().newBodyInvoker().withCallback(new Callback()).start();
             }
+
             private final class Callback extends TailCall {
-                @Override protected void finished(StepContext context) throws Exception {
+
+                @Override
+                protected void finished(StepContext context) throws Exception {
                     println("body completed, starting background end part of step");
                     endEnter.release();
                     endExit.acquire();
                     println("ending step");
                 }
             }
         }
-        @TestExtension public static final class DescriptorImpl extends StepDescriptor {
-            @Override public String getFunctionName() {
+        @TestExtension
+        public static final class DescriptorImpl extends StepDescriptor {
+
+            @Override
+            public String getFunctionName() {
                 return "slowBlock";
             }
-            @Override public Set<? extends Class<?>> getRequiredContext() {
+
+            @Override
+            public Set<? extends Class<?>> getRequiredContext() {
                 return Collections.singleton(TaskListener.class);
             }
-            @Override public boolean takesImplicitBlockArgument() {
+
+            @Override
+            public boolean takesImplicitBlockArgument() {
                 return true;
             }
         }
     }
-
 }

src/test/java/org/jenkinsci/plugins/workflow/steps/MissingContextVariableExceptionTest.java

@@ -27,18 +27,25 @@
 import hudson.model.Result;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
-import org.junit.Rule;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.junit.jupiter.WithJenkins;
 
-public final class MissingContextVariableExceptionTest {
+@WithJenkins
+class MissingContextVariableExceptionTest {
 
-    @Rule public JenkinsRule r = new JenkinsRule();
+    private JenkinsRule r;
 
-    @Test public void message() throws Exception {
+    @BeforeEach
+    void setUp(JenkinsRule rule) {
+        r = rule;
+    }
+
+    @Test
+    void message() throws Exception {
         WorkflowJob p = r.createProject(WorkflowJob.class, "p");
         p.setDefinition(new CpsFlowDefinition("sh 'oops'", true));
         r.assertLogContains("Perhaps you forgot to surround the sh step with a step that provides this, such as: node", r.buildAndAssertStatus(Result.FAILURE, p));
     }
-
 }