diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/durable_task/DurableTaskStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/durable_task/DurableTaskStep.java index c073ee26..7d9c3214 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/steps/durable_task/DurableTaskStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/steps/durable_task/DurableTaskStep.java @@ -494,21 +494,19 @@ private static final class NewlineSafeTaskListener implements TaskListener { if (workspace != null) { listener().getLogger().println("Sending interrupt signal to process"); LOGGER.log(Level.FINE, "stopping process", cause); - stopTask = Timer.get().schedule(new Runnable() { - @Override public void run() { - stopTask = null; - if (recurrencePeriod > 0) { - recurrencePeriod = 0; - listener().getLogger().println("After " + REMOTE_TIMEOUT + "s process did not stop"); - getContext().onFailure(cause); - try { - FilePath workspace = getWorkspace(); - if (workspace != null) { - controller.cleanup(workspace); - } - } catch (IOException | InterruptedException x) { - Functions.printStackTrace(x, listener().getLogger()); + stopTask = Timer.get().schedule(() -> { + stopTask = null; + if (recurrencePeriod > 0) { + recurrencePeriod = 0; + listener().getLogger().println("After " + REMOTE_TIMEOUT + "s process did not stop"); + getContext().onFailure(cause); + try { + FilePath taskWorkspace = getWorkspace(); + if (taskWorkspace != null) { + controller.cleanup(taskWorkspace); } + } catch (IOException | InterruptedException x) { + Functions.printStackTrace(x, listener().getLogger()); } } }, REMOTE_TIMEOUT, TimeUnit.SECONDS); diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/pickles/ExecutorPickle.java b/src/main/java/org/jenkinsci/plugins/workflow/support/pickles/ExecutorPickle.java index e89a54cf..e15ea507 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/pickles/ExecutorPickle.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/pickles/ExecutorPickle.java @@ -129,7 +129,7 @@ protected Executor tryResolve() throws Exception { if (System.nanoTime() > endTimeNanos) { Queue.getInstance().cancel(item); throw new AbortException(MessageFormat.format("Killed {0} after waiting for {1} ms because we assume unknown Node {2} is never going to appear!", - new Object[]{item, TIMEOUT_WAITING_FOR_NODE_MILLIS, placeholder.getAssignedLabel().toString()})); + item, TIMEOUT_WAITING_FOR_NODE_MILLIS, placeholder.getAssignedLabel().toString())); } } } diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStep.java b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStep.java index e9af7c94..161caf7e 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStep.java @@ -30,12 +30,10 @@ import hudson.FilePath; import hudson.Launcher; import hudson.Util; -import hudson.model.AbstractProject; import hudson.model.AutoCompletionCandidates; import hudson.model.Computer; import hudson.model.Executor; import hudson.model.Job; -import hudson.model.Label; import hudson.model.Node; import hudson.model.Run; import hudson.model.TaskListener; @@ -44,7 +42,6 @@ import java.io.Serializable; import java.util.Set; import javax.annotation.CheckForNull; -import jenkins.model.Jenkins; import org.jenkinsci.plugins.workflow.flow.FlowExecution; import org.jenkinsci.plugins.workflow.graph.FlowNode; import org.jenkinsci.plugins.workflow.steps.Step; diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecution.java b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecution.java index 885b96b2..74e42beb 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecution.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecution.java @@ -108,22 +108,20 @@ public boolean start() throws Exception { } getContext().get(FlowNode.class).addAction(new QueueItemActionImpl(waitingItem.getId())); - Timer.get().schedule(new Runnable() { - @Override public void run() { - Queue.Item item = Queue.getInstance().getItem(task); - if (item != null) { - TaskListener listener; - try { - listener = getContext().get(TaskListener.class); - } catch (Exception x) { // IOException, InterruptedException - LOGGER.log(FINE, "could not print message to build about " + item + "; perhaps it is already completed", x); - return; - } - listener.getLogger().println("Still waiting to schedule task"); - CauseOfBlockage cob = item.getCauseOfBlockage(); - if (cob != null) { - cob.print(listener); - } + Timer.get().schedule(() -> { + Queue.Item item = Queue.getInstance().getItem(task); + if (item != null) { + TaskListener listener; + try { + listener = getContext().get(TaskListener.class); + } catch (Exception x) { // IOException, InterruptedException + LOGGER.log(FINE, "could not print message to build about " + item + "; perhaps it is already completed", x); + return; + } + listener.getLogger().println("Still waiting to schedule task"); + CauseOfBlockage cob = item.getCauseOfBlockage(); + if (cob != null) { + cob.print(listener); } } }, 15, TimeUnit.SECONDS); @@ -193,7 +191,7 @@ public void stop(Throwable cause) throws Exception { } Jenkins j = Jenkins.getInstanceOrNull(); if (j != null) { - COMPUTERS: for (Computer c : j.getComputers()) { + for (Computer c : j.getComputers()) { for (Executor e : c.getExecutors()) { Queue.Executable exec = e.getCurrentExecutable(); if (exec instanceof PlaceholderTask.PlaceholderExecutable && ((PlaceholderTask.PlaceholderExecutable) exec).getParent().context.equals(getContext())) { @@ -224,7 +222,7 @@ public void stop(Throwable cause) throws Exception { } Jenkins j = Jenkins.getInstanceOrNull(); if (j != null) { - COMPUTERS: for (Computer c : j.getComputers()) { + for (Computer c : j.getComputers()) { for (Executor e : c.getExecutors()) { Queue.Executable exec = e.getCurrentExecutable(); if (exec instanceof PlaceholderTask.PlaceholderExecutable && ((PlaceholderTask.PlaceholderExecutable) exec).getParent().context.equals(getContext())) { @@ -309,7 +307,7 @@ private static final class RunningTask { public static final class PlaceholderTask implements ContinuedTask, Serializable, AccessControlled { /** keys are {@link #cookie}s */ - private static final Map runningTasks = new HashMap(); + private static final Map runningTasks = new HashMap<>(); private final StepContext context; /** Initially set to {@link ExecutorStep#getLabel}, if any; later switched to actual self-label when block runs. */ @@ -732,23 +730,16 @@ private static void finish(@CheckForNull final String cookie) { return; } assert runningTask.launcher != null; - Timer.get().submit(new Runnable() { // JENKINS-31614 - @Override public void run() { - execution.completed(null); - } - }); - Computer.threadPoolForRemoting.submit(new Runnable() { // JENKINS-34542, JENKINS-45553 - @Override - public void run() { - try { - runningTask.launcher.kill(Collections.singletonMap(COOKIE_VAR, cookie)); - } catch (ChannelClosedException x) { - // fine, Jenkins was shutting down - } catch (RequestAbortedException x) { - // slave was exiting; too late to kill subprocesses - } catch (Exception x) { - LOGGER.log(Level.WARNING, "failed to shut down " + cookie, x); - } + Timer.get().submit(() -> execution.completed(null)); // JENKINS-31614 + Computer.threadPoolForRemoting.submit(() -> { // JENKINS-34542, JENKINS-45553 + try { + runningTask.launcher.kill(Collections.singletonMap(COOKIE_VAR, cookie)); + } catch (ChannelClosedException x) { + // fine, Jenkins was shutting down + } catch (RequestAbortedException x) { + // slave was exiting; too late to kill subprocesses + } catch (Exception x) { + LOGGER.log(Level.WARNING, "failed to shut down " + cookie, x); } }); } @@ -889,18 +880,16 @@ private final class PlaceholderExecutable implements ContinuableExecutable, Acce } LOGGER.log(FINE, "interrupted {0}", cookie); // TODO save the BodyExecution somehow and call .cancel() here; currently we just interrupt the build as a whole: - Timer.get().submit(new Runnable() { // JENKINS-46738 - @Override public void run() { - Executor masterExecutor = r.getExecutor(); - if (masterExecutor != null) { - masterExecutor.interrupt(); - } else { // anomalous state; perhaps build already aborted but this was left behind; let user manually cancel executor slot - Executor thisExecutor = /* AsynchronousExecution. */getExecutor(); - if (thisExecutor != null) { - thisExecutor.recordCauseOfInterruption(r, _listener); - } - completed(null); + Timer.get().submit(() -> { // JENKINS-46738 + Executor masterExecutor = r.getExecutor(); + if (masterExecutor != null) { + masterExecutor.interrupt(); + } else { // anomalous state; perhaps build already aborted but this was left behind; let user manually cancel executor slot + Executor thisExecutor = /* AsynchronousExecution. */getExecutor(); + if (thisExecutor != null) { + thisExecutor.recordCauseOfInterruption(r, _listener); } + completed(null); } }); } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/BuildQueueTasksTest.java b/src/test/java/org/jenkinsci/plugins/workflow/BuildQueueTasksTest.java index 3755a91a..053976d6 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/BuildQueueTasksTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/BuildQueueTasksTest.java @@ -54,7 +54,7 @@ public class BuildQueueTasksTest { @Rule public RestartableJenkinsRule story = new RestartableJenkinsRule(); @Issue("JENKINS-28649") - @Test public void queueAPI() throws Exception { + @Test public void queueAPI() { // This is implicitly testing ExecutorStepExecution$PlaceholderTask as exported bean story.addStep(new Statement() { @Override public void evaluate() throws Throwable { @@ -69,7 +69,7 @@ public class BuildQueueTasksTest { } @Issue("JENKINS-28649") - @Test public void queueAPIRestartable() throws Exception { + @Test public void queueAPIRestartable() { // This is implicitly testing AfterRestartTask as exported bean story.addStep(new Statement() { @Override public void evaluate() throws Throwable { @@ -91,7 +91,7 @@ public class BuildQueueTasksTest { } @Issue("JENKINS-28649") - @Test public void computerAPI() throws Exception { + @Test public void computerAPI() { // This is implicitly testing ExecutorStepExecution$PlaceholderTask$PlaceholderExecutable as exported bean story.addStep(new Statement() { @Override public void evaluate() throws Throwable { diff --git a/src/test/java/org/jenkinsci/plugins/workflow/steps/durable_task/ShellStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/steps/durable_task/ShellStepTest.java index 481cb8b7..53b4a89b 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/steps/durable_task/ShellStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/steps/durable_task/ShellStepTest.java @@ -91,7 +91,12 @@ import org.jenkinsci.plugins.workflow.support.steps.ExecutorStepExecution; import org.jenkinsci.plugins.workflow.support.visualization.table.FlowGraphTable; import org.jenkinsci.plugins.workflow.support.visualization.table.FlowGraphTable.Row; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertTrue; import org.junit.Assume; import static org.junit.Assume.assumeFalse; import org.junit.ClassRule; @@ -244,7 +249,7 @@ public void abort() throws Exception { } public static class NiceStep extends Step { @DataBoundConstructor public NiceStep() {} - @Override public StepExecution start(StepContext context) throws Exception { + @Override public StepExecution start(StepContext context) { return new Execution(context); } public static class Execution extends StepExecution { @@ -297,7 +302,7 @@ private static class Decorator extends LauncherDecorator implements Serializable assertFalse(s.isReturnStdout()); assertNull(s.getEncoding()); assertFalse(s.isReturnStatus()); - assertEquals(null, s.getLabel()); + assertNull(s.getLabel()); s.setReturnStdout(true); s.setEncoding("ISO-8859-1"); @@ -306,7 +311,7 @@ private static class Decorator extends LauncherDecorator implements Serializable assertTrue(s.isReturnStdout()); assertEquals("ISO-8859-1", s.getEncoding()); assertFalse(s.isReturnStatus()); - assertEquals(null, s.getLabel()); + assertNull(s.getLabel()); s.setReturnStdout(false); s.setEncoding("UTF-8"); @@ -316,7 +321,7 @@ private static class Decorator extends LauncherDecorator implements Serializable assertFalse(s.isReturnStdout()); assertEquals("UTF-8", s.getEncoding()); assertTrue(s.isReturnStatus()); - assertEquals(null, s.getLabel()); + assertNull(s.getLabel()); s.setLabel("Round Trip Test"); s = new StepConfigTester(j).configRoundTrip(s); @@ -523,7 +528,7 @@ private Object writeReplace() { public static final class MarkUpStep extends Step { @DataBoundSetter public boolean smart; @DataBoundConstructor public MarkUpStep() {} - @Override public StepExecution start(StepContext context) throws Exception { + @Override public StepExecution start(StepContext context) { return new Exec(context, smart); } private static final class Exec extends StepExecution { @@ -755,7 +760,7 @@ private static final class HelloNote extends ConsoleNote> { } @Issue("JENKINS-62014") - @Test public void ensureTypes() throws Exception { + @Test public void ensureTypes() { final List descriptors = BuilderUtil.allDescriptors(); MatcherAssert.assertThat(descriptors , containsInAnyOrder( diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/pickles/ExecutorPickleTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/pickles/ExecutorPickleTest.java index 4ac1c3f5..849e29b8 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/pickles/ExecutorPickleTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/pickles/ExecutorPickleTest.java @@ -60,7 +60,7 @@ public class ExecutorPickleTest { @Rule public TemporaryFolder tmp = new TemporaryFolder(); //@Rule public LoggerRule logging = new LoggerRule().record(Queue.class, Level.FINE); - @Test public void canceledQueueItem() throws Exception { + @Test public void canceledQueueItem() { r.addStep(new Statement() { @Override public void evaluate() throws Throwable { DumbSlave s = r.j.createSlave(Label.get("remote")); @@ -121,7 +121,7 @@ public class ExecutorPickleTest { * I.E. cases where the {@link RetentionStrategy} is {@link RetentionStrategy#NOOP}. */ @Issue("JENKINS-36013") - @Test public void normalNodeDisappearance() throws Exception { + @Test public void normalNodeDisappearance() { r.addStep(new Statement() { // Start up a build that needs executor and then reboot and take the node offline @Override public void evaluate() throws Throwable { @@ -155,7 +155,7 @@ public class ExecutorPickleTest { r.j.assertBuildStatus(Result.FAILURE, run); Assert.assertEquals(0, r.j.jenkins.getQueue().getItems().length); } catch (InterruptedIOException ioe) { - Assert.fail("Waited for build to detect loss of node and it didn't!"); + throw new AssertionError("Waited for build to detect loss of node and it didn't!", ioe); } finally { if (run.isBuilding()) { run.doKill(); diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepTest.java index 9d69a81b..dfff5a43 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepTest.java @@ -99,7 +99,13 @@ import org.jenkinsci.plugins.workflow.steps.durable_task.Messages; import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep; import org.junit.AfterClass; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import org.junit.Assume; import org.junit.ClassRule; import org.junit.Rule; @@ -131,7 +137,7 @@ public class ExecutorStepTest { * This ensures that the context variable overrides are working as expected, and * that they are persisted and resurrected. */ - @Test public void buildShellScriptOnSlave() throws Exception { + @Test public void buildShellScriptOnSlave() { story.addStep(new Statement() { @Override public void evaluate() throws Throwable { DumbSlave s = story.j.createOnlineSlave(); @@ -179,7 +185,7 @@ public class ExecutorStepTest { * This ensures that the context variable overrides are working as expected, and * that they are persisted and resurrected. */ - @Test public void buildShellScriptWithPersistentProcesses() throws Exception { + @Test public void buildShellScriptWithPersistentProcesses() { story.addStep(new Statement() { @Override public void evaluate() throws Throwable { DumbSlave s = story.j.createOnlineSlave(); @@ -228,7 +234,7 @@ private void startJnlpProc() throws Exception { } } - @Test public void buildShellScriptAcrossRestart() throws Exception { + @Test public void buildShellScriptAcrossRestart() { Assume.assumeFalse("TODO not sure how to write a corresponding batch script", Functions.isWindows()); story.addStep(new Statement() { @SuppressWarnings("SleepWhileInLoop") @@ -281,7 +287,7 @@ private void startJnlpProc() throws Exception { } @Issue("JENKINS-52165") - @Test public void shellOutputAcrossRestart() throws Exception { + @Test public void shellOutputAcrossRestart() { Assume.assumeFalse("TODO not sure how to write a corresponding batch script", Functions.isWindows()); // TODO does not assert anything in watch mode, just informational. // There is no way for FileMonitoringTask.Watcher to know when content has been written through to the sink @@ -323,7 +329,7 @@ private void startJnlpProc() throws Exception { }); } - @Test public void buildShellScriptAcrossDisconnect() throws Exception { + @Test public void buildShellScriptAcrossDisconnect() { Assume.assumeFalse("TODO not sure how to write a corresponding batch script", Functions.isWindows()); story.addStep(new Statement() { @SuppressWarnings("SleepWhileInLoop") @@ -374,7 +380,7 @@ private void startJnlpProc() throws Exception { @Issue({"JENKINS-41854", "JENKINS-50504"}) @Test - public void contextualizeFreshFilePathAfterAgentReconnection() throws Exception { + public void contextualizeFreshFilePathAfterAgentReconnection() { Assume.assumeFalse("TODO not sure how to write a corresponding batch script", Functions.isWindows()); story.addStep(new Statement() { @SuppressWarnings("SleepWhileInLoop") @@ -453,7 +459,7 @@ private static void assertWorkspaceLocked(Computer computer, String workspacePat } } - @Test public void buildShellScriptQuick() throws Exception { + @Test public void buildShellScriptQuick() { story.addStep(new Statement() { @Override public void evaluate() throws Throwable { DumbSlave s = story.j.createOnlineSlave(); @@ -471,7 +477,7 @@ private static void assertWorkspaceLocked(Computer computer, String workspacePat }); } - @Test public void acquireWorkspace() throws Exception { + @Test public void acquireWorkspace() { story.addStep(new Statement() { @Override public void evaluate() throws Throwable { String slaveRoot = tmp.newFolder().getPath(); @@ -580,7 +586,7 @@ void assertLogMatches(WorkflowRun build, String regexp) throws IOException { // }); } - @Test public void detailsExported() throws Exception { + @Test public void detailsExported() { story.addStep(new Statement() { @Override public void evaluate() throws Throwable { @@ -600,7 +606,7 @@ public void evaluate() throws Throwable { .goTo("computer/" + s.getNodeName() + "/api/json?tree=executors[currentExecutable[number,displayName,fullDisplayName,url,timestamp]]", "application/json"); - JSONObject propertiesJSON = (JSONObject) (new JsonSlurper()).parseText(page.getWebResponse().getContentAsString()); + JSONObject propertiesJSON = (JSONObject) new JsonSlurper().parseText(page.getWebResponse().getContentAsString()); JSONArray executors = propertiesJSON.getJSONArray("executors"); JSONObject executor = executors.getJSONObject(0); JSONObject currentExecutable = executor.getJSONObject("currentExecutable"); @@ -1202,7 +1208,7 @@ List currentLabels() { @Test @Issue("JENKINS-60634") - public void tempDirVariable() throws Exception { + public void tempDirVariable() { story.then(r -> { WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition("node {if (isUnix()) {sh 'set -u && touch \"$WORKSPACE_TMP/x\"'} else {bat(/echo ok > \"%WORKSPACE_TMP%\\x\"/)}}", true));