From fcdf74991226fb6869052caa89ed8d678944b6fc Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 10 Dec 2013 17:49:41 -0500 Subject: [PATCH] [FIXED JENKINS-18410] AbstractTestResultAction should be a RunAction2 so it need not persist its owning build. --- changelog.html | 3 ++ .../tasks/junit/JUnitResultArchiver.java | 3 +- .../hudson/tasks/junit/TestResultAction.java | 6 ++++ .../tasks/test/AbstractTestResultAction.java | 32 +++++++++++++------ .../test/AggregatedTestResultAction.java | 4 +++ .../test/AggregatedTestResultPublisher.java | 7 +++- .../hudson/tasks/test/MatrixTestResult.java | 5 +++ .../tasks/test/TestResultAggregator.java | 2 +- .../tasks/test/TrivialTestResultAction.java | 7 ++++ .../tasks/test/TrivialTestResultRecorder.java | 4 +-- 10 files changed, 59 insertions(+), 14 deletions(-) diff --git a/changelog.html b/changelog.html index d51476d812..58ccb7ea64 100644 --- a/changelog.html +++ b/changelog.html @@ -61,6 +61,9 @@ Upcoming changes
  • Hudson shows 0GB free space when space available drops below 1GB. (issue 7776) +
  • + Avoiding serializing the owning build as part of a test result action, as this can lead to errors later. + (issue 18410)
  • Added filter field for installed plugins tab. (issue 20219) diff --git a/core/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java b/core/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java index 50d5d784e8..6dea15b9ed 100644 --- a/core/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java +++ b/core/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java @@ -133,6 +133,7 @@ public class JUnitResultArchiver extends Recorder implements MatrixAggregatable TestResult result = parse(testResults, build, launcher, listener); try { + // TODO can the build argument be omitted now, or is it used prior to the call to addAction? action = new TestResultAction(build, result, listener); } catch (NullPointerException npe) { throw new AbortException(Messages.JUnitResultArchiver_BadXML(testResults)); @@ -173,7 +174,7 @@ public class JUnitResultArchiver extends Recorder implements MatrixAggregatable return true; } - build.getActions().add(action); + build.addAction(action); CHECKPOINT.report(); if (action.getResult().getFailCount() > 0) diff --git a/core/src/main/java/hudson/tasks/junit/TestResultAction.java b/core/src/main/java/hudson/tasks/junit/TestResultAction.java index c27053e7a3..4c48e1b74b 100644 --- a/core/src/main/java/hudson/tasks/junit/TestResultAction.java +++ b/core/src/main/java/hudson/tasks/junit/TestResultAction.java @@ -62,11 +62,17 @@ public class TestResultAction extends AbstractTestResultAction private Integer totalCount; private List testData = new ArrayList(); + @Deprecated public TestResultAction(AbstractBuild owner, TestResult result, BuildListener listener) { super(owner); setResult(result, listener); } + /** @since 1.544 */ + public TestResultAction(TestResult result, BuildListener listener) { + this(null, result, listener); + } + /** * Overwrites the {@link TestResult} by a new data set. */ diff --git a/core/src/main/java/hudson/tasks/test/AbstractTestResultAction.java b/core/src/main/java/hudson/tasks/test/AbstractTestResultAction.java index 16ac32ad1d..925e0fb99b 100644 --- a/core/src/main/java/hudson/tasks/test/AbstractTestResultAction.java +++ b/core/src/main/java/hudson/tasks/test/AbstractTestResultAction.java @@ -28,6 +28,14 @@ import hudson.model.*; import hudson.tasks.junit.CaseResult; import hudson.util.*; import hudson.util.ChartUtil.NumberOnlyBuildLabel; + +import java.awt.*; +import java.io.IOException; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import jenkins.model.RunAction2; import org.jfree.chart.ChartFactory; import org.jfree.chart.JFreeChart; import org.jfree.chart.axis.CategoryAxis; @@ -44,13 +52,6 @@ import org.kohsuke.stapler.StaplerResponse; import org.kohsuke.stapler.export.Exported; import org.kohsuke.stapler.export.ExportedBean; -import java.awt.*; -import java.io.IOException; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; - /** * Common base class for recording test result. * @@ -61,15 +62,28 @@ import java.util.concurrent.ConcurrentHashMap; * @author Kohsuke Kawaguchi */ @ExportedBean -public abstract class AbstractTestResultAction implements HealthReportingAction { - public final AbstractBuild owner; +public abstract class AbstractTestResultAction implements HealthReportingAction, RunAction2 { + public transient AbstractBuild owner; private Map descriptions = new ConcurrentHashMap(); + /** @since 1.544 */ + protected AbstractTestResultAction() {} + + /** @deprecated Use the default constructor and just call {@link Run#addAction} to associate the build with the action. */ + @Deprecated protected AbstractTestResultAction(AbstractBuild owner) { this.owner = owner; } + @Override public void onAttached(Run r) { + this.owner = (AbstractBuild) r; + } + + @Override public void onLoad(Run r) { + this.owner = (AbstractBuild) r; + } + /** * Gets the number of failed tests. */ diff --git a/core/src/main/java/hudson/tasks/test/AggregatedTestResultAction.java b/core/src/main/java/hudson/tasks/test/AggregatedTestResultAction.java index 91d97ac64d..877cc0af6d 100644 --- a/core/src/main/java/hudson/tasks/test/AggregatedTestResultAction.java +++ b/core/src/main/java/hudson/tasks/test/AggregatedTestResultAction.java @@ -67,10 +67,14 @@ public abstract class AggregatedTestResultAction extends AbstractTestResultActio */ public final List children = new ArrayList(); + @Deprecated public AggregatedTestResultAction(AbstractBuild owner) { super(owner); } + /** @since 1.544 */ + public AggregatedTestResultAction() {} + protected void update(List children) { failCount = skipCount = totalCount = 0; this.children.clear(); diff --git a/core/src/main/java/hudson/tasks/test/AggregatedTestResultPublisher.java b/core/src/main/java/hudson/tasks/test/AggregatedTestResultPublisher.java index 2b4bd00174..709908d688 100644 --- a/core/src/main/java/hudson/tasks/test/AggregatedTestResultPublisher.java +++ b/core/src/main/java/hudson/tasks/test/AggregatedTestResultPublisher.java @@ -87,7 +87,7 @@ public class AggregatedTestResultPublisher extends Recorder { public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException { // add a TestResult just so that it can show up later. - build.addAction(new TestResultAction(jobs,includeFailedBuilds,build)); + build.addAction(new TestResultAction(jobs, includeFailedBuilds)); return true; } @@ -131,6 +131,11 @@ public class AggregatedTestResultPublisher extends Recorder { private transient List didntRun; private transient List noFingerprints; + public TestResultAction(String jobs, boolean includeFailedBuilds) { + this(jobs, includeFailedBuilds, null); + } + + @Deprecated public TestResultAction(String jobs, boolean includeFailedBuilds, AbstractBuild owner) { super(owner); this.includeFailedBuilds = includeFailedBuilds; diff --git a/core/src/main/java/hudson/tasks/test/MatrixTestResult.java b/core/src/main/java/hudson/tasks/test/MatrixTestResult.java index 5f6fa77c8c..d023fc9a6c 100644 --- a/core/src/main/java/hudson/tasks/test/MatrixTestResult.java +++ b/core/src/main/java/hudson/tasks/test/MatrixTestResult.java @@ -38,10 +38,15 @@ import hudson.model.Action; * @author Kohsuke Kawaguchi */ public class MatrixTestResult extends AggregatedTestResultAction { + + @Deprecated public MatrixTestResult(MatrixBuild owner) { super(owner); } + /** @since 1.544 */ + public MatrixTestResult() {} + /** * Use the configuration name. */ diff --git a/core/src/main/java/hudson/tasks/test/TestResultAggregator.java b/core/src/main/java/hudson/tasks/test/TestResultAggregator.java index 22d90b9134..72b76fd708 100644 --- a/core/src/main/java/hudson/tasks/test/TestResultAggregator.java +++ b/core/src/main/java/hudson/tasks/test/TestResultAggregator.java @@ -46,7 +46,7 @@ public class TestResultAggregator extends MatrixAggregator { @Override public boolean startBuild() throws InterruptedException, IOException { - result = new MatrixTestResult(build); + result = new MatrixTestResult(); build.addAction(result); return true; } diff --git a/test/src/test/java/hudson/tasks/test/TrivialTestResultAction.java b/test/src/test/java/hudson/tasks/test/TrivialTestResultAction.java index efa33f76de..993c05bf44 100644 --- a/test/src/test/java/hudson/tasks/test/TrivialTestResultAction.java +++ b/test/src/test/java/hudson/tasks/test/TrivialTestResultAction.java @@ -32,12 +32,19 @@ import org.kohsuke.stapler.StaplerProxy; public class TrivialTestResultAction extends AbstractTestResultAction implements StaplerProxy { protected TrivialTestResult result; + + @Deprecated protected TrivialTestResultAction(AbstractBuild owner, TrivialTestResult result) { super(owner); this.result = result; this.result.setParentAction(this); } + /** @since 1.544 */ + protected TrivialTestResultAction(TrivialTestResult result) { + this(null, result); + } + /** * Gets the number of failed tests. */ diff --git a/test/src/test/java/hudson/tasks/test/TrivialTestResultRecorder.java b/test/src/test/java/hudson/tasks/test/TrivialTestResultRecorder.java index 3958f1a340..aaa272bb10 100644 --- a/test/src/test/java/hudson/tasks/test/TrivialTestResultRecorder.java +++ b/test/src/test/java/hudson/tasks/test/TrivialTestResultRecorder.java @@ -48,9 +48,9 @@ public class TrivialTestResultRecorder extends Recorder implements Serializable System.out.println("performing TrviialTestResultRecorder"); listener.getLogger().println("perfoming TrivialTestResultRecorder"); TrivialTestResult r = new TrivialTestResult("gubernatorial"); - TrivialTestResultAction action = new TrivialTestResultAction(build, r); + TrivialTestResultAction action = new TrivialTestResultAction(r); r.setParentAction(action); - build.getActions().add(action); + build.addAction(action); listener.getLogger().println("done with TrivialTestResultRecorder"); System.out.println("done with TrivialTestResultRecorder"); return true;