From 05a2c0d27f2e867d5146c0b13874ca45caef031a Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Fri, 5 Feb 2021 23:05:46 +0000 Subject: [PATCH 1/7] Add failure to title --- .../checks/status/FlowExecutionAnalyzer.java | 33 +++++++++++++++++-- .../BuildStatusChecksPublisherITest.java | 4 +-- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java b/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java index 299ae580..27b4ae21 100644 --- a/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java +++ b/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java @@ -15,6 +15,8 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Optional; @@ -22,6 +24,7 @@ import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; +import java.util.stream.Stream; class FlowExecutionAnalyzer { @@ -137,6 +140,8 @@ ChecksOutput extractOutput() { .withTruncationText(TRUNCATED_MESSAGE); indentationStack.clear(); + List potentialTitles = new ArrayList<>(); + table.getRows().forEach(row -> { final FlowNode flowNode = row.getNode(); @@ -153,17 +158,38 @@ ChecksOutput extractOutput() { final Pair nodeInfo = stageOrBranchName.map(s -> processStageOrBranchRow(row, s)) .orElseGet(() -> processErrorOrWarningRow(row, errorAction, warningAction)); + // the last title will be used in the ChecksOutput (if any are found) + if (!stageOrBranchName.isPresent()) { + potentialTitles.add(getPotentialTitle(flowNode, errorAction)); + } + textBuilder.addText(nodeInfo.getLeft()); summaryBuilder.addText(nodeInfo.getRight()); }); + + String title = extractOutputTitle(); + if (!potentialTitles.isEmpty()) { + title = potentialTitles.get(potentialTitles.size() - 1); + } + return new ChecksOutput.ChecksOutputBuilder() - .withTitle(extractOutputTitle()) + .withTitle(title) .withSummary(summaryBuilder.build()) .withText(textBuilder.build()) .build(); } + private String getPotentialTitle(FlowNode flowNode, ErrorAction errorAction) { + final String whereBuildFailed = String.format("%s in '%s' step", errorAction == null ? "warning" : "error", flowNode.getDisplayFunctionName()); + return flowNode.getParents().stream() + .map(FlowExecutionAnalyzer::getStageOrBranchName) + .flatMap(o -> o.map(Stream::of).orElseGet(Stream::empty)) + .map(blockName -> blockName + ": " + whereBuildFailed) + .findFirst() + .orElse(whereBuildFailed); + } + @CheckForNull private static String getLog(final FlowNode flowNode) { LogAction logAction = flowNode.getAction(LogAction.class); @@ -174,7 +200,10 @@ private static String getLog(final FlowNode flowNode) { if (logAction.getLogText().writeLogTo(0, out) == 0) { return null; } - return out.toString(StandardCharsets.UTF_8.toString()); + + String outputString = out.toString(StandardCharsets.UTF_8.toString()); + // strip ansi color codes + return outputString.replaceAll("\u001B\\[[;\\d]*m", ""); } catch (IOException e) { LOGGER.log(Level.WARNING, String.format("Failed to extract logs for step '%s'", flowNode.getDisplayName()).replaceAll("[\r\n]", ""), e); diff --git a/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java b/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java index e921ba53..205cb488 100644 --- a/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java +++ b/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java @@ -172,7 +172,7 @@ public void shouldPublishStageDetails() { // Details 6, p1s1 has finished and emitted unstable details = checksDetails.get(6); assertThat(details.getOutput()).isPresent().get().satisfies(output -> { - assertThat(output.getTitle()).isPresent().get().isEqualTo("Unstable"); + assertThat(output.getTitle()).isPresent().get().isEqualTo("p1s1: warning in 'unstable' step"); assertThat(output.getSummary()).isPresent().get().asString().isEqualToIgnoringNewLines("" + "### `In parallel / p1 / p1s1 / Set stage result to unstable`\n" + "Warning in `unstable` step, with arguments `something went wrong`.\n" @@ -195,7 +195,7 @@ public void shouldPublishStageDetails() { assertThat(details.getStatus()).isEqualTo(ChecksStatus.COMPLETED); assertThat(details.getConclusion()).isEqualTo(ChecksConclusion.FAILURE); assertThat(details.getOutput()).isPresent().get().satisfies(output -> { - assertThat(output.getTitle()).isPresent().get().isEqualTo("Failure"); + assertThat(output.getTitle()).isPresent().get().isEqualTo("Fails: error in 'archiveArtifacts' step"); assertThat(output.getSummary()).isPresent().get().asString().matches(Pattern.compile(".*" + "### `In parallel / p1 / p1s1 / Set stage result to unstable`\\s+" + "Warning in `unstable` step, with arguments `something went wrong`\\.\\s+" From 741966f59a3d649ceb6f527f20e1a317d0ea257a Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Fri, 5 Feb 2021 23:25:30 +0000 Subject: [PATCH 2/7] Copy code from junit plugin --- .../checks/status/FlowExecutionAnalyzer.java | 63 +++++++++++++++++-- .../BuildStatusChecksPublisherITest.java | 2 +- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java b/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java index 27b4ae21..f8b9d0bf 100644 --- a/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java +++ b/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java @@ -1,15 +1,19 @@ package io.jenkins.plugins.checks.status; import edu.umd.cs.findbugs.annotations.CheckForNull; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.model.Result; import hudson.model.Run; import io.jenkins.plugins.checks.api.ChecksOutput; import io.jenkins.plugins.checks.api.TruncatedString; +import org.apache.commons.collections.iterators.ReverseListIterator; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.tuple.Pair; import org.jenkinsci.plugins.workflow.actions.*; import org.jenkinsci.plugins.workflow.flow.FlowExecution; import org.jenkinsci.plugins.workflow.graph.FlowNode; +import org.jenkinsci.plugins.workflow.graph.StepNode; +import org.jenkinsci.plugins.workflow.steps.StepDescriptor; import org.jenkinsci.plugins.workflow.support.visualization.table.FlowGraphTable; import java.io.ByteArrayOutputStream; @@ -182,12 +186,59 @@ ChecksOutput extractOutput() { private String getPotentialTitle(FlowNode flowNode, ErrorAction errorAction) { final String whereBuildFailed = String.format("%s in '%s' step", errorAction == null ? "warning" : "error", flowNode.getDisplayFunctionName()); - return flowNode.getParents().stream() - .map(FlowExecutionAnalyzer::getStageOrBranchName) - .flatMap(o -> o.map(Stream::of).orElseGet(Stream::empty)) - .map(blockName -> blockName + ": " + whereBuildFailed) - .findFirst() - .orElse(whereBuildFailed); + + List enclosingStagesAndParallels = getEnclosingStagesAndParallels(flowNode); + List enclosingBlockNames = getEnclosingBlockNames(enclosingStagesAndParallels); + + return StringUtils.join(new ReverseListIterator(enclosingBlockNames), "/") + ": " + whereBuildFailed; + } + + private static boolean isStageNode(@NonNull FlowNode node) { + if (node instanceof StepNode) { + StepDescriptor d = ((StepNode) node).getDescriptor(); + return d != null && d.getFunctionName().equals("stage"); + } else { + return false; + } + } + + /** + * Get the stage and parallel branch start node IDs (not the body nodes) for this node, innermost first. + * @param node A flownode. + * @return A nonnull, possibly empty list of stage/parallel branch start nodes, innermost first. + */ + @NonNull + private static List getEnclosingStagesAndParallels(FlowNode node) { + List enclosingBlocks = new ArrayList<>(); + for (FlowNode enclosing : node.getEnclosingBlocks()) { + if (enclosing != null && enclosing.getAction(LabelAction.class) != null) { + if (isStageNode(enclosing) || + (enclosing.getAction(ThreadNameAction.class) != null)) { + enclosingBlocks.add(enclosing); + } + } + } + + return enclosingBlocks; + } + + @NonNull + private static List getEnclosingBlockNames(@NonNull List nodes) { + List names = new ArrayList<>(); + for (FlowNode n : nodes) { + ThreadNameAction threadNameAction = n.getPersistentAction(ThreadNameAction.class); + LabelAction labelAction = n.getPersistentAction(LabelAction.class); + if (threadNameAction != null) { + // If we're on a parallel branch with the same name as the previous (inner) node, that generally + // means we're in a Declarative parallel stages situation, so don't add the redundant branch name. + if (names.isEmpty() || !threadNameAction.getThreadName().equals(names.get(names.size()-1))) { + names.add(threadNameAction.getThreadName()); + } + } else if (labelAction != null) { + names.add(labelAction.getDisplayName()); + } + } + return names; } @CheckForNull diff --git a/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java b/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java index 205cb488..f28ceff7 100644 --- a/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java +++ b/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java @@ -172,7 +172,7 @@ public void shouldPublishStageDetails() { // Details 6, p1s1 has finished and emitted unstable details = checksDetails.get(6); assertThat(details.getOutput()).isPresent().get().satisfies(output -> { - assertThat(output.getTitle()).isPresent().get().isEqualTo("p1s1: warning in 'unstable' step"); + assertThat(output.getTitle()).isPresent().get().isEqualTo("In parallel/p1/p1s1: warning in 'unstable' step"); assertThat(output.getSummary()).isPresent().get().asString().isEqualToIgnoringNewLines("" + "### `In parallel / p1 / p1s1 / Set stage result to unstable`\n" + "Warning in `unstable` step, with arguments `something went wrong`.\n" From dc22fbe5bfb8eceec5cc21897510b18738927e7d Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Fri, 5 Feb 2021 23:29:17 +0000 Subject: [PATCH 3/7] Remove unused imports --- .../io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java b/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java index f8b9d0bf..f187fb09 100644 --- a/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java +++ b/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java @@ -20,7 +20,6 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Optional; @@ -28,7 +27,6 @@ import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; -import java.util.stream.Stream; class FlowExecutionAnalyzer { From 538f96bb7dae7009e0cfb685de9f338095d2fea1 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sat, 6 Feb 2021 08:31:52 +0000 Subject: [PATCH 4/7] Minor refactoring --- .../checks/status/FlowExecutionAnalyzer.java | 42 ++++++++----------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java b/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java index f187fb09..d2973655 100644 --- a/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java +++ b/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java @@ -142,41 +142,30 @@ ChecksOutput extractOutput() { .withTruncationText(TRUNCATED_MESSAGE); indentationStack.clear(); - List potentialTitles = new ArrayList<>(); - - table.getRows().forEach(row -> { + String title = null; + for (FlowGraphTable.Row row : table.getRows()) { final FlowNode flowNode = row.getNode(); Optional stageOrBranchName = getStageOrBranchName(flowNode); ErrorAction errorAction = flowNode.getError(); WarningAction warningAction = flowNode.getPersistentAction(WarningAction.class); - if (!stageOrBranchName.isPresent() - && errorAction == null - && warningAction == null) { - return; - } + if (stageOrBranchName.isPresent() || errorAction != null || warningAction != null) { + final Pair nodeInfo = stageOrBranchName.map(s -> processStageOrBranchRow(row, s)) + .orElseGet(() -> processErrorOrWarningRow(row, errorAction, warningAction)); - final Pair nodeInfo = stageOrBranchName.map(s -> processStageOrBranchRow(row, s)) - .orElseGet(() -> processErrorOrWarningRow(row, errorAction, warningAction)); + // the last title will be used in the ChecksOutput (if any are found) + if (!stageOrBranchName.isPresent()) { + title = getPotentialTitle(flowNode, errorAction); + } - // the last title will be used in the ChecksOutput (if any are found) - if (!stageOrBranchName.isPresent()) { - potentialTitles.add(getPotentialTitle(flowNode, errorAction)); + textBuilder.addText(nodeInfo.getLeft()); + summaryBuilder.addText(nodeInfo.getRight()); } - - textBuilder.addText(nodeInfo.getLeft()); - summaryBuilder.addText(nodeInfo.getRight()); - }); - - - String title = extractOutputTitle(); - if (!potentialTitles.isEmpty()) { - title = potentialTitles.get(potentialTitles.size() - 1); } return new ChecksOutput.ChecksOutputBuilder() - .withTitle(title) + .withTitle(extractOutputTitle(title)) .withSummary(summaryBuilder.build()) .withText(textBuilder.build()) .build(); @@ -260,7 +249,7 @@ private static String getLog(final FlowNode flowNode) { } } - private String extractOutputTitle() { + private String extractOutputTitle(String title) { Result result = run.getResult(); if (result == null) { return "In progress"; @@ -268,6 +257,11 @@ private String extractOutputTitle() { if (result.isBetterOrEqualTo(Result.SUCCESS)) { return "Success"; } + + if (title != null) { + return title; + } + if (result.isBetterOrEqualTo(Result.UNSTABLE)) { return "Unstable"; } From 88c8dd650a1cf726431071097447e132048554e0 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sat, 6 Feb 2021 08:42:32 +0000 Subject: [PATCH 5/7] Checkstyle --- .../plugins/checks/status/FlowExecutionAnalyzer.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java b/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java index d2973655..0e970352 100644 --- a/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java +++ b/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java @@ -184,7 +184,8 @@ private static boolean isStageNode(@NonNull FlowNode node) { if (node instanceof StepNode) { StepDescriptor d = ((StepNode) node).getDescriptor(); return d != null && d.getFunctionName().equals("stage"); - } else { + } + else { return false; } } @@ -199,8 +200,7 @@ private static List getEnclosingStagesAndParallels(FlowNode node) { List enclosingBlocks = new ArrayList<>(); for (FlowNode enclosing : node.getEnclosingBlocks()) { if (enclosing != null && enclosing.getAction(LabelAction.class) != null) { - if (isStageNode(enclosing) || - (enclosing.getAction(ThreadNameAction.class) != null)) { + if (isStageNode(enclosing) || enclosing.getAction(ThreadNameAction.class) != null) { enclosingBlocks.add(enclosing); } } @@ -218,10 +218,11 @@ private static List getEnclosingBlockNames(@NonNull List nodes if (threadNameAction != null) { // If we're on a parallel branch with the same name as the previous (inner) node, that generally // means we're in a Declarative parallel stages situation, so don't add the redundant branch name. - if (names.isEmpty() || !threadNameAction.getThreadName().equals(names.get(names.size()-1))) { + if (names.isEmpty() || !threadNameAction.getThreadName().equals(names.get(names.size() - 1))) { names.add(threadNameAction.getThreadName()); } - } else if (labelAction != null) { + } + else if (labelAction != null) { names.add(labelAction.getDisplayName()); } } From 6388b262acb4bcfc41d3145162f38df7e8893543 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sat, 6 Feb 2021 09:03:05 +0000 Subject: [PATCH 6/7] Add a bit more coverage --- .../BuildStatusChecksPublisherITest.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java b/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java index f28ceff7..5f37f028 100644 --- a/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java +++ b/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java @@ -229,6 +229,26 @@ public void shouldPublishStageDetails() { }); } + @Test + public void shouldPublishSimplePipeline() { + PROPERTIES.setApplicable(true); + PROPERTIES.setSkipped(false); + PROPERTIES.setName("Test Status"); + WorkflowJob job = createPipeline(); + + job.setDefinition(new CpsFlowDefinition("" + + "node {\n" + + " echo 'Hello, world'" + + "}", true)); + + buildWithResult(job, Result.SUCCESS); + + List checksDetails = PUBLISHER_FACTORY.getPublishedChecks(); + + ChecksDetails details = checksDetails.get(1); + assertThat(details.getOutput()).isPresent().get().satisfies(output -> assertThat(output.getTitle()).isPresent().get().isEqualTo("Success")); + } + static class ChecksProperties extends AbstractStatusChecksProperties { private boolean applicable; private boolean skipped; From f08b9a9ff2548d8aec88caef172d508acfee6f98 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sat, 6 Feb 2021 09:14:41 +0000 Subject: [PATCH 7/7] Javadoc on test --- .../plugins/checks/status/BuildStatusChecksPublisherITest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java b/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java index 5f37f028..e5180775 100644 --- a/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java +++ b/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java @@ -229,6 +229,9 @@ public void shouldPublishStageDetails() { }); } + /** + * Validates the a simple successful pipeline works. + */ @Test public void shouldPublishSimplePipeline() { PROPERTIES.setApplicable(true);