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.

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

Add a list of all Skipped tests to the Test Result report #106

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
45 changes: 42 additions & 3 deletions src/main/java/hudson/tasks/junit/CaseResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,13 @@ public class CaseResult extends TestResult implements Comparable<CaseResult> {
*/
private /*final*/ int failedSince;

/**
* This test has been skipped since this build number (not id.)
*
* If {@link #isPassed() passing}, this field is left unused to 0.
*/
private int skippedSince;

private static float parseTime(Element testCase) {
String time = testCase.attributeValue("time");
return new TimeToFloat(time).parse();
Expand Down Expand Up @@ -420,10 +427,40 @@ else if (getRun() != null) {
}
}

private void recomputeSkippedSinceIfNeeded() {
if (skippedSince==0 && getSkipCount()==1) {
CaseResult prev = getPreviousResult();
if(prev!=null && prev.isSkipped()){
this.skippedSince = prev.getSkippedSince();
Copy link
Member

Choose a reason for hiding this comment

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

What happens if in previous the test failed and therefore it has been disabled now? Then previous.isPassed is false, too. Might be a good setup for an integration test.

Copy link
Author

Choose a reason for hiding this comment

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

You're absolutely right - when a test becomes skipped, its skippedSince counter is set to the current build number.
In a subsequent build:

  • If the test passes or fails, the skippedSince counter will be reset to 0.
  • If the test is skipped again, the skippedSince is carried over from the previous build.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unlike failedSince, skippedSince is not available in XML, so this could recursively load all the skipped builds leading to StackOverflowError like JENKINS-31660 . IMHO it would be safer to save skippedSince in XML too and assume current build is the first skipped one if no information is found in XML, avoiding recursion. (Related to #117.)

Copy link
Author

Choose a reason for hiding this comment

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

Good catch! I will review this and suggest a fix

Copy link
Author

@nirgilboa nirgilboa May 17, 2019

Choose a reason for hiding this comment

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

@zbynek I've given this some thought.. as I understand we only parse the XML and have no control over its contents. Do you see any other way to protect against a StackOverflowError ?

}
else if (getRun() != null) {
this.skippedSince = getRun().getNumber();
} else {
LOGGER.warning("trouble calculating getSkippedSince. We've got prev, but no owner.");
// skippedSince will be 0, which isn't correct.
}
}
}

/**
* If this test was skipped, then return the build number
* when this test was first skipped.
*/
@Exported(visibility=9)
public int getSkippedSince() {
// If we haven't calculated skippedSince yet, and we should, do it now.
recomputeSkippedSinceIfNeeded();
return skippedSince;
}

public Run<?,?> getFailedSinceRun() {
return getRun().getParent().getBuildByNumber(getFailedSince());
}

public Run<?,?> getSkippedSinceRun() {
return getRun().getParent().getBuildByNumber(getSkippedSince());
}

/**
* Gets the number of consecutive builds (including this)
* that this test case has been failing.
Expand All @@ -434,12 +471,14 @@ public Run<?,?> getFailedSinceRun() {
public int getAge() {
if(isPassed())
return 0;
else if (getRun() != null) {
else if (isFailed() && getRun() != null)
return getRun().getNumber()-getFailedSince()+1;
else if (isSkipped() && getRun() != null) {
return getRun().getNumber()-getSkippedSince()+1;
} else {
LOGGER.fine("Trying to get age of a CaseResult without an owner");
return 0;
}
return 0;
}
}

/**
Expand Down
18 changes: 13 additions & 5 deletions src/main/java/hudson/tasks/test/MetaTabulatedResult.java
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
/*
* The MIT License
*
*
* Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi, Yahoo!, Inc.
*
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
Expand All @@ -25,10 +25,11 @@


import java.util.Collection;
import java.util.Collections;

/**
* The purpose of this class is to provide a good place for the
* jelly to bind to.
* jelly to bind to.
* {@link TabulatedResult} whose immediate children
* are other {@link TabulatedResult}s.
*
Expand All @@ -41,4 +42,11 @@ public abstract class MetaTabulatedResult extends TabulatedResult {
*/
public abstract Collection<? extends TestResult> getFailedTests();

/**
* All skipped tests.
*/
public Collection<? extends TestResult> getSkippedTests() {
return Collections.emptyList();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ THE SOFTWARE.
<j:set var="id" value="${attrs.id}-${attrs.name}"/>
<j:set var="display" value="${attrs.opened ? '' : 'none'}"/>
<j:set var="idisplay" value="${attrs.opened ? 'none' : ''}"/>
<j:set var="open" value="javascript:showFailureSummary('${id}')"/>
<j:set var="close" value="javascript:hideFailureSummary('${id}')"/>
<j:set var="open" value="javascript:showSummary('${id}')"/>
<j:set var="close" value="javascript:hideSummary('${id}')"/>
<h4>
<a id="${id}-showlink" href="${open}" title="Show ${title}" style="display: ${idisplay}">
<l:icon class="icon-document-add icon-sm"/><st:nbsp/>${title}
Expand All @@ -54,6 +54,7 @@ THE SOFTWARE.

<j:set var="id" value="${h.generateId()}"/>

<local:item id="${id}" name="skipMessage" title="${%Skip Message}" value="${it.skippedMessage}" opened="true"/>
<local:item id="${id}" name="error" title="${%Error Details}" value="${it.errorDetails}" opened="true"/>
<local:item id="${id}" name="stacktrace" title="${%Stack Trace}" value="${it.errorStackTrace}"/>
<local:item id="${id}" name="stdout" title="${%Standard Output}" value="${it.stdout}"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,44 @@ THE SOFTWARE.

<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson/test" xmlns:f="/lib/form">
<st:once>
<script type="text/javascript">
function showSkipped() {
const skippedTestsTable = document.getElementsByClassName("block collapse skipped")[0];
const showSkippedLink = document.getElementById("showSkippedLink");
skippedTestsTable.classList.add("show");
showSkippedLink.style.display = "none";
}
</script>
<style type="text/css">
*, ::after, ::before {
box-sizing: border-box;
}

.collapse {
display: block;
max-height: 0px;
overflow: hidden;
transition: max-height 0.5s cubic-bezier(0, 1, 0, 1);
}
.collapse.show {
max-height: 99em;
transition: max-height .5s ease-in-out;
}

.block {
margin-top: 10px;
padding: 0;
}
.block__content {
height: 100%;
}
a:link#showSkippedLink {
margin-top: 0.83em;
}
</style>
</st:once>

<j:if test="${it.failCount!=0}">
<h2>${%All Failed Tests}</h2>
<table class="pane sortable bigtable stripped">
Expand Down Expand Up @@ -95,4 +133,34 @@ THE SOFTWARE.
</tbody>
</table>
</j:if>

<j:if test="${it.skipCount!=0}">
<a id="showSkippedLink" name="editSkipped"
href="#showskipped"
onclick="javascript:showSkipped()">${%Show all skipped tests} ${">>>"}</a>
<div class="block collapse skipped">
<div class="block__content">
<h2>${%All Skipped Tests}</h2>
<table class="pane sortable bigtable stripped">
<tr>
<td class="pane-header">${%Test Name}</td>
<td class="pane-header" style="width:4em">${%Duration}</td>
<td class="pane-header" style="width:3em">${%Age}</td>
</tr>
<j:forEach var="f" items="${it.skippedTests}" varStatus="i">
<tr>
<td class="pane no-wrap"><t:failed-test result="${f}" url="${f.getRelativePathFrom(it)}"/></td>
<td class="pane no-wrap" style="text-align:right;" data="${f.duration}">
${f.durationString}
</td>
<td class="pane" style="text-align:right;">
<a href="${rootURL}/${f.skippedSinceRun.url}" class="model-link inside">${f.age}</a>
</td>
</tr>
</j:forEach>
</table>
</div>
</div>
</j:if>

</j:jelly>
26 changes: 13 additions & 13 deletions src/main/resources/lib/hudson/test/failed-test.jelly
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,19 @@ THE SOFTWARE.
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
<st:documentation>
Display link to the failed test.
Display link to the test.
@since 1.538
<st:attribute name="url" type="String">
Path to the failed test.
Path to the test.
</st:attribute>
<st:attribute name="result" type="TestObject">
Failed test object
Test object
</st:attribute>
</st:documentation>
<st:once>
<script type="text/javascript">
<!-- TODO make sure load doesn't happen every time -->
function showFailureSummary(id,query) {
function showSummary(id,query) {
var element = document.getElementById(id)
element.style.display = "";
document.getElementById(id + "-showlink").style.display = "none";
Expand All @@ -51,39 +51,39 @@ THE SOFTWARE.
}
}

function hideFailureSummary(id) {
function hideSummary(id) {
document.getElementById(id).style.display = "none";
document.getElementById(id + "-showlink").style.display = "";
document.getElementById(id + "-hidelink").style.display = "none";
}
</script>
<style type="text/css">
.failure-summary {
.summary {
margin-left: 2em;
}

.failure-summary h4 {
.summary h4 {
margin: 0.5em 0 0.5em 0;
}

.failure-summary h4 a {
.summary h4 a {
text-decoration: none;
color: inherit;
}

.failure-summary h4 a img {
.summary h4 a img {
width: 8px;
height: 8px;
}

.failure-summary pre {
.summary pre {
margin-left: 2em;
}
</style>
</st:once>
<j:set var="id" value="${h.jsStringEscape(url)}"/>
<j:set var="open" value="showFailureSummary('test-${id}','${url}/summary')"/>
<j:set var="close" value="hideFailureSummary('test-${id}')"/>
<j:set var="open" value="showSummary('test-${id}','${url}/summary')"/>
<j:set var="close" value="hideSummary('test-${id}')"/>
<a id="test-${id}-showlink" onclick="${open}" title="${%Show details}">
<l:icon class="icon-document-add icon-sm"/>
</a>
Expand All @@ -95,7 +95,7 @@ THE SOFTWARE.
<j:forEach var="badge" items="${result.testActions}">
<st:include it="${badge}" page="badge.jelly" optional="true"/>
</j:forEach>
<div id="test-${id}" class="failure-summary" style="display: none;">
<div id="test-${id}" class="summary" style="display: none;">
${%Loading...}
</div>
</j:jelly>
82 changes: 81 additions & 1 deletion src/test/java/hudson/tasks/junit/HistoryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,87 @@ public void testFailedSince() throws Exception {
PackageResult smallPackage = tr5.byPackage("org.jvnet.hudson.examples.small");
ClassResult miscClass = smallPackage.getClassResult("MiscTest");
CaseResult eleanorCase = miscClass.getCaseResult("testEleanor");
assertNotNull(eleanorCase);
assertTrue("eleanor failed", !eleanorCase.isPassed());
assertEquals("eleanor has failed since build 3", 3, eleanorCase.getFailedSince());
assertEquals("eleanor has failed since build 3", 3, eleanorCase.getFailedSince());
}

@LocalData
@Test
public void testSkippedSince() throws Exception {
assertNotNull("project should exist", project);
nirgilboa marked this conversation as resolved.
Show resolved Hide resolved

// Check the status of a few builds
FreeStyleBuild build8 = project.getBuildByNumber(8);
assertNotNull("build8", build8);
rule.assertBuildStatus(Result.SUCCESS, build8);

FreeStyleBuild build10 = project.getBuildByNumber(10);
assertNotNull("build10", build10);
rule.assertBuildStatus(Result.FAILURE, build10);

TestResult tr8 = build8.getAction(TestResultAction.class).getResult();
assertEquals(1,tr8.getSkippedTests().size());

// In build 8 and 9, we expect these tests to have skippedSince:
// org.jvnet.hudson.examples.small.deep.DeepTest.testScubaOnlyInRedSea skipped since 8

PackageResult deepPackage = tr8.byPackage("org.jvnet.hudson.examples.small.deep");
assertNotNull("deepPackage", deepPackage);
assertTrue("package is failed", !deepPackage.isPassed());
ClassResult deepClass = deepPackage.getClassResult("DeepTest");
assertNotNull(deepClass);
assertTrue("class is failed", !deepClass.isPassed());
CaseResult testScubaOnlyInRedSeaCase = deepClass.getCaseResult("testScubaOnlyInRedSea");
assertNotNull(testScubaOnlyInRedSeaCase);
assertTrue("scubaCase case is not skipped", testScubaOnlyInRedSeaCase.isSkipped());
int scubaSkippedSince = testScubaOnlyInRedSeaCase.getSkippedSince();
assertEquals("scubaCase should have skipped since build 8", 8, scubaSkippedSince);

// In build 9 the scuba in red sea test is still skipped
TestResult tr9 = project.getBuildByNumber(9).getAction(TestResultAction.class).getResult();
assertEquals(1,tr9.getSkippedTests().size());
deepPackage = tr9.byPackage("org.jvnet.hudson.examples.small.deep");
assertNotNull("deepPackage", deepPackage);
assertTrue("package is failed", !deepPackage.isPassed());
deepClass = deepPackage.getClassResult("DeepTest");
assertNotNull(deepClass);
assertTrue("class is failed", !deepClass.isPassed());
testScubaOnlyInRedSeaCase = deepClass.getCaseResult("testScubaOnlyInRedSea");
assertNotNull(testScubaOnlyInRedSeaCase);
assertTrue("testScubaOnlyInRedSea case is skipped", testScubaOnlyInRedSeaCase.isSkipped());
scubaSkippedSince = testScubaOnlyInRedSeaCase.getSkippedSince();
assertEquals("scubatestScubaOnlyInRedSeaCase should have skipped since build 8", 8, scubaSkippedSince);

// In build 10 the scuba in red sea test begins to fail
TestResult tr10 = project.getBuildByNumber(10).getAction(TestResultAction.class).getResult();
assertEquals(1,tr10.getFailedTests().size());
deepPackage = tr10.byPackage("org.jvnet.hudson.examples.small.deep");
assertNotNull("deepPackage", deepPackage);
assertTrue("package is failed", !deepPackage.isPassed());
deepClass = deepPackage.getClassResult("DeepTest");
assertNotNull(deepClass);
assertTrue("class is failed", !deepClass.isPassed());
testScubaOnlyInRedSeaCase = deepClass.getCaseResult("testScubaOnlyInRedSea");
assertNotNull(testScubaOnlyInRedSeaCase);
assertTrue("testScubaOnlyInRedSea case is failed", testScubaOnlyInRedSeaCase.isFailed());
scubaSkippedSince = testScubaOnlyInRedSeaCase.getSkippedSince();
assertEquals("scubatestScubaOnlyInRedSeaCase should have skipped since build 0", 0, scubaSkippedSince);

// In build11, testScubaOnlyInRedSea is skipped again - skippedSince becomes 11
TestResult tr11 = project.getBuildByNumber(11).getAction(TestResultAction.class).getResult();
assertEquals(0,tr11.getFailedTests().size());
assertEquals(1,tr11.getSkippedTests().size());
deepPackage = tr11.byPackage("org.jvnet.hudson.examples.small.deep");
assertNotNull("deepPackage", deepPackage);
assertTrue("package is failed", !deepPackage.isPassed());
deepClass = deepPackage.getClassResult("DeepTest");
assertNotNull(deepClass);
assertTrue("class is failed", !deepClass.isPassed());
testScubaOnlyInRedSeaCase = deepClass.getCaseResult("testScubaOnlyInRedSea");
assertNotNull(testScubaOnlyInRedSeaCase);
assertTrue("testScubaOnlyInRedSea case is skipped", testScubaOnlyInRedSeaCase.isSkipped());
scubaSkippedSince = testScubaOnlyInRedSeaCase.getSkippedSince();
assertEquals("scubatestScubaOnlyInRedSeaCase should have skipped since build 11", 11, scubaSkippedSince);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public void testPreviousBuildNotLoaded() throws IOException, URISyntaxException
FreeStyleBuild build = new FreeStyleBuild(project) {
@Override
public FreeStyleBuild getPreviousBuild() {
fail("When no tests fail, we don't need tp load previous builds (expensive)");
fail("When no tests fail, we don't need to load previous builds (expensive)");
return null;
}
};
Expand Down
Binary file modified src/test/resources/hudson/tasks/junit/HistoryTest.zip
Binary file not shown.