Merge pull request #16 from cliffmeyers/bug/JENKINS-47219-skipped-par…

…allel-stage-wrong-status Bug/jenkins 47219 skipped parallel stage wrong status
jenkinsci · Jan 2, 2018 · f1b74e91bbb9824618c4848f319b438c302eb396 · f1b74e9
2 parents 7ea371d + 40dc7ba
commit f1b74e91bbb9824618c4848f319b438c302eb396
diff --git a/pom.xml b/pom.xml
@@ -56,7 +56,7 @@
        <dependency>
            <groupId>org.jenkins-ci.plugins.workflow</groupId>
            <artifactId>workflow-job</artifactId>
-  
-            <version>2.0</version>
+  
+            <version>2.4</version>
        </dependency>
        <dependency>
            <groupId>org.jenkins-ci.plugins.workflow</groupId>
@@ -131,5 +131,11 @@
            <version>2.13</version>
            <scope>test</scope>
        </dependency>
+  
+        <dependency>
+  
+            <groupId>org.jenkinsci.plugins</groupId>
+  
+            <artifactId>pipeline-model-definition</artifactId>
+  
+            <version>1.2.5</version>
+  
+            <scope>test</scope>
+  
+        </dependency>
    </dependencies>
 </project>
diff --git a/src/main/java/org/jenkinsci/plugins/workflow/pipelinegraphanalysis/StageStatus.java b/src/main/java/org/jenkinsci/plugins/workflow/pipelinegraphanalysis/StageStatus.java
@@ -0,0 +1,20 @@
+  
+package org.jenkinsci.plugins.workflow.pipelinegraphanalysis;
+  
+
+  
+/**
+  
+ * Duplicates some string constants used in pipeline-stage-tags-metadata plugin to avoid introducing the dependency.
+  
+ * See: https://github.com/jenkinsci/pipeline-model-definition-plugin/tree/master/pipeline-stage-tags-metadata
+  
+ *
+  
+ * @author cliffmeyers
+  
+ */
+  
+public class StageStatus {
+  
+
+  
+    public static final String TAG_NAME = "STAGE_STATUS";
+  
+
+  
+    public String getTagName() {
+  
+        return TAG_NAME;
+  
+    }
+  
+
+  
+    public static String getSkippedForConditional() {
+  
+        return "SKIPPED_FOR_CONDITIONAL";
+  
+    }
+  
+}
diff --git a/src/main/java/org/jenkinsci/plugins/workflow/pipelinegraphanalysis/StatusAndTiming.java b/src/main/java/org/jenkinsci/plugins/workflow/pipelinegraphanalysis/StatusAndTiming.java
@@ -35,6 +35,7 @@
 import org.jenkinsci.plugins.workflow.actions.ErrorAction;
 import org.jenkinsci.plugins.workflow.actions.NotExecutedNodeAction;
 import org.jenkinsci.plugins.workflow.actions.QueueItemAction;
+  
+import org.jenkinsci.plugins.workflow.actions.TagsAction;
 import org.jenkinsci.plugins.workflow.actions.ThreadNameAction;
 import org.jenkinsci.plugins.workflow.actions.TimingAction;
 import org.jenkinsci.plugins.workflow.cps.nodes.StepStartNode;
@@ -58,7 +59,6 @@
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import java.util.ArrayList;
-  
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
@@ -242,7 +242,7 @@ public static GenericStatus computeChunkStatus2(@Nonnull WorkflowRun run,
        if (exec == null) {
            return null;
        }
-  
-        if (!NotExecutedNodeAction.isExecuted(lastNode)) {
+  
+        if (!NotExecutedNodeAction.isExecuted(lastNode) || wasStageSkippedForConditional(firstNode)) {
            return GenericStatus.NOT_EXECUTED;
        }
        boolean isLastChunk = after == null || exec.isCurrentHead(lastNode);
@@ -299,6 +299,16 @@ public static GenericStatus computeChunkStatus2(@Nonnull WorkflowRun run,
        return (run.getResult() == Result.UNSTABLE) ? GenericStatus.UNSTABLE : GenericStatus.SUCCESS;
    }
+  
+    /**
+  
+     * Check if the specified {@link FlowNode} corresponds to a stage that was skipped via conditional block.
+  
+     * @param node
+  
+     * @return
+  
+     */
+  
+    private static boolean wasStageSkippedForConditional(FlowNode node) {
+  
+        TagsAction tags = node.getAction(TagsAction.class);
+  
+        return tags != null && StageStatus.getSkippedForConditional().equals(tags.getTagValue(StageStatus.TAG_NAME));
+  
+    }
+  
+
    @CheckForNull
    public static TimingInfo computeChunkTiming(@Nonnull WorkflowRun run, long internalPauseDuration, @Nonnull MemoryFlowChunk chunk) {
        return computeChunkTiming(run, internalPauseDuration, chunk.getFirstNode(), chunk.getLastNode(), chunk.getNodeAfter());

diff --git a/src/test/java/org/jenkinsci/plugins/workflow/pipelinegraphanalysis/StageTest.java b/src/test/java/org/jenkinsci/plugins/workflow/pipelinegraphanalysis/StageTest.java
@@ -1,13 +1,13 @@
 package org.jenkinsci.plugins.workflow.pipelinegraphanalysis;
-  
-import junit.framework.Assert;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
 import org.jenkinsci.plugins.workflow.graphanalysis.FlowChunkWithContext;
 import org.jenkinsci.plugins.workflow.graphanalysis.ForkScanner;
 import org.jenkinsci.plugins.workflow.graphanalysis.MemoryFlowChunk;
 import org.jenkinsci.plugins.workflow.graphanalysis.StandardChunkVisitor;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
+  
+import org.junit.Assert;
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.JenkinsRule;

diff --git a/src/test/java/org/jenkinsci/plugins/workflow/pipelinegraphanalysis/StatusAndTimingTest.java b/src/test/java/org/jenkinsci/plugins/workflow/pipelinegraphanalysis/StatusAndTimingTest.java
@@ -36,7 +36,9 @@
 import org.jenkinsci.plugins.workflow.flow.FlowExecution;
 import org.jenkinsci.plugins.workflow.graph.BlockStartNode;
 import org.jenkinsci.plugins.workflow.graph.FlowNode;
+  
+import org.jenkinsci.plugins.workflow.graphanalysis.FlowChunkWithContext;
 import org.jenkinsci.plugins.workflow.graphanalysis.ForkScanner;
+  
+import org.jenkinsci.plugins.workflow.graphanalysis.MemoryFlowChunk;
 import org.jenkinsci.plugins.workflow.graphanalysis.NoOpChunkFinder;
 import org.jenkinsci.plugins.workflow.graphanalysis.TestVisitor;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
@@ -711,4 +713,60 @@ public void queuedAndParallel() throws Exception {
        SemaphoreStep.success("wait-a/1", null);
        j.assertBuildStatusSuccess(j.waitForCompletion(b1));
    }
+  
+
+  
+    @Test
+  
+    @Issue("JENKINS-47219")
+  
+    public void parallelStagesOneSkipped() throws Exception {
+  
+        WorkflowJob job = j.jenkins.createProject(WorkflowJob.class, "parallel stages, one skipped job");
+  
+        job.setDefinition(new CpsFlowDefinition("" +
+  
+                "pipeline { \n" +
+  
+                "   agent any \n" +
+  
+                "   stages { \n" +
+  
+                "       stage('Run Tests') { \n"+
+  
+                "           parallel { \n" +
+  
+                "               stage('Test on Windows') { \n" +
+  
+                "                   when { \n" +
+  
+                "                       branch 'cake' \n" +
+  
+                "                   } \n"+
+  
+                "                   steps { \n"+
+  
+                "                       echo 'hello world' \n"+
+  
+                "                   } \n"+
+  
+                "               } \n"+
+  
+                "               stage('Test on Linux') { \n" +
+  
+                "                   steps { \n"+
+  
+                "                       echo 'hello world' \n"+
+  
+                "                   } \n"+
+  
+                "               } \n"+
+  
+                "           } \n"+
+  
+                "       } \n"+
+  
+                "   } \n"+
+  
+                "} \n",
+  
+                false));
+  
+
+  
+        WorkflowRun build = j.assertBuildStatusSuccess(job.scheduleBuild2(0));
+  
+
+  
+        ForkScanner scan = new ForkScanner();
+  
+        scan.setup(build.getExecution().getCurrentHeads());
+  
+        StageTest.CollectingChunkVisitor visitor = new StageTest.CollectingChunkVisitor();
+  
+        scan.visitSimpleChunks(visitor, new StageChunkFinder());
+  
+        ArrayList<MemoryFlowChunk> stages = visitor.getChunks();
+  
+
+  
+        Assert.assertEquals(3, stages.size());
+  
+
+  
+        MemoryFlowChunk chunkTests = stages.get(0);
+  
+        Assert.assertEquals("Run Tests", chunkTests.getFirstNode().getDisplayName());
+  
+        Assert.assertEquals(GenericStatus.SUCCESS, getStatusForChunk(build, chunkTests));
+  
+
+  
+        MemoryFlowChunk chunkWindows = stages.get(1);
+  
+        Assert.assertEquals("Test on Windows", chunkWindows.getFirstNode().getDisplayName());
+  
+        Assert.assertEquals(GenericStatus.NOT_EXECUTED, getStatusForChunk(build, chunkWindows));
+  
+
+  
+        MemoryFlowChunk chunkLinux = stages.get(2);
+  
+        Assert.assertEquals("Test on Linux", chunkLinux.getFirstNode().getDisplayName());
+  
+        Assert.assertEquals(GenericStatus.SUCCESS, getStatusForChunk(build, chunkLinux));
+  
+    }
+  
+
+  
+    private static GenericStatus getStatusForChunk(WorkflowRun run, FlowChunkWithContext chunk) {
+  
+        return StatusAndTiming.computeChunkStatus2(run, chunk.getNodeBefore(), chunk.getFirstNode(), chunk.getLastNode(), chunk.getNodeAfter());
+  
+    }
 }