Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JENKINS-27395] Removed per-run node tracking
On further inspection, I came to the conclusion that since
multiple-runs-per-TestResult was only possible for
`AggregatedTestResultAction`, which doesn't support Pipeline jobs in
the first place, it made more sense to simplify things and get rid of
the run-and-nodes tracking of suites in favor of just nodes.

This also necessitated gathreing the enclosing block names earlier,
but that's probably a good change anyway.
  • Loading branch information
abayer committed Oct 19, 2017
1 parent 1c00c38 commit 147692d
Show file tree
Hide file tree
Showing 11 changed files with 176 additions and 209 deletions.
19 changes: 10 additions & 9 deletions src/main/java/hudson/tasks/junit/CaseResult.java
Expand Up @@ -273,7 +273,7 @@ public String getDisplayName() {
Run<?, ?> r = getRun();
if (r != null) {
TestResultAction action = r.getAction(TestResultAction.class);
if (action != null && action.getResult().hasMultipleBlocksForRun(r.getExternalizableId())) {
if (action != null && action.getResult().hasMultipleBlocks()) {
return StringUtils.join(new ReverseListIterator(getEnclosingFlowNodeNames()), " / ") + " / " + getTransformedTestName();
}
}
Expand Down Expand Up @@ -605,16 +605,17 @@ public List<String> getEnclosingFlowNodeNames() {
public Run<?,?> getRun() {
SuiteResult sr = getSuiteResult();
if (sr==null) {
LOGGER.warning("In getOwner(), getSuiteResult is null"); return null; }
LOGGER.warning("In getOwner(), getSuiteResult is null");
return null;
}

hudson.tasks.junit.TestResult tr = sr.getParent();
if (tr==null) {
if (sr.getRunId() != null) {
return Run.fromExternalizableId(sr.getRunId());
} else {
LOGGER.warning("In getOwner(), suiteResult.getParent() is null and suiteResult.getRunId() is null.");
return null;
}

if (tr == null) {
LOGGER.warning("In getOwner(), suiteResult.getParent() is null.");
return null;
}

return tr.getRun();
}

Expand Down
23 changes: 11 additions & 12 deletions src/main/java/hudson/tasks/junit/JUnitParser.java
Expand Up @@ -97,22 +97,21 @@ public String getTestResultLocationMessage() {
public TestResult parseResult(String testResultLocations, Run<?,?> build, FilePath workspace,
Launcher launcher, TaskListener listener)
throws InterruptedException, IOException {
return parseResult(testResultLocations, build, null, null, workspace, launcher, listener);
return parseResult(testResultLocations, build, null, null, null, workspace, launcher, listener);
}

@Override
public TestResult parseResult(String testResultLocations, Run<?,?> build, @CheckForNull String nodeId,
List<String> enclosingBlocks, FilePath workspace, Launcher launcher,
TaskListener listener) throws InterruptedException, IOException {
List<String> enclosingBlocks, List<String> enclosingBlockNames, FilePath workspace,
Launcher launcher, TaskListener listener) throws InterruptedException, IOException {
final long buildTime = build.getTimestamp().getTimeInMillis();
final long timeOnMaster = System.currentTimeMillis();

// [BUG 3123310] TODO - Test Result Refactor: review and fix TestDataPublisher/TestAction subsystem]
// also get code that deals with testDataPublishers from JUnitResultArchiver.perform

return workspace.act(new ParseResultCallable(testResultLocations, buildTime,
timeOnMaster, keepLongStdio, allowEmptyResults,
build.getExternalizableId(), nodeId, enclosingBlocks));
return workspace.act(new ParseResultCallable(testResultLocations, buildTime, timeOnMaster, keepLongStdio,
allowEmptyResults, nodeId, enclosingBlocks, enclosingBlockNames));
}

private static final class ParseResultCallable extends MasterToSlaveFileCallable<TestResult> {
Expand All @@ -121,21 +120,21 @@ private static final class ParseResultCallable extends MasterToSlaveFileCallable
private final long nowMaster;
private final boolean keepLongStdio;
private final boolean allowEmptyResults;
private final String runId;
private final String nodeId;
private final List<String> enclosingBlocks;
private final List<String> enclosingBlockNames;

private ParseResultCallable(String testResults, long buildTime, long nowMaster,
boolean keepLongStdio, boolean allowEmptyResults, String runId, @CheckForNull String nodeId,
List<String> enclosingBlocks) {
boolean keepLongStdio, boolean allowEmptyResults, @CheckForNull String nodeId,
List<String> enclosingBlocks, List<String> enclosingBlockNames) {
this.buildTime = buildTime;
this.testResults = testResults;
this.nowMaster = nowMaster;
this.keepLongStdio = keepLongStdio;
this.allowEmptyResults = allowEmptyResults;
this.runId = runId;
this.nodeId = nodeId;
this.enclosingBlocks = enclosingBlocks;
this.enclosingBlockNames = enclosingBlockNames;
}

public TestResult invoke(File ws, VirtualChannel channel) throws IOException {
Expand All @@ -147,8 +146,8 @@ public TestResult invoke(File ws, VirtualChannel channel) throws IOException {

String[] files = ds.getIncludedFiles();
if (files.length > 0) {
result = new TestResult(buildTime + (nowSlave - nowMaster), ds, keepLongStdio, runId, nodeId,
enclosingBlocks);
result = new TestResult(buildTime + (nowSlave - nowMaster), ds, keepLongStdio, nodeId,
enclosingBlocks, enclosingBlockNames);
result.tally();
} else {
if (this.allowEmptyResults) {
Expand Down
14 changes: 7 additions & 7 deletions src/main/java/hudson/tasks/junit/JUnitResultArchiver.java
Expand Up @@ -124,16 +124,16 @@ public JUnitResultArchiver(
private TestResult parse(String expandedTestResults, Run<?,?> run, @Nonnull FilePath workspace, Launcher launcher, TaskListener listener)
throws IOException, InterruptedException
{
return parse(this, null, null, expandedTestResults, run, workspace, launcher, listener);
return parse(this, null, null, null, expandedTestResults, run, workspace, launcher, listener);

}

private static TestResult parse(@Nonnull JUnitTask task, @CheckForNull String nodeId, List<String> enclosingBlocks,
String expandedTestResults, Run<?,?> run, @Nonnull FilePath workspace,
List<String> enclosingBlockNames, String expandedTestResults, Run<?,?> run, @Nonnull FilePath workspace,
Launcher launcher, TaskListener listener)
throws IOException, InterruptedException {
return new JUnitParser(task.isKeepLongStdio(), task.isAllowEmptyResults())
.parseResult(expandedTestResults, run, nodeId, enclosingBlocks, workspace, launcher, listener);
.parseResult(expandedTestResults, run, nodeId, enclosingBlocks, enclosingBlockNames, workspace, launcher, listener);
}

@Deprecated
Expand All @@ -150,21 +150,21 @@ protected TestResult parse(String expandedTestResults, AbstractBuild build, Laun
@Override
public void perform(Run build, FilePath workspace, Launcher launcher,
TaskListener listener) throws InterruptedException, IOException {
TestResultAction action = parseAndAttach(this, null, null, build, workspace, launcher, listener);
TestResultAction action = parseAndAttach(this, null, null, null, build, workspace, launcher, listener);

if (action != null && action.getResult().getFailCount() > 0)
build.setResult(Result.UNSTABLE);
}

public static TestResultAction parseAndAttach(@Nonnull JUnitTask task, @CheckForNull String nodeId,
List<String> enclosingBlocks, Run build, FilePath workspace,
Launcher launcher, TaskListener listener)
List<String> enclosingBlocks, List<String> enclosingBlockNames,
Run build, FilePath workspace, Launcher launcher, TaskListener listener)
throws InterruptedException, IOException {
listener.getLogger().println(Messages.JUnitResultArchiver_Recording());

final String testResults = build.getEnvironment(listener).expand(task.getTestResults());

TestResult result = parse(task, nodeId, enclosingBlocks, testResults, build, workspace, launcher, listener);
TestResult result = parse(task, nodeId, enclosingBlocks, enclosingBlockNames, testResults, build, workspace, launcher, listener);

synchronized (build) {
// TODO can the build argument be omitted now, or is it used prior to the call to addAction?
Expand Down
88 changes: 24 additions & 64 deletions src/main/java/hudson/tasks/junit/SuiteResult.java
Expand Up @@ -91,11 +91,6 @@ public final class SuiteResult implements Serializable {
**/
private String time;

/**
* Optional {@link Run#getExternalizableId()} this suite was generated in.
*/
private String runId;

/**
* Optional {@link FlowNode#getId()} this suite was generated in.
*/
Expand All @@ -118,22 +113,23 @@ public final class SuiteResult implements Serializable {
}

/**
* @since 1.21
* @since 1.22
*/
SuiteResult(String name, String stdout, String stderr, @CheckForNull String runId, @CheckForNull String nodeId,
@CheckForNull List<String> enclosingBlocks) {
SuiteResult(String name, String stdout, String stderr, @CheckForNull String nodeId,
@CheckForNull List<String> enclosingBlocks, @CheckForNull List<String> enclosingBlockNames) {
this.name = name;
this.stderr = stderr;
this.stdout = stdout;
// runId is generally going to be not null, but we only care about it if both it and nodeId are not null.
if (runId != null && nodeId != null) {
this.runId = runId;
if (nodeId != null) {
this.nodeId = nodeId;
if (enclosingBlocks != null) {
this.enclosingBlocks.addAll(enclosingBlocks);
}
if (enclosingBlockNames != null) {
this.enclosingBlockNames.addAll(enclosingBlockNames);
}
} else {
this.runId = null;
this.nodeId = null;
}
this.file = null;
Expand Down Expand Up @@ -169,8 +165,8 @@ public static class SuiteResultParserConfigurationContext {
* This method returns a collection, as a single XML may have multiple &lt;testsuite>
* elements wrapped into the top-level &lt;testsuites>.
*/
static List<SuiteResult> parse(File xmlReport, boolean keepLongStdio, @CheckForNull String runId, @CheckForNull String nodeId,
@CheckForNull List<String> enclosingBlocks)
static List<SuiteResult> parse(File xmlReport, boolean keepLongStdio, @CheckForNull String nodeId,
@CheckForNull List<String> enclosingBlocks, @CheckForNull List<String> enclosingBlockNames)
throws DocumentException, IOException, InterruptedException {
List<SuiteResult> r = new ArrayList<SuiteResult>();

Expand All @@ -183,34 +179,35 @@ static List<SuiteResult> parse(File xmlReport, boolean keepLongStdio, @CheckForN
Document result = saxReader.read(xmlReportStream);
Element root = result.getRootElement();

parseSuite(xmlReport, keepLongStdio, r, root, runId, nodeId, enclosingBlocks);
parseSuite(xmlReport, keepLongStdio, r, root, nodeId, enclosingBlocks, enclosingBlockNames);
} finally {
xmlReportStream.close();
}

return r;
}

private static void parseSuite(File xmlReport, boolean keepLongStdio, List<SuiteResult> r, Element root, @CheckForNull String runId,
@CheckForNull String nodeId, @CheckForNull List<String> enclosingBlocks) throws DocumentException, IOException {
private static void parseSuite(File xmlReport, boolean keepLongStdio, List<SuiteResult> r, Element root,
@CheckForNull String nodeId, @CheckForNull List<String> enclosingBlocks,
@CheckForNull List<String> enclosingBlockNames) throws DocumentException, IOException {
// nested test suites
@SuppressWarnings("unchecked")
List<Element> testSuites = (List<Element>) root.elements("testsuite");
for (Element suite : testSuites)
parseSuite(xmlReport, keepLongStdio, r, suite, runId, nodeId, enclosingBlocks);
parseSuite(xmlReport, keepLongStdio, r, suite, nodeId, enclosingBlocks, enclosingBlockNames);

// child test cases
// FIXME: do this also if no testcases!
if (root.element("testcase") != null || root.element("error") != null)
r.add(new SuiteResult(xmlReport, root, keepLongStdio, runId, nodeId, enclosingBlocks));
r.add(new SuiteResult(xmlReport, root, keepLongStdio, nodeId, enclosingBlocks, enclosingBlockNames));
}

/**
* @param xmlReport A JUnit XML report file whose top level element is 'testsuite'.
* @param suite The parsed result of {@code xmlReport}
*/
private SuiteResult(File xmlReport, Element suite, boolean keepLongStdio, @CheckForNull String runId,
@CheckForNull String nodeId, @CheckForNull List<String> enclosingBlocks)
private SuiteResult(File xmlReport, Element suite, boolean keepLongStdio, @CheckForNull String nodeId,
@CheckForNull List<String> enclosingBlocks, @CheckForNull List<String> enclosingBlockNames)
throws DocumentException, IOException {
this.file = xmlReport.getAbsolutePath();
String name = suite.attributeValue("name");
Expand All @@ -225,18 +222,14 @@ private SuiteResult(File xmlReport, Element suite, boolean keepLongStdio, @Check
this.name = TestObject.safe(name);
this.timestamp = suite.attributeValue("timestamp");
this.id = suite.attributeValue("id");
if (runId != null && nodeId != null) {
this.runId = runId;
if (nodeId != null) {
this.nodeId = nodeId;
if (enclosingBlocks != null) {
Run<?, ?> r = Run.fromExternalizableId(runId);
if (r instanceof WorkflowRun) {
for (String enc : enclosingBlocks) {
this.enclosingBlockNames.add(getEnclosingNodeDisplayName((WorkflowRun)r, enc));
}
}
this.enclosingBlocks.addAll(enclosingBlocks);
}
if (enclosingBlockNames != null) {
this.enclosingBlockNames.addAll(enclosingBlockNames);
}
}

// check for test suite time attribute
Expand Down Expand Up @@ -307,28 +300,6 @@ private SuiteResult(File xmlReport, Element suite, boolean keepLongStdio, @Check
}
}

@Nonnull
private String getEnclosingNodeDisplayName(@Nonnull WorkflowRun run, @Nonnull String nodeId) {
FlowExecution execution = run.getExecution();
if (execution != null) {
try {
FlowNode node = execution.getNode(nodeId);
if (node != null) {
if (node.getAction(LabelAction.class) != null) {
ThreadNameAction threadNameAction = node.getAction(ThreadNameAction.class);
if (threadNameAction != null) {
return threadNameAction.getThreadName();
}
}
return node.getDisplayName();
}
} catch (Exception e) {
// TODO: Something in that odd case where we can get an NPE
}
}
return "";
}

/**
* Returns true if the time attribute is present in this Suite.
*/
Expand All @@ -346,21 +317,10 @@ public float getDuration() {
return duration;
}

/**
* The possibly-null {@link Run#getExternalizableId()} this suite was generated in.
*
* @since 1.21
*/
@CheckForNull
@Exported(visibility=9)
public String getRunId() {
return runId;
}

/**
* The possibly-null {@link FlowNode#id} this suite was generated in.
*
* @since 1.21
* @since 1.22
*/
@Exported(visibility=9)
@CheckForNull
Expand All @@ -371,7 +331,7 @@ public String getNodeId() {
/**
* The possibly-empty list of {@link FlowNode#id}s for enclosing blocks within which this suite was generated.
*
* @since 1.21
* @since 1.22
*/
@Exported(visibility=9)
@Nonnull
Expand All @@ -386,7 +346,7 @@ public List<String> getEnclosingBlocks() {
/**
* The possibly-empty list of display names of enclosing blocks within which this suite was generated.
*
* @since 1.21
* @since 1.22
*/
@Exported(visibility=9)
@Nonnull
Expand Down

0 comments on commit 147692d

Please sign in to comment.