From eb1d8053c77045fd6a8116f0f781bee8e6c443ec Mon Sep 17 00:00:00 2001 From: strangelookingnerd <49242855+strangelookingnerd@users.noreply.github.com> Date: Mon, 28 Jul 2025 12:42:31 +0200 Subject: [PATCH 1/7] Migrate tests to JUnit5 * Migrate annotations and imports * Migrate assertions * Remove public visibility for test classes and methods * Minor code cleanup --- pom.xml | 2 + .../plugins/workflow/ArtifactManagerTest.java | 52 +- .../DirectArtifactManagerFactory.java | 4 +- .../workflow/actions/ErrorActionTest.java | 89 +-- .../workflow/flow/DurabilityBasicsTest.java | 37 +- .../workflow/flow/FlowExecutionListTest.java | 71 ++- .../plugins/workflow/graph/FlowNodeTest.java | 510 +++++++++-------- .../graphanalysis/FlowScannerTest.java | 244 ++++---- .../workflow/graphanalysis/FlowTestUtils.java | 37 +- .../graphanalysis/ForkScannerTest.java | 531 +++++++++--------- .../graphanalysis/MemoryFlowChunkTest.java | 6 +- .../graphanalysis/NoOpChunkFinder.java | 1 + .../workflow/graphanalysis/TestVisitor.java | 58 +- .../workflow/log/FileLogStorageTest.java | 34 +- .../workflow/log/LogStorageTestBase.java | 55 +- .../workflow/log/SpanCoalescerTest.java | 9 +- .../log/TaskListenerDecoratorOrderTest.java | 34 +- .../log/TaskListenerDecoratorTest.java | 28 +- 18 files changed, 983 insertions(+), 819 deletions(-) diff --git a/pom.xml b/pom.xml index a81594b1..c71d4641 100644 --- a/pom.xml +++ b/pom.xml @@ -68,6 +68,8 @@ ${jenkins.baseline}.1 false jenkinsci/${project.artifactId}-plugin + + 2486.v14ec80488532 diff --git a/src/test/java/org/jenkinsci/plugins/workflow/ArtifactManagerTest.java b/src/test/java/org/jenkinsci/plugins/workflow/ArtifactManagerTest.java index c8d4f8b6..91e2af30 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/ArtifactManagerTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/ArtifactManagerTest.java @@ -30,10 +30,10 @@ import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import hudson.AbortException; import hudson.EnvVars; @@ -52,6 +52,7 @@ import java.io.IOException; import java.io.InputStream; import java.net.URL; +import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collections; @@ -66,19 +67,27 @@ import jenkins.util.VirtualFile; import org.apache.commons.io.IOUtils; import org.jenkinsci.plugins.workflow.flow.StashManager; -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.LoggerRule; +import org.jvnet.hudson.test.LogRecorder; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; import test.ssh_agent.OutboundAgent; /** * {@link #artifactArchiveAndDelete} and variants allow an implementation of {@link ArtifactManager} plus {@link VirtualFile} to be run through a standard gantlet of tests. */ -public class ArtifactManagerTest { +@WithJenkins +class ArtifactManagerTest { - @Rule public JenkinsRule r = new JenkinsRule(); - @Rule public LoggerRule logging = new LoggerRule(); + private final LogRecorder logging = new LogRecorder(); + + private JenkinsRule r; + + @BeforeEach + void setUp(JenkinsRule rule) { + r = rule; + } /** * Creates an agent, in a Docker container when possible, calls {@link #setUpWorkspace}, then runs some tests. @@ -180,7 +189,7 @@ private static void setUpWorkspace(FilePath workspace, boolean weirdCharacters) } private static class FindEncoding extends MasterToSlaveCallable { @Override public String call() { - return System.getProperty("file.encoding") + " vs. " + System.getProperty("sun.jnu.encoding"); + return Charset.defaultCharset().displayName() + " vs. " + System.getProperty("sun.jnu.encoding"); } } @@ -349,9 +358,9 @@ private void test() throws Exception { */ private void assertFile(VirtualFile f, String contents) throws Exception { System.err.println("Asserting file: " + f); - assertTrue("Not a file: " + f, f.isFile()); - assertFalse("Unexpected directory: " + f, f.isDirectory()); - assertTrue("Does not exist: " + f, f.exists()); + assertTrue(f.isFile(), "Not a file: " + f); + assertFalse(f.isDirectory(), "Unexpected directory: " + f); + assertTrue(f.exists(), "Does not exist: " + f); assertEquals(contents.length(), f.length()); assertThat(f.lastModified(), not(is(0))); try (InputStream is = f.open()) { @@ -381,9 +390,9 @@ private static final class RemoteOpenURL extends MasterToSlaveCallable { String method = request.getRequestLine().getMethod(); - String contents = URLDecoder.decode(request.getRequestLine().getUri().substring(1), "UTF-8"); + String contents = URLDecoder.decode(request.getRequestLine().getUri().substring(1), StandardCharsets.UTF_8); switch (method) { case "GET": { response.setStatusCode(200); @@ -183,7 +183,7 @@ private static final class NoOpenVF extends VirtualFile { try (InputStream is = delegate.open()) { contents = IOUtils.toString(is, StandardCharsets.UTF_8); } - return new URL(null, baseURL + URLEncoder.encode(contents, "UTF-8"), new URLStreamHandler() { + return new URL(null, baseURL + URLEncoder.encode(contents, StandardCharsets.UTF_8), new URLStreamHandler() { @Override protected URLConnection openConnection(URL u) throws IOException { throw new IOException("not allowed to open " + u + " from this JVM"); } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/actions/ErrorActionTest.java b/src/test/java/org/jenkinsci/plugins/workflow/actions/ErrorActionTest.java index 6ec2ac99..e55ae99a 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/actions/ErrorActionTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/actions/ErrorActionTest.java @@ -25,8 +25,8 @@ package org.jenkinsci.plugins.workflow.actions; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import java.util.ArrayList; import java.util.List; @@ -39,12 +39,9 @@ import org.jenkinsci.plugins.workflow.graph.FlowNode; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; -import org.junit.ClassRule; -import org.junit.Rule; -import org.junit.Test; -import org.jvnet.hudson.test.BuildWatcher; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import org.jvnet.hudson.test.Issue; -import org.jvnet.hudson.test.JenkinsSessionRule; import groovy.lang.MissingMethodException; import hudson.FilePath; @@ -65,25 +62,28 @@ import org.jenkinsci.plugins.workflow.steps.StepDescriptor; import org.jenkinsci.plugins.workflow.steps.StepExecution; import org.jenkinsci.plugins.workflow.steps.StepExecutions; -import org.jvnet.hudson.test.InboundAgentRule; -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.InboundAgentExtension; +import org.jvnet.hudson.test.junit.jupiter.JenkinsSessionExtension; import org.kohsuke.stapler.DataBoundConstructor; /** * Tests for {@link ErrorAction} */ -public class ErrorActionTest { +class ErrorActionTest { - @ClassRule - public static BuildWatcher buildWatcher = new BuildWatcher(); + @RegisterExtension + private static final BuildWatcherExtension buildWatcher = new BuildWatcherExtension(); - @Rule - public JenkinsSessionRule rr = new JenkinsSessionRule(); + @RegisterExtension + private final JenkinsSessionExtension rr = new JenkinsSessionExtension(); - @Rule public InboundAgentRule agents = new InboundAgentRule(); + @RegisterExtension + private final InboundAgentExtension agents = new InboundAgentExtension(); - @Rule public LoggerRule logging = new LoggerRule().record(ErrorAction.class, Level.FINE); + private final LogRecorder logging = new LogRecorder().record(ErrorAction.class, Level.FINE); private List extractErrorActions(FlowExecution exec) { List ret = new ArrayList<>(); @@ -99,14 +99,15 @@ private List extractErrorActions(FlowExecution exec) { } @Test - public void simpleException() throws Throwable { + void simpleException() throws Throwable { rr.then(r -> { final String EXPECTED = "For testing purpose"; WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "p"); job.setDefinition(new CpsFlowDefinition(String.format( - "node {\n" - + "throw new Exception('%s');\n" - + "}" + """ + node { + throw new Exception('%s'); + }""" , EXPECTED ), true)); WorkflowRun b = r.assertBuildStatus(Result.FAILURE, job.scheduleBuild2(0).get()); @@ -121,16 +122,17 @@ public void simpleException() throws Throwable { @Issue("JENKINS-34488") @Test - public void unserializableForSecurityReason() throws Throwable { + void unserializableForSecurityReason() throws Throwable { rr.then(r -> { final String FAILING_EXPRESSION = "(2 + 2) == 5"; WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "p"); // "assert false" throws org.codehaus.groovy.runtime.powerassert.PowerAssertionError, // which is rejected by remoting. job.setDefinition(new CpsFlowDefinition(String.format( - "node {\n" - + "assert %s;\n" - + "}", + """ + node { + assert %s; + }""", FAILING_EXPRESSION ), true)); WorkflowRun b = r.assertBuildStatus(Result.FAILURE, job.scheduleBuild2(0).get()); @@ -144,7 +146,8 @@ public void unserializableForSecurityReason() throws Throwable { } @Issue("JENKINS-39346") - @Test public void wrappedUnserializableException() throws Throwable { + @Test + void wrappedUnserializableException() throws Throwable { rr.then(r -> { WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition( @@ -167,7 +170,8 @@ public void unserializableForSecurityReason() throws Throwable { } @Issue("JENKINS-49025") - @Test public void nestedFieldUnserializable() throws Throwable { + @Test + void nestedFieldUnserializable() throws Throwable { rr.then(r -> { WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition( @@ -189,21 +193,25 @@ public static class X extends Exception { final NullObject nil = NullObject.getNullObject(); } - @Test public void userDefinedError() throws Throwable { + @Test + void userDefinedError() throws Throwable { rr.then(r -> { WorkflowJob p = r.createProject(WorkflowJob.class); p.setDefinition(new CpsFlowDefinition( - "class MyException extends Exception {\n" + - " MyException(String message) { super(message) }\n" + - "}\n" + - "throw new MyException('test')\n", + """ + class MyException extends Exception { + MyException(String message) { super(message) } + } + throw new MyException('test') + """, true)); WorkflowRun b = r.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0)); assertThat(b.getExecution().getCauseOfFailure(), Matchers.instanceOf(ProxyException.class)); }); } - @Test public void missingPropertyExceptionMemoryLeak() throws Throwable { + @Test + void missingPropertyExceptionMemoryLeak() throws Throwable { rr.then(r -> { WorkflowJob p = r.createProject(WorkflowJob.class); p.setDefinition(new CpsFlowDefinition("FOO", false)); @@ -212,7 +220,8 @@ public static class X extends Exception { }); } - @Test public void findOriginOfAsyncErrorAcrossRestart() throws Throwable { + @Test + void findOriginOfAsyncErrorAcrossRestart() throws Throwable { String name = "restart"; AtomicReference origin = new AtomicReference<>(); rr.then(r -> { @@ -232,7 +241,8 @@ public static class X extends Exception { }); } - @Test public void findOriginOfSyncErrorAcrossRestart() throws Throwable { + @Test + void findOriginOfSyncErrorAcrossRestart() throws Throwable { String name = "restart"; AtomicReference origin = new AtomicReference<>(); rr.then(r -> { @@ -251,7 +261,8 @@ public static class X extends Exception { }); } - @Test public void findOriginFromBodyExecutionCallback() throws Throwable { + @Test + void findOriginFromBodyExecutionCallback() throws Throwable { rr.then(r -> { agents.createAgent(r, "remote"); var p = r.createProject(WorkflowJob.class); @@ -263,6 +274,7 @@ public static class X extends Exception { r.assertLogContains("Found in: fails", b); }); } + public static final class WrapperStep extends Step { @DataBoundConstructor public WrapperStep() {} @Override public StepExecution start(StepContext context) throws Exception { @@ -305,6 +317,7 @@ public void onFailure(StepContext context, Throwable t) { } } } + public static final class FailingStep extends Step { @DataBoundConstructor public FailingStep() {} @Override public StepExecution start(StepContext context) throws Exception { @@ -332,7 +345,8 @@ private static final class Sleep extends MasterToSlaveFileCallable { } } - @Test public void cyclicErrorsAreSupported() throws Throwable { + @Test + void cyclicErrorsAreSupported() throws Throwable { Exception cyclic1 = new Exception(); Exception cyclic2 = new Exception(cyclic1); cyclic1.initCause(cyclic2); @@ -340,7 +354,8 @@ private static final class Sleep extends MasterToSlaveFileCallable { assertNotNull(new ErrorAction(cyclic2)); } - @Test public void unserializableCyclicErrorsAreSupported() throws Throwable { + @Test + void unserializableCyclicErrorsAreSupported() throws Throwable { Exception unserializable = new MissingMethodException("thisMethodDoesNotExist", String.class, new Object[0]); Exception cyclic = new Exception(unserializable); unserializable.initCause(cyclic); diff --git a/src/test/java/org/jenkinsci/plugins/workflow/flow/DurabilityBasicsTest.java b/src/test/java/org/jenkinsci/plugins/workflow/flow/DurabilityBasicsTest.java index 97ce4bdb..b458a9de 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/flow/DurabilityBasicsTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/flow/DurabilityBasicsTest.java @@ -6,60 +6,61 @@ import hudson.security.ACL; import hudson.security.ACLContext; import jenkins.model.Jenkins; -import org.junit.Assert; -import org.junit.Rule; -import org.junit.Test; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import org.jvnet.hudson.test.MockAuthorizationStrategy; -import org.jvnet.hudson.test.JenkinsSessionRule; +import org.jvnet.hudson.test.junit.jupiter.JenkinsSessionExtension; import java.util.Collection; import java.util.Optional; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * @author Sam Van Oort */ -public class DurabilityBasicsTest { +class DurabilityBasicsTest { - @Rule - public JenkinsSessionRule sessions = new JenkinsSessionRule(); + @RegisterExtension + private final JenkinsSessionExtension sessions = new JenkinsSessionExtension(); @Test - public void configRoundTrip() throws Throwable { + void configRoundTrip() throws Throwable { sessions.then(j -> { GlobalDefaultFlowDurabilityLevel.DescriptorImpl level = j.jenkins.getExtensionList(GlobalDefaultFlowDurabilityLevel.DescriptorImpl.class).get(0); level.setDurabilityHint(FlowDurabilityHint.PERFORMANCE_OPTIMIZED); j.configRoundtrip(); - Assert.assertEquals(FlowDurabilityHint.PERFORMANCE_OPTIMIZED, level.getDurabilityHint()); + assertEquals(FlowDurabilityHint.PERFORMANCE_OPTIMIZED, level.getDurabilityHint()); level.setDurabilityHint(null); j.configRoundtrip(); - Assert.assertNull(level.getDurabilityHint()); + assertNull(level.getDurabilityHint()); // Customize again so we can check for persistence level.setDurabilityHint(FlowDurabilityHint.PERFORMANCE_OPTIMIZED); - Assert.assertEquals(FlowDurabilityHint.PERFORMANCE_OPTIMIZED, level.getDurabilityHint()); + assertEquals(FlowDurabilityHint.PERFORMANCE_OPTIMIZED, level.getDurabilityHint()); }); sessions.then(j -> { GlobalDefaultFlowDurabilityLevel.DescriptorImpl level = j.jenkins.getExtensionList(GlobalDefaultFlowDurabilityLevel.DescriptorImpl.class).get(0); - Assert.assertEquals(FlowDurabilityHint.PERFORMANCE_OPTIMIZED, level.getDurabilityHint()); + assertEquals(FlowDurabilityHint.PERFORMANCE_OPTIMIZED, level.getDurabilityHint()); }); } @Test - public void defaultHandling() throws Throwable { + void defaultHandling() throws Throwable { sessions.then(j -> { - Assert.assertEquals(GlobalDefaultFlowDurabilityLevel.SUGGESTED_DURABILITY_HINT, GlobalDefaultFlowDurabilityLevel.getDefaultDurabilityHint()); + assertEquals(GlobalDefaultFlowDurabilityLevel.SUGGESTED_DURABILITY_HINT, GlobalDefaultFlowDurabilityLevel.getDefaultDurabilityHint()); GlobalDefaultFlowDurabilityLevel.DescriptorImpl level = j.jenkins.getExtensionList(GlobalDefaultFlowDurabilityLevel.DescriptorImpl.class).get(0); level.setDurabilityHint(FlowDurabilityHint.PERFORMANCE_OPTIMIZED); - Assert.assertEquals(FlowDurabilityHint.PERFORMANCE_OPTIMIZED, GlobalDefaultFlowDurabilityLevel.getDefaultDurabilityHint()); + assertEquals(FlowDurabilityHint.PERFORMANCE_OPTIMIZED, GlobalDefaultFlowDurabilityLevel.getDefaultDurabilityHint()); }); } @Test - public void managePermissionShouldAccessGlobalConfig() throws Throwable { + void managePermissionShouldAccessGlobalConfig() throws Throwable { sessions.then(j -> { final String USER = "user"; final String MANAGER = "manager"; @@ -80,7 +81,7 @@ public void managePermissionShouldAccessGlobalConfig() throws Throwable { try (ACLContext c = ACL.as(User.getById(MANAGER, true))) { Collection descriptors = Functions.getSortedDescriptorsForGlobalConfigUnclassified(); Optional found = descriptors.stream().filter(descriptor -> descriptor instanceof GlobalDefaultFlowDurabilityLevel.DescriptorImpl).findFirst(); - assertTrue("Global configuration should be accessible to MANAGE users", found.isPresent()); + assertTrue(found.isPresent(), "Global configuration should be accessible to MANAGE users"); } }); } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest.java b/src/test/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest.java index d122897f..9367fd38 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest.java @@ -28,7 +28,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.hasItem; -import static org.junit.Assert.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotNull; import com.google.common.util.concurrent.ListenableFuture; import hudson.AbortException; @@ -40,6 +40,8 @@ import hudson.model.StringParameterValue; import hudson.model.TaskListener; import hudson.model.queue.QueueTaskFuture; + +import java.io.Serial; import java.io.Serializable; import java.lang.ref.WeakReference; import java.util.Collections; @@ -64,26 +66,30 @@ import org.jenkinsci.plugins.workflow.support.pickles.SingleTypedPickleFactory; import org.jenkinsci.plugins.workflow.support.pickles.TryRepeatedly; import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep; -import org.junit.ClassRule; -import org.junit.Test; -import org.junit.Rule; -import org.jvnet.hudson.test.BuildWatcher; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import org.jvnet.hudson.test.Issue; -import org.jvnet.hudson.test.LoggerRule; -import org.jvnet.hudson.test.JenkinsSessionRule; +import org.jvnet.hudson.test.LogRecorder; import org.jvnet.hudson.test.MemoryAssert; import org.jvnet.hudson.test.TestExtension; +import org.jvnet.hudson.test.junit.jupiter.BuildWatcherExtension; +import org.jvnet.hudson.test.junit.jupiter.JenkinsSessionExtension; import org.jvnet.hudson.test.recipes.LocalData; import org.kohsuke.stapler.DataBoundConstructor; -public class FlowExecutionListTest { +class FlowExecutionListTest { + + @RegisterExtension + private static final BuildWatcherExtension buildWatcher = new BuildWatcherExtension(); - @ClassRule public static BuildWatcher buildWatcher = new BuildWatcher(); - @Rule public JenkinsSessionRule sessions = new JenkinsSessionRule(); - @Rule public LoggerRule logging = new LoggerRule().record(FlowExecutionList.class, Level.FINE); + @RegisterExtension + private final JenkinsSessionExtension sessions = new JenkinsSessionExtension(); + + private final LogRecorder logging = new LogRecorder().record(FlowExecutionList.class, Level.FINE); @Issue("JENKINS-40771") - @Test public void simultaneousRegister() throws Throwable { + @Test + void simultaneousRegister() throws Throwable { sessions.then(j -> { WorkflowJob p = j.createProject(WorkflowJob.class, "p"); { // make sure there is an initial FlowExecutionList.xml @@ -112,7 +118,8 @@ public class FlowExecutionListTest { }); } - @Test public void forceLoadRunningExecutionsAfterRestart() throws Throwable { + @Test + void forceLoadRunningExecutionsAfterRestart() throws Throwable { logging.capture(50); sessions.then(r -> { WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p"); @@ -155,7 +162,8 @@ public class FlowExecutionListTest { } @Issue("JENKINS-67164") - @Test public void resumeStepExecutions() throws Throwable { + @Test + void resumeStepExecutions() throws Throwable { sessions.then(r -> { WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition("noResume()", true)); @@ -174,7 +182,8 @@ public class FlowExecutionListTest { } @LocalData - @Test public void resumeStepExecutionsWithCorruptFlowGraphWithCycle() throws Throwable { + @Test + void resumeStepExecutionsWithCorruptFlowGraphWithCycle() throws Throwable { // LocalData created using the following snippet while the build was waiting in the _second_ sleep, except // for build.xml, which was captured during the sleep step. The StepEndNode for the stage was then adjusted to // have its startId point to the timeout step's StepStartNode, creating a loop. @@ -197,12 +206,13 @@ public class FlowExecutionListTest { .map(LogRecord::getThrown) .filter(Objects::nonNull) .map(Throwable::toString) - .collect(Collectors.toList()); + .toList(); assertThat(loggedExceptions, hasItem(containsString("Cycle in flow graph"))); }); } - @Test public void stepExecutionIteratorDoesNotLeakBuildsWhenCpsVmIsStuck() throws Throwable { + @Test + void stepExecutionIteratorDoesNotLeakBuildsWhenCpsVmIsStuck() throws Throwable { sessions.then(r -> { var notStuck = r.createProject(WorkflowJob.class, "not-stuck"); notStuck.setDefinition(new CpsFlowDefinition("semaphore 'wait'", true)); @@ -230,13 +240,15 @@ public class FlowExecutionListTest { }); } - @Test public void stepExecutionIteratorDoesNotLeakBuildsWhenProgramPromiseIsStuck() throws Throwable { + @Test + void stepExecutionIteratorDoesNotLeakBuildsWhenProgramPromiseIsStuck() throws Throwable { sessions.then(r -> { var stuck = r.createProject(WorkflowJob.class, "stuck"); stuck.setDefinition(new CpsFlowDefinition( - "def x = new org.jenkinsci.plugins.workflow.flow.FlowExecutionListTest.StuckPickle.Marker()\n" + - "semaphore 'stuckWait'\n" + - "echo x.getClass().getName()", false)); + """ + def x = new org.jenkinsci.plugins.workflow.flow.FlowExecutionListTest.StuckPickle.Marker() + semaphore 'stuckWait' + echo x.getClass().getName()""", false)); var stuckBuild = stuck.scheduleBuild2(0).waitForStart(); SemaphoreStep.waitForStart("stuckWait/1", stuckBuild); }); @@ -267,7 +279,8 @@ public class FlowExecutionListTest { } public static class NonResumableStep extends Step implements Serializable { - public static final long serialVersionUID = 1L; + @Serial + private static final long serialVersionUID = 1L; @DataBoundConstructor public NonResumableStep() { } @Override @@ -276,7 +289,8 @@ public StepExecution start(StepContext sc) { } private static class ExecutionImpl extends StepExecution implements Serializable { - public static final long serialVersionUID = 1L; + @Serial + private static final long serialVersionUID = 1L; private ExecutionImpl(StepContext sc) { super(sc); } @@ -307,6 +321,7 @@ public String getFunctionName() { * Blocks the CPS VM thread synchronously (bad!) to test related problems. */ public static class SynchronousBlockingStep extends Step implements Serializable { + @Serial private static final long serialVersionUID = 1L; private static final Map blocked = new HashMap<>(); private final String id; @@ -362,15 +377,19 @@ public String getFunctionName() { public static class StuckPickle extends Pickle { @Override public ListenableFuture rehydrate(FlowExecutionOwner owner) { - return new TryRepeatedly(1) { + return new TryRepeatedly<>(1) { @Override protected Marker tryResolve() { return ExtensionList.lookupSingleton(Factory.class).resolved; } - @Override protected FlowExecutionOwner getOwner() { + + @Override + protected FlowExecutionOwner getOwner() { return owner; } - @Override public String toString() { + + @Override + public String toString() { return "StuckPickle for " + owner; } }; diff --git a/src/test/java/org/jenkinsci/plugins/workflow/graph/FlowNodeTest.java b/src/test/java/org/jenkinsci/plugins/workflow/graph/FlowNodeTest.java index f713d901..6c734767 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/graph/FlowNodeTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/graph/FlowNodeTest.java @@ -27,7 +27,6 @@ import hudson.model.BallColor; import hudson.model.Result; import java.lang.reflect.Method; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.text.MessageFormat; import java.util.ArrayList; @@ -51,88 +50,85 @@ import org.jenkinsci.plugins.workflow.steps.StepDescriptor; import org.jenkinsci.plugins.workflow.steps.StepExecution; import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep; -import org.junit.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.jvnet.hudson.test.LogRecorder; +import org.jvnet.hudson.test.junit.jupiter.JenkinsSessionExtension; import org.kohsuke.stapler.DataBoundConstructor; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasSize; -import static org.junit.Assert.assertArrayEquals; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; -import org.junit.Rule; -import org.junit.runners.model.Statement; +import org.junit.jupiter.api.Test; import org.jvnet.hudson.test.Issue; -import org.jvnet.hudson.test.LoggerRule; -import org.jvnet.hudson.test.RestartableJenkinsRule; import org.jvnet.hudson.test.TestExtension; -public class FlowNodeTest { +class FlowNodeTest { - @Rule public RestartableJenkinsRule rr = new RestartableJenkinsRule(); - @Rule public LoggerRule logging = new LoggerRule().record(FlowNode.class, Level.FINER); + @RegisterExtension + private final JenkinsSessionExtension rr = new JenkinsSessionExtension(); + + private final LogRecorder logging = new LogRecorder().record(FlowNode.class, Level.FINER); @Issue("JENKINS-38223") - @Test public void isActive() { - rr.addStep(new Statement() { - @Override public void evaluate() throws Throwable { - WorkflowJob p = rr.j.createProject(WorkflowJob.class, "p"); - p.setDefinition(new CpsFlowDefinition( - "semaphore 'pre-outer'\n" + - "stage('outer') {\n" + - " semaphore 'pre-inner'\n" + - " stage('inner') {\n" + - " semaphore 'inner'\n" + - " }\n" + - " semaphore 'post-inner'\n" + - "}\n" + - "semaphore 'post-outer'\n" + - "parallel a: {\n" + - " semaphore 'branch-a'\n" + - "}, b: {\n" + - " semaphore 'branch-b'\n" + - "}\n" + - "semaphore 'last'", true)); - WorkflowRun b = p.scheduleBuild2(0).waitForStart(); - SemaphoreStep.waitForStart("pre-outer/1", b); - assertActiveSteps(b, "Start of Pipeline", "semaphore: pre-outer"); - SemaphoreStep.success("pre-outer/1", null); - SemaphoreStep.waitForStart("pre-inner/1", b); - } + @Test + void isActive() throws Throwable { + rr.then(j -> { + WorkflowJob p = j.createProject(WorkflowJob.class, "p"); + p.setDefinition(new CpsFlowDefinition( + """ + semaphore 'pre-outer' + stage('outer') { + semaphore 'pre-inner' + stage('inner') { + semaphore 'inner' + } + semaphore 'post-inner' + } + semaphore 'post-outer' + parallel a: { + semaphore 'branch-a' + }, b: { + semaphore 'branch-b' + } + semaphore 'last'""", true)); + WorkflowRun b = p.scheduleBuild2(0).waitForStart(); + SemaphoreStep.waitForStart("pre-outer/1", b); + assertActiveSteps(b, "Start of Pipeline", "semaphore: pre-outer"); + SemaphoreStep.success("pre-outer/1", null); + SemaphoreStep.waitForStart("pre-inner/1", b); }); - rr.addStep(new Statement() { - @Override public void evaluate() throws Throwable { - WorkflowRun b = rr.j.jenkins.getItemByFullName("p", WorkflowJob.class).getLastBuild(); - assertActiveSteps(b, "Start of Pipeline", "stage: outer", "{ (outer)", "semaphore: pre-inner"); - SemaphoreStep.success("pre-inner/1", null); - SemaphoreStep.waitForStart("inner/1", b); - assertActiveSteps(b, "Start of Pipeline", "stage: outer", "{ (outer)", "stage: inner", "{ (inner)", "semaphore: inner"); - SemaphoreStep.success("inner/1", null); - SemaphoreStep.waitForStart("post-inner/1", b); - assertActiveSteps(b, "Start of Pipeline", "stage: outer", "{ (outer)", "semaphore: post-inner"); - SemaphoreStep.success("post-inner/1", null); - SemaphoreStep.waitForStart("post-outer/1", b); - assertActiveSteps(b, "Start of Pipeline", "semaphore: post-outer"); - SemaphoreStep.success("post-outer/1", null); - SemaphoreStep.waitForStart("branch-a/1", b); - SemaphoreStep.waitForStart("branch-b/1", b); - } + rr.then(j -> { + WorkflowRun b = j.jenkins.getItemByFullName("p", WorkflowJob.class).getLastBuild(); + assertActiveSteps(b, "Start of Pipeline", "stage: outer", "{ (outer)", "semaphore: pre-inner"); + SemaphoreStep.success("pre-inner/1", null); + SemaphoreStep.waitForStart("inner/1", b); + assertActiveSteps(b, "Start of Pipeline", "stage: outer", "{ (outer)", "stage: inner", "{ (inner)", "semaphore: inner"); + SemaphoreStep.success("inner/1", null); + SemaphoreStep.waitForStart("post-inner/1", b); + assertActiveSteps(b, "Start of Pipeline", "stage: outer", "{ (outer)", "semaphore: post-inner"); + SemaphoreStep.success("post-inner/1", null); + SemaphoreStep.waitForStart("post-outer/1", b); + assertActiveSteps(b, "Start of Pipeline", "semaphore: post-outer"); + SemaphoreStep.success("post-outer/1", null); + SemaphoreStep.waitForStart("branch-a/1", b); + SemaphoreStep.waitForStart("branch-b/1", b); }); - rr.addStep(new Statement() { - @Override public void evaluate() throws Throwable { - WorkflowRun b = rr.j.jenkins.getItemByFullName("p", WorkflowJob.class).getLastBuild(); - // weird order caused by FlowGraphWalker DFS - assertActiveSteps(b, "{ (Branch: a)", "semaphore: branch-a", "Start of Pipeline", "parallel", "{ (Branch: b)", "semaphore: branch-b"); - SemaphoreStep.success("branch-a/1", null); - SemaphoreStep.success("branch-b/1", null); - SemaphoreStep.waitForStart("last/1", b); - assertActiveSteps(b, "Start of Pipeline", "semaphore: last"); - SemaphoreStep.success("last/1", null); - rr.j.waitForCompletion(b); - assertActiveSteps(b); - } + rr.then(j -> { + WorkflowRun b = j.jenkins.getItemByFullName("p", WorkflowJob.class).getLastBuild(); + // weird order caused by FlowGraphWalker DFS + assertActiveSteps(b, "{ (Branch: a)", "semaphore: branch-a", "Start of Pipeline", "parallel", "{ (Branch: b)", "semaphore: branch-b"); + SemaphoreStep.success("branch-a/1", null); + SemaphoreStep.success("branch-b/1", null); + SemaphoreStep.waitForStart("last/1", b); + assertActiveSteps(b, "Start of Pipeline", "semaphore: last"); + SemaphoreStep.success("last/1", null); + j.waitForCompletion(b); + assertActiveSteps(b); }); } @@ -172,272 +168,270 @@ private static void assertActiveSteps(WorkflowRun b, String... expected) throws @Issue("JENKINS-27395") @Test - public void enclosingBlocksSingleBlock() { - rr.addStep(new Statement() { - @Override - public void evaluate() throws Throwable { - WorkflowJob job = rr.j.createProject(WorkflowJob.class, "enclosingBlocks"); - job.setDefinition(new CpsFlowDefinition( - "catchError {echo 'bob';}", true)); + void enclosingBlocksSingleBlock() throws Throwable { + rr.then(j -> { + WorkflowJob job = j.createProject(WorkflowJob.class, "enclosingBlocks"); + job.setDefinition(new CpsFlowDefinition( + "catchError {echo 'bob';}", true)); - WorkflowRun r = rr.j.buildAndAssertSuccess(job); + WorkflowRun r = j.buildAndAssertSuccess(job); - FlowExecution execution = r.getExecution(); + FlowExecution execution = r.getExecution(); - // FlowStartNode - assertExpectedEnclosing(execution, "2", null); + // FlowStartNode + assertExpectedEnclosing(execution, "2", null); - // CatchError block, outer block - assertExpectedEnclosing(execution, "3", "2"); + // CatchError block, outer block + assertExpectedEnclosing(execution, "3", "2"); - // CatchError block, inner block (body) - assertExpectedEnclosing(execution, "4", "3"); + // CatchError block, inner block (body) + assertExpectedEnclosing(execution, "4", "3"); - // Echo step, enclosed in body block for catchError step - assertExpectedEnclosing(execution, "5", "4"); + // Echo step, enclosed in body block for catchError step + assertExpectedEnclosing(execution, "5", "4"); - // End of inner catchError block (end of body), enclosed by outer block - assertExpectedEnclosing(execution, "6", "3"); + // End of inner catchError block (end of body), enclosed by outer block + assertExpectedEnclosing(execution, "6", "3"); - // End of outer catchError block, enclosed by FlowStartNode - assertExpectedEnclosing(execution, "7", "2"); + // End of outer catchError block, enclosed by FlowStartNode + assertExpectedEnclosing(execution, "7", "2"); - // FlowEndNode, not inside anything at all - assertExpectedEnclosing(execution, "8", null); - } + // FlowEndNode, not inside anything at all + assertExpectedEnclosing(execution, "8", null); }); } @Issue("JENKINS-27395") @Test - public void enclosingBlocks() { - rr.addStep(new Statement() { - @Override - public void evaluate() throws Throwable { - WorkflowJob job = rr.j.createProject(WorkflowJob.class, "enclosingBlocks"); - /* - Node dump follows, format: + void enclosingBlocks() throws Throwable { + rr.then(j -> { + WorkflowJob job = j.createProject(WorkflowJob.class, "enclosingBlocks"); + /* + Node dump follows, format: [ID]{parent,ids}(millisSinceStartOfRun) flowNodeClassName stepDisplayName [st=startId if a block end node] Action format: - - actionClassName actionDisplayName +- actionClassName actionDisplayName ------------------------------------------------------------------------------------------ [2]{}FlowStartNode Start of Pipeline [3]{2}StepStartNode Stage : Start - -LogActionImpl Console Output - -ArgumentsActionImpl null +-LogActionImpl Console Output +-ArgumentsActionImpl null [4]{3}StepStartNode outermost - -BodyInvocationAction null - -LabelAction outermost +-BodyInvocationAction null +-LabelAction outermost [5]{4}StepAtomNode Print Message - -LogActionImpl Console Output - -ArgumentsActionImpl null +-LogActionImpl Console Output +-ArgumentsActionImpl null [6]{5}StepStartNode Execute in parallel : Start - -LogActionImpl Console Output +-LogActionImpl Console Output [8]{6}StepStartNode Branch: a - -BodyInvocationAction null - -ParallelLabelAction Branch: a +-BodyInvocationAction null +-ParallelLabelAction Branch: a [9]{6}StepStartNode Branch: b - -BodyInvocationAction null - -ParallelLabelAction Branch: b +-BodyInvocationAction null +-ParallelLabelAction Branch: b [10]{8}StepStartNode Stage : Start - -LogActionImpl Console Output - -ArgumentsActionImpl null +-LogActionImpl Console Output +-ArgumentsActionImpl null [11]{10}StepStartNode inner-a - -BodyInvocationAction null - -LabelAction inner-a +-BodyInvocationAction null +-LabelAction inner-a [12]{9}StepStartNode Stage : Start - -LogActionImpl Console Output - -ArgumentsActionImpl null +-LogActionImpl Console Output +-ArgumentsActionImpl null [13]{12}StepStartNode inner-b - -BodyInvocationAction null - -LabelAction inner-b +-BodyInvocationAction null +-LabelAction inner-b [14]{11}StepAtomNode Print Message - -LogActionImpl Console Output - -ArgumentsActionImpl null +-LogActionImpl Console Output +-ArgumentsActionImpl null [15]{14}StepStartNode Stage : Start - -LogActionImpl Console Output - -ArgumentsActionImpl null +-LogActionImpl Console Output +-ArgumentsActionImpl null [16]{15}StepStartNode innermost-a - -BodyInvocationAction null - -LabelAction innermost-a +-BodyInvocationAction null +-LabelAction innermost-a [17]{13}StepAtomNode Print Message - -LogActionImpl Console Output - -ArgumentsActionImpl null +-LogActionImpl Console Output +-ArgumentsActionImpl null [18]{17}StepStartNode Stage : Start - -LogActionImpl Console Output - -ArgumentsActionImpl null +-LogActionImpl Console Output +-ArgumentsActionImpl null [19]{18}StepStartNode innermost-b - -BodyInvocationAction null - -LabelAction innermost-b +-BodyInvocationAction null +-LabelAction innermost-b [20]{16}StepAtomNode Print Message - -LogActionImpl Console Output - -ArgumentsActionImpl null +-LogActionImpl Console Output +-ArgumentsActionImpl null [21]{20}StepEndNode Stage : Body : End [st=16] - -BodyInvocationAction null +-BodyInvocationAction null [22]{19}StepAtomNode Print Message - -LogActionImpl Console Output - -ArgumentsActionImpl null +-LogActionImpl Console Output +-ArgumentsActionImpl null [23]{22}StepEndNode Stage : Body : End [st=19] - -BodyInvocationAction null +-BodyInvocationAction null [24]{21}StepEndNode Stage : End [st=15] [25]{23}StepEndNode Stage : End [st=18] [26]{24}StepEndNode Stage : Body : End [st=11] - -BodyInvocationAction null +-BodyInvocationAction null [27]{25}StepEndNode Stage : Body : End [st=13] - -BodyInvocationAction null +-BodyInvocationAction null [28]{26}StepEndNode Stage : End [st=10] [29]{27}StepEndNode Stage : End [st=12] [30]{28}StepEndNode Execute in parallel : Body : End [st=8] - -BodyInvocationAction null +-BodyInvocationAction null [31]{29}StepEndNode Execute in parallel : Body : End [st=9] - -BodyInvocationAction null +-BodyInvocationAction null [32]{30,31}StepEndNode Execute in parallel : End [st=6] [33]{32}StepEndNode Stage : Body : End [st=4] - -BodyInvocationAction null +-BodyInvocationAction null [34]{33}StepEndNode Stage : End [st=3] [35]{34}FlowEndNode End of Pipeline [st=2] ------------------------------------------------------------------------------------------ - */ - job.setDefinition(new CpsFlowDefinition( - "stage('outermost') {\n" + - " echo 'in outermost'\n" + - " parallel(a: {\n" + - " stage('inner-a') {\n" + - " echo 'in inner-a'\n" + - " stage('innermost-a') {\n" + - " echo 'in innermost-a'\n" + - " }\n" + - " }\n" + - " },\n" + - " b: {\n" + - " stage('inner-b') {\n" + - " echo 'in inner-b'\n" + - " stage('innermost-b') {\n" + - " echo 'in innermost-b'\n" + - " }\n" + - " }\n" + - " })\n" + - "}\n", true)); + */ + job.setDefinition(new CpsFlowDefinition( + """ + stage('outermost') { + echo 'in outermost' + parallel(a: { + stage('inner-a') { + echo 'in inner-a' + stage('innermost-a') { + echo 'in innermost-a' + } + } + }, + b: { + stage('inner-b') { + echo 'in inner-b' + stage('innermost-b') { + echo 'in innermost-b' + } + } + }) + } + """, true)); - WorkflowRun r = rr.j.buildAndAssertSuccess(job); + WorkflowRun r = j.buildAndAssertSuccess(job); - FlowExecution execution = r.getExecution(); + FlowExecution execution = r.getExecution(); - // FlowStartNode - assertExpectedEnclosing(execution, "2", null); + // FlowStartNode + assertExpectedEnclosing(execution, "2", null); - // outermost stage start - assertExpectedEnclosing(execution, "3", "2"); + // outermost stage start + assertExpectedEnclosing(execution, "3", "2"); - // outermost stage body - assertExpectedEnclosing(execution, "4", "3"); + // outermost stage body + assertExpectedEnclosing(execution, "4", "3"); - // outermost echo - assertExpectedEnclosing(execution, "5", "4"); + // outermost echo + assertExpectedEnclosing(execution, "5", "4"); - // parallel start - assertExpectedEnclosing(execution, "6", "4"); + // parallel start + assertExpectedEnclosing(execution, "6", "4"); - // Branch a start - assertExpectedEnclosing(execution, "8", "6"); + // Branch a start + assertExpectedEnclosing(execution, "8", "6"); - // Branch b start - assertExpectedEnclosing(execution, "9", "6"); + // Branch b start + assertExpectedEnclosing(execution, "9", "6"); - // Stage inner-a start - assertExpectedEnclosing(execution, "10", "8"); + // Stage inner-a start + assertExpectedEnclosing(execution, "10", "8"); - // Stage inner-a body - assertExpectedEnclosing(execution, "11", "10"); + // Stage inner-a body + assertExpectedEnclosing(execution, "11", "10"); - // Stage inner-b start - assertExpectedEnclosing(execution, "12", "9"); + // Stage inner-b start + assertExpectedEnclosing(execution, "12", "9"); - // Stage inner-b body - assertExpectedEnclosing(execution, "13", "12"); + // Stage inner-b body + assertExpectedEnclosing(execution, "13", "12"); - // echo inner-a - assertExpectedEnclosing(execution, "14", "11"); + // echo inner-a + assertExpectedEnclosing(execution, "14", "11"); - // Stage innermost-a start - assertExpectedEnclosing(execution, "15", "11"); + // Stage innermost-a start + assertExpectedEnclosing(execution, "15", "11"); - // Stage innermost-a body - assertExpectedEnclosing(execution, "16", "15"); + // Stage innermost-a body + assertExpectedEnclosing(execution, "16", "15"); - // echo inner-b - assertExpectedEnclosing(execution, "17", "13"); + // echo inner-b + assertExpectedEnclosing(execution, "17", "13"); - // Stage innermost-b start - assertExpectedEnclosing(execution, "18", "13"); + // Stage innermost-b start + assertExpectedEnclosing(execution, "18", "13"); - // Stage innermost-b body - assertExpectedEnclosing(execution, "19", "18"); + // Stage innermost-b body + assertExpectedEnclosing(execution, "19", "18"); - // echo innermost-a - assertExpectedEnclosing(execution, "20", "16"); + // echo innermost-a + assertExpectedEnclosing(execution, "20", "16"); - // Stage innermost-a body end - assertExpectedEnclosing(execution, "21", "15"); + // Stage innermost-a body end + assertExpectedEnclosing(execution, "21", "15"); - // echo innermost-b - assertExpectedEnclosing(execution, "22", "19"); + // echo innermost-b + assertExpectedEnclosing(execution, "22", "19"); - // Stage innermost-b body end - assertExpectedEnclosing(execution, "23", "18"); + // Stage innermost-b body end + assertExpectedEnclosing(execution, "23", "18"); - // Stage innermost-a end - assertExpectedEnclosing(execution, "24", "11"); + // Stage innermost-a end + assertExpectedEnclosing(execution, "24", "11"); - // Stage innermost-b end - assertExpectedEnclosing(execution, "25", "13"); + // Stage innermost-b end + assertExpectedEnclosing(execution, "25", "13"); - // Stage inner-a body end - assertExpectedEnclosing(execution, "26", "10"); + // Stage inner-a body end + assertExpectedEnclosing(execution, "26", "10"); - // Stage inner-b body end - assertExpectedEnclosing(execution, "27", "12"); + // Stage inner-b body end + assertExpectedEnclosing(execution, "27", "12"); - // Stage inner-a end - assertExpectedEnclosing(execution, "28", "8"); + // Stage inner-a end + assertExpectedEnclosing(execution, "28", "8"); - // Stage inner-b end - assertExpectedEnclosing(execution, "29", "9"); + // Stage inner-b end + assertExpectedEnclosing(execution, "29", "9"); - // Branch a end - assertExpectedEnclosing(execution, "30", "6"); + // Branch a end + assertExpectedEnclosing(execution, "30", "6"); - // Branch b end - assertExpectedEnclosing(execution, "31", "6"); + // Branch b end + assertExpectedEnclosing(execution, "31", "6"); - // parallel end - assertExpectedEnclosing(execution, "32", "4"); + // parallel end + assertExpectedEnclosing(execution, "32", "4"); - // outermost stage body end - assertExpectedEnclosing(execution, "33", "3"); + // outermost stage body end + assertExpectedEnclosing(execution, "33", "3"); - // outermost stage end - assertExpectedEnclosing(execution, "34", "2"); + // outermost stage end + assertExpectedEnclosing(execution, "34", "2"); - // FlowEndNode - assertExpectedEnclosing(execution, "35", null); - } + // FlowEndNode + assertExpectedEnclosing(execution, "35", null); }); } - @Test public void useAbortedStatusWhenFailFast() { + @Test + void useAbortedStatusWhenFailFast() throws Throwable { rr.then(r -> { WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "p"); job.setDefinition(new CpsFlowDefinition( - "jobs = [failFast:true]\n" + - "jobs['one'] = {\n" + - " sleep 5\n" + - "}\n" + - "jobs['two'] = {\n" + - " error 'failing'\n" + - "}\n" + - "parallel jobs", true)); + """ + jobs = [failFast:true] + jobs['one'] = { + sleep 5 + } + jobs['two'] = { + error 'failing' + } + parallel jobs""", true)); WorkflowRun b = r.assertBuildStatus(Result.FAILURE, job.scheduleBuild2(0).get()); List coreStepNodes = new DepthFirstScanner().filteredNodes(b.getExecution(), new NodeStepTypePredicate("sleep")); @@ -449,12 +443,15 @@ public void evaluate() throws Throwable { }); } - @Test public void iconColorUsesWarningActionResult() { + @Test + void iconColorUsesWarningActionResult() throws Throwable { rr.then(r -> { WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "p"); job.setDefinition(new CpsFlowDefinition( - "warning('UNSTABLE')\n" + - "warning('FAILURE')\n", true)); + """ + warning('UNSTABLE') + warning('FAILURE') + """, true)); WorkflowRun b = r.assertBuildStatus(Result.SUCCESS, job.scheduleBuild2(0).get()); List nodes = new DepthFirstScanner().filteredNodes(b.getExecution(), new NodeStepTypePredicate("warning")); assertThat(nodes, hasSize(2)); @@ -465,9 +462,9 @@ public void evaluate() throws Throwable { @Issue("JENKINS-57805") @Test - public void nodeWithNoParentsInBruteForceScanForEnclosingBlock() { + void nodeWithNoParentsInBruteForceScanForEnclosingBlock() throws Throwable { logging.capture(10); - rr.thenWithHardShutdown(j -> { + rr.then(j -> { WorkflowJob job = j.jenkins.createProject(WorkflowJob.class, "p"); job.setDefinition(new CpsFlowDefinition( "echo 'to-corrupt'\n" + @@ -477,7 +474,7 @@ public void nodeWithNoParentsInBruteForceScanForEnclosingBlock() { SemaphoreStep.waitForStart("marker/1", run); CpsFlowExecution cpsExec = (CpsFlowExecution)run.getExecution(); // Corrupt a flow node so that we get an error when trying to load it. - Files.write(cpsExec.getStorageDir().toPath().resolve("3.xml"), "garbage".getBytes(StandardCharsets.UTF_8)); + Files.writeString(cpsExec.getStorageDir().toPath().resolve("3.xml"), "garbage"); }); rr.then(j -> { WorkflowJob job = j.jenkins.getItemByFullName("p", WorkflowJob.class); @@ -492,7 +489,7 @@ public void nodeWithNoParentsInBruteForceScanForEnclosingBlock() { @Issue("JENKINS-64438") @Test - public void addOrReplaceActionWorks() { + void addOrReplaceActionWorks() throws Throwable { rr.then(r -> { WorkflowJob j = r.createProject(WorkflowJob.class); j.setDefinition(new CpsFlowDefinition("doubleWarning()", true)); @@ -512,17 +509,16 @@ private void assertExpectedEnclosing(FlowExecution execution, String nodeId, Str assertNotNull(node); String secondEnclosingId = node.getEnclosingId(); - assertEquals(MessageFormat.format("Node {0}: enclosingID doesn't match between first and second fetches", node), - enclosingId, secondEnclosingId); + assertEquals(enclosingId, secondEnclosingId, MessageFormat.format("Node {0}: enclosingID doesn't match between first and second fetches", node)); if (enclosingId == null) { - assertTrue(MessageFormat.format("Node {0} and enclosingId {1}: null enclosing ID, but non-empty list of enclosing IDs", node, enclosingId), - node.getAllEnclosingIds().isEmpty()); - assertTrue(MessageFormat.format("Node {0} and enclosingId {1}: null enclosing ID, but non-empty list of enclosing blocks", node, enclosingId), - node.getEnclosingBlocks().isEmpty()); + assertTrue(node.getAllEnclosingIds().isEmpty(), + MessageFormat.format("Node {0} and enclosingId {1}: null enclosing ID, but non-empty list of enclosing IDs", node, enclosingId)); + assertTrue(node.getEnclosingBlocks().isEmpty(), + MessageFormat.format("Node {0} and enclosingId {1}: null enclosing ID, but non-empty list of enclosing blocks", node, enclosingId)); } else { FlowNode enclosingNode = execution.getNode(enclosingId); - assertNotNull(MessageFormat.format("Node {0} and enclosingId {1}: no node with enclosing ID exists", node, enclosingId), - enclosingNode); + assertNotNull(enclosingNode, + MessageFormat.format("Node {0} and enclosingId {1}: no node with enclosing ID exists", node, enclosingId)); List enclosingIds = node.getAllEnclosingIds(); List enclosingIdsIncludingNode = enclosingIdsIncludingNode(enclosingNode); List iteratedEnclosingBlockIds = new ArrayList<>(); diff --git a/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/FlowScannerTest.java b/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/FlowScannerTest.java index 2732725e..5a3a971c 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/FlowScannerTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/FlowScannerTest.java @@ -24,6 +24,14 @@ package org.jenkinsci.plugins.workflow.graphanalysis; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + import com.google.common.base.Predicate; import com.google.common.base.Predicates; import com.google.common.collect.Iterators; @@ -33,12 +41,12 @@ import org.jenkinsci.plugins.workflow.graph.FlowNode; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; -import org.junit.Assert; -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.JenkinsRule; +import org.jvnet.hudson.test.junit.jupiter.BuildWatcherExtension; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; import java.util.AbstractSet; import java.util.ArrayList; @@ -54,22 +62,28 @@ * Tests for all the core parts of graph analysis except the ForkScanner, internals which is complex enough to merit its own tests * @author Sam Van Oort */ -public class FlowScannerTest { +@WithJenkins +class FlowScannerTest { - @ClassRule - public static BuildWatcher buildWatcher = new BuildWatcher(); + @RegisterExtension + private static final BuildWatcherExtension buildWatcher = new BuildWatcherExtension(); - @Rule public JenkinsRule r = new JenkinsRule(); + private JenkinsRule r; + @BeforeEach + void setUp(JenkinsRule rule) { + r = rule; + } /** Tests the core logic separately from each implementation's scanner */ @Test - public void testAbstractScanner() throws Exception { + void testAbstractScanner() throws Exception { WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "SimpleLinear"); job.setDefinition(new CpsFlowDefinition( - "sleep 2 \n" + - "echo 'donothing'\n" + - "echo 'doitagain'", + """ + sleep 2 + echo 'donothing' + echo 'doitagain'""", true)); /* Flow structure (ID - type) @@ -87,36 +101,36 @@ public void testAbstractScanner() throws Exception { AbstractFlowScanner linear = new LinearScanner(); // ## Bunch of tests for convertToFastCheckable ## - Assert.assertEquals(Collections.emptySet(), linear.convertToFastCheckable(null)); - Assert.assertEquals(Collections.emptySet(), linear.convertToFastCheckable(new ArrayList<>())); + assertEquals(Collections.emptySet(), linear.convertToFastCheckable(null)); + assertEquals(Collections.emptySet(), linear.convertToFastCheckable(new ArrayList<>())); Collection coll = linear.convertToFastCheckable(Collections.singletonList(intermediateNode)); - Assert.assertTrue("Singleton set used for one element", coll instanceof AbstractSet); - Assert.assertEquals(1, coll.size()); + assertInstanceOf(AbstractSet.class, coll, "Singleton set used for one element"); + assertEquals(1, coll.size()); Collection multipleItems = Arrays.asList(exec.getNode("3"), exec.getNode("2")); coll = linear.convertToFastCheckable(multipleItems); - Assert.assertTrue("Original used for short list", coll instanceof List); - Assert.assertEquals(2, coll.size()); + assertInstanceOf(List.class, coll, "Original used for short list"); + assertEquals(2, coll.size()); coll = linear.convertToFastCheckable(new LinkedHashSet<>(multipleItems)); - Assert.assertTrue("Original used where set", coll instanceof LinkedHashSet); + assertInstanceOf(LinkedHashSet.class, coll, "Original used where set"); multipleItems = new ArrayList<>(); for (int i=0; i < 3; i++) { multipleItems.add(intermediateNode); } coll = linear.convertToFastCheckable(multipleItems); - Assert.assertTrue("Original used for short list", coll instanceof List); - Assert.assertEquals(3, coll.size()); + assertInstanceOf(List.class, coll, "Original used for short list"); + assertEquals(3, coll.size()); multipleItems = new ArrayList<>(); for (int i=0; i < 10; i++) { multipleItems.add(intermediateNode); } coll = linear.convertToFastCheckable(multipleItems); - Assert.assertTrue("Original used for short list", coll instanceof HashSet); - Assert.assertEquals(1, coll.size()); + assertInstanceOf(HashSet.class, coll, "Original used for short list"); + assertEquals(1, coll.size()); // Setup, return false if no nodes to iterate, else true @@ -124,43 +138,43 @@ public void testAbstractScanner() throws Exception { FlowNode nullNode = null; Collection nullColl = null; - Assert.assertTrue(linear.setup(heads, null)); - Assert.assertTrue(linear.setup(heads, Collections.emptySet())); - Assert.assertFalse(linear.setup(nullColl, heads)); - Assert.assertFalse(linear.setup(nullColl, null)); - Assert.assertFalse(linear.setup(heads, heads)); - Assert.assertTrue(linear.setup(heads)); - Assert.assertFalse(linear.setup(nullColl)); - Assert.assertFalse(linear.setup(Collections.emptySet())); - Assert.assertTrue(linear.setup(lastNode)); - Assert.assertTrue(linear.setup(lastNode, nullColl)); - Assert.assertFalse(linear.setup(nullNode)); - Assert.assertFalse(linear.setup(nullNode, heads)); - Assert.assertFalse(linear.setup(nullNode, nullColl)); - Assert.assertTrue(linear.setup(Arrays.asList(intermediateNode, lastNode), Collections.singleton(intermediateNode))); - Assert.assertEquals(lastNode, linear.myCurrent); + assertTrue(linear.setup(heads, null)); + assertTrue(linear.setup(heads, Collections.emptySet())); + assertFalse(linear.setup(nullColl, heads)); + assertFalse(linear.setup(nullColl, null)); + assertFalse(linear.setup(heads, heads)); + assertTrue(linear.setup(heads)); + assertFalse(linear.setup(nullColl)); + assertFalse(linear.setup(Collections.emptySet())); + assertTrue(linear.setup(lastNode)); + assertTrue(linear.setup(lastNode, nullColl)); + assertFalse(linear.setup(nullNode)); + assertFalse(linear.setup(nullNode, heads)); + assertFalse(linear.setup(nullNode, nullColl)); + assertTrue(linear.setup(Arrays.asList(intermediateNode, lastNode), Collections.singleton(intermediateNode))); + assertEquals(lastNode, linear.myCurrent); // First match, with no blacklist int[] ids = {6, 5, 4, 3, 2}; FlowNode firstEchoNode = exec.getNode("5"); FlowExecution nullExecution = null; - Assert.assertEquals(firstEchoNode, linear.findFirstMatch(heads, Collections.emptySet(), FlowTestUtils.MATCH_ECHO_STEP)); - Assert.assertEquals(firstEchoNode, linear.findFirstMatch(heads, FlowTestUtils.MATCH_ECHO_STEP)); - Assert.assertEquals(firstEchoNode, linear.findFirstMatch(lastNode, FlowTestUtils.MATCH_ECHO_STEP)); - Assert.assertEquals(firstEchoNode, linear.findFirstMatch(exec, FlowTestUtils.MATCH_ECHO_STEP)); - Assert.assertNull(linear.findFirstMatch(nullColl, FlowTestUtils.MATCH_ECHO_STEP)); - Assert.assertNull(linear.findFirstMatch(Collections.emptySet(), FlowTestUtils.MATCH_ECHO_STEP)); - Assert.assertNull(linear.findFirstMatch(nullNode, FlowTestUtils.MATCH_ECHO_STEP)); - Assert.assertNull(linear.findFirstMatch(nullExecution, FlowTestUtils.MATCH_ECHO_STEP)); + assertEquals(firstEchoNode, linear.findFirstMatch(heads, Collections.emptySet(), FlowTestUtils.MATCH_ECHO_STEP)); + assertEquals(firstEchoNode, linear.findFirstMatch(heads, FlowTestUtils.MATCH_ECHO_STEP)); + assertEquals(firstEchoNode, linear.findFirstMatch(lastNode, FlowTestUtils.MATCH_ECHO_STEP)); + assertEquals(firstEchoNode, linear.findFirstMatch(exec, FlowTestUtils.MATCH_ECHO_STEP)); + assertNull(linear.findFirstMatch(nullColl, FlowTestUtils.MATCH_ECHO_STEP)); + assertNull(linear.findFirstMatch(Collections.emptySet(), FlowTestUtils.MATCH_ECHO_STEP)); + assertNull(linear.findFirstMatch(nullNode, FlowTestUtils.MATCH_ECHO_STEP)); + assertNull(linear.findFirstMatch(nullExecution, FlowTestUtils.MATCH_ECHO_STEP)); // Filtered nodes FlowTestUtils.assertNodeOrder("Filtered echo nodes", linear.filteredNodes(heads, FlowTestUtils.MATCH_ECHO_STEP), 5, 4); FlowTestUtils.assertNodeOrder("Filtered echo nodes", linear.filteredNodes(heads, Collections.singleton(intermediateNode), FlowTestUtils.MATCH_ECHO_STEP), 5); - Assert.assertEquals(0, linear.filteredNodes(heads, null, Predicates.alwaysFalse()).size()); - Assert.assertEquals(0, linear.filteredNodes(nullNode, FlowTestUtils.MATCH_ECHO_STEP).size()); - Assert.assertEquals(0, linear.filteredNodes(Collections.emptySet(), FlowTestUtils.MATCH_ECHO_STEP).size()); + assertEquals(0, linear.filteredNodes(heads, null, Predicates.alwaysFalse()).size()); + assertEquals(0, linear.filteredNodes(nullNode, FlowTestUtils.MATCH_ECHO_STEP).size()); + assertEquals(0, linear.filteredNodes(Collections.emptySet(), FlowTestUtils.MATCH_ECHO_STEP).size()); // Same filter using the filterator linear.setup(heads); @@ -175,7 +189,7 @@ public void testAbstractScanner() throws Exception { // Visitor pattern tests FlowTestUtils.CollectingVisitor visitor = new FlowTestUtils.CollectingVisitor(); linear.visitAll(Collections.emptySet(), visitor); - Assert.assertEquals(0, visitor.getVisited().size()); + assertEquals(0, visitor.getVisited().size()); visitor.reset(); linear.visitAll(heads, visitor); @@ -188,17 +202,17 @@ public void testAbstractScanner() throws Exception { // Tests for edge cases of the various basic APIs linear.myNext = null; - Assert.assertFalse(linear.hasNext()); + assertFalse(linear.hasNext()); try { linear.next(); - Assert.fail("Should throw NoSuchElement exception"); + fail("Should throw NoSuchElement exception"); } catch (NoSuchElementException nsee) { // Passing case } - Assert.assertSame(linear.iterator(), linear); + assertSame(linear.iterator(), linear); try { linear.remove(); - Assert.fail("Should throw UnsupportedOperation exception"); + fail("Should throw UnsupportedOperation exception"); } catch (UnsupportedOperationException usoe) { // Passing case } @@ -206,12 +220,13 @@ public void testAbstractScanner() throws Exception { /** Tests the basic scan algorithm, predicate use, start/stop nodes */ @Test - public void testSimpleScan() throws Exception { + void testSimpleScan() throws Exception { WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "Convoluted"); job.setDefinition(new CpsFlowDefinition( - "sleep 2 \n" + - "echo 'donothing'\n" + - "echo 'doitagain'", + """ + sleep 2 + echo 'donothing' + echo 'doitagain'""", true)); /* Flow structure (ID - type) @@ -236,27 +251,28 @@ public void testSimpleScan() throws Exception { System.out.println("Iteration test with scanner: " + scan.getClass()); scan.setup(heads, null); FlowTestUtils.assertNodeOrder("Testing full scan for scanner " + scan.getClass(), scan, 6, 5, 4, 3, 2); - Assert.assertFalse(scan.hasNext()); + assertFalse(scan.hasNext()); // Blacklist tests scan.setup(heads, Collections.singleton(exec.getNode("4"))); FlowTestUtils.assertNodeOrder("Testing full scan for scanner " + scan.getClass(), scan, 6, 5); FlowNode f = scan.findFirstMatch(heads, Collections.singleton(exec.getNode("6")), Predicates.alwaysTrue()); - Assert.assertNull(f); + assertNull(f); } } /** Tests the basic scan algorithm where blocks are involved */ @Test - public void testBasicScanWithBlock() throws Exception { + void testBasicScanWithBlock() throws Exception { WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "Convoluted"); job.setDefinition(new CpsFlowDefinition( - "echo 'first'\n" + - "timeout(time: 10, unit: 'SECONDS') {\n" + - " echo 'second'\n" + - " echo 'third'\n" + - "}\n" + - "sleep 1", + """ + echo 'first' + timeout(time: 10, unit: 'SECONDS') { + echo 'second' + echo 'third' + } + sleep 1""", true)); /* Flow structure (ID - type) 2 - FlowStartNode @@ -287,12 +303,12 @@ public void testBasicScanWithBlock() throws Exception { // // Test block jump core FlowNode headCandidate = exec.getNode("8"); - Assert.assertEquals(exec.getNode("4"), linearBlockHoppingScanner.jumpBlockScan(headCandidate, Collections.emptySet())); - Assert.assertTrue("Setup should return true if we can iterate", linearBlockHoppingScanner.setup(headCandidate, null)); + assertEquals(exec.getNode("4"), linearBlockHoppingScanner.jumpBlockScan(headCandidate, Collections.emptySet())); + assertTrue(linearBlockHoppingScanner.setup(headCandidate, null), "Setup should return true if we can iterate"); // Test the actual iteration linearBlockHoppingScanner.setup(heads); - Assert.assertFalse(linearBlockHoppingScanner.hasNext()); + assertFalse(linearBlockHoppingScanner.hasNext()); linearBlockHoppingScanner.setup(exec.getNode("8")); FlowTestUtils.assertNodeOrder("Hopping over one block", linearBlockHoppingScanner, 4, 3, 2); linearBlockHoppingScanner.setup(exec.getNode("7")); @@ -300,28 +316,29 @@ public void testBasicScanWithBlock() throws Exception { // Test the black list in combination with hopping linearBlockHoppingScanner.setup(exec.getNode("8"), Collections.singleton(exec.getNode("5"))); - Assert.assertFalse(linearBlockHoppingScanner.hasNext()); + assertFalse(linearBlockHoppingScanner.hasNext()); linearBlockHoppingScanner.setup(exec.getNode("8"), Collections.singleton(exec.getNode("4"))); - Assert.assertFalse(linearBlockHoppingScanner.hasNext()); + assertFalse(linearBlockHoppingScanner.hasNext()); } /** And the parallel case */ @Test - public void testParallelScan() throws Exception { + void testParallelScan() throws Exception { WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "Convoluted"); job.setDefinition(new CpsFlowDefinition( - "echo 'first'\n" + - "def steps = [:]\n" + - "steps['1'] = {\n" + - " echo 'do 1 stuff'\n" + - "}\n" + - "steps['2'] = {\n" + - " echo '2a'\n" + - " echo '2b'\n" + - "}\n" + - "parallel steps\n" + - "echo 'final'", + """ + echo 'first' + def steps = [:] + steps['1'] = { + echo 'do 1 stuff' + } + steps['2'] = { + echo '2a' + echo '2b' + } + parallel steps + echo 'final'""", true)); /* Flow structure (ID - type) @@ -399,36 +416,37 @@ public void testParallelScan() throws Exception { // Filtering at different points within branches List blackList = Arrays.asList(exec.getNode("6"), exec.getNode("7")); - Assert.assertEquals(4, scanner.filteredNodes(heads, blackList, FlowTestUtils.MATCH_ECHO_STEP).size()); - Assert.assertEquals(4, scanner.filteredNodes(heads, Collections.singletonList(exec.getNode("4")), FlowTestUtils.MATCH_ECHO_STEP).size()); + assertEquals(4, scanner.filteredNodes(heads, blackList, FlowTestUtils.MATCH_ECHO_STEP).size()); + assertEquals(4, scanner.filteredNodes(heads, Collections.singletonList(exec.getNode("4")), FlowTestUtils.MATCH_ECHO_STEP).size()); blackList = Arrays.asList(exec.getNode("6"), exec.getNode("10")); - Assert.assertEquals(3, scanner.filteredNodes(heads, blackList, FlowTestUtils.MATCH_ECHO_STEP).size()); + assertEquals(3, scanner.filteredNodes(heads, blackList, FlowTestUtils.MATCH_ECHO_STEP).size()); } @Test - public void testNestedParallelScan() throws Exception { + void testNestedParallelScan() throws Exception { WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "Convoluted"); job.setDefinition(new CpsFlowDefinition( - "echo 'first'\n" + - "def steps = [:]\n" + - "steps['1'] = {\n" + - " echo 'do 1 stuff'\n" + - "}\n" + - "steps['2'] = {\n" + - " echo '2a'\n" + - " def nested = [:]\n" + - " nested['2-1'] = {\n" + - " echo 'do 2-1'\n" + - " } \n" + - " nested['2-2'] = {\n" + - " sleep 1\n" + - " echo '2 section 2'\n" + - " }\n" + - " echo '2b'\n" + - " parallel nested\n" + - "}\n" + - "parallel steps\n" + - "echo 'final'", + """ + echo 'first' + def steps = [:] + steps['1'] = { + echo 'do 1 stuff' + } + steps['2'] = { + echo '2a' + def nested = [:] + nested['2-1'] = { + echo 'do 2-1' + } + nested['2-2'] = { + sleep 1 + echo '2 section 2' + } + echo '2b' + parallel nested + } + parallel steps + echo 'final'""", true)); /* Parallel nested in parallel (ID-type) @@ -463,19 +481,19 @@ public void testNestedParallelScan() throws Exception { // Basic test of DepthFirstScanner AbstractFlowScanner scanner = new DepthFirstScanner(); Collection matches = scanner.filteredNodes(heads, null, FlowTestUtils.MATCH_ECHO_STEP); - Assert.assertEquals(7, matches.size()); + assertEquals(7, matches.size()); scanner.setup(heads); - Assert.assertTrue("FlowGraphWalker differs from DepthFirstScanner", Iterators.elementsEqual(new FlowGraphWalker(exec).iterator(), scanner.iterator())); + assertTrue(Iterators.elementsEqual(new FlowGraphWalker(exec).iterator(), scanner.iterator()), "FlowGraphWalker differs from DepthFirstScanner"); // We're going to test the ForkScanner in more depth since this is its natural use scanner = new ForkScanner(); matches = scanner.filteredNodes(heads, null, FlowTestUtils.MATCH_ECHO_STEP); - Assert.assertEquals(7, matches.size()); + assertEquals(7, matches.size()); heads = Arrays.asList(exec.getNode("20"), exec.getNode("17"), exec.getNode("9")); matches = scanner.filteredNodes(heads, null, FlowTestUtils.MATCH_ECHO_STEP); - Assert.assertEquals(6, matches.size()); // Commented out since temporarily failing + assertEquals(6, matches.size()); // Commented out since temporarily failing } } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/FlowTestUtils.java b/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/FlowTestUtils.java index 56ea42f3..f5e58ca4 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/FlowTestUtils.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/FlowTestUtils.java @@ -1,4 +1,31 @@ package org.jenkinsci.plugins.workflow.graphanalysis; +/* + * The MIT License + * + * Copyright (c) 2016, CloudBees, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + /* * The MIT License * @@ -28,8 +55,6 @@ import org.jenkinsci.plugins.workflow.flow.FlowExecution; import org.jenkinsci.plugins.workflow.graph.FlowNode; import org.jenkinsci.plugins.workflow.steps.StepDescriptor; -import org.junit.Assert; - import edu.umd.cs.findbugs.annotations.NonNull; import java.io.IOException; import java.util.ArrayList; @@ -40,10 +65,10 @@ * @author Sam Van Oort */ public class FlowTestUtils { + public static Predicate predicateMatchStepDescriptor(@NonNull final String descriptorId) { return input -> { - if (input instanceof StepAtomNode) { - StepAtomNode san = (StepAtomNode)input; + if (input instanceof StepAtomNode san) { StepDescriptor sd = san.getDescriptor(); return sd != null && descriptorId.equals(sd.getId()); } @@ -75,10 +100,10 @@ public ArrayList getVisited() { public static void assertNodeOrder(String description, Iterable nodes, String... nodeIds) { ArrayList realIds = new ArrayList<>(); for (FlowNode f: nodes) { - Assert.assertNotNull(f); + assertNotNull(f); realIds.add(f.getId()); } - Assert.assertArrayEquals(description, nodeIds, realIds.toArray()); + assertArrayEquals(nodeIds, realIds.toArray(), description); } /** Assert node ordering using iotas for their ids */ diff --git a/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScannerTest.java b/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScannerTest.java index 846f7592..bd46b3ce 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScannerTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScannerTest.java @@ -37,14 +37,14 @@ import org.jenkinsci.plugins.workflow.job.WorkflowRun; import org.jenkinsci.plugins.workflow.steps.EchoStep; 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.Disabled; +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.junit.Assert; +import org.jvnet.hudson.test.junit.jupiter.BuildWatcherExtension; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; import java.util.ArrayDeque; import java.util.ArrayList; @@ -61,22 +61,29 @@ import java.util.stream.Collectors; import java.util.stream.StreamSupport; -import org.junit.Ignore; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; /** * Tests for internals of ForkScanner */ -public class ForkScannerTest { - @ClassRule - public static BuildWatcher buildWatcher = new BuildWatcher(); +@WithJenkins +class ForkScannerTest { - @Rule - public JenkinsRule r = new JenkinsRule(); + @RegisterExtension + private static final BuildWatcherExtension buildWatcher = new BuildWatcherExtension(); + + private JenkinsRule r; public static Predicate predicateForCallEntryType(final TestVisitor.CallType type) { - return new Predicate() { + return new Predicate<>() { final TestVisitor.CallType myType = type; @Override @@ -129,49 +136,53 @@ public boolean test(TestVisitor.CallEntry input) { */ WorkflowRun NESTED_PARALLEL_RUN; - @Before - public void setUp() throws Exception { + @BeforeEach + void setUp(JenkinsRule rule) throws Exception { + r = rule; + r.jenkins.getInjector().injectMembers(this); WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "SimpleParallel"); job.setDefinition(new CpsFlowDefinition( - "echo 'first'\n" + - "def steps = [:]\n" + - "steps['1'] = {\n" + - " echo 'do 1 stuff'\n" + - "}\n" + - "steps['2'] = {\n" + - " echo '2a'\n" + - " echo '2b'\n" + - "}\n" + - "parallel steps\n" + - "echo 'final'", + """ + echo 'first' + def steps = [:] + steps['1'] = { + echo 'do 1 stuff' + } + steps['2'] = { + echo '2a' + echo '2b' + } + parallel steps + echo 'final'""", true)); WorkflowRun b = r.assertBuildStatusSuccess(job.scheduleBuild2(0)); this.SIMPLE_PARALLEL_RUN = b; job = r.jenkins.createProject(WorkflowJob.class, "NestedParallel"); job.setDefinition(new CpsFlowDefinition( - "echo 'first'\n" + - "def steps = [:]\n" + - "steps['1'] = {\n" + - " echo 'do 1 stuff'\n" + - "}\n" + - "steps['2'] = {\n" + - " echo '2a'\n" + - " echo '2b'\n" + - " def nested = [:]\n" + - " nested['2-1'] = {\n" + - " echo 'do 2-1'\n" + - " } \n" + - " nested['2-2'] = {\n" + - " sleep 1\n" + - " echo '2 section 2'\n" + - " }\n" + - " parallel nested\n" + - "}\n" + - "parallel steps\n" + - "echo 'final'", + """ + echo 'first' + def steps = [:] + steps['1'] = { + echo 'do 1 stuff' + } + steps['2'] = { + echo '2a' + echo '2b' + def nested = [:] + nested['2-1'] = { + echo 'do 2-1' + } + nested['2-2'] = { + sleep 1 + echo '2 section 2' + } + parallel nested + } + parallel steps + echo 'final'""", true)); b = r.assertBuildStatusSuccess(job.scheduleBuild2(0)); this.NESTED_PARALLEL_RUN = b; @@ -188,7 +199,7 @@ private void assertIncompleteParallelsHaveEventsForEnd(List heads, Tes List parallelEnds = test.filteredCallsByType(TestVisitor.CallType.PARALLEL_END).stream() .map(CALL_TO_NODE_ID) - .collect(Collectors.toList()); + .toList(); boolean hasMatchingEnd = false; for (FlowNode f : heads) { if (parallelEnds.contains(f.getId())) { @@ -196,17 +207,17 @@ private void assertIncompleteParallelsHaveEventsForEnd(List heads, Tes break; } } - Assert.assertTrue("If there are multiple heads, we MUST be in a parallel and have an event for the end", hasMatchingEnd); + assertTrue(hasMatchingEnd, "If there are multiple heads, we MUST be in a parallel and have an event for the end"); List branchEnds = test.filteredCallsByType(TestVisitor.CallType.PARALLEL_BRANCH_END).stream() .map(CALL_TO_NODE_ID) - .collect(Collectors.toList()); + .toList(); // Verify each branch has a branch end event for (FlowNode f : heads) { // Below can be used if we harden up the guarantees with incomplete parallels - Assert.assertTrue("Must have a parallel branch end for each branch we know of, but didn't, for nodeId: "+f.getId(), - branchEnds.contains(f.getId())); + assertTrue(branchEnds.contains(f.getId()), + "Must have a parallel branch end for each branch we know of, but didn't, for nodeId: "+f.getId()); } } @@ -225,13 +236,13 @@ private void sanityTestIterationAndVisiter(List heads) { test.assertNoIllegalNullsInEvents(); test.assertNoDupes(); int nodeCount = new DepthFirstScanner().allNodes(heads).size(); - Assert.assertEquals("ForkScanner should visit the same number of nodes as DepthFirstScanner", - nodeCount, - new ForkScanner().allNodes(heads).size()); + assertEquals(nodeCount, + new ForkScanner().allNodes(heads).size(), + "ForkScanner should visit the same number of nodes as DepthFirstScanner"); test.assertMatchingParallelStartEnd(); test.assertAllNodesGotChunkEvents(new DepthFirstScanner().allNodes(heads)); assertNoMissingParallelEvents(heads); - if (heads.size() > 0) { + if (!heads.isEmpty()) { test.assertMatchingParallelBranchStartEnd(); } @@ -250,11 +261,11 @@ private void sanityTestIterationAndVisiter(List heads) { lastId = entry.getNodeId(); } if (TestVisitor.CHUNK_EVENTS.contains(entry.type)) { - Assert.assertEquals(TestVisitor.CallType.CHUNK_END, entry.type); + assertEquals(TestVisitor.CallType.CHUNK_END, entry.type); break; } } - Assert.assertEquals(nodeCount, + assertEquals(nodeCount, new ForkScanner().allNodes(heads).size()); test.assertMatchingParallelStartEnd(); test.assertMatchingParallelBranchStartEnd(); @@ -263,34 +274,34 @@ private void sanityTestIterationAndVisiter(List heads) { } @Test - public void testForkedScanner() throws Exception { + void testForkedScanner() throws Exception { FlowExecution exec = SIMPLE_PARALLEL_RUN.getExecution(); List heads = SIMPLE_PARALLEL_RUN.getExecution().getCurrentHeads(); // Initial case ForkScanner scanner = new ForkScanner(); scanner.setup(heads, null); - Assert.assertNull(scanner.currentParallelStart); - Assert.assertNull(scanner.currentParallelStartNode); - Assert.assertNotNull(scanner.parallelBlockStartStack); - Assert.assertEquals(0, scanner.parallelBlockStartStack.size()); - Assert.assertTrue(scanner.isWalkingFromFinish()); + assertNull(scanner.currentParallelStart); + assertNull(scanner.currentParallelStartNode); + assertNotNull(scanner.parallelBlockStartStack); + assertEquals(0, scanner.parallelBlockStartStack.size()); + assertTrue(scanner.isWalkingFromFinish()); sanityTestIterationAndVisiter(heads); // Fork case scanner.setup(exec.getNode("13")); - Assert.assertFalse(scanner.isWalkingFromFinish()); - Assert.assertNull(scanner.currentType); - Assert.assertEquals(ForkScanner.NodeType.PARALLEL_END, scanner.nextType); - Assert.assertEquals("13", scanner.next().getId()); - Assert.assertNotNull(scanner.parallelBlockStartStack); - Assert.assertEquals(0, scanner.parallelBlockStartStack.size()); - Assert.assertEquals(exec.getNode("4"), scanner.currentParallelStartNode); + assertFalse(scanner.isWalkingFromFinish()); + assertNull(scanner.currentType); + assertEquals(ForkScanner.NodeType.PARALLEL_END, scanner.nextType); + assertEquals("13", scanner.next().getId()); + assertNotNull(scanner.parallelBlockStartStack); + assertEquals(0, scanner.parallelBlockStartStack.size()); + assertEquals(exec.getNode("4"), scanner.currentParallelStartNode); sanityTestIterationAndVisiter(Collections.singletonList(exec.getNode("13"))); ForkScanner.ParallelBlockStart start = scanner.currentParallelStart; - Assert.assertEquals(1, start.unvisited.size()); - Assert.assertEquals(exec.getNode("4"), start.forkStart); + assertEquals(1, start.unvisited.size()); + assertEquals(exec.getNode("4"), start.forkStart); /* Flow structure (ID - type) 2 - FlowStartNode (BlockStartNode) @@ -308,31 +319,31 @@ public void testForkedScanner() throws Exception { 15 - FlowEndNode (BlockEndNode) */ - Assert.assertEquals(exec.getNode("12"), scanner.next()); //12 - Assert.assertEquals(ForkScanner.NodeType.PARALLEL_BRANCH_END, scanner.getCurrentType()); - Assert.assertEquals(ForkScanner.NodeType.NORMAL, scanner.getNextType()); - Assert.assertEquals(exec.getNode("11"), scanner.next()); - Assert.assertEquals(ForkScanner.NodeType.NORMAL, scanner.getCurrentType()); - Assert.assertEquals(exec.getNode("10"), scanner.next()); - Assert.assertEquals(ForkScanner.NodeType.NORMAL, scanner.getCurrentType()); - Assert.assertEquals(ForkScanner.NodeType.PARALLEL_BRANCH_START, scanner.getNextType()); - Assert.assertEquals(exec.getNode("7"), scanner.next()); - Assert.assertEquals(ForkScanner.NodeType.PARALLEL_BRANCH_START, scanner.getCurrentType()); + assertEquals(exec.getNode("12"), scanner.next()); //12 + assertEquals(ForkScanner.NodeType.PARALLEL_BRANCH_END, scanner.getCurrentType()); + assertEquals(ForkScanner.NodeType.NORMAL, scanner.getNextType()); + assertEquals(exec.getNode("11"), scanner.next()); + assertEquals(ForkScanner.NodeType.NORMAL, scanner.getCurrentType()); + assertEquals(exec.getNode("10"), scanner.next()); + assertEquals(ForkScanner.NodeType.NORMAL, scanner.getCurrentType()); + assertEquals(ForkScanner.NodeType.PARALLEL_BRANCH_START, scanner.getNextType()); + assertEquals(exec.getNode("7"), scanner.next()); + assertEquals(ForkScanner.NodeType.PARALLEL_BRANCH_START, scanner.getCurrentType()); // Next branch, branch 1 (since we visit in reverse) - Assert.assertEquals(ForkScanner.NodeType.PARALLEL_BRANCH_END, scanner.getNextType()); - Assert.assertEquals(exec.getNode("9"), scanner.next()); - Assert.assertEquals(ForkScanner.NodeType.PARALLEL_BRANCH_END, scanner.getCurrentType()); - Assert.assertEquals(exec.getNode("8"), scanner.next()); - Assert.assertEquals(ForkScanner.NodeType.NORMAL, scanner.getCurrentType()); - Assert.assertEquals(exec.getNode("6"), scanner.next()); - Assert.assertEquals(ForkScanner.NodeType.PARALLEL_BRANCH_START, scanner.getCurrentType()); - Assert.assertEquals(ForkScanner.NodeType.PARALLEL_START, scanner.getNextType()); + assertEquals(ForkScanner.NodeType.PARALLEL_BRANCH_END, scanner.getNextType()); + assertEquals(exec.getNode("9"), scanner.next()); + assertEquals(ForkScanner.NodeType.PARALLEL_BRANCH_END, scanner.getCurrentType()); + assertEquals(exec.getNode("8"), scanner.next()); + assertEquals(ForkScanner.NodeType.NORMAL, scanner.getCurrentType()); + assertEquals(exec.getNode("6"), scanner.next()); + assertEquals(ForkScanner.NodeType.PARALLEL_BRANCH_START, scanner.getCurrentType()); + assertEquals(ForkScanner.NodeType.PARALLEL_START, scanner.getNextType()); } /** Reference the flow graphs in {@link #SIMPLE_PARALLEL_RUN} and {@link #NESTED_PARALLEL_RUN} */ @Test - public void testFlowSegmentSplit() throws Exception { + void testFlowSegmentSplit() throws Exception { FlowExecution exec = SIMPLE_PARALLEL_RUN.getExecution(); /* Flow structure (ID - type) @@ -381,20 +392,20 @@ public void testFlowSegmentSplit() throws Exception { ForkScanner.Fork forked = mainBranch.split(nodeMap, (BlockStartNode)exec.getNode("4"), sideBranch); ForkScanner.FlowSegment splitSegment = (ForkScanner.FlowSegment)nodeMap.get(BRANCH1_END); // New branch - Assert.assertNull(splitSegment.after); + assertNull(splitSegment.after); FlowTestUtils.assertNodeOrder("Branch 1 split after fork", splitSegment.visited, 9, 8, 6); // Just the single node before the fork - Assert.assertEquals(forked, mainBranch.after); + assertEquals(forked, mainBranch.after); FlowTestUtils.assertNodeOrder("Head of flow, pre-fork", mainBranch.visited, 3); // Fork point - Assert.assertEquals(forked, nodeMap.get(START_PARALLEL)); + assertEquals(forked, nodeMap.get(START_PARALLEL)); ForkScanner.FlowPiece[] follows = {splitSegment, sideBranch}; - Assert.assertArrayEquals(follows, forked.following.toArray()); + assertArrayEquals(follows, forked.following.toArray()); // Branch 2 - Assert.assertEquals(sideBranch, nodeMap.get(BRANCH2_END)); + assertEquals(sideBranch, nodeMap.get(BRANCH2_END)); FlowTestUtils.assertNodeOrder("Branch 2", sideBranch.visited, 12, 11, 10, 7); // Test me where splitting right at a fork point, where we should have a fork with and main branch shoudl become following @@ -414,18 +425,18 @@ public void testFlowSegmentSplit() throws Exception { follows = new ForkScanner.FlowSegment[2]; follows[0] = mainBranch; follows[1] = sideBranch; - Assert.assertArrayEquals(follows, forked.following.toArray()); + assertArrayEquals(follows, forked.following.toArray()); FlowTestUtils.assertNodeOrder("Branch1", mainBranch.visited, 6); - Assert.assertNull(mainBranch.after); + assertNull(mainBranch.after); FlowTestUtils.assertNodeOrder("Branch2", sideBranch.visited, 7); - Assert.assertNull(sideBranch.after); - Assert.assertEquals(forked, nodeMap.get(START_PARALLEL)); - Assert.assertEquals(mainBranch, nodeMap.get(exec.getNode("6"))); - Assert.assertEquals(sideBranch, nodeMap.get(exec.getNode("7"))); + assertNull(sideBranch.after); + assertEquals(forked, nodeMap.get(START_PARALLEL)); + assertEquals(mainBranch, nodeMap.get(exec.getNode("6"))); + assertEquals(sideBranch, nodeMap.get(exec.getNode("7"))); } @Test - public void testEmptyParallel() throws Exception { + void testEmptyParallel() throws Exception { WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "EmptyParallel"); job.setDefinition(new CpsFlowDefinition( "parallel 'empty1': {}, 'empty2':{} \n" + @@ -435,7 +446,7 @@ public void testEmptyParallel() throws Exception { ForkScanner scan = new ForkScanner(); List outputs = scan.filteredNodes(b.getExecution().getCurrentHeads(), x -> true); - Assert.assertEquals(9, outputs.size()); + assertEquals(9, outputs.size()); } private final Function NODE_TO_ID = input -> input != null ? input.getId() : null; @@ -460,9 +471,9 @@ private void assertNoMissingParallelEvents(List heads) { visit.filteredCallsByType(TestVisitor.CallType.PARALLEL_BRANCH_START).stream() .map(CALL_TO_NODE_ID) .collect(Collectors.toSet()); - for (String id : matches.stream().map(NODE_TO_ID).collect(Collectors.toList())) { + for (String id : matches.stream().map(NODE_TO_ID).toList()) { if (!callIds.contains(id)) { - Assert.fail("Parallel Branch start node without an appropriate parallelBranchStart callback: "+id); + fail("Parallel Branch start node without an appropriate parallelBranchStart callback: "+id); } } @@ -484,9 +495,9 @@ private void assertNoMissingParallelEvents(List heads) { visit.filteredCallsByType(TestVisitor.CallType.PARALLEL_START).stream() .map(CALL_TO_NODE_ID) .collect(Collectors.toSet()); - for (String id : matches.stream().map(NODE_TO_ID).collect(Collectors.toList())) { + for (String id : matches.stream().map(NODE_TO_ID).toList()) { if (!callIds.contains(id)) { - Assert.fail("Parallel start node without an appropriate parallelStart callback: "+id); + fail("Parallel start node without an appropriate parallelStart callback: "+id); } } @@ -495,18 +506,19 @@ private void assertNoMissingParallelEvents(List heads) { visit.filteredCallsByType(TestVisitor.CallType.PARALLEL_END).stream() .map(CALL_TO_NODE_ID) .collect(Collectors.toSet()); - for (String id : parallelEnds.stream().map(NODE_TO_ID).collect(Collectors.toList())) { + for (String id : parallelEnds.stream().map(NODE_TO_ID).toList()) { if (!callIds.contains(id)) { - Assert.fail("Parallel END node without an appropriate parallelEnd callback: "+id); + fail("Parallel END node without an appropriate parallelEnd callback: "+id); } } // Parallel Ends should be handled by the checks that blocks are balanced. } + // Implicitly covers JENKINS-39841 too though @Test - @Issue("JENKINS-39839") // Implicitly covers JENKINS-39841 too though - public void testSingleNestedParallelBranches() throws Exception { + @Issue("JENKINS-39839") + void testSingleNestedParallelBranches() throws Exception { String script = "node {\n" + " echo ('Testing')\n" + " parallel nestedBranch: {\n" + @@ -523,7 +535,7 @@ public void testSingleNestedParallelBranches() throws Exception { job.setDefinition(new CpsFlowDefinition(script, true)); WorkflowRun b = r.assertBuildStatusSuccess(job.scheduleBuild2(0)); FlowNode echoNode = new DepthFirstScanner().findFirstMatch(b.getExecution(), new NodeStepTypePredicate(EchoStep.DescriptorImpl.byFunctionName("echo"))); - Assert.assertNotNull(echoNode); + assertNotNull(echoNode); sanityTestIterationAndVisiter(b.getExecution().getCurrentHeads()); sanityTestIterationAndVisiter(Collections.singletonList(echoNode)); @@ -531,43 +543,43 @@ public void testSingleNestedParallelBranches() throws Exception { ForkScanner scanner = new ForkScanner(); scanner.setup(b.getExecution().getCurrentHeads()); scanner.visitSimpleChunks(visitor, new NoOpChunkFinder()); - Assert.assertEquals(2, visitor.filteredCallsByType(TestVisitor.CallType.PARALLEL_START).size()); - Assert.assertEquals(2, visitor.filteredCallsByType(TestVisitor.CallType.PARALLEL_END).size()); - Assert.assertEquals(2, visitor.filteredCallsByType(TestVisitor.CallType.PARALLEL_BRANCH_START).size()); - Assert.assertEquals(2, visitor.filteredCallsByType(TestVisitor.CallType.PARALLEL_BRANCH_END).size()); + assertEquals(2, visitor.filteredCallsByType(TestVisitor.CallType.PARALLEL_START).size()); + assertEquals(2, visitor.filteredCallsByType(TestVisitor.CallType.PARALLEL_END).size()); + assertEquals(2, visitor.filteredCallsByType(TestVisitor.CallType.PARALLEL_BRANCH_START).size()); + assertEquals(2, visitor.filteredCallsByType(TestVisitor.CallType.PARALLEL_BRANCH_END).size()); } /** Reference the flow graphs in {@link #SIMPLE_PARALLEL_RUN} and {@link #NESTED_PARALLEL_RUN} */ @Test - public void testLeastCommonAncestor() throws Exception { + void testLeastCommonAncestor() throws Exception { FlowExecution exec = SIMPLE_PARALLEL_RUN.getExecution(); ForkScanner scan = new ForkScanner(); // Starts at the ends of the parallel branches Set heads = new LinkedHashSet<>(Arrays.asList(exec.getNode("12"), exec.getNode("9"))); ArrayDeque starts = scan.leastCommonAncestor(heads); - Assert.assertEquals(1, starts.size()); + assertEquals(1, starts.size()); ForkScanner.ParallelBlockStart start = starts.peek(); - Assert.assertEquals(2, start.unvisited.size()); - Assert.assertEquals(exec.getNode("4"), start.forkStart); - Assert.assertArrayEquals(heads.toArray(), start.unvisited.toArray()); + assertEquals(2, start.unvisited.size()); + assertEquals(exec.getNode("4"), start.forkStart); + assertArrayEquals(heads.toArray(), start.unvisited.toArray()); // Ensure no issues with single start triggering least common ancestor heads = new LinkedHashSet<>(Collections.singletonList(exec.getNode("4"))); scan.setup(heads); - Assert.assertNull(scan.currentParallelStart); - Assert.assertTrue(scan.parallelBlockStartStack == null || scan.parallelBlockStartStack.isEmpty()); + assertNull(scan.currentParallelStart); + assertTrue(scan.parallelBlockStartStack == null || scan.parallelBlockStartStack.isEmpty()); // Empty fork heads = new LinkedHashSet<>(Arrays.asList(exec.getNode("6"), exec.getNode("7"))); starts = scan.leastCommonAncestor(heads); - Assert.assertEquals(1, starts.size()); + assertEquals(1, starts.size()); ForkScanner.ParallelBlockStart pbs = starts.pop(); - Assert.assertEquals(exec.getNode("4"), pbs.forkStart); - Assert.assertEquals(2, pbs.unvisited.size()); - Assert.assertTrue(pbs.unvisited.contains(exec.getNode("6"))); - Assert.assertTrue(pbs.unvisited.contains(exec.getNode("7"))); + assertEquals(exec.getNode("4"), pbs.forkStart); + assertEquals(2, pbs.unvisited.size()); + assertTrue(pbs.unvisited.contains(exec.getNode("6"))); + assertTrue(pbs.unvisited.contains(exec.getNode("7"))); sanityTestIterationAndVisiter(new ArrayList<>(heads)); /* Now we do the same with nested run */ @@ -576,43 +588,44 @@ public void testLeastCommonAncestor() throws Exception { // Problem: we get a parallel start with the same flowsegment in the following for more than one parallel start starts = scan.leastCommonAncestor(heads); - Assert.assertEquals(2, starts.size()); + assertEquals(2, starts.size()); ForkScanner.ParallelBlockStart inner = starts.getFirst(); ForkScanner.ParallelBlockStart outer = starts.getLast(); - Assert.assertEquals(2, inner.unvisited.size()); - Assert.assertEquals(exec.getNode("12"), inner.forkStart); + assertEquals(2, inner.unvisited.size()); + assertEquals(exec.getNode("12"), inner.forkStart); - Assert.assertEquals(1, outer.unvisited.size()); - Assert.assertEquals(exec.getNode("9"), outer.unvisited.peek()); - Assert.assertEquals(exec.getNode("4"), outer.forkStart); + assertEquals(1, outer.unvisited.size()); + assertEquals(exec.getNode("9"), outer.unvisited.peek()); + assertEquals(exec.getNode("4"), outer.forkStart); sanityTestIterationAndVisiter(new ArrayList<>(heads)); heads = new LinkedHashSet<>(Arrays.asList(exec.getNode("9"), exec.getNode("17"), exec.getNode("20"))); starts = scan.leastCommonAncestor(heads); - Assert.assertEquals(2, starts.size()); + assertEquals(2, starts.size()); sanityTestIterationAndVisiter(new ArrayList<>(heads)); } @Test @Issue("JENKINS-38089") - public void testVariousParallelCombos() throws Exception { + void testVariousParallelCombos() throws Exception { WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "ParallelTimingBug"); job.setDefinition(new CpsFlowDefinition( // Seemingly gratuitous sleep steps are because original issue required specific timing to reproduce - "echo 'test stage' \n" + - " parallel 'unit': {\n" + - " retry(1) {\n" + - " sleep 1;\n" + - " sleep 10; echo 'hello'; \n" + - " }\n" + - " }, 'otherunit': {\n" + - " retry(1) {\n" + - " sleep 1;\n" + - " sleep 5; \n" + - " echo 'goodbye' \n" + - " }\n" + - " }", + """ + echo 'test stage' + parallel 'unit': { + retry(1) { + sleep 1; + sleep 10; echo 'hello'; + } + }, 'otherunit': { + retry(1) { + sleep 1; + sleep 5; + echo 'goodbye' + } + }""", true)); /*Node dump follows, format: [ID]{parent,ids}(millisSinceStartOfRun) flowNodeClassName stepDisplayName [st=startId if a block end node] @@ -655,7 +668,7 @@ public void testVariousParallelCombos() throws Exception { ArrayList starts = new ArrayList<>(); FlowTestUtils.addNodesById(starts, exec, branchANodeId, branchBNodeId); List all = scan.filteredNodes(starts, x -> true); - Assert.assertEquals(new HashSet<>(all).size(), all.size()); + assertEquals(new HashSet<>(all).size(), all.size()); scan.reset(); } } @@ -663,58 +676,59 @@ public void testVariousParallelCombos() throws Exception { @Test @Issue("JENKINS-42895") - public void testMissingHeadErrorWithZeroBranchParallel() throws Exception { + void testMissingHeadErrorWithZeroBranchParallel() throws Exception { WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "MissingHeadBug"); - job.setDefinition(new CpsFlowDefinition("" + - "stage('Stage A') {\n" + - " echo \"A\"\n" + - "}\n" + - "// Works\n" + - "stage('Stage B') {\n" + - " parallel a: {\n" + - " echo \"B.A\"\n" + - " }, b: {\n" + - " echo \"B.B\"\n" + - " }\n" + - "}\n" + - "// Breaks\n" + - "stage('Stage C') {\n" + - " def steps = [:]\n" + - " // Empty map\n" + - " parallel steps\n" + - "}\n", true)); + job.setDefinition(new CpsFlowDefinition(""" + stage('Stage A') { + echo "A" + } + // Works + stage('Stage B') { + parallel a: { + echo "B.A" + }, b: { + echo "B.B" + } + } + // Breaks + stage('Stage C') { + def steps = [:] + // Empty map + parallel steps + } + """, true)); WorkflowRun run = r.buildAndAssertSuccess(job); FlowExecution exec = run.getExecution(); sanityTestIterationAndVisiter(exec.getCurrentHeads()); } @Test - public void testParallelPredicate() throws Exception { + void testParallelPredicate() throws Exception { FlowExecution exec = SIMPLE_PARALLEL_RUN.getExecution(); - Assert.assertTrue(new ForkScanner.IsParallelStartPredicate().apply(exec.getNode("4"))); - Assert.assertFalse(new ForkScanner.IsParallelStartPredicate().apply(exec.getNode("6"))); - Assert.assertFalse(new ForkScanner.IsParallelStartPredicate().apply(exec.getNode("8"))); + assertTrue(new ForkScanner.IsParallelStartPredicate().apply(exec.getNode("4"))); + assertFalse(new ForkScanner.IsParallelStartPredicate().apply(exec.getNode("6"))); + assertFalse(new ForkScanner.IsParallelStartPredicate().apply(exec.getNode("8"))); } @Test - public void testGetNodeType() throws Exception { + void testGetNodeType() throws Exception { FlowExecution exec = SIMPLE_PARALLEL_RUN.getExecution(); - Assert.assertEquals(ForkScanner.NodeType.NORMAL, ForkScanner.getNodeType(exec.getNode("2"))); - Assert.assertEquals(ForkScanner.NodeType.PARALLEL_START, ForkScanner.getNodeType(exec.getNode("4"))); - Assert.assertEquals(ForkScanner.NodeType.PARALLEL_BRANCH_START, ForkScanner.getNodeType(exec.getNode("6"))); - Assert.assertEquals(ForkScanner.NodeType.PARALLEL_BRANCH_END, ForkScanner.getNodeType(exec.getNode("9"))); - Assert.assertEquals(ForkScanner.NodeType.PARALLEL_END, ForkScanner.getNodeType(exec.getNode("13"))); + assertEquals(ForkScanner.NodeType.NORMAL, ForkScanner.getNodeType(exec.getNode("2"))); + assertEquals(ForkScanner.NodeType.PARALLEL_START, ForkScanner.getNodeType(exec.getNode("4"))); + assertEquals(ForkScanner.NodeType.PARALLEL_BRANCH_START, ForkScanner.getNodeType(exec.getNode("6"))); + assertEquals(ForkScanner.NodeType.PARALLEL_BRANCH_END, ForkScanner.getNodeType(exec.getNode("9"))); + assertEquals(ForkScanner.NodeType.PARALLEL_END, ForkScanner.getNodeType(exec.getNode("13"))); - Assert.assertEquals(ForkScanner.NodeType.NORMAL, ForkScanner.getNodeType(exec.getNode("8"))); + assertEquals(ForkScanner.NodeType.NORMAL, ForkScanner.getNodeType(exec.getNode("8"))); } /** For nodes, see {@link #SIMPLE_PARALLEL_RUN} */ @Test - public void testSimpleVisitor() throws Exception { + void testSimpleVisitor() throws Exception { FlowExecution exec = this.SIMPLE_PARALLEL_RUN.getExecution(); ForkScanner f = new ForkScanner(); f.setup(exec.getCurrentHeads()); - Assert.assertArrayEquals(new HashSet<>(exec.getCurrentHeads()).toArray(), new HashSet<>(f.currentParallelHeads()).toArray()); + assertArrayEquals(new HashSet<>(exec.getCurrentHeads()).toArray(), new HashSet<>(f.currentParallelHeads()).toArray()); List expectedHeads = f.currentParallelHeads(); sanityTestIterationAndVisiter(exec.getCurrentHeads()); @@ -723,7 +737,7 @@ public void testSimpleVisitor() throws Exception { f.visitSimpleChunks(visitor, new BlockChunkFinder()); // 13 calls for chunk/atoms, 6 for parallels - Assert.assertEquals(19, visitor.calls.size()); + assertEquals(19, visitor.calls.size()); // End has nothing after it, just last node (15) TestVisitor.CallEntry last = new TestVisitor.CallEntry(TestVisitor.CallType.CHUNK_END, 15, -1, -1, -1); @@ -741,26 +755,26 @@ public void testSimpleVisitor() throws Exception { visitor.calls.stream() .filter(predicateForCallEntryType(TestVisitor.CallType.CHUNK_END)) .count(); - Assert.assertEquals(4L, chunkStartCount); - Assert.assertEquals(4L, chunkEndCount); + assertEquals(4L, chunkStartCount); + assertEquals(4L, chunkEndCount); // Verify the AtomNode calls are correct List atomNodeCalls = visitor.calls.stream() .filter(predicateForCallEntryType(TestVisitor.CallType.ATOM_NODE)) - .collect(Collectors.toList()); - Assert.assertEquals(5, atomNodeCalls.size()); + .toList(); + assertEquals(5, atomNodeCalls.size()); for (TestVisitor.CallEntry ce : atomNodeCalls) { int beforeId = ce.ids[0]; int atomNodeId = ce.ids[1]; int afterId = ce.ids[2]; int alwaysEmpty = ce.ids[3]; - Assert.assertTrue(ce+" beforeNodeId <= 0: "+beforeId, beforeId > 0); - Assert.assertTrue(ce + " atomNodeId <= 0: " + atomNodeId, atomNodeId > 0); - Assert.assertTrue(ce+" afterNodeId <= 0: "+afterId, afterId > 0); - Assert.assertEquals(-1, alwaysEmpty); - Assert.assertTrue(ce + "AtomNodeId >= afterNodeId", atomNodeId < afterId); - Assert.assertTrue(ce+ "beforeNodeId >= atomNodeId", beforeId < atomNodeId); + assertTrue(beforeId > 0, ce+" beforeNodeId <= 0: "+beforeId); + assertTrue(atomNodeId > 0, ce + " atomNodeId <= 0: " + atomNodeId); + assertTrue(afterId > 0, ce+" afterNodeId <= 0: "+afterId); + assertEquals(-1, alwaysEmpty); + assertTrue(atomNodeId < afterId, ce + "AtomNodeId >= afterNodeId"); + assertTrue(beforeId < atomNodeId, ce+ "beforeNodeId >= atomNodeId"); } List parallelCalls = visitor.calls.stream() @@ -769,8 +783,8 @@ public void testSimpleVisitor() throws Exception { && input.type != TestVisitor.CallType.ATOM_NODE && input.type != TestVisitor.CallType.CHUNK_START && input.type != TestVisitor.CallType.CHUNK_END) - .collect(Collectors.toList()); - Assert.assertEquals(6, parallelCalls.size()); + .toList(); + assertEquals(6, parallelCalls.size()); // Start to end new TestVisitor.CallEntry(TestVisitor.CallType.PARALLEL_END, 4, 13).assertEquals(parallelCalls.get(0)); @@ -787,7 +801,7 @@ public void testSimpleVisitor() throws Exception { /** Checks for off-by one cases with multiple parallel, and with the leastCommonAncestor */ @Test - public void testTripleParallel() throws Exception { + void testTripleParallel() throws Exception { WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "TripleParallel"); job.setDefinition(new CpsFlowDefinition( "echo 'test stage'\n"+ // Id 3, Id 2 before that has the FlowStartNode @@ -814,7 +828,7 @@ public void testTripleParallel() throws Exception { .or( predicateForCallEntryType(TestVisitor.CallType.PARALLEL_BRANCH_END))) .collect(Collectors.toList()); - Assert.assertEquals(6, parallels.size()); + assertEquals(6, parallels.size()); // Visiting from partially completed branches // Verify we still get appropriate parallels callbacks for a branch end @@ -823,7 +837,7 @@ public void testTripleParallel() throws Exception { ends.add(exec.getNode("11")); ends.add(exec.getNode("12")); ends.add(exec.getNode("14")); - Assert.assertEquals(new DepthFirstScanner().allNodes(ends).size(), + assertEquals(new DepthFirstScanner().allNodes(ends).size(), new ForkScanner().allNodes(ends).size()); visitor = new TestVisitor(); f.setup(ends); @@ -836,20 +850,20 @@ public void testTripleParallel() throws Exception { predicateForCallEntryType(TestVisitor.CallType.PARALLEL_BRANCH_START) .or( predicateForCallEntryType(TestVisitor.CallType.PARALLEL_BRANCH_END))) - .collect(Collectors.toList()); - Assert.assertEquals(6, parallels.size()); - Assert.assertEquals(18, visitor.calls.size()); + .toList(); + assertEquals(6, parallels.size()); + assertEquals(18, visitor.calls.size()); // Test the least common ancestor implementation with triplicate FlowNode[] branchHeads = {exec.getNode("7"), exec.getNode("8"), exec.getNode("9")}; ArrayDeque starts = f.leastCommonAncestor(new HashSet<>(Arrays.asList(branchHeads))); - Assert.assertEquals(1, starts.size()); + assertEquals(1, starts.size()); ForkScanner.ParallelBlockStart pbs = starts.pop(); - Assert.assertEquals(exec.getNode("4"), pbs.forkStart); - Assert.assertEquals(3, pbs.unvisited.size()); - Assert.assertTrue(pbs.unvisited.contains(exec.getNode("7"))); - Assert.assertTrue(pbs.unvisited.contains(exec.getNode("8"))); - Assert.assertTrue(pbs.unvisited.contains(exec.getNode("9"))); + assertEquals(exec.getNode("4"), pbs.forkStart); + assertEquals(3, pbs.unvisited.size()); + assertTrue(pbs.unvisited.contains(exec.getNode("7"))); + assertTrue(pbs.unvisited.contains(exec.getNode("8"))); + assertTrue(pbs.unvisited.contains(exec.getNode("9"))); } private void testParallelFindsLast(WorkflowJob job, String semaphoreName) throws Exception { @@ -867,14 +881,14 @@ private void testParallelFindsLast(WorkflowJob job, String semaphoreName) throws scan.setup(heads); // Check the right number of branches are set up - Assert.assertEquals(run.getExecution().getCurrentHeads().size()-1, scan.currentParallelStart.unvisited.size()); + assertEquals(run.getExecution().getCurrentHeads().size()-1, scan.currentParallelStart.unvisited.size()); // Check visitor handling for parallel end scan.visitSimpleChunks(visitor, labelFinder); TestVisitor.CallEntry parallelEnd = visitor.calls.get(0); - Assert.assertEquals(TestVisitor.CallType.PARALLEL_END, parallelEnd.type); - Assert.assertEquals("Wrong End Node: ("+parallelEnd.getNodeId()+")", semaphoreNode.getId(), parallelEnd.getNodeId().toString()); - Assert.assertEquals(semaphoreNode.getId(), parallelEnd.getNodeId().toString()); + assertEquals(TestVisitor.CallType.PARALLEL_END, parallelEnd.type); + assertEquals(semaphoreNode.getId(), parallelEnd.getNodeId().toString(), "Wrong End Node: ("+parallelEnd.getNodeId()+")"); + assertEquals(semaphoreNode.getId(), parallelEnd.getNodeId().toString()); SemaphoreStep.success(semaphoreName+"/1", null); r.waitForCompletion(run); @@ -884,21 +898,22 @@ private void testParallelFindsLast(WorkflowJob job, String semaphoreName) throws /** Reproduce issues with in-progress parallels */ @Test @Issue("JENKINS-41685") - public void testParallelsWithDuplicateEvents() throws Exception { + void testParallelsWithDuplicateEvents() throws Exception { //https://gist.github.com/vivek/ccf3a4ef25fbff267c76c962d265041d WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "ParallelInsanity"); - job.setDefinition(new CpsFlowDefinition("" + - "echo 'first stage'\n" + - "parallel left : {\n" + - " echo 'run a bit'\n" + - " echo 'run a bit more'\n" + - " semaphore 'wait1'\n" + - "}, right : {\n" + - " echo 'wozzle'\n" + - " semaphore 'wait2'\n" + - "}\n" + - "echo 'last stage'\n" + - "echo \"last done\"\n", + job.setDefinition(new CpsFlowDefinition(""" + echo 'first stage' + parallel left : { + echo 'run a bit' + echo 'run a bit more' + semaphore 'wait1' + }, right : { + echo 'wozzle' + semaphore 'wait2' + } + echo 'last stage' + echo "last done" + """, true)); ForkScanner scan = new ForkScanner(); ChunkFinder labelFinder = new NoOpChunkFinder(); @@ -935,9 +950,9 @@ public void testParallelsWithDuplicateEvents() throws Exception { } sanityTestIterationAndVisiter(heads); - Assert.assertEquals(10, atomEventCount); - Assert.assertEquals(1, parallelStartCount); - Assert.assertEquals(2, parallelBranchEndCount); + assertEquals(10, atomEventCount); + assertEquals(1, parallelStartCount); + assertEquals(2, parallelBranchEndCount); } /** Covers finding the right parallel end node in cases where we have one long-running step on an incomplete branch. @@ -946,7 +961,7 @@ public void testParallelsWithDuplicateEvents() throws Exception { */ @Issue("JENKINS-38536") @Test - public void testPartlyCompletedParallels() throws Exception { + void testPartlyCompletedParallels() throws Exception { String jobScript = ""+ "echo 'first stage'\n" + "parallel 'long' : { sleep 60; }, \n" + // Needs to be in-progress @@ -964,7 +979,7 @@ public void testPartlyCompletedParallels() throws Exception { scan.setup(heads); scan.visitSimpleChunks(tv, new NoOpChunkFinder()); FlowNode endNode = exec.getNode(tv.filteredCallsByType(TestVisitor.CallType.PARALLEL_END).get(0).getNodeId().toString()); - Assert.assertEquals("sleep", endNode.getDisplayFunctionName()); + assertEquals("sleep", endNode.getDisplayFunctionName()); sanityTestIterationAndVisiter(heads); run.doKill(); // Avoid waiting for long branch completion } @@ -972,28 +987,29 @@ public void testPartlyCompletedParallels() throws Exception { /** Covers finding the right parallel end node in cases we have not written a TimingAction or are using SemaphoreStep */ @Issue("JENKINS-38536") @Test - public void testParallelCorrectEndNodeForVisitor() throws Exception { + void testParallelCorrectEndNodeForVisitor() throws Exception { // Verify that SimpleBlockVisitor actually gets the *real* last node not just the last declared branch WorkflowJob jobPauseFirst = r.jenkins.createProject(WorkflowJob.class, "PauseFirst"); - jobPauseFirst.setDefinition(new CpsFlowDefinition("" + - "echo 'primero stage'\n" + - "parallel 'wait' : {sleep 1; semaphore 'wait1';}, \n" + - " 'final': { echo 'succeed';} ", + jobPauseFirst.setDefinition(new CpsFlowDefinition(""" + echo 'primero stage' + parallel 'wait' : {sleep 1; semaphore 'wait1';}, + 'final': { echo 'succeed';}""", true)); WorkflowJob jobPauseSecond = r.jenkins.createProject(WorkflowJob.class, "PauseSecond"); - jobPauseSecond.setDefinition(new CpsFlowDefinition("" + - "echo 'primero stage'\n" + - "parallel 'success' : {echo 'succeed'}, \n" + - " 'pause':{ sleep 1; semaphore 'wait2'; }\n", + jobPauseSecond.setDefinition(new CpsFlowDefinition(""" + echo 'primero stage' + parallel 'success' : {echo 'succeed'}, + 'pause':{ sleep 1; semaphore 'wait2'; } + """, true)); WorkflowJob jobPauseMiddle = r.jenkins.createProject(WorkflowJob.class, "PauseMiddle"); - jobPauseMiddle.setDefinition(new CpsFlowDefinition("" + - "echo 'primero stage'\n" + - "parallel 'success' : {echo 'succeed'}, \n" + - " 'pause':{ sleep 1; semaphore 'wait3'; }, \n" + - " 'final': { echo 'succeed-final';} ", + jobPauseMiddle.setDefinition(new CpsFlowDefinition(""" + echo 'primero stage' + parallel 'success' : {echo 'succeed'}, + 'pause':{ sleep 1; semaphore 'wait3'; }, + 'final': { echo 'succeed-final';}""", true)); testParallelFindsLast(jobPauseFirst, "wait1"); testParallelFindsLast(jobPauseSecond, "wait2"); @@ -1001,7 +1017,7 @@ public void testParallelCorrectEndNodeForVisitor() throws Exception { } @Test - public void inProgressParallelInParallel() throws Exception { + void inProgressParallelInParallel() throws Exception { WorkflowJob p = r.createProject(WorkflowJob.class); p.setDefinition(new CpsFlowDefinition( "stage('MyStage') {\n" + // 3, 4 @@ -1038,9 +1054,9 @@ public void inProgressParallelInParallel() throws Exception { assertBranchOrder(b.getExecution(), "innerB", "innerA", "outerC", "outerB", "outerA"); } - @Ignore("outerA gets skipped when iterating, see warning in ForkScanner Javadoc") + @Disabled("outerA gets skipped when iterating, see warning in ForkScanner Javadoc") @Test - public void inProgressParallelInParallelOneNestedBranch() throws Exception { + void inProgressParallelInParallelOneNestedBranch() throws Exception { WorkflowJob p = r.createProject(WorkflowJob.class); p.setDefinition(new CpsFlowDefinition( "parallel(\n" + // 3 @@ -1066,18 +1082,19 @@ public void inProgressParallelInParallelOneNestedBranch() throws Exception { assertBranchOrder(b.getExecution(), "outerB", "innerA", "outerA"); } - @Ignore("Actual ordering is all complete branches and then all incomplete branches, see warning in ForkScanner Javadoc") + @Disabled("Actual ordering is all complete branches and then all incomplete branches, see warning in ForkScanner Javadoc") @Test - public void parallelBranchOrdering() throws Exception { + void parallelBranchOrdering() throws Exception { WorkflowJob p = r.createProject(WorkflowJob.class); p.setDefinition(new CpsFlowDefinition( - "parallel(\n" + - " '1': { echo '1' },\n" + - " '2': { semaphore('2') },\n" + - " '3': { echo '3' },\n" + - " '4': { semaphore('4') },\n" + - " '5': { echo '5' },\n" + - ")", true)); + """ + parallel( + '1': { echo '1' }, + '2': { semaphore('2') }, + '3': { echo '3' }, + '4': { semaphore('4') }, + '5': { echo '5' }, + )""", true)); WorkflowRun b = p.scheduleBuild2(0).waitForStart(); SemaphoreStep.waitForStart("2/1", b); SemaphoreStep.waitForStart("4/1", b); @@ -1097,7 +1114,7 @@ private static void assertBranchOrder(FlowExecution execution, String... expecte .map(n -> n.getPersistentAction(ThreadNameAction.class)) .filter(Objects::nonNull) .map(ThreadNameAction::getThreadName) - .collect(Collectors.toList()) + .toList() .toArray(String[]::new); assertThat(branches, equalTo(expectedBranchNames)); } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/MemoryFlowChunkTest.java b/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/MemoryFlowChunkTest.java index 794e12d3..41a8be1e 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/MemoryFlowChunkTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/MemoryFlowChunkTest.java @@ -25,15 +25,15 @@ package org.jenkinsci.plugins.workflow.graphanalysis; import org.jenkinsci.plugins.workflow.graph.FlowNode; -import org.junit.Test; +import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; -public class MemoryFlowChunkTest { +class MemoryFlowChunkTest { @Test - public void constructor() { + void constructor() { MockFlowNode start = new MockFlowNode("1"); MockFlowNode blockStart = new MockFlowNode("2", start); MockFlowNode blockEnd = new MockFlowNode("3", blockStart); diff --git a/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/NoOpChunkFinder.java b/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/NoOpChunkFinder.java index 2d902d41..c0418b7e 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/NoOpChunkFinder.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/NoOpChunkFinder.java @@ -10,6 +10,7 @@ * All {@link FlowNode}s will this result in calling {@link SimpleChunkVisitor#atomNode(FlowNode, FlowNode, FlowNode, ForkScanner)} */ public class NoOpChunkFinder implements ChunkFinder { + @Override public boolean isStartInsideChunk() { return false; diff --git a/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/TestVisitor.java b/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/TestVisitor.java index f71f2325..7a28b3e2 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/TestVisitor.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/TestVisitor.java @@ -1,10 +1,17 @@ package org.jenkinsci.plugins.workflow.graphanalysis; import org.jenkinsci.plugins.workflow.graph.FlowNode; -import org.junit.Assert; import org.jvnet.hudson.test.Issue; import edu.umd.cs.findbugs.annotations.CheckForNull; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + import edu.umd.cs.findbugs.annotations.NonNull; import java.util.ArrayDeque; import java.util.ArrayList; @@ -19,6 +26,7 @@ * Test visitor class, tracks invocations of methods */ public class TestVisitor implements SimpleChunkVisitor { + public enum CallType { ATOM_NODE, CHUNK_START, @@ -64,17 +72,16 @@ public CallEntry(CallType type, int... vals) { @Override public boolean equals(Object o) { - if (!(o instanceof CallEntry)) { + if (!(o instanceof CallEntry entry)) { return false; } - CallEntry entry = (CallEntry)o; return this.type == entry.type && Arrays.equals(this.ids, entry.ids); } public void assertEquals(CallEntry test) { - Assert.assertNotNull(test); - Assert.assertNotNull(test.type); - Assert.assertArrayEquals(this.ids, test.ids); + assertNotNull(test); + assertNotNull(test.type); + assertArrayEquals(this.ids, test.ids); } /** Return ID of the node pointed at by this event or null if none */ @@ -202,30 +209,30 @@ public void assertNoDupes() { for (CallEntry ce : this.calls) { // Complete equality check if (entries.contains(ce)) { - Assert.fail("Duplicate call: "+ce.toString()); + fail("Duplicate call: "+ce.toString()); } // A node is either a start or end to a chunk, or an atom (a node within a chunk) if (CHUNK_EVENTS.contains(ce.type)) { int idToCheck = (ce.type == CallType.ATOM_NODE) ? ce.ids[1] : ce.ids[0]; if (ce.type == CallType.ATOM_NODE) { if (visitedAtomNodes.contains(idToCheck)) { - Assert.fail("Duplicate atomNode callback for node "+idToCheck+" with "+ce); + fail("Duplicate atomNode callback for node "+idToCheck+" with "+ce); } else if (visitedChunkStartNodes.contains(idToCheck)) { - Assert.fail("Illegal atomNode callback where chunkStart callback existed for node "+idToCheck+" with "+ce); + fail("Illegal atomNode callback where chunkStart callback existed for node "+idToCheck+" with "+ce); } else if (visitedChunkEndNodes.contains(idToCheck)) { - Assert.fail("Illegal atomNode callback where chunkEnd callback existed for node "+idToCheck+" with "+ce); + fail("Illegal atomNode callback where chunkEnd callback existed for node "+idToCheck+" with "+ce); } visitedAtomNodes.add(idToCheck); } else { // Start/end if (visitedAtomNodes.contains(idToCheck)) { - Assert.fail("Illegal chunk start/end callback where atomNode callback existed for node "+idToCheck+" with "+ce); + fail("Illegal chunk start/end callback where atomNode callback existed for node "+idToCheck+" with "+ce); } if (ce.type == CallType.CHUNK_START){ boolean added = visitedChunkStartNodes.add(idToCheck); - Assert.assertTrue("Duplicate chunkStart callback for node "+idToCheck+" with "+ce, added); + assertTrue(added, "Duplicate chunkStart callback for node "+idToCheck+" with "+ce); } else { // ChunkEnd boolean added = visitedChunkEndNodes.add(idToCheck); - Assert.assertTrue("Duplicate chunkEnd callback for node "+idToCheck+" with "+ce, added); + assertTrue(added, "Duplicate chunkEnd callback for node "+idToCheck+" with "+ce); } } } @@ -237,10 +244,10 @@ public void assertNoDupes() { public void assertNoIllegalNullsInEvents() { for (CallEntry ce : calls) { Integer id = ce.getNodeId(); - Assert.assertNotNull("Callback with illegally null node: "+ce, id); + assertNotNull(id, "Callback with illegally null node: "+ce); if (ce.type == CallType.PARALLEL_START || ce.type == CallType.PARALLEL_END || ce.type == CallType.PARALLEL_BRANCH_START || ce.type == CallType.PARALLEL_BRANCH_END) { - Assert.assertNotEquals("Parallel event with illegally null parallel start node ID: " + ce, -1, ce.ids[0]); + assertNotEquals(-1, ce.ids[0], "Parallel event with illegally null parallel start node ID: " + ce); } } } @@ -256,7 +263,7 @@ public void assertAllNodesGotChunkEvents(Iterable nodes) { } for (FlowNode f : nodes) { if(!ids.contains(f.getId())) { - Assert.fail("No chunk callbacks for flownode: "+f); + fail("No chunk callbacks for flownode: "+f); } } } @@ -290,10 +297,10 @@ public void assertMatchingParallelBranchStartEnd() { List ends = branchEndIds.get(startEntry.getKey()); // Branch start without branch end is legal due to incomplete flows, but not when complete! if (this.isFromCompleteRun != null && this.isFromCompleteRun) { - Assert.assertNotNull(ends); + assertNotNull(ends); } else if (ends != null) { // Can have starts without ends due to single-branch parallels with incomplete branches that are unterminated - Assert.assertEquals("Parallels must have matching numbers of start and end events, but don't -- for parallel starting with: " + - startEntry.getKey(), startEntry.getValue().size(), ends.size()); + assertEquals(startEntry.getValue().size(), ends.size(), "Parallels must have matching numbers of start and end events, but don't -- for parallel starting with: " + + startEntry.getKey()); } } } @@ -301,7 +308,7 @@ public void assertMatchingParallelBranchStartEnd() { // Verify the reverse is true: if we have a branch end, there are branch starts (count equality was checked above) for (Map.Entry> endEntry : branchEndIds.entrySet()) { List starts = branchStartIds.get(endEntry.getKey()); - Assert.assertNotNull("Parallels with a branch end event(s) but no matching branch start event(s), parallel start node id: "+endEntry.getKey(), starts); + assertNotNull(starts, "Parallels with a branch end event(s) but no matching branch start event(s), parallel start node id: "+endEntry.getKey()); } } @@ -314,25 +321,24 @@ public void assertMatchingParallelStartEnd() { if (ce.type == CallType.PARALLEL_END) { openParallelStarts.push(ce.ids[0]); } else if (ce.type == CallType.PARALLEL_START) { - if (openParallelStarts.size() > 0) { - Assert.assertEquals("Parallel start and end events must point to the same parallel start node ID", - openParallelStarts.peekFirst(), Integer.valueOf(ce.ids[0]) + if (!openParallelStarts.isEmpty()) { + assertEquals(openParallelStarts.peekFirst(), Integer.valueOf(ce.ids[0]), "Parallel start and end events must point to the same parallel start node ID" ); openParallelStarts.pop(); } else if (isFromCompleteRun != null && isFromCompleteRun) { // For a complete flow, every start must have an end, for an incomplete one we may have // an incomplete block (still running) - Assert.fail("Found a parallel start without a matching end, with CallEntry: "+ce); + fail("Found a parallel start without a matching end, with CallEntry: "+ce); } } } - if (openParallelStarts.size() > 0) { + if (!openParallelStarts.isEmpty()) { StringBuilder sb = new StringBuilder(); for (Integer parallelStartId : openParallelStarts) { sb.append(parallelStartId).append(','); } - Assert.fail("Parallel ends with no starts, for parallel(s) with start nodes IDs: " + sb); + fail("Parallel ends with no starts, for parallel(s) with start nodes IDs: " + sb); } } } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/log/FileLogStorageTest.java b/src/test/java/org/jenkinsci/plugins/workflow/log/FileLogStorageTest.java index fb35e8fe..25788b13 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/log/FileLogStorageTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/log/FileLogStorageTest.java @@ -24,29 +24,38 @@ package org.jenkinsci.plugins.workflow.log; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertTrue; import hudson.model.TaskListener; import java.io.File; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.CleanupMode; +import org.junit.jupiter.api.io.TempDir; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; -public class FileLogStorageTest extends LogStorageTestBase { +@WithJenkins +class FileLogStorageTest extends LogStorageTestBase { - @Rule public TemporaryFolder tmp = new TemporaryFolder(); + @TempDir(cleanup = CleanupMode.NEVER) + private File tmp; private File log; - @Before public void log() throws Exception { - log = tmp.newFile(); + @BeforeEach + @Override + void setUp(JenkinsRule rule) throws Exception { + super.setUp(rule); + log = File.createTempFile("junit", null, tmp); } - @Override protected LogStorage createStorage() { + @Override + protected LogStorage createStorage() { return FileLogStorage.forFile(log); } - @Test public void oldFormat() throws Exception { + @Test + void oldFormat() throws Exception { LogStorage ls = createStorage(); TaskListener overall = ls.overallListener(); overall.getLogger().println("stuff"); @@ -55,7 +64,8 @@ public class FileLogStorageTest extends LogStorageTestBase { assertOverallLog(0, lines("stuff"), true); } - @Test public void interruptionDoesNotCloseStream() throws Exception { + @Test + void interruptionDoesNotCloseStream() throws Exception { LogStorage ls = createStorage(); TaskListener overall = ls.overallListener(); overall.getLogger().println("overall 1"); diff --git a/src/test/java/org/jenkinsci/plugins/workflow/log/LogStorageTestBase.java b/src/test/java/org/jenkinsci/plugins/workflow/log/LogStorageTestBase.java index a8cabbb8..36808455 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/log/LogStorageTestBase.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/log/LogStorageTestBase.java @@ -26,7 +26,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; -import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; import edu.umd.cs.findbugs.annotations.NonNull; import hudson.console.AnnotatedLargeText; @@ -62,26 +62,33 @@ import org.jenkinsci.plugins.workflow.graph.FlowNode; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; -import org.junit.Before; -import org.junit.ClassRule; -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.LoggerRule; +import org.jvnet.hudson.test.LogRecorder; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; import org.springframework.security.core.Authentication; /** * Foundation for compliance tests of {@link LogStorage} implementations. */ -public abstract class LogStorageTestBase { +@WithJenkins +abstract class LogStorageTestBase { - @ClassRule public static JenkinsRule r = new JenkinsRule(); + protected static LogRecorder logging = new LogRecorder(); - @ClassRule public static LoggerRule logging = new LoggerRule(); + protected JenkinsRule r; - /** Create a new storage implementation, but potentially reusing any data initialized in the last {@link Before} setup. */ + @BeforeEach + void setUp(JenkinsRule rule) throws Exception { + r = rule; + } + + /** Create a new storage implementation, but potentially reusing any data initialized in the last {@link BeforeEach} setup. */ protected abstract LogStorage createStorage(); - @Test public void smokes() throws Exception { + @Test + void smokes() throws Exception { LogStorage ls = createStorage(); TaskListener overall = ls.overallListener(); overall.getLogger().println("starting"); @@ -89,7 +96,7 @@ public abstract class LogStorageTestBase { step1.getLogger().println("one #1"); TaskListener step2 = ls.nodeListener(new MockNode("2")); step2.getLogger().println("two #1"); - long betweenStep2Lines = text().writeHtmlTo(0, new NullWriter()); + long betweenStep2Lines = text().writeHtmlTo(0, NullWriter.INSTANCE); step2.getLogger().println("two #2"); overall.getLogger().println("interrupting"); /* We do not really care much whether nodes are annotated when we start display in the middle; the UI will not do anything with it anyway: @@ -170,7 +177,8 @@ protected static void close(TaskListener listener) throws Exception { } } - @Test public void remoting() throws Exception { + @Test + void remoting() throws Exception { logging.capture(100).record(Channel.class, Level.WARNING); LogStorage ls = createStorage(); TaskListener overall = ls.overallListener(); @@ -197,9 +205,11 @@ protected static void close(TaskListener listener) throws Exception { assertEquals(stepPos, assertStepLog("1", stepPos, "", true)); assertThat(logging.getMessages(), empty()); } + protected Map agentLoggers() { return Collections.singletonMap(LogStorageTestBase.class.getPackage().getName(), Level.FINER); } + private static final class RemotePrint extends MasterToSlaveCallable { private final String message; private final TaskListener listener; @@ -213,6 +223,7 @@ private static final class RemotePrint extends MasterToSlaveCallable { @Override public Void call() { @@ -228,7 +239,8 @@ private static final class GC extends MasterToSlaveCallable { * Failures to do this can cause output from different steps (or general build output) to be interleaved at a sub-line level. * This might not render well (depending on the implementation), but we need to ensure that the entire build log is not broken as a result. */ - @Test public void mangledLines() throws Exception { + @Test + void mangledLines() throws Exception { Random r = new Random(); BiFunction thread = (c, l) -> new Thread(() -> { for (int i = 0; i < 1000; i++) { @@ -258,23 +270,24 @@ private static final class GC extends MasterToSlaveCallable { x.printStackTrace(); } }); - long pos = text().writeHtmlTo(0, new NullWriter()); + long pos = text().writeHtmlTo(0, NullWriter.INSTANCE); // TODO detailed assertions would need to take into account completion flag: // assertLength(pos); // assertOverallLog(pos, "", true); - text().writeRawLogTo(0, new NullOutputStream()); - pos = text("1").writeHtmlTo(0, new NullWriter()); + text().writeRawLogTo(0, NullOutputStream.INSTANCE); + pos = text("1").writeHtmlTo(0, NullWriter.INSTANCE); // assertLength("1", pos); // assertStepLog("1", pos, "", true); - text("1").writeRawLogTo(0, new NullOutputStream()); - pos = text("2").writeHtmlTo(0, new NullWriter()); + text("1").writeRawLogTo(0, NullOutputStream.INSTANCE); + pos = text("2").writeHtmlTo(0, NullWriter.INSTANCE); // assertLength("2", pos); // assertStepLog("2", pos, "", true); - text("2").writeRawLogTo(0, new NullOutputStream()); + text("2").writeRawLogTo(0, NullOutputStream.INSTANCE); } @SuppressWarnings("deprecation") - @Test public void getLogFile() throws Exception { + @Test + void getLogFile() throws Exception { LogStorage ls = createStorage(); TaskListener overall = ls.overallListener(); overall.getLogger().println("starting"); @@ -312,7 +325,7 @@ private long assertLog(Callable> text, long start, String if (html) { pos = oneText.writeHtmlTo(pos, sw); } else { - pos = oneText.writeRawLogTo(pos, new WriterOutputStream(sw, StandardCharsets.UTF_8)); + pos = oneText.writeRawLogTo(pos, WriterOutputStream.builder().setWriter(sw).setCharset(StandardCharsets.UTF_8).get()); } } while (!oneText.isComplete()); String result = sw.toString(); diff --git a/src/test/java/org/jenkinsci/plugins/workflow/log/SpanCoalescerTest.java b/src/test/java/org/jenkinsci/plugins/workflow/log/SpanCoalescerTest.java index b000c2a0..34487dc0 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/log/SpanCoalescerTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/log/SpanCoalescerTest.java @@ -24,16 +24,17 @@ package org.jenkinsci.plugins.workflow.log; -import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; import hudson.console.AnnotatedLargeText; import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.junit.Test; +import org.junit.jupiter.api.Test; -public class SpanCoalescerTest { +class SpanCoalescerTest { - @Test public void works() { + @Test + void works() { assertUncoalesced("plain\n"); assertUncoalesced("one\n"); assertUncoalesced("plain\n1a\n1b\n2a\n2b\nmore plain\n"); diff --git a/src/test/java/org/jenkinsci/plugins/workflow/log/TaskListenerDecoratorOrderTest.java b/src/test/java/org/jenkinsci/plugins/workflow/log/TaskListenerDecoratorOrderTest.java index 8848b0e8..15c922d4 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/log/TaskListenerDecoratorOrderTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/log/TaskListenerDecoratorOrderTest.java @@ -14,25 +14,37 @@ import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; import org.jenkinsci.plugins.workflow.steps.*; -import org.junit.ClassRule; -import org.junit.Rule; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import org.jvnet.hudson.test.*; +import org.jvnet.hudson.test.junit.jupiter.BuildWatcherExtension; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; import org.kohsuke.stapler.DataBoundConstructor; import java.io.IOException; import java.io.OutputStream; +import java.io.Serial; import java.io.Serializable; import java.util.Collections; import java.util.Set; @For(TaskListenerDecorator.class) -public class TaskListenerDecoratorOrderTest { - @ClassRule public static BuildWatcher buildWatcher = new BuildWatcher(); +@WithJenkins +class TaskListenerDecoratorOrderTest { - @Rule public JenkinsRule r = new JenkinsRule(); + @RegisterExtension + private static final BuildWatcherExtension buildWatcher = new BuildWatcherExtension(); - @Test public void verifyTaskListenerDecoratorOrder() throws Exception { + private JenkinsRule r; + + @BeforeEach + void setUp(JenkinsRule rule) { + r = rule; + } + + @Test + void verifyTaskListenerDecoratorOrder() throws Exception { r.createSlave("remote", null, null); WorkflowJob p = r.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition("filter {node('remote') {remotePrint()}}", true)); @@ -51,12 +63,14 @@ public class TaskListenerDecoratorOrderTest { } private static final class DecoratorImpl extends TaskListenerDecorator { + @Serial private static final long serialVersionUID = 1L; private final String tagName; DecoratorImpl(String tagName) { this.tagName = tagName; } + @Serial private Object writeReplace() { Channel ch = Channel.current(); return ch != null ? new DecoratorImpl(tagName + " via " + ch.getName()) : this; @@ -107,6 +121,7 @@ public static final class FilterStep extends Step { return new Execution(context); } private static final class Execution extends StepExecution { + @Serial private static final long serialVersionUID = 1L; Execution(StepContext context) { @@ -118,17 +133,18 @@ private static final class Execution extends StepExecution { } } private static final class Filter extends ConsoleLogFilter implements Serializable { + @Serial private static final long serialVersionUID = 1L; private final String message; Filter(String message) { this.message = message; } + @Serial private Object writeReplace() { Channel ch = Channel.current(); return ch != null ? new Filter(message + " via " + ch.getName()) : this; } - @SuppressWarnings("rawtypes") @Override public OutputStream decorateLogger(AbstractBuild _ignore, OutputStream logger) throws IOException, InterruptedException { return new DecoratorImpl(message).decorate(logger); } @@ -155,6 +171,7 @@ public static final class RemotePrintStep extends Step { return new Execution(context); } private static final class Execution extends SynchronousNonBlockingStepExecution { + @Serial private static final long serialVersionUID = 1L; Execution(StepContext context) { @@ -165,6 +182,7 @@ private static final class Execution extends SynchronousNonBlockingStepExecution } } private static final class PrintCallable extends MasterToSlaveCallable { + @Serial private static final long serialVersionUID = 1L; private final TaskListener listener; diff --git a/src/test/java/org/jenkinsci/plugins/workflow/log/TaskListenerDecoratorTest.java b/src/test/java/org/jenkinsci/plugins/workflow/log/TaskListenerDecoratorTest.java index d359bf1f..cbdd7314 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/log/TaskListenerDecoratorTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/log/TaskListenerDecoratorTest.java @@ -27,6 +27,7 @@ import hudson.console.LineTransformationOutputStream; import java.io.IOException; import java.io.OutputStream; +import java.io.Serial; import java.nio.charset.StandardCharsets; import java.util.Set; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; @@ -36,21 +37,30 @@ import org.jenkinsci.plugins.workflow.steps.StepContext; import org.jenkinsci.plugins.workflow.steps.StepDescriptor; import org.jenkinsci.plugins.workflow.steps.StepExecution; -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.JenkinsRule; 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 final class TaskListenerDecoratorTest { +@WithJenkins +class TaskListenerDecoratorTest { - @ClassRule public static BuildWatcher buildWatcher = new BuildWatcher(); + @RegisterExtension + private static final BuildWatcherExtension buildWatcher = new BuildWatcherExtension(); - @Rule public JenkinsRule r = new JenkinsRule(); + private JenkinsRule r; - @Test public void brokenMergedTaskListenerDecorator() throws Exception { + @BeforeEach + void setUp(JenkinsRule rule) { + r = rule; + } + + @Test + void brokenMergedTaskListenerDecorator() throws Exception { var p = r.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition("mask {broken {echo 'please mask s3cr3t'}}; broken {mask {echo 'please also mask s3cr3t'}}", true)); var b = r.buildAndAssertSuccess(p); @@ -67,6 +77,7 @@ public static final class MaskStep extends Step { return new Execution(context); } private static final class Execution extends StepExecution { + @Serial private static final long serialVersionUID = 1L; Execution(StepContext context) { super(context); @@ -104,6 +115,7 @@ public static final class BrokenStep extends Step { return new Execution(context); } private static final class Execution extends StepExecution { + @Serial private static final long serialVersionUID = 1L; Execution(StepContext context) { super(context); From ba95370a012471edae442c70e47db2d63a3fb8e3 Mon Sep 17 00:00:00 2001 From: strangelookingnerd <49242855+strangelookingnerd@users.noreply.github.com> Date: Fri, 8 Aug 2025 19:50:03 +0200 Subject: [PATCH 2/7] Update parent and remove jth version --- pom.xml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index c71d4641..2ed73194 100644 --- a/pom.xml +++ b/pom.xml @@ -28,7 +28,7 @@ org.jenkins-ci.plugins plugin - 5.9 + 5.19 org.jenkins-ci.plugins.workflow @@ -68,8 +68,6 @@ ${jenkins.baseline}.1 false jenkinsci/${project.artifactId}-plugin - - 2486.v14ec80488532 From 6d02dfeb62bb0b935629a1137ca2b7e987298b09 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 15 Sep 2025 01:01:13 +0000 Subject: [PATCH 3/7] Bump org.jenkins-ci.plugins:plugin from 5.24 to 5.26 Bumps [org.jenkins-ci.plugins:plugin](https://github.com/jenkinsci/plugin-pom) from 5.24 to 5.26. - [Release notes](https://github.com/jenkinsci/plugin-pom/releases) - [Changelog](https://github.com/jenkinsci/plugin-pom/blob/master/CHANGELOG.md) - [Commits](https://github.com/jenkinsci/plugin-pom/compare/plugin-5.24...plugin-5.26) --- updated-dependencies: - dependency-name: org.jenkins-ci.plugins:plugin dependency-version: '5.26' dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 03d3e2b6..9636a600 100644 --- a/pom.xml +++ b/pom.xml @@ -28,7 +28,7 @@ org.jenkins-ci.plugins plugin - 5.24 + 5.26 org.jenkins-ci.plugins.workflow From 67badaec25cad72e4f9c3fb1a5b9ce7b23e3a82a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 24 Feb 2025 01:53:22 +0000 Subject: [PATCH 4/7] Bump org.awaitility:awaitility from 4.2.2 to 4.3.0 Bumps [org.awaitility:awaitility](https://github.com/awaitility/awaitility) from 4.2.2 to 4.3.0. - [Changelog](https://github.com/awaitility/awaitility/blob/master/changelog.txt) - [Commits](https://github.com/awaitility/awaitility/compare/awaitility-4.2.2...awaitility-4.3.0) --- updated-dependencies: - dependency-name: org.awaitility:awaitility dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 9636a600..b67732f6 100644 --- a/pom.xml +++ b/pom.xml @@ -105,7 +105,7 @@ org.awaitility awaitility - 4.2.2 + 4.3.0 test From 8a9a4dde69d75947063da41fc318a988f33de17e Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Tue, 30 Sep 2025 17:41:25 -0400 Subject: [PATCH 5/7] Disallow soft references in MemoryAssert-related tests in FlowExecutionListTest --- .../plugins/workflow/flow/FlowExecutionListTest.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest.java b/src/test/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest.java index 9367fd38..e000b22b 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionListTest.java @@ -49,6 +49,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Set; +import java.util.concurrent.ForkJoinPool; import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.LogRecord; @@ -213,6 +214,8 @@ void resumeStepExecutionsWithCorruptFlowGraphWithCycle() throws Throwable { @Test void stepExecutionIteratorDoesNotLeakBuildsWhenCpsVmIsStuck() throws Throwable { + // Make sure ForkJoinPool is not initialized on a thread where its inheritedAccessControlContext would point to a CpsGroovyShell$CleanGroovyClassLoader + ForkJoinPool.commonPool().execute(() -> {}); sessions.then(r -> { var notStuck = r.createProject(WorkflowJob.class, "not-stuck"); notStuck.setDefinition(new CpsFlowDefinition("semaphore 'wait'", true)); @@ -230,10 +233,11 @@ void stepExecutionIteratorDoesNotLeakBuildsWhenCpsVmIsStuck() throws Throwable { // Let notStuckBuild complete and clean up all references. SemaphoreStep.success("wait/1", null); r.waitForCompletion(notStuckBuild); + notStuck.getLazyBuildMixIn().removeRun(notStuckBuild); notStuckBuild = null; // Clear out the local variable in this thread. Jenkins.get().getQueue().clearLeftItems(); // Otherwise we'd have to wait 5 minutes for the cache to be cleared. // Make sure that the reference can be GC'd. - MemoryAssert.assertGC(notStuckBuildRef, true); + MemoryAssert.assertGC(notStuckBuildRef, false); // Allow stuck #1 to complete so the test can be cleaned up promptly. SynchronousBlockingStep.unblock("stuck"); r.waitForCompletion(stuckBuild); @@ -242,6 +246,8 @@ void stepExecutionIteratorDoesNotLeakBuildsWhenCpsVmIsStuck() throws Throwable { @Test void stepExecutionIteratorDoesNotLeakBuildsWhenProgramPromiseIsStuck() throws Throwable { + // Make sure ForkJoinPool is not initialized on a thread where its inheritedAccessControlContext would point to a CpsGroovyShell$CleanGroovyClassLoader + ForkJoinPool.commonPool().execute(() -> {}); sessions.then(r -> { var stuck = r.createProject(WorkflowJob.class, "stuck"); stuck.setDefinition(new CpsFlowDefinition( @@ -266,10 +272,11 @@ void stepExecutionIteratorDoesNotLeakBuildsWhenProgramPromiseIsStuck() throws Th // Let notStuckBuild complete and clean up all references. SemaphoreStep.success("wait/1", null); r.waitForCompletion(notStuckBuild); + notStuck.getLazyBuildMixIn().removeRun(notStuckBuild); notStuckBuild = null; // Clear out the local variable in this thread. Jenkins.get().getQueue().clearLeftItems(); // Otherwise we'd have to wait 5 minutes for the cache to be cleared. // Make sure that the reference can be GC'd. - MemoryAssert.assertGC(notStuckBuildRef, true); + MemoryAssert.assertGC(notStuckBuildRef, false); // Allow stuck #1 to complete so the test can be cleaned up promptly. r.waitForMessage("Still trying to load StuckPickle for", stuckBuild); ExtensionList.lookupSingleton(StuckPickle.Factory.class).resolved = new StuckPickle.Marker(); From 2617bcfaff031a45e8fd579229972ae00b17de8f Mon Sep 17 00:00:00 2001 From: strangelookingnerd <49242855+strangelookingnerd@users.noreply.github.com> Date: Thu, 23 Oct 2025 09:26:28 +0200 Subject: [PATCH 6/7] Change LogStorageTestBase class to public --- .../org/jenkinsci/plugins/workflow/log/LogStorageTestBase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/log/LogStorageTestBase.java b/src/test/java/org/jenkinsci/plugins/workflow/log/LogStorageTestBase.java index 36808455..b72c79f4 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/log/LogStorageTestBase.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/log/LogStorageTestBase.java @@ -73,7 +73,7 @@ * Foundation for compliance tests of {@link LogStorage} implementations. */ @WithJenkins -abstract class LogStorageTestBase { +public abstract class LogStorageTestBase { protected static LogRecorder logging = new LogRecorder(); From 26b795fedd3029df837e5948c46580450c19f87b Mon Sep 17 00:00:00 2001 From: strangelookingnerd <49242855+strangelookingnerd@users.noreply.github.com> Date: Fri, 24 Oct 2025 08:57:17 +0200 Subject: [PATCH 7/7] Change ArtifactManagerTest class to public --- .../org/jenkinsci/plugins/workflow/ArtifactManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/ArtifactManagerTest.java b/src/test/java/org/jenkinsci/plugins/workflow/ArtifactManagerTest.java index 91e2af30..0c0e14fc 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/ArtifactManagerTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/ArtifactManagerTest.java @@ -78,7 +78,7 @@ * {@link #artifactArchiveAndDelete} and variants allow an implementation of {@link ArtifactManager} plus {@link VirtualFile} to be run through a standard gantlet of tests. */ @WithJenkins -class ArtifactManagerTest { +public class ArtifactManagerTest { private final LogRecorder logging = new LogRecorder();