From c6e125f24d4c7240444d10f195eae859996d51e1 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sun, 5 Sep 2021 21:21:26 +0300 Subject: [PATCH 1/2] Use message from "error" step in title In status checks, if the run failed because of an "error" pipeline step, then use the first line of the error message in the title of ChecksOutput, instead of "error in 'error' step". However, keep the original behaviour if the error message does not match the "message" argument of the step, if the step has other arguments, or if the first line of the message is empty. Update the test so that it keeps passing, --- .../checks/status/FlowExecutionAnalyzer.java | 52 +++++++++++++++++-- .../BuildStatusChecksPublisherITest.java | 2 +- 2 files changed, 50 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 4ed9298a..a6dde93c 100644 --- a/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java +++ b/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Stack; import java.util.logging.Level; @@ -175,9 +176,16 @@ ChecksOutput extractOutput() { .build(); } - private String getPotentialTitle(final FlowNode flowNode, final ErrorAction errorAction) { - final String whereBuildFailed = String.format("%s in '%s' step", errorAction == null ? "warning" : "error", - flowNode.getDisplayFunctionName()); + private String getPotentialTitle(@NonNull final FlowNode flowNode, final ErrorAction errorAction) { + String whereBuildFailed = null; + if (errorAction != null + && isTrivialErrorOrUnstable(flowNode, "error", errorAction.getDisplayName())) { + whereBuildFailed = summarize(errorAction.getDisplayName()); + } + if (whereBuildFailed == null) { + whereBuildFailed = String.format("%s in '%s' step", errorAction == null ? "warning" : "error", + flowNode.getDisplayFunctionName()); + } List enclosingStagesAndParallels = getEnclosingStagesAndParallels(flowNode); List enclosingBlockNames = getEnclosingBlockNames(enclosingStagesAndParallels); @@ -185,6 +193,44 @@ private String getPotentialTitle(final FlowNode flowNode, final ErrorAction erro return StringUtils.join(new ReverseListIterator(enclosingBlockNames), "/") + ": " + whereBuildFailed; } + /** + * Check whether the node is an "error" or "unstable" step with the + * specified message and no other arguments. In that case, the caller + * can simplify the output. + * + * @param node The flow node to examine. + * @param name The expected name of the step; either "error" or "unstable". + * @param message The error or warning that was reported from the node. + */ + private static boolean isTrivialErrorOrUnstable(@NonNull final FlowNode node, + @NonNull final String name, + final String message) { + if (node instanceof StepNode) { + StepDescriptor d = ((StepNode) node).getDescriptor(); + if (d != null && d.getFunctionName().equals(name)) { + Map arguments = ArgumentsAction.getArguments(node); + if (arguments != null && arguments.size() == 1) { + Object argument = arguments.get("message"); + return argument instanceof String + && argument.equals(message); + } + } + } + + return false; + } + + private static String summarize(String message) { + if (message != null) { + final String firstLine = message.split("\r?\n", 2)[0]; + if (!firstLine.isEmpty()) { + return firstLine; + } + } + + return null; + } + private static boolean isStageNode(@NonNull final FlowNode node) { if (node instanceof StepNode) { StepDescriptor d = ((StepNode) node).getDescriptor(); 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 6f8609f3..f40683fa 100644 --- a/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java +++ b/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java @@ -199,7 +199,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("Fails: error in 'error' step"); + assertThat(output.getTitle()).isPresent().get().isEqualTo("Fails: a fatal error occurs"); 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 8ba8c0dbdbf98921cfc81ba60a1f803a574eb721 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sun, 5 Sep 2021 22:00:27 +0300 Subject: [PATCH 2/2] Omit step name and arguments if trivial In status checks, omit the name and arguments of the "error" or "unstable" step from the output if the error or warning message matches the "message" argument of the step and there are no other arguments. Update the tests so that they keep passing. --- .../checks/status/FlowExecutionAnalyzer.java | 22 ++++++++++++------- .../BuildStatusChecksPublisherITest.java | 4 ---- 2 files changed, 14 insertions(+), 12 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 a6dde93c..239fe146 100644 --- a/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java +++ b/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java @@ -107,14 +107,20 @@ private Pair processErrorOrWarningRow(final FlowGraphTable.Row r nodeSummaryBuilder.append(String.format("### `%s`%n", String.join(" / ", location))); - nodeSummaryBuilder.append(String.format("%s in `%s` step", errorAction == null ? "Warning" : "Error", - flowNode.getDisplayFunctionName())); - String arguments = ArgumentsAction.getStepArgumentsAsString(flowNode); - if (arguments == null) { - nodeSummaryBuilder.append(".\n"); - } - else { - nodeSummaryBuilder.append(String.format(", with arguments `%s`.%n", arguments)); + if (errorAction != null && isTrivialErrorOrUnstable(flowNode, "error", errorAction.getDisplayName()) + || warningAction != null && isTrivialErrorOrUnstable(flowNode, "unstable", warningAction.getMessage())) { + // Suppress the step name and arguments because they + // would be mostly redundant with other text. + } else { + nodeSummaryBuilder.append(String.format("%s in `%s` step", errorAction == null ? "Warning" : "Error", + flowNode.getDisplayFunctionName())); + String arguments = ArgumentsAction.getStepArgumentsAsString(flowNode); + if (arguments == null) { + nodeSummaryBuilder.append(".\n"); + } + else { + nodeSummaryBuilder.append(String.format(", with arguments `%s`.%n", arguments)); + } } nodeTextBuilder.append(String.join("", Collections.nCopies(indentationStack.size() + 1, " "))); 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 f40683fa..19d212c5 100644 --- a/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java +++ b/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java @@ -179,7 +179,6 @@ public void shouldPublishStageDetails() { 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" + "```\n" + "something went wrong\n" + "```\n" @@ -202,12 +201,10 @@ public void shouldPublishStageDetails() { assertThat(output.getTitle()).isPresent().get().isEqualTo("Fails: a fatal error occurs"); 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+" + "```\\s+" + "something went wrong\\s+" + "```\\s+" + "### `Fails / Error signal`\\s+" - + "Error in `error` step, with arguments `a fatal error occurs`\\.\\s+" + "```\\s+" + "a fatal error occurs\\s+" + "```\\s+", Pattern.DOTALL)); @@ -267,7 +264,6 @@ public void shouldPublishStageDetailsWithoutLogsIfRequested() { 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+" + "```\\s+" + "something went wrong\\s+" + "```\\s+"