From c6f203181b25a9744bf2bcaee756f1893f59ca7c Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Thu, 18 Apr 2019 14:17:17 -0400 Subject: [PATCH 1/7] [JENKINS-39203] Create new action to mark a FlowNode as unstable --- .../plugins/workflow/actions/ErrorAction.java | 2 +- .../workflow/actions/WarningAction.java | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 src/main/java/org/jenkinsci/plugins/workflow/actions/WarningAction.java diff --git a/src/main/java/org/jenkinsci/plugins/workflow/actions/ErrorAction.java b/src/main/java/org/jenkinsci/plugins/workflow/actions/ErrorAction.java index cbc27d2e..43e27cb4 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/actions/ErrorAction.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/actions/ErrorAction.java @@ -35,7 +35,7 @@ import org.apache.commons.io.output.NullOutputStream; /** - * Attached to {@link AtomNode} that caused an error. + * Attached to {@link FlowNode} that caused an error. * * This has to be Action because it's added after a node is created. */ diff --git a/src/main/java/org/jenkinsci/plugins/workflow/actions/WarningAction.java b/src/main/java/org/jenkinsci/plugins/workflow/actions/WarningAction.java new file mode 100644 index 00000000..71f25a63 --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/workflow/actions/WarningAction.java @@ -0,0 +1,36 @@ +package org.jenkinsci.plugins.workflow.actions; + +import hudson.model.Result; +import javax.annotation.Nonnull; + +/** + * This action can be attached to a {@link FlowNode} to signify that some non-fatal + * warning occurred during execution. Visualizations should treat FlowNodes with this + * action as if the FlowNode's result was {@link Result#UNSTABLE}. + */ +public final class WarningAction implements PersistentAction { + private final @Nonnull String message; + + public WarningAction(@Nonnull String message) { + this.message = message; + } + + public @Nonnull String getMessage() { + return message; + } + + @Override + public String getDisplayName() { + return "Warning: " + message; + } + + @Override + public String getIconFileName() { + return null; + } + + @Override + public String getUrlName() { + return null; + } +} From 5084a2d6602df86f4a6d120b1e7ccd593c823511 Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Mon, 22 Apr 2019 10:34:45 -0400 Subject: [PATCH 2/7] [JENKINS-39203] Fix broken Javadoc link --- .../org/jenkinsci/plugins/workflow/actions/WarningAction.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/actions/WarningAction.java b/src/main/java/org/jenkinsci/plugins/workflow/actions/WarningAction.java index 71f25a63..7979d24c 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/actions/WarningAction.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/actions/WarningAction.java @@ -2,6 +2,7 @@ import hudson.model.Result; import javax.annotation.Nonnull; +import org.jenkinsci.plugins.workflow.graph.FlowNode; /** * This action can be attached to a {@link FlowNode} to signify that some non-fatal From b179faa50943f474ef95cc7e5e7f855ca9f22bcb Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Mon, 22 Apr 2019 10:55:08 -0400 Subject: [PATCH 3/7] [JENKINS-39203] Fix another broken Javadoc link --- .../org/jenkinsci/plugins/workflow/actions/ErrorAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/actions/ErrorAction.java b/src/main/java/org/jenkinsci/plugins/workflow/actions/ErrorAction.java index 43e27cb4..90ddfda5 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/actions/ErrorAction.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/actions/ErrorAction.java @@ -29,7 +29,7 @@ import hudson.remoting.ProxyException; import javax.annotation.CheckForNull; import org.codehaus.groovy.control.MultipleCompilationErrorsException; -import org.jenkinsci.plugins.workflow.graph.AtomNode; +import org.jenkinsci.plugins.workflow.graph.FlowNode; import javax.annotation.Nonnull; import jenkins.model.Jenkins; import org.apache.commons.io.output.NullOutputStream; From 7d94c661c55a3a34dd73d1f6ed7bdd37224f814e Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Tue, 23 Apr 2019 12:56:29 -0400 Subject: [PATCH 4/7] [JENKINS-39203] Visually separate Javadoc summary line --- .../jenkinsci/plugins/workflow/actions/WarningAction.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/actions/WarningAction.java b/src/main/java/org/jenkinsci/plugins/workflow/actions/WarningAction.java index 7979d24c..739ab583 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/actions/WarningAction.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/actions/WarningAction.java @@ -5,9 +5,11 @@ import org.jenkinsci.plugins.workflow.graph.FlowNode; /** - * This action can be attached to a {@link FlowNode} to signify that some non-fatal - * warning occurred during execution. Visualizations should treat FlowNodes with this - * action as if the FlowNode's result was {@link Result#UNSTABLE}. + * Action to be attached to a {@link FlowNode} to signify that some non-fatal warning occurred + * during execution of a {@code Step}. + * + * Visualizations should treat FlowNodes with this action as if the FlowNode's result was + * {@link Result#UNSTABLE}. */ public final class WarningAction implements PersistentAction { private final @Nonnull String message; From 9a08467f4ffbda886816b4937d82842c4035d2c8 Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Tue, 23 Apr 2019 12:57:49 -0400 Subject: [PATCH 5/7] Show steps with WarningAction as unstable on the Pipeline Steps page --- .../org/jenkinsci/plugins/workflow/graph/FlowNode.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/graph/FlowNode.java b/src/main/java/org/jenkinsci/plugins/workflow/graph/FlowNode.java index d1f1d390..58cd5631 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/graph/FlowNode.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/graph/FlowNode.java @@ -48,6 +48,7 @@ import org.jenkinsci.plugins.workflow.actions.ErrorAction; import org.jenkinsci.plugins.workflow.actions.LabelAction; import org.jenkinsci.plugins.workflow.actions.PersistentAction; +import org.jenkinsci.plugins.workflow.actions.WarningAction; import org.jenkinsci.plugins.workflow.flow.FlowExecution; import org.jenkinsci.plugins.workflow.graphanalysis.FlowScanningUtils; import org.jenkinsci.plugins.workflow.graphanalysis.NodeStepTypePredicate; @@ -274,9 +275,6 @@ public String getDisplayFunctionName() { /** * Returns colored orb that represents the current state of this node. - * - * TODO: this makes me wonder if we should support other colored states, - * like unstable and aborted --- seems useful. */ @Exported public BallColor getIconColor() { @@ -284,10 +282,13 @@ public BallColor getIconColor() { BallColor c = null; if(error != null) { if(error.getError() instanceof FlowInterruptedException) { + // TODO: Use FlowInterruptedException.getResult() to determine ball color. c = BallColor.ABORTED; } else { c = BallColor.RED; } + } else if (getPersistentAction(WarningAction.class) != null) { + c = BallColor.YELLOW; } else { c = BallColor.BLUE; } From 068083104b030e67298892aea9ce76361f1332d6 Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Tue, 30 Apr 2019 14:20:02 -0400 Subject: [PATCH 6/7] [JENKINS-39203] Allow WarningAction to have a custom result and make the message optional --- .../workflow/actions/WarningAction.java | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/actions/WarningAction.java b/src/main/java/org/jenkinsci/plugins/workflow/actions/WarningAction.java index 739ab583..182f0b71 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/actions/WarningAction.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/actions/WarningAction.java @@ -1,30 +1,41 @@ package org.jenkinsci.plugins.workflow.actions; import hudson.model.Result; +import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import org.jenkinsci.plugins.workflow.graph.FlowNode; /** - * Action to be attached to a {@link FlowNode} to signify that some non-fatal warning occurred - * during execution of a {@code Step}. + * Action to be attached to a {@link FlowNode} to signify that some non-fatal event occurred + * during execution of a {@code Step} but execution continued normally. * - * Visualizations should treat FlowNodes with this action as if the FlowNode's result was - * {@link Result#UNSTABLE}. + * {@link #withMessage} should be used whenever possible to give context to the warning. + * Visualizations should treat FlowNodes with this action as if the FlowNode's result was {@link #result}. */ -public final class WarningAction implements PersistentAction { - private final @Nonnull String message; +public class WarningAction implements PersistentAction { + private @Nonnull Result result; + private @CheckForNull String message; - public WarningAction(@Nonnull String message) { + public WarningAction(@Nonnull Result result) { + this.result = result; + } + + public WarningAction withMessage(String message) { this.message = message; + return this; } - public @Nonnull String getMessage() { + public @CheckForNull String getMessage() { return message; } + public @Nonnull Result getResult() { + return result; + } + @Override public String getDisplayName() { - return "Warning: " + message; + return "Warning" + (message != null ? ": " + message : ""); } @Override From 240afee962d23d0102a29583a603d7dbc653d6e8 Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Tue, 30 Apr 2019 16:57:20 -0400 Subject: [PATCH 7/7] [JENKINS-39203] Update FlowNode.getIconColor to use WarningAction.getResult --- .../plugins/workflow/graph/FlowNode.java | 27 ++++++-- .../plugins/workflow/graph/FlowNodeTest.java | 66 ++++++++++++++++++- 2 files changed, 87 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/graph/FlowNode.java b/src/main/java/org/jenkinsci/plugins/workflow/graph/FlowNode.java index 3fa56624..a394a0bb 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/graph/FlowNode.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/graph/FlowNode.java @@ -29,6 +29,7 @@ import hudson.model.Action; import hudson.model.Actionable; import hudson.model.BallColor; +import hudson.model.Result; import hudson.model.Saveable; import hudson.search.SearchItem; import java.io.IOException; @@ -276,16 +277,16 @@ public String getDisplayFunctionName() { @Exported public BallColor getIconColor() { ErrorAction error = getError(); + WarningAction warning = getPersistentAction(WarningAction.class); BallColor c = null; if(error != null) { - if(error.getError() instanceof FlowInterruptedException) { - // TODO: Use FlowInterruptedException.getResult() to determine ball color. - c = BallColor.ABORTED; + if (error.getError() instanceof FlowInterruptedException) { + c = resultToBallColor(((FlowInterruptedException) error.getError()).getResult()); } else { c = BallColor.RED; } - } else if (getPersistentAction(WarningAction.class) != null) { - c = BallColor.YELLOW; + } else if (warning != null) { + c = resultToBallColor(warning.getResult()); } else { c = BallColor.BLUE; } @@ -295,6 +296,22 @@ public BallColor getIconColor() { return c; } + private static BallColor resultToBallColor(@Nonnull Result result) { + if (result == Result.SUCCESS) { + return BallColor.BLUE; + } else if (result == Result.UNSTABLE) { + return BallColor.YELLOW; + } else if (result == Result.FAILURE) { + return BallColor.RED; + } else if (result == Result.NOT_BUILT) { + return BallColor.NOTBUILT; + } else if (result == Result.ABORTED) { + return BallColor.ABORTED; + } else { + return BallColor.GREY; + } + } + /** * Gets a human readable name for this type of the node. * diff --git a/src/test/java/org/jenkinsci/plugins/workflow/graph/FlowNodeTest.java b/src/test/java/org/jenkinsci/plugins/workflow/graph/FlowNodeTest.java index 72f14f68..46d556ba 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/graph/FlowNodeTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/graph/FlowNodeTest.java @@ -35,8 +35,9 @@ import java.util.List; import java.util.logging.Level; -import com.google.common.collect.Lists; +import java.util.Set; import org.jenkinsci.plugins.workflow.actions.ArgumentsAction; +import org.jenkinsci.plugins.workflow.actions.WarningAction; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; import org.jenkinsci.plugins.workflow.flow.FlowExecution; import org.jenkinsci.plugins.workflow.graphanalysis.DepthFirstScanner; @@ -44,18 +45,25 @@ import org.jenkinsci.plugins.workflow.graphanalysis.NodeStepTypePredicate; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; +import org.jenkinsci.plugins.workflow.steps.Step; +import org.jenkinsci.plugins.workflow.steps.StepContext; +import org.jenkinsci.plugins.workflow.steps.StepDescriptor; +import org.jenkinsci.plugins.workflow.steps.StepExecution; import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep; import org.junit.Ignore; import org.junit.Test; +import org.kohsuke.stapler.DataBoundConstructor; import static org.hamcrest.Matchers.hasSize; import static org.junit.Assert.*; + import org.junit.Rule; import org.junit.runners.model.Statement; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.LoggerRule; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.RestartableJenkinsRule; +import org.jvnet.hudson.test.TestExtension; public class FlowNodeTest { @@ -436,6 +444,25 @@ public void evaluate() throws Throwable { assertEquals(BallColor.ABORTED, coreStepNodes.get(0).getIconColor()); } + @Test public void iconColorUsesWarningActionResult() throws Exception { + WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "p"); + job.setDefinition(new CpsFlowDefinition( + "warning('UNSTABLE')\n" + + "warning('FAILURE')\n", true)); + WorkflowRun b = r.assertBuildStatus(Result.SUCCESS, job.scheduleBuild2(0).get()); + List nodes = new DepthFirstScanner().filteredNodes(b.getExecution(), new NodeStepTypePredicate("warning")); + assertThat(nodes, hasSize(2)); + assertWarning(nodes.get(0), Result.FAILURE, BallColor.RED); + assertWarning(nodes.get(1), Result.UNSTABLE, BallColor.YELLOW); + } + + private void assertWarning(FlowNode node, Result expectedResult, BallColor expectedColor) { + WarningAction warningAction = node.getPersistentAction(WarningAction.class); + assertNotNull(warningAction); + assertEquals(expectedResult, warningAction.getResult()); + assertEquals(expectedColor, node.getIconColor()); + } + private void assertExpectedEnclosing(FlowExecution execution, String nodeId, String enclosingId) throws Exception { FlowNode node = execution.getNode(nodeId); assertNotNull(node); @@ -478,5 +505,42 @@ private List enclosingBlocksIncludingNode(FlowNode node) { encl.addAll(node.getEnclosingBlocks()); return encl; } + + // TODO: Delete and replace with UnstableStep once workflow-basic-steps dependency has it available. + public static class WarningStep extends Step { + private final Result result; + @DataBoundConstructor + public WarningStep(String result) { + this.result = Result.fromString(result); + } + @Override + public StepExecution start(StepContext sc) throws Exception { + class Execution extends StepExecution { + private final Result result; + public Execution(StepContext sc, Result result) { + super(sc); + this.result = result; + } + @Override + public boolean start() throws Exception { + getContext().get(FlowNode.class).addAction(new WarningAction(result)); + getContext().onSuccess(null); + return true; + } + } + return new Execution(sc, this.result); + } + @TestExtension("iconColorUsesWarningActionResult") + public static class DescriptorImpl extends StepDescriptor { + @Override + public String getFunctionName() { + return "warning"; + } + @Override + public Set> getRequiredContext() { + return Collections.singleton(FlowNode.class); + } + } + } }