Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[JENKINS-39203] Mark JUnit steps with failing tests with WarningAction #118

Merged
merged 6 commits into from May 13, 2019
Merged

[JENKINS-39203] Mark JUnit steps with failing tests with WarningAction #118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
30b3875
[JENKINS-39203] Mark JUnit steps with failing tests with WarningAction
abayer Dec 7, 2017
cbf34db
[JENKINS-39203] switch to incremental dependency
dwnusbaum Apr 22, 2019
ee54440
Incrementalify the plugin
dwnusbaum Apr 22, 2019
030f0db
[JENKINS-39203] Reuse node-specific TestResult
dwnusbaum Apr 22, 2019
228bc79
[JENKINS-39203] Update to latest API iteration
dwnusbaum May 3, 2019
9eb6bf1
[JENKINS-39203] Update to released version of workflow-api
dwnusbaum May 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions .mvn/extensions.xml
View file
Open in desktop
@@ -0,0 +1,7 @@
<extensions xmlns="http://maven.apache.org/EXTENSIONS/1.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/EXTENSIONS/1.0.0 http://maven.apache.org/xsd/core-extensions-1.0.0.xsd">
<extension>
<groupId>io.jenkins.tools.incrementals</groupId>
<artifactId>git-changelist-maven-extension</artifactId>
<version>1.0-beta-7</version>
</extension>
</extensions>
2 changes: 2 additions & 0 deletions .mvn/maven.config
View file
Open in desktop
@@ -0,0 +1,2 @@
-Pconsume-incrementals
-Pmight-produce-incrementals
2 changes: 1 addition & 1 deletion Jenkinsfile
View file
Open in desktop
@@ -1 +1 @@
buildPlugin(jenkinsVersions: [null, '2.60.1'])
buildPlugin()
30 changes: 16 additions & 14 deletions pom.xml
View file
Open in desktop
Expand Up @@ -3,21 +3,23 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>3.9</version>
<version>3.43</version>
<relativePath />
</parent>
<artifactId>junit</artifactId>
<version>1.28-SNAPSHOT</version>
<version>${revision}${changelist}</version>
<packaging>hpi</packaging>
<name>JUnit Plugin</name>
<description>Allows JUnit-format test results to be published.</description>
<url>http://wiki.jenkins-ci.org/display/JENKINS/JUnit+Plugin</url>
<properties>
<jenkins.version>2.7.3</jenkins.version>
<java.level>7</java.level>
<revision>1.28</revision>
<changelist>-SNAPSHOT</changelist>
<jenkins.version>2.138.4</jenkins.version>
<java.level>8</java.level>
<no-test-jar>false</no-test-jar>
<workflow-cps.version>2.37</workflow-cps.version>
<workflow-support.version>2.14</workflow-support.version>
<workflow-cps.version>2.66</workflow-cps.version>
<workflow-support.version>3.2</workflow-support.version>
</properties>
<licenses>
<license>
Expand All @@ -29,7 +31,7 @@
<connection>scm:git:git://github.com/jenkinsci/${project.artifactId}-plugin.git</connection>
<developerConnection>scm:git:git@github.com:jenkinsci/${project.artifactId}-plugin.git</developerConnection>
<url>https://github.com/jenkinsci/${project.artifactId}-plugin</url>
<tag>HEAD</tag>
<tag>${scmTag}</tag>
</scm>
<repositories>
<repository>
Expand All @@ -47,22 +49,22 @@
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>structs</artifactId>
<version>1.7</version>
<version>1.17</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-step-api</artifactId>
<version>2.12</version>
<version>2.19</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-api</artifactId>
<version>2.22</version>
<version>2.34</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>script-security</artifactId>
<version>1.30</version>
<version>1.56</version>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
Expand Down Expand Up @@ -91,7 +93,7 @@
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-job</artifactId>
<version>2.11.1</version>
<version>2.32</version>
<scope>test</scope>
</dependency>
<dependency>
Expand All @@ -104,7 +106,7 @@
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-basic-steps</artifactId>
<version>2.6</version>
<version>2.15</version>
<scope>test</scope>
</dependency>
<dependency>
Expand All @@ -129,7 +131,7 @@
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-durable-task-step</artifactId>
<version>2.13</version>
<version>2.28</version>
<scope>test</scope>
</dependency>
</dependencies>
Expand Down
10 changes: 7 additions & 3 deletions src/main/java/hudson/tasks/junit/pipeline/JUnitResultsStepExecution.java
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.tasks.junit.JUnitResultArchiver;
import hudson.tasks.junit.TestResult;
import hudson.tasks.junit.TestResultAction;
import hudson.tasks.junit.TestResultSummary;
import hudson.tasks.test.PipelineTestDetails;
Expand All @@ -14,6 +15,7 @@
import javax.annotation.Nonnull;
import org.jenkinsci.plugins.workflow.actions.LabelAction;
import org.jenkinsci.plugins.workflow.actions.ThreadNameAction;
import org.jenkinsci.plugins.workflow.actions.WarningAction;
import org.jenkinsci.plugins.workflow.graph.FlowNode;
import org.jenkinsci.plugins.workflow.graph.StepNode;
import org.jenkinsci.plugins.workflow.steps.StepContext;
Expand Down Expand Up @@ -50,11 +52,13 @@ protected TestResultSummary run() throws Exception {
TestResultAction testResultAction = JUnitResultArchiver.parseAndAttach(step, pipelineTestDetails, run, workspace, launcher, listener);

if (testResultAction != null) {
// TODO: Once JENKINS-43995 lands, update this to set the step status instead of the entire build.
if (testResultAction.getResult().getFailCount() > 0) {
TestResult testResult = testResultAction.getResult().getResultByNode(nodeId);
int testFailures = testResult.getFailCount();
if (testFailures > 0) {
node.addOrReplaceAction(new WarningAction(Result.UNSTABLE).withMessage(testFailures + " tests failed"));
run.setResult(Result.UNSTABLE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option would be for WarningAction.onAttached to call Run.setResult automatically. On the other hand I can imagine wanting this drastic effect to be explicit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't familiar with onAttached previously, but I think you are referring to RunAction2.onAttached. We certainly could make some changes in FlowNode so that onAttached and onLoad would be called for actions implementing RunAction2, but as you mentioned I think this particular effect is best left explicit.

}
return new TestResultSummary(testResultAction.getResult().getResultByNode(nodeId));
return new TestResultSummary(testResult);
}
} catch (Exception e) {
listener.getLogger().println(e.getMessage());
Expand Down
61 changes: 41 additions & 20 deletions src/test/java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java
View file
Open in desktop
Expand Up @@ -16,14 +16,18 @@
import hudson.tasks.junit.TestResultTest;
import hudson.tasks.test.PipelineBlockWithTests;
import org.hamcrest.CoreMatchers;
import org.hamcrest.BaseMatcher;
import org.hamcrest.Description;
import org.jenkinsci.plugins.workflow.actions.LabelAction;
import org.jenkinsci.plugins.workflow.actions.LogAction;
import org.jenkinsci.plugins.workflow.actions.ThreadNameAction;
import org.jenkinsci.plugins.workflow.actions.WarningAction;
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
import org.jenkinsci.plugins.workflow.cps.SnippetizerTester;
import org.jenkinsci.plugins.workflow.cps.nodes.StepAtomNode;
import org.jenkinsci.plugins.workflow.cps.nodes.StepStartNode;
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.graphanalysis.DepthFirstScanner;
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
Expand Down Expand Up @@ -264,10 +268,10 @@ public void parallelInStage() throws Exception {
assertEquals(5, action.getResult().getSuites().size());
assertEquals(10, action.getTotalCount());

assertBranchResults(r, 1, 6, "a", "first");
assertBranchResults(r, 1, 1, "b", "first");
assertBranchResults(r, 3, 3, "c", "first");
assertStageResults(r, 5, 10, "first");
assertBranchResults(r, 1, 6, 0, "a", "first", null);
assertBranchResults(r, 1, 1, 0, "b", "first", null);
assertBranchResults(r, 3, 3, 1, "c", "first", null);
assertStageResults(r, 5, 10, 1, "first");
}

@Issue("JENKINS-48196")
Expand Down Expand Up @@ -300,11 +304,11 @@ public void stageInParallel() throws Exception {
// assertBranchResults looks to make sure the display names for tests are "(stageName) / (branchName) / (testName)"
// That should still effectively be the case here, even though there's a stage inside each branch, because the
// branch and nested stage have the same name.
assertBranchResults(r, 1, 6, "a", "outer");
assertBranchResults(r, 1, 1, "b", "outer");
assertBranchResults(r, 1, 6, 0, "a", "outer", null);
assertBranchResults(r, 1, 1, 0, "b", "outer", null);
// ...except for branch c. That contains a stage named 'd', so its test should have display names like
// "outer / c / d / (testName)"
assertBranchResults(r, 3, 3, -1, "c", "outer", "d");
assertBranchResults(r, 3, 3, 1, "c", "outer", "d");
}

@Test
Expand Down Expand Up @@ -411,15 +415,11 @@ public boolean apply(@Nullable FlowNode input) {
};
}

public static void assertBranchResults(WorkflowRun run, int suiteCount, int testCount, String branchName, String stageName) {
assertBranchResults(run, suiteCount, testCount, -1, branchName, stageName, null);
}

public static void assertBranchResults(WorkflowRun run, int suiteCount, int testCount, int failCount, String branchName, String stageName,
String innerStageName) {
FlowExecution execution = run.getExecution();
DepthFirstScanner scanner = new DepthFirstScanner();
FlowNode aBranch = scanner.findFirstMatch(execution, branchForName(branchName));
BlockStartNode aBranch = (BlockStartNode)scanner.findFirstMatch(execution, branchForName(branchName));
assertNotNull(aBranch);
TestResult branchResult = assertBlockResults(run, suiteCount, testCount, failCount, aBranch);
String namePrefix = stageName + " / " + branchName;
Expand All @@ -431,19 +431,15 @@ public static void assertBranchResults(WorkflowRun run, int suiteCount, int test
}
}

public static void assertStageResults(WorkflowRun run, int suiteCount, int testCount, String stageName) {
assertStageResults(run, suiteCount, testCount, -1, stageName);
}

public static void assertStageResults(WorkflowRun run, int suiteCount, int testCount, int failCount, String stageName) {
FlowExecution execution = run.getExecution();
DepthFirstScanner scanner = new DepthFirstScanner();
FlowNode aStage = scanner.findFirstMatch(execution, stageForName(stageName));
BlockStartNode aStage = (BlockStartNode)scanner.findFirstMatch(execution, stageForName(stageName));
assertNotNull(aStage);
assertBlockResults(run, suiteCount, testCount, failCount, aStage);
}

private static TestResult assertBlockResults(WorkflowRun run, int suiteCount, int testCount, int failCount, FlowNode blockNode) {
private static TestResult assertBlockResults(WorkflowRun run, int suiteCount, int testCount, int failCount, BlockStartNode blockNode) {
assertNotNull(blockNode);

TestResultAction action = run.getAction(TestResultAction.class);
Expand All @@ -454,8 +450,11 @@ private static TestResult assertBlockResults(WorkflowRun run, int suiteCount, in

assertEquals(suiteCount, aResult.getSuites().size());
assertEquals(testCount, aResult.getTotalCount());
if (failCount > -1) {
assertEquals(failCount, aResult.getFailCount());
assertEquals(failCount, aResult.getFailCount());
if (failCount > 0) {
assertThat(findJUnitSteps(blockNode), CoreMatchers.hasItem(hasWarningAction()));
} else {
assertThat(findJUnitSteps(blockNode), CoreMatchers.not(CoreMatchers.hasItem(hasWarningAction())));
}

PipelineBlockWithTests aBlock = action.getResult().getPipelineBlockWithTests(blockNode.getId());
Expand All @@ -482,6 +481,28 @@ private void assertExpectedResults(Run<?,?> run, int suiteCount, int testCount,
assertEquals(testCount, result.getTotalCount());
}

private static List<FlowNode> findJUnitSteps(BlockStartNode blockStart) {
return new DepthFirstScanner().filteredNodes(
Collections.singletonList(blockStart.getEndNode()),
Collections.singletonList(blockStart),
node -> node instanceof StepAtomNode &&
((StepAtomNode) node).getDescriptor() instanceof JUnitResultsStep.DescriptorImpl
);
}

private static BaseMatcher<FlowNode> hasWarningAction() {
return new BaseMatcher<FlowNode>() {
@Override
public boolean matches(Object item) {
return item instanceof FlowNode && ((FlowNode) item).getPersistentAction(WarningAction.class) != null;
}
@Override
public void describeTo(Description description) {
description.appendText("a FlowNode with a WarningAction");
}
};
}

public static class MockTestDataPublisher extends TestDataPublisher {
private final String name;
@DataBoundConstructor
Expand Down