Add includeStage to allow starting check immediately for JUnit (#254)

Co-authored-by: Ullrich Hafner <ullrich.hafner@gmail.com>
jenkinsci · Apr 4, 2024 · eb5589d · eb5589d
1 parent fb71c46
commit eb5589d
Show file tree

Hide file tree

Showing 9 changed files with 191 additions and 71 deletions.
diff --git a/README.md b/README.md
@@ -53,12 +53,57 @@ see [the handler](https://github.com/jenkinsci/github-checks-plugin/blob/ea060be
 
 - withChecks: you can inject the check's name into the closure for other steps to use:
 
-```
-withChecks(name: 'injected name') {
+```groovy
+withChecks('injected name') {
     // some other steps that will extract the name
 }
 ```
 
+`withChecks` will publish an in progress check immediately and then other consuming plugins will publish the final check.
+
+You can also include the checks stage name with `includeStage`:
+
+```groovy
+withChecks(name: 'Tests', includeStage: true) {
+    sh 'mvn -Dmaven.test.failure.ignore=true clean verify'
+    junit '**/target/surefire-reports/TEST-*.xml'
+}
+```
+
+Combining `includeStage` with the JUnit plugin works well to publish checks for each test suite:
+
+![With Checks multiple stages](docs/images/github-status.png)
+
+<details>
+
+<summary>Example full pipeline with parallel stages</summary>
+
+```groovy
+def axes = [
+        platforms: ['linux', 'windows'],
+        jdks: [17, 21],
+]
+def builds = [:]
+axes.values().combinations {
+    def (platform, jdk) = it
+    builds["${platform}-jdk${jdk}"] = {
+        node(platform) {
+            stage("${platform.capitalize()} - JDK ${jdk} - Test") {
+                checkout scm
+                withChecks(name: 'Tests', includeStage: true) {
+                    sh 'mvn -Dmaven.test.failure.ignore=true clean verify'
+                    junit '**/target/surefire-reports/TEST-*.xml'
+                }
+            }
+        }
+    }
+}
+
+parallel builds
+```
+
+</details>
+
 ## Guides
 
 - [Consumers Guide](docs/consumers-guide.md)

diff --git a/docs/images/multiple-with-checks.png b/docs/images/multiple-with-checks.png
diff --git a/pom.xml b/pom.xml
@@ -18,7 +18,7 @@
   <description>Defines an API for Jenkins to publish checks to SCM platforms.</description>
 
   <properties>
-    <revision>2.1.0</revision>
+    <revision>2.2.0</revision>
     <changelist>-SNAPSHOT</changelist>
     <gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
 
@@ -166,6 +166,12 @@
                 <serialVersionUID>1</serialVersionUID>
                 <justification>Adding actions list with an initial empty value should not break the compatibility.</justification>
               </item>
+              <item>
+                <code>java.field.serialVersionUIDUnchanged</code>
+                <old>field io.jenkins.plugins.checks.steps.WithChecksStep.serialVersionUID</old>
+                <serialVersionUID>1</serialVersionUID>
+                <justification>Boolean value added, will have no impact on backwards compat.</justification>
+              </item>
               <item>
                 <regex>true</regex>
                 <code>java.annotation.*</code>

diff --git a/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java b/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java
@@ -1,24 +1,26 @@
 package io.jenkins.plugins.checks.status;
 
+import static io.jenkins.plugins.checks.utils.FlowNodeUtils.getEnclosingBlockNames;
+import static io.jenkins.plugins.checks.utils.FlowNodeUtils.getEnclosingStagesAndParallels;
+
+import edu.umd.cs.findbugs.annotations.CheckForNull;
+import hudson.model.Result;
+import hudson.model.Run;
+import io.jenkins.plugins.checks.api.ChecksOutput;
+import io.jenkins.plugins.checks.api.TruncatedString;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.Stack;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
-
 import org.apache.commons.collections.iterators.ReverseListIterator;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.Pair;
-
-import edu.umd.cs.findbugs.annotations.CheckForNull;
-import edu.umd.cs.findbugs.annotations.NonNull;
-
 import org.jenkinsci.plugins.workflow.actions.ArgumentsAction;
 import org.jenkinsci.plugins.workflow.actions.ErrorAction;
 import org.jenkinsci.plugins.workflow.actions.LabelAction;
@@ -28,14 +30,7 @@
 import org.jenkinsci.plugins.workflow.flow.FlowExecution;
 import org.jenkinsci.plugins.workflow.graph.BlockStartNode;
 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 hudson.model.Result;
-import hudson.model.Run;
-
-import io.jenkins.plugins.checks.api.ChecksOutput;
-import io.jenkins.plugins.checks.api.TruncatedString;
 
 @SuppressWarnings("PMD.GodClass")
 class FlowExecutionAnalyzer {
@@ -200,54 +195,6 @@ private String getPotentialTitle(final FlowNode flowNode, final ErrorAction erro
         return StringUtils.join(new ReverseListIterator(enclosingBlockNames), "/") + ": " + whereBuildFailed;
     }
 
-    private static boolean isStageNode(@NonNull final 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<FlowNode> getEnclosingStagesAndParallels(final FlowNode node) {
-        List<FlowNode> enclosingBlocks = new ArrayList<>();
-        for (FlowNode enclosing : node.getEnclosingBlocks()) {
-            if (enclosing != null && enclosing.getAction(LabelAction.class) != null
-                    && (isStageNode(enclosing) || enclosing.getAction(ThreadNameAction.class) != null)) {
-                enclosingBlocks.add(enclosing);
-            }
-        }
-
-        return enclosingBlocks;
-    }
-
-    @NonNull
-    private static List<String> getEnclosingBlockNames(@NonNull final List<FlowNode> nodes) {
-        List<String> 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
     private static String getLog(final FlowNode flowNode) {
         LogAction logAction = flowNode.getAction(LogAction.class);

diff --git a/src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java b/src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java
@@ -6,9 +6,13 @@
 import hudson.model.Run;
 import hudson.model.TaskListener;
 import io.jenkins.plugins.checks.api.*;
+import io.jenkins.plugins.checks.utils.FlowNodeUtils;
 import io.jenkins.plugins.util.PluginLogger;
 import jenkins.model.CauseOfInterruption;
+import org.apache.commons.collections.iterators.ReverseListIterator;
+import org.apache.commons.lang3.StringUtils;
 import org.jenkinsci.plugins.displayurlapi.DisplayURLProvider;
+import org.jenkinsci.plugins.workflow.graph.FlowNode;
 import org.jenkinsci.plugins.workflow.steps.*;
 import org.kohsuke.stapler.DataBoundConstructor;
 
@@ -18,6 +22,7 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
+import org.kohsuke.stapler.DataBoundSetter;
 
 import static hudson.Util.fixNull;
 
@@ -28,6 +33,7 @@ public class WithChecksStep extends Step implements Serializable {
     private static final long serialVersionUID = 1L;
 
     private final String name;
+    private boolean includeStage;
 
     /**
      * Creates the step with a name to inject.
@@ -45,6 +51,15 @@ public String getName() {
         return name;
     }
 
+    public boolean isIncludeStage() {
+        return includeStage;
+    }
+
+    @DataBoundSetter
+    public void setIncludeStage(boolean includeStage) {
+        this.includeStage = includeStage;
+    }
+
     @Override
     public StepExecution start(final StepContext stepContext) {
         return new WithChecksStepExecution(stepContext, this);
@@ -109,7 +124,7 @@ static class WithChecksStepExecution extends AbstractStepExecutionImpl {
         }
 
         @Override
-        public boolean start() {
+        public boolean start() throws IOException, InterruptedException {
             ChecksInfo info = extractChecksInfo();
             getContext().newBodyInvoker()
                     .withContext(info)
@@ -119,19 +134,36 @@ public boolean start() {
         }
 
         @VisibleForTesting
-        ChecksInfo extractChecksInfo() {
-            return new ChecksInfo(step.name);
+        ChecksInfo extractChecksInfo() throws IOException, InterruptedException {
+            return new ChecksInfo(getName());
+        }
+
+        private String getName() throws IOException, InterruptedException {
+            if (step.isIncludeStage()) {
+                FlowNode flowNode = getContext().get(FlowNode.class);
+                if (flowNode == null) {
+                    throw new IllegalStateException("No FlowNode found in the context.");
+                }
+
+                List<FlowNode> enclosingStagesAndParallels = FlowNodeUtils.getEnclosingStagesAndParallels(flowNode);
+                List<String> checksComponents = FlowNodeUtils.getEnclosingBlockNames(enclosingStagesAndParallels);
+
+                checksComponents.add(step.getName());
+
+                return StringUtils.join(new ReverseListIterator(checksComponents), " / ");
+            }
+            return step.getName();
         }
 
         @Override
         public void stop(final Throwable cause) {
             try {
                 publish(getContext(), new ChecksDetails.ChecksDetailsBuilder()
-                        .withName(step.getName())
+                        .withName(getName())
                         .withStatus(ChecksStatus.COMPLETED)
                         .withConclusion(ChecksConclusion.CANCELED));
             }
-            catch (WithChecksPublishException e) {
+            catch (WithChecksPublishException | IOException | InterruptedException e) {
                 cause.addSuppressed(e);
             }
             getContext().onFailure(cause);

diff --git a/src/main/java/io/jenkins/plugins/checks/utils/FlowNodeUtils.java b/src/main/java/io/jenkins/plugins/checks/utils/FlowNodeUtils.java
@@ -0,0 +1,71 @@
+package io.jenkins.plugins.checks.utils;
+
+import edu.umd.cs.findbugs.annotations.NonNull;
+import java.util.ArrayList;
+import java.util.List;
+import org.jenkinsci.plugins.workflow.actions.LabelAction;
+import org.jenkinsci.plugins.workflow.actions.ThreadNameAction;
+import org.jenkinsci.plugins.workflow.graph.FlowNode;
+import org.jenkinsci.plugins.workflow.graph.StepNode;
+import org.jenkinsci.plugins.workflow.steps.StepDescriptor;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+
+/**
+ * Utility methods for working with FlowNodes.
+ */
+@Restricted(NoExternalUse.class)
+public class FlowNodeUtils {
+    /**
+     * 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
+    public static List<FlowNode> getEnclosingStagesAndParallels(final FlowNode node) {
+        List<FlowNode> enclosingBlocks = new ArrayList<>();
+        for (FlowNode enclosing : node.getEnclosingBlocks()) {
+            if (enclosing != null && enclosing.getAction(LabelAction.class) != null
+                    && (isStageNode(enclosing) || enclosing.getAction(ThreadNameAction.class) != null)) {
+                enclosingBlocks.add(enclosing);
+            }
+        }
+
+        return enclosingBlocks;
+    }
+
+    /**
+     * Get the stage and parallel branch names for these nodes, innermost first.
+     * @param nodes A flownode.
+     * @return A nonnull, possibly empty list of stage/parallel branch names, innermost first.
+     */
+    @NonNull
+    public static List<String> getEnclosingBlockNames(@NonNull final List<FlowNode> nodes) {
+        List<String> 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;
+    }
+
+    private static boolean isStageNode(@NonNull final FlowNode node) {
+        if (node instanceof StepNode) {
+            StepDescriptor d = ((StepNode) node).getDescriptor();
+            return d != null && d.getFunctionName().equals("stage");
+        }
+        else {
+            return false;
+        }
+    }
+}
diff --git a/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java b/src/test/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisherITest.java
@@ -88,13 +88,14 @@ public void shouldNotPublishStatusWhenSkipped() {
      * a status checks using the specified name should be published.
      */
     @Test
-    public void shouldPublishStatusWithProperties() {
+    public void shouldPublishStatusWithProperties() throws Exception {
         getProperties().setApplicable(true);
         getProperties().setSkipped(false);
         getProperties().setName("Test Status");
 
         buildSuccessfully(createFreeStyleProject());
-
+        // Wait for the job to finish to work around slow Windows builds sometimes
+        this.getJenkins().waitUntilNoActivity();
         assertThat(getFactory().getPublishedChecks()).hasSize(3);
 
         ChecksDetails details = getFactory().getPublishedChecks().get(0);

diff --git a/src/test/java/io/jenkins/plugins/checks/steps/WithChecksStepITest.java b/src/test/java/io/jenkins/plugins/checks/steps/WithChecksStepITest.java
@@ -82,6 +82,21 @@ public void publishChecksShouldTakeNameFromWithChecks() {
         assertThat(manualChecks.getConclusion()).isEqualTo(ChecksConclusion.SUCCESS);
     }
 
+    @Test
+    public void publishChecksShouldIncludeEnclosingBlocksWhenEnabled() {
+        WorkflowJob job = createPipeline();
+        job.setDefinition(asStage("withChecks(name: 'tests', includeStage: true) {}"));
+
+        buildSuccessfully(job);
+
+        assertThat(getFactory().getPublishedChecks()).hasSize(1);
+        ChecksDetails autoChecks = getFactory().getPublishedChecks().get(0);
+
+        assertThat(autoChecks.getName()).contains("tests / Integration Test");
+        assertThat(autoChecks.getStatus()).isEqualTo(ChecksStatus.IN_PROGRESS);
+        assertThat(autoChecks.getConclusion()).isEqualTo(ChecksConclusion.NONE);
+    }
+
     /**
      * Tests that withChecks step ignores names from the withChecks context if one has been explicitly set.
      */