Skip to content

Commit

Permalink
Merge pull request #83 from abayer/jenkins-27395-remove-run-id
Browse files Browse the repository at this point in the history 
[JENKINS-27395] Removed per-run node tracking
  • Loading branch information
@abayer
abayer committed Nov 2, 2017
2 parents 1c00c38 + 279ffb9 commit b98cb6e
Show file tree
Hide file tree
Showing 15 changed files with 250 additions and 272 deletions.
19 changes: 10 additions & 9 deletions src/main/java/hudson/tasks/junit/CaseResult.java
View file
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
31 changes: 13 additions & 18 deletions src/main/java/hudson/tasks/junit/JUnitParser.java
View file
Expand Up @@ -24,6 +24,7 @@
package hudson.tasks.junit;

import hudson.model.TaskListener;
import hudson.tasks.test.PipelineTestDetails;
import hudson.tasks.test.TestResultParser;
import hudson.*;
import hudson.model.AbstractBuild;
Expand All @@ -32,14 +33,14 @@

import java.io.IOException;
import java.io.File;
import java.util.List;

import jenkins.MasterToSlaveFileCallable;

import org.apache.tools.ant.types.FileSet;
import org.apache.tools.ant.DirectoryScanner;

import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;

/**
* Parse some JUnit xml files and generate a TestResult containing all the
Expand Down Expand Up @@ -97,22 +98,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, 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 {
public TestResult parseResult(String testResultLocations, Run<?,?> build, PipelineTestDetails pipelineTestDetails,
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, pipelineTestDetails));
}

private static final class ParseResultCallable extends MasterToSlaveFileCallable<TestResult> {
Expand All @@ -121,21 +121,17 @@ 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 PipelineTestDetails pipelineTestDetails;

private ParseResultCallable(String testResults, long buildTime, long nowMaster,
boolean keepLongStdio, boolean allowEmptyResults, String runId, @CheckForNull String nodeId,
List<String> enclosingBlocks) {
boolean keepLongStdio, boolean allowEmptyResults,
PipelineTestDetails pipelineTestDetails) {
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.pipelineTestDetails = pipelineTestDetails;
}

public TestResult invoke(File ws, VirtualChannel channel) throws IOException {
Expand All @@ -147,8 +143,7 @@ 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, pipelineTestDetails);
result.tally();
} else {
if (this.allowEmptyResults) {
Expand Down
17 changes: 8 additions & 9 deletions src/main/java/hudson/tasks/junit/JUnitResultArchiver.java
View file
Expand Up @@ -41,6 +41,7 @@
import hudson.tasks.Publisher;
import hudson.tasks.Recorder;
import hudson.tasks.junit.TestResultAction.Data;
import hudson.tasks.test.PipelineTestDetails;
import hudson.util.DescribableList;
import hudson.util.FormValidation;
import org.apache.tools.ant.DirectoryScanner;
Expand All @@ -52,7 +53,6 @@
import java.io.IOException;
import java.util.Collections;
import java.util.List;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import jenkins.tasks.SimpleBuildStep;
import org.kohsuke.stapler.DataBoundSetter;
Expand Down 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, expandedTestResults, run, workspace, launcher, listener);

}

private static TestResult parse(@Nonnull JUnitTask task, @CheckForNull String nodeId, List<String> enclosingBlocks,
private static TestResult parse(@Nonnull JUnitTask task, PipelineTestDetails pipelineTestDetails,
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, pipelineTestDetails, workspace, launcher, listener);
}

@Deprecated
Expand All @@ -150,21 +150,20 @@ 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, 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)
public static TestResultAction parseAndAttach(@Nonnull JUnitTask task, PipelineTestDetails pipelineTestDetails,
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, pipelineTestDetails, 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
100 changes: 22 additions & 78 deletions src/main/java/hudson/tasks/junit/SuiteResult.java
View file
Expand Up @@ -23,18 +23,14 @@
*/
package hudson.tasks.junit;

import hudson.model.Run;
import hudson.tasks.test.PipelineTestDetails;
import hudson.tasks.test.TestObject;
import hudson.util.io.ParserConfigurator;
import org.dom4j.Document;
import org.dom4j.DocumentException;
import org.dom4j.Element;
import org.dom4j.io.SAXReader;
import org.jenkinsci.plugins.workflow.actions.LabelAction;
import org.jenkinsci.plugins.workflow.actions.ThreadNameAction;
import org.jenkinsci.plugins.workflow.flow.FlowExecution;
import org.jenkinsci.plugins.workflow.graph.FlowNode;
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
import org.kohsuke.stapler.export.Exported;
import org.kohsuke.stapler.export.ExportedBean;

Expand Down Expand Up @@ -91,11 +87,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 @@ -114,26 +105,22 @@ public final class SuiteResult implements Serializable {

@Deprecated
SuiteResult(String name, String stdout, String stderr) {
this(name, stdout, stderr, null, null, null);
this(name, stdout, stderr, null);
}

/**
* @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 PipelineTestDetails pipelineTestDetails) {
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;
this.nodeId = nodeId;
if (enclosingBlocks != null) {
this.enclosingBlocks.addAll(enclosingBlocks);
}
if (pipelineTestDetails != null && pipelineTestDetails.getNodeId() != null) {
this.nodeId = pipelineTestDetails.getNodeId();
this.enclosingBlocks.addAll(pipelineTestDetails.getEnclosingBlocks());
this.enclosingBlockNames.addAll(pipelineTestDetails.getEnclosingBlockNames());
} else {
this.runId = null;
this.nodeId = null;
}
this.file = null;
Expand Down Expand Up @@ -169,8 +156,7 @@ 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, PipelineTestDetails pipelineTestDetails)
throws DocumentException, IOException, InterruptedException {
List<SuiteResult> r = new ArrayList<SuiteResult>();

Expand All @@ -183,34 +169,33 @@ 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, pipelineTestDetails);
} 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,
PipelineTestDetails pipelineTestDetails) 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, pipelineTestDetails);

// 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, pipelineTestDetails));
}

/**
* @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 PipelineTestDetails pipelineTestDetails)
throws DocumentException, IOException {
this.file = xmlReport.getAbsolutePath();
String name = suite.attributeValue("name");
Expand All @@ -225,18 +210,10 @@ 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;
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 (pipelineTestDetails != null && pipelineTestDetails.getNodeId() != null) {
this.nodeId = pipelineTestDetails.getNodeId();
this.enclosingBlocks.addAll(pipelineTestDetails.getEnclosingBlocks());
this.enclosingBlockNames.addAll(pipelineTestDetails.getEnclosingBlockNames());
}

// check for test suite time attribute
Expand Down Expand Up @@ -307,28 +284,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 +301,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 +315,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 +330,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 b98cb6e

Please sign in to comment.