Merge pull request #39 from jenkinsci/fix-bismuth-long-parallelbranch…

…-JENKINS-38536 Fix finding the last node of a parallel with one long-running step [JENKINS-38536]
jenkinsci · Jun 5, 2017 · c591aef · c591aef
2 parents af2aec2 + f750d44
commit c591aef
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 5 deletions.
diff --git a/pom.xml b/pom.xml
@@ -63,6 +63,7 @@
     </pluginRepositories>
     <properties>
         <jenkins.version>1.642.3</jenkins.version>
+        <no-test-jar>false</no-test-jar>
     </properties>
     <dependencies>
         <dependency>

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScanner.java b/src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScanner.java
@@ -646,18 +646,24 @@ static FlowNode findLastRunningNode(@Nonnull List<FlowNode> candidates) {
         } else if (candidates.size() == 1) {
             return candidates.get(0);
         } else {
-            FlowNode returnOut = candidates.get(0);
+            FlowNode lastFound = candidates.get(0);
             long startTime = Long.MIN_VALUE;
             for(FlowNode f : candidates) {
                 TimingAction ta = f.getAction(TimingAction.class);
-                // Null timing with multiple heads is probably a node where the GraphListener hasn't fired to add TimingAction yet
+                // Null timing with multiple heads is probably the newest node where the GraphListener hasn't fired to add TimingAction yet
                 long myStart = (ta == null) ? System.currentTimeMillis() : ta.getStartTime();
-                if (myStart > startTime) {
-                    returnOut = f;
+                if (f instanceof BlockEndNode != lastFound instanceof BlockEndNode) {
+                    // A BlockEndNode isn't currently running, this represents the case where some branches are done
+                    if (!(f instanceof BlockEndNode)) {
+                        lastFound = f;
+                        startTime = myStart;
+                    }
+                } else if (myStart > startTime) {
+                    lastFound = f;
                     startTime = myStart;
                 }
             }
-            return returnOut;
+            return lastFound;
         }
     }
 

diff --git a/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScannerTest.java b/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScannerTest.java
@@ -934,6 +934,36 @@ public void testParallelsWithDuplicateEvents() throws Exception {
         Assert.assertEquals(2, parallelBranchEndCount);
     }
 
+    /** Covers finding the right parallel end node in cases where we have one long-running step on an incomplete branch.
+     *  Previously we'd assume the BlockEndNode of the completed branch was the last-running branch because it had the
+     *   most recent node addition, but the longer-running non-end node needs to take precedence.
+     */
+    @Issue("JENKINS-38536")
+    @Test
+    public void testPartlyCompletedParallels() throws Exception {
+        String jobScript = ""+
+                "stage 'first'\n" +
+                "parallel 'long' : { sleep 60; }, \n" +  // Needs to be in-progress
+                "         'short': { sleep 2; }";  // Needs to have completed, and SemaphoreStep alone doesn't cut it
+
+        // This must be amateur science fiction because the exposition for the setting goes on FOREVER
+        ForkScanner scan = new ForkScanner();
+        TestVisitor tv = new TestVisitor();
+        WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "parallelTimes");
+        job.setDefinition(new CpsFlowDefinition(jobScript));
+        WorkflowRun run = job.scheduleBuild2(0).getStartCondition().get();
+        Thread.sleep(4000);  // Allows enough time for the shorter branch to finish and write its BlockEndNode
+        FlowExecution exec = run.getExecution();
+        List<FlowNode> heads = exec.getCurrentHeads();
+        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());
+        sanityTestIterationAndVisiter(heads);
+        run.doKill(); // Avoid waiting for long branch completion
+    }
+
+    /** 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 {