Merge pull request #171 from jglick/StepExecution.stop-JENKINS-26148

[JENKINS-26148] Using default implementation of StepExecution.stop
jenkinsci · Sep 27, 2017 · aa4c4e7 · aa4c4e7
2 parents 1988c8f + d461897
commit aa4c4e7
Show file tree

Hide file tree

Showing 11 changed files with 28 additions and 31 deletions.
diff --git a/pom.xml b/pom.xml
@@ -73,7 +73,7 @@
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-step-api</artifactId>
-            <version>2.10</version>
+            <version>2.13</version>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/cps/steps/LoadStepExecution.java b/src/main/java/org/jenkinsci/plugins/workflow/cps/steps/LoadStepExecution.java
@@ -54,13 +54,6 @@ public boolean start() throws Exception {
         return false;
     }
 
-    @Override
-    public void stop(Throwable cause) throws Exception {
-        // noop
-        //
-        // the head of the CPS thread that's executing the body should stop and that's all we need to do.
-    }
-
     private static final long serialVersionUID = 1L;
 
 }
diff --git a/src/main/java/org/jenkinsci/plugins/workflow/cps/steps/ParallelStepExecution.java b/src/main/java/org/jenkinsci/plugins/workflow/cps/steps/ParallelStepExecution.java
@@ -62,6 +62,7 @@ public boolean start() throws Exception {
 
     @Override
     public void stop(Throwable cause) throws Exception {
+        // Despite suggestion in JENKINS-26148, super.stop does not work here, even accounting for the direct call from checkAllDone.
         for (BodyExecution body : bodies) {
             body.cancel(cause);
         }

diff --git a/src/test/java/org/jenkinsci/plugins/workflow/DynamicEnvironmentExpanderTest.java b/src/test/java/org/jenkinsci/plugins/workflow/DynamicEnvironmentExpanderTest.java
@@ -100,7 +100,6 @@ private static class Execution extends StepExecution {
                 super.onResume();
                 value = "two";
             }
-            @Override public void stop(Throwable cause) throws Exception {}
         }
         private static class ExpanderImpl extends EnvironmentExpander {
             private static final long serialVersionUID = 1;

diff --git a/src/test/java/org/jenkinsci/plugins/workflow/SerializationTest.java b/src/test/java/org/jenkinsci/plugins/workflow/SerializationTest.java
@@ -103,10 +103,6 @@ private Object writeReplace() {
             public boolean start() throws Exception {
                 return false;
             }
-            @Override
-            public void stop(Throwable cause) throws Exception {
-                // nothing to do here
-            }
         }
     }
 

diff --git a/src/test/java/org/jenkinsci/plugins/workflow/SubtypeInjectingStepExecution.java b/src/test/java/org/jenkinsci/plugins/workflow/SubtypeInjectingStepExecution.java
@@ -6,7 +6,6 @@
 import org.jenkinsci.plugins.workflow.cps.CpsFlowExecution;
 import org.jenkinsci.plugins.workflow.flow.FlowExecution;
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
-import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl;
 import org.jenkinsci.plugins.workflow.steps.AbstractSynchronousStepExecution;
 import org.jenkinsci.plugins.workflow.steps.StepContextParameter;
 
@@ -39,9 +38,4 @@ protected Void run() throws Exception {
         if (r!=r2 || n!=n2 || f!=f2)        throw new AssertionError("What!?");
         return null;
     }
-
-    @Override
-    public void stop(Throwable cause) throws Exception {
-        // nothing to do here
-    }
 }
diff --git a/src/test/java/org/jenkinsci/plugins/workflow/WorkflowTest.java b/src/test/java/org/jenkinsci/plugins/workflow/WorkflowTest.java
@@ -201,7 +201,6 @@ public static final class Execution extends AbstractStepExecutionImpl {
                 listener.getLogger().println("running as " + Jenkins.getAuthentication().getName() + " from " + Thread.currentThread().getName());
                 return false;
             }
-            @Override public void stop(Throwable cause) throws Exception {}
             @Override public void onResume() {
                 super.onResume();
                 try {

diff --git a/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsBodyExecutionTest.java b/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsBodyExecutionTest.java
@@ -125,11 +125,6 @@ public boolean start() throws Exception {
                         .start();
                 return false;
             }
-
-            @Override
-            public void stop(@Nonnull Throwable cause) throws Exception {
-                throw new UnsupportedOperationException();
-            }
         }
 
     }

diff --git a/src/test/java/org/jenkinsci/plugins/workflow/cps/steps/ParallelStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/cps/steps/ParallelStepTest.java
@@ -226,6 +226,32 @@ private static void failureInB() {
         throw new IllegalStateException("second problem");
     }
 
+    @Issue("JENKINS-26148")
+    @Test public void abort() {
+        story.addStep(new Statement() {
+            @Override public void evaluate() throws Throwable {
+                WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p");
+                p.setDefinition(new CpsFlowDefinition("parallel a: {def r = semaphore 'a'; echo r}, b: {semaphore 'b'}", true));
+                WorkflowRun b1 = p.scheduleBuild2(0).waitForStart();
+                SemaphoreStep.waitForStart("a/1", b1);
+                SemaphoreStep.waitForStart("b/1", b1);
+                b1.getExecutor().interrupt();
+                story.j.assertBuildStatus(Result.ABORTED, story.j.waitForCompletion(b1));
+                story.j.assertLogContains("Failed in branch a", b1);
+                story.j.assertLogContains("Failed in branch b", b1);
+                WorkflowRun b2 = p.scheduleBuild2(0).waitForStart();
+                SemaphoreStep.waitForStart("a/2", b2);
+                SemaphoreStep.waitForStart("b/2", b2);
+                SemaphoreStep.success("a/2", "finished branch a");
+                story.j.waitForMessage("finished branch a", b2);
+                b2.getExecutor().interrupt();
+                story.j.assertBuildStatus(Result.ABORTED, story.j.waitForCompletion(b2));
+                story.j.assertLogNotContains("Failed in branch a", b2);
+                story.j.assertLogContains("Failed in branch b", b2);
+            }
+        });
+    }
+
     @Test
     public void localMethodCallWithinBranch() {
         story.addStep(new Statement() {

diff --git a/src/test/java/org/jenkinsci/plugins/workflow/testMetaStep/CurveMetaStep.java b/src/test/java/org/jenkinsci/plugins/workflow/testMetaStep/CurveMetaStep.java
@@ -55,8 +55,6 @@ public static final class Execution extends AbstractStepExecutionImpl {
             return false;
         }
 
-        @Override public void stop(Throwable cause) {}
-
     }
 
     @Extension public static class DescriptorImpl extends AbstractStepDescriptorImpl {

diff --git a/src/test/java/org/jenkinsci/plugins/workflow/testMetaStep/EchoResultStep.java b/src/test/java/org/jenkinsci/plugins/workflow/testMetaStep/EchoResultStep.java
@@ -61,10 +61,6 @@ public boolean start() throws Exception {
 
             return true;
         }
-
-        @Override
-        public void stop(Throwable cause) throws Exception {
-        }
     }
 
     private static final long serialVersionUID = 1L;