Merge pull request #30 from jenkinsci/fix-node-display-JENKINS-37945

Fix info popup for stages with errors deeply inside blocks [JENKINS-37945]
jenkinsci · Sep 30, 2016 · 7e01342 · 7e01342
2 parents ef8a7c7 + 15e15a1
commit 7e01342
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 16 deletions.
diff --git a/rest-api/pom.xml b/rest-api/pom.xml
@@ -47,7 +47,7 @@
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-api</artifactId>
-            <version>2.2</version> <!-- allows consuming the new APIs -->
+            <version>2.4</version> <!-- allows consuming the new APIs -->
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins</groupId>

diff --git a/rest-api/src/main/java/com/cloudbees/workflow/rest/external/ChunkVisitor.java b/rest-api/src/main/java/com/cloudbees/workflow/rest/external/ChunkVisitor.java
@@ -6,6 +6,7 @@
 import org.jenkinsci.plugins.workflow.actions.ErrorAction;
 import org.jenkinsci.plugins.workflow.actions.NotExecutedNodeAction;
 import org.jenkinsci.plugins.workflow.actions.TimingAction;
+import org.jenkinsci.plugins.workflow.graph.AtomNode;
 import org.jenkinsci.plugins.workflow.graph.BlockEndNode;
 import org.jenkinsci.plugins.workflow.graph.FlowNode;
 import org.jenkinsci.plugins.workflow.graphanalysis.ForkScanner;
@@ -44,6 +45,10 @@ public Collection<StageNodeExt> getStages() {
     }
 
     public static AtomFlowNodeExt makeAtomNode(@Nonnull WorkflowRun run, @CheckForNull FlowNode beforeNode, @Nonnull FlowNode node, @CheckForNull FlowNode next) {
+        if (!(node instanceof AtomNode)) {
+            return null;
+        }
+
         long pause = PauseAction.getPauseDuration(node);
         TimingInfo times = StatusAndTiming.computeChunkTiming(run, pause, node, node, next);
         ExecDuration dur = (times == null) ? new ExecDuration() : new ExecDuration(times);
@@ -145,7 +150,10 @@ public void atomNode(@CheckForNull FlowNode before, @Nonnull FlowNode atomNode,
 
         // TODO this is rather inefficient, we should optimize to use a circular buffer or ArrayList with limited size
         // And then only create the node container objects when we hit the start (doing timing ETC at that point)
-        stageContents.push(makeAtomNode(run, before, atomNode, after));
+        AtomFlowNodeExt ext = makeAtomNode(run, before, atomNode, after);
+        if (ext != null) {
+            stageContents.push(ext);
+        }
         stageNodeIds.add(atomNode.getId());
     }
 }
diff --git a/rest-api/src/test/java/com/cloudbees/workflow/rest/endpoints/FlowNodeAPITest.java b/rest-api/src/test/java/com/cloudbees/workflow/rest/endpoints/FlowNodeAPITest.java
@@ -193,7 +193,7 @@ public void test_failed_flow() throws Exception {
         Assert.assertEquals("Build", stageDesc.getName());
         Assert.assertEquals(StatusExt.FAILED, stageDesc.getStatus());
         Assert.assertEquals("/jenkins/job/Noddy%20Job/1/execution/node/5/wfapi/describe", stageDesc.get_links().self.href);
-        Assert.assertEquals(4, stageDesc.getStageFlowNodes().size());
+        Assert.assertEquals(1, stageDesc.getStageFlowNodes().size());
         Assert.assertEquals("6", stageDesc.getStageFlowNodes().get(0).getId());
         Assert.assertEquals(jenkinsRule.jenkins.getDescriptorByType(ErrorStep.DescriptorImpl.class).getDisplayName(), stageDesc.getStageFlowNodes().get(0).getName());
         Assert.assertEquals("/jenkins/job/Noddy%20Job/1/execution/node/6/wfapi/describe", stageDesc.getStageFlowNodes().get(0).get_links().self.href);

diff --git a/rest-api/src/test/java/com/cloudbees/workflow/rest/endpoints/JobAndRunAPITest.java b/rest-api/src/test/java/com/cloudbees/workflow/rest/endpoints/JobAndRunAPITest.java
@@ -178,7 +178,7 @@ public void testEmptyStageHandling() throws Exception {
         StageNodeExt first = run.getStages().get(0);
         StageNodeExt second = run.getStages().get(1);
         Assert.assertEquals(0, first.getStageFlowNodes().size());
-        Assert.assertEquals(2, second.getStageFlowNodes().size());
+        Assert.assertEquals(1, second.getStageFlowNodes().size());
         assertStageInfoOkay(first, false);
         assertStageInfoOkay(second, true);
         Assert.assertTrue(first.getDurationMillis() > 0);
@@ -229,10 +229,7 @@ public void testErrorHandling() throws Exception {
 
         StageNodeExt finalStage = stages.get(2);
         AtomFlowNodeExt finalNode = finalStage.getStageFlowNodes().get(finalStage.getStageFlowNodes().size()-1);
-        Assert.assertEquals(StatusExt.FAILED, finalNode.getStatus());
-        Assert.assertNotNull(finalNode.getError());
-        Assert.assertNotNull(finalNode.getError().getMessage());
-        Assert.assertNotNull(finalNode.getError().getType());
+        Assert.assertEquals(StatusExt.SUCCESS, finalNode.getStatus());
     }
 
 

diff --git a/rest-api/src/test/java/com/cloudbees/workflow/rest/endpoints/ParallelStepTest.java b/rest-api/src/test/java/com/cloudbees/workflow/rest/endpoints/ParallelStepTest.java
@@ -52,10 +52,10 @@ public void test_success_flow() throws Exception {
                 "  stage ('Stage 1');" +
                 "  parallel( " +
                 "       a: { " +
-                "           echo('echo a'); " +
+                "           echo('echo a'); " + // ID=10
                 "       }, " +
                 "       b: { " +
-                "           echo('echo b'); " +
+                "           echo('echo b'); " + // ID=12
                 "       } " +
                 "  );" +
                 "  stage ('Stage 2');" +
@@ -92,17 +92,17 @@ public void test_success_flow() throws Exception {
         jsonResponse = stageDescription.getWebResponse().getContentAsString();
 
         StageNodeExt stage1Desc = jsonReadWrite.fromString(jsonResponse, StageNodeExt.class);
-        Assert.assertEquals(7, stage1Desc.getStageFlowNodes().size());
-        Assert.assertEquals("6", stage1Desc.getStageFlowNodes().get(0).getId());
-        Assert.assertEquals("Print Message", stage1Desc.getStageFlowNodes().get(2).getName());
-        Assert.assertEquals("10", stage1Desc.getStageFlowNodes().get(5).getId());
-        Assert.assertEquals("Print Message", stage1Desc.getStageFlowNodes().get(5).getName());
+        Assert.assertEquals(2, stage1Desc.getStageFlowNodes().size());
+        Assert.assertEquals("10", stage1Desc.getStageFlowNodes().get(0).getId());
+        Assert.assertEquals("Print Message", stage1Desc.getStageFlowNodes().get(0).getName());
+        Assert.assertEquals("12", stage1Desc.getStageFlowNodes().get(1).getId());
+        Assert.assertEquals("Print Message", stage1Desc.getStageFlowNodes().get(1).getName());
 
         stageDescription = webClient.goTo("job/Noddy%20Job/1/execution/node/15/wfapi/describe", "application/json");
         jsonResponse = stageDescription.getWebResponse().getContentAsString();
 
         StageNodeExt stage2Desc = jsonReadWrite.fromString(jsonResponse, StageNodeExt.class);
-        Assert.assertEquals(4, stage2Desc.getStageFlowNodes().size());
+        Assert.assertEquals(1, stage2Desc.getStageFlowNodes().size());
         Assert.assertEquals("16", stage2Desc.getStageFlowNodes().get(0).getId());
         Assert.assertEquals("Print Message", stage2Desc.getStageFlowNodes().get(0).getName());
     }

diff --git a/ui/src/main/js/view/templates/info-action-popover.less b/ui/src/main/js/view/templates/info-action-popover.less
@@ -19,6 +19,7 @@
     font-family: inherit;
     font-size: inherit;
     color: inherit;
+    word-break: break-all;
   }
   .btn-toolbar:before, .btn-toolbar:after{
     display: table;