Skip to content

Commit

Permalink
[FIXED JENKINS-48020] Show stage in failed tests, fix warnings
Browse files Browse the repository at this point in the history 
First, this changes `CaseResult#getFullDisplayName()` to include
stage/branch names like `CaseResult#getDisplayName()`, so that the
full information will show up in lists of failed tests, for example.

Second, stop using `CaseResult#getDisplayName()` for indexing
`CaseResult`s in `SuiteResult`. That method will spam warnings if
called too early in the run now due to `CaseResult#getRun()` being
called before the `SuiteResult`'s parent is set.

Third, fix `JUnitResultsStepTest#testTrends` to actually test what
it's supposed to, to fail if it doesn't find the expected display
names, to test full display names as well, and to use the same
filename for the test results each time due to one of the stages' test
file not containing an explicit suite name, leading to its suite's
name being set to the filename, breaking `SuiteResult#getPreviousResult()`.
  • Loading branch information
@abayer
abayer committed Nov 15, 2017
1 parent ef6bb87 commit 9ec9395
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 18 deletions.
12 changes: 8 additions & 4 deletions src/main/java/hudson/tasks/junit/CaseResult.java
View file
Expand Up @@ -271,18 +271,22 @@ public String getTransformedTestName() {
}

public String getDisplayName() {
return getNameWithEnclosingBlocks(getTransformedTestName());
}

private String getNameWithEnclosingBlocks(String rawName) {
// Only prepend the enclosing flow node names if there are any and the run this is in has multiple blocks directly containing
// test results.
if (!getEnclosingFlowNodeNames().isEmpty()) {
Run<?, ?> r = getRun();
if (r != null) {
TestResultAction action = r.getAction(TestResultAction.class);
if (action != null && action.getResult().hasMultipleBlocks()) {
return StringUtils.join(new ReverseListIterator(getEnclosingFlowNodeNames()), " / ") + " / " + getTransformedTestName();
return StringUtils.join(new ReverseListIterator(getEnclosingFlowNodeNames()), " / ") + " / " + rawName;
}
}
}
return getTransformedTestName();
return rawName;
}

/**
Expand Down Expand Up @@ -369,7 +373,7 @@ public String getFullName() {
* @since 1.515
*/
public String getFullDisplayName() {
return TestNameTransformer.getTransformedName(getFullName());
return getNameWithEnclosingBlocks(TestNameTransformer.getTransformedName(getFullName()));
}

@Override
Expand Down Expand Up @@ -471,7 +475,7 @@ public CaseResult getPreviousResult() {
if (parent == null) return null;
SuiteResult pr = parent.getPreviousResult();
if(pr==null) return null;
return pr.getCase(getDisplayName());
return pr.getCase(getTransformedTestName());
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/hudson/tasks/junit/SuiteResult.java
View file
Expand Up @@ -130,7 +130,7 @@ private synchronized Map<String, CaseResult> casesByName() {
if (casesByName == null) {
casesByName = new HashMap<>();
for (CaseResult c : cases) {
casesByName.put(c.getDisplayName(), c);
casesByName.put(c.getTransformedTestName(), c);
}
}
return casesByName;
Expand Down Expand Up @@ -276,7 +276,7 @@ private SuiteResult(File xmlReport, Element suite, boolean keepLongStdio, @Check

/*package*/ void addCase(CaseResult cr) {
cases.add(cr);
casesByName().put(cr.getDisplayName(), cr);
casesByName().put(cr.getTransformedTestName(), cr);

//if suite time was not specified use sum of the cases' times
if( !hasTimeAttr() ){
Expand Down
43 changes: 31 additions & 12 deletions src/test/java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java
View file
Expand Up @@ -42,6 +42,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.fail;

public class JUnitResultsStepTest {
@Rule
Expand Down Expand Up @@ -245,26 +246,36 @@ public void testTrends() throws Exception {
WorkflowJob j = rule.jenkins.createProject(WorkflowJob.class, "testTrends");
j.setDefinition(new CpsFlowDefinition("node {\n" +
" stage('first') {\n" +
" def first = junit(testResults: \"junit-report-testTrends-first-${env.BUILD_NUMBER}.xml\")\n" +
" def first = junit(testResults: \"junit-report-testTrends-first.xml\")\n" +
" }\n" +
" stage('second') {\n" +
" def second = junit(testResults: \"junit-report-testTrends-second-${env.BUILD_NUMBER}.xml\")\n" +
" def second = junit(testResults: \"junit-report-testTrends-second.xml\")\n" +
" }\n" +
"}\n", true));
FilePath ws = rule.jenkins.getWorkspaceFor(j);
for (int i = 1; i < 4; i++) {
FilePath firstFile = ws.child("junit-report-testTrends-first-" + i + ".xml");
firstFile.copyFrom(JUnitResultsStepTest.class.getResource("junit-report-testTrends-first-" + i + ".xml"));
FilePath secondFile = ws.child("junit-report-testTrends-second-" + i + ".xml");
secondFile.copyFrom(JUnitResultsStepTest.class.getResource("junit-report-testTrends-second-" + i + ".xml"));
}
FilePath firstFile = ws.child("junit-report-testTrends-first.xml");
FilePath secondFile = ws.child("junit-report-testTrends-second.xml");

// Populate first run's tests.
firstFile.copyFrom(JUnitResultsStepTest.class.getResource("junit-report-testTrends-first-1.xml"));
secondFile.copyFrom(JUnitResultsStepTest.class.getResource("junit-report-testTrends-second-1.xml"));

WorkflowRun firstRun = rule.buildAndAssertSuccess(j);
assertStageResults(firstRun, 1, 8, 0, "first");
assertStageResults(firstRun, 1, 1, 0, "second");

// Populate second run's tests.
firstFile.copyFrom(JUnitResultsStepTest.class.getResource("junit-report-testTrends-first-2.xml"));
secondFile.copyFrom(JUnitResultsStepTest.class.getResource("junit-report-testTrends-second-2.xml"));

WorkflowRun secondRun = rule.assertBuildStatus(Result.UNSTABLE, rule.waitForCompletion(j.scheduleBuild2(0).waitForStart()));
assertStageResults(secondRun, 1, 8, 3, "first");
assertStageResults(secondRun, 1, 1, 0, "second");

// Populate third run's tests
firstFile.copyFrom(JUnitResultsStepTest.class.getResource("junit-report-testTrends-first-3.xml"));
secondFile.copyFrom(JUnitResultsStepTest.class.getResource("junit-report-testTrends-second-3.xml"));

WorkflowRun thirdRun = rule.assertBuildStatus(Result.UNSTABLE, rule.waitForCompletion(j.scheduleBuild2(0).waitForStart()));
assertStageResults(thirdRun, 1, 8, 3, "first");
assertStageResults(thirdRun, 1, 1, 0, "second");
Expand All @@ -273,17 +284,25 @@ public void testTrends() throws Exception {

for (CaseResult failed : thirdAction.getFailedTests()) {
if (failed.getDisplayName() != null) {
if (failed.getDisplayName().equals("first / org.twia.vendor.VendorManagerTest.testGetVendorFirmKeyForVendorRep") ||
failed.getDisplayName().equals("first / org.twia.vendor.VendorManagerTest.testCreateAdjustingFirm")) {
if (failed.getDisplayName().equals("first / testGetVendorFirmKeyForVendorRep")) {
assertEquals("first / org.twia.vendor.VendorManagerTest.testGetVendorFirmKeyForVendorRep",
failed.getFullDisplayName());
assertEquals(2, failed.getFailedSince());
} else if (failed.getDisplayName().equals("first / org.twia.vendor.VendorManagerTest.testCreateVendorFirm")) {
} else if (failed.getDisplayName().equals("first / testCreateAdjustingFirm")) {
assertEquals("first / org.twia.vendor.VendorManagerTest.testCreateAdjustingFirm",
failed.getFullDisplayName());
assertEquals(2, failed.getFailedSince());
} else if (failed.getDisplayName().equals("first / testCreateVendorFirm")) {
assertEquals("first / org.twia.vendor.VendorManagerTest.testCreateVendorFirm",
failed.getFullDisplayName());
assertEquals(3, failed.getFailedSince());
} else {
fail("Failed test displayName " + failed.getDisplayName() + " is unexpected.");
}
}
}
}


private static Predicate<FlowNode> branchForName(final String name) {
return new Predicate<FlowNode>() {
@Override
Expand Down

0 comments on commit 9ec9395

Please sign in to comment.