From cfa55a82b3f585dfa45bceaf42ad9ccc5f820b52 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 24 Apr 2024 15:31:53 -0400 Subject: [PATCH 1/2] Log a note when `catchError` sets build result --- .../org/jenkinsci/plugins/workflow/steps/CatchErrorStep.java | 1 + .../org/jenkinsci/plugins/workflow/steps/CatchErrorStepTest.java | 1 + 2 files changed, 2 insertions(+) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/CatchErrorStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/CatchErrorStep.java index d5b3d32d..bba8aca7 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/steps/CatchErrorStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/steps/CatchErrorStep.java @@ -248,6 +248,7 @@ public Object readResolve() { Functions.printStackTrace(t, listener.getLogger()); } if (buildResult.isWorseThan(Result.SUCCESS)) { + listener.getLogger().println("Setting overall build result to " + buildResult); context.get(Run.class).setResult(buildResult); } if (stepResult.isWorseThan(Result.SUCCESS)) { diff --git a/src/test/java/org/jenkinsci/plugins/workflow/steps/CatchErrorStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/steps/CatchErrorStepTest.java index 0721c91e..5c82975c 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/steps/CatchErrorStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/steps/CatchErrorStepTest.java @@ -118,6 +118,7 @@ public class CatchErrorStepTest { "}", true)); WorkflowRun b = p.scheduleBuild2(0).waitForStart(); assertCatchError(r, b, Result.UNSTABLE, null, true); + r.assertLogContains("Setting overall build result to UNSTABLE", b); } @Test public void invalidBuildResult() throws Exception { From 6a6e3416ea7d262b4f18b186bb701a3c1f8225c6 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 26 Apr 2024 07:10:51 -0400 Subject: [PATCH 2/2] Avoid logging when `setResult` is predicted to do nothing https://github.com/jenkinsci/workflow-basic-steps-plugin/pull/308#discussion_r1580724541 --- .../jenkinsci/plugins/workflow/steps/CatchErrorStep.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/CatchErrorStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/CatchErrorStep.java index bba8aca7..33f706c0 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/steps/CatchErrorStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/steps/CatchErrorStep.java @@ -248,8 +248,12 @@ public Object readResolve() { Functions.printStackTrace(t, listener.getLogger()); } if (buildResult.isWorseThan(Result.SUCCESS)) { - listener.getLogger().println("Setting overall build result to " + buildResult); - context.get(Run.class).setResult(buildResult); + Run build = context.get(Run.class); + Result currentResult = build.getResult(); + if (currentResult == null || buildResult.isWorseThan(currentResult)) { + listener.getLogger().println("Setting overall build result to " + buildResult); + } // otherwise WorkflowRun.setResult should be a no-op, so do not log anything + build.setResult(buildResult); } if (stepResult.isWorseThan(Result.SUCCESS)) { context.get(FlowNode.class).addOrReplaceAction(new WarningAction(stepResult).withMessage(message));