From 04d85eb476bdc57eb7ac3c2bb34e91be5b55c487 Mon Sep 17 00:00:00 2001 From: Christoph Kutzinski Date: Sat, 6 Jul 2013 12:49:47 +0200 Subject: [PATCH] Loading the last 3 successful builds may be very expensive in certain rare, but possible situations (all newer builds failed, only some very old builds were successful). Go only up to 6 build into the past for calculating the estimated duration. Also take failed builds into account, if we don't find any successful ones. --- core/src/main/java/hudson/model/Job.java | 49 ++++++++++++++++++- core/src/main/java/hudson/model/Result.java | 27 +++++++--- .../hudson/maven/MavenModuleSetBuild.java | 2 +- .../test/java/hudson/model/SimpleJobTest.java | 47 +++++++++++++----- 4 files changed, 106 insertions(+), 19 deletions(-) diff --git a/core/src/main/java/hudson/model/Job.java b/core/src/main/java/hudson/model/Job.java index 523e3de856..c7723bf9b2 100644 --- a/core/src/main/java/hudson/model/Job.java +++ b/core/src/main/java/hudson/model/Job.java @@ -26,6 +26,7 @@ package hudson.model; import com.google.common.base.Function; import com.google.common.collect.Collections2; import com.infradna.tool.bridge_method_injector.WithBridgeMethods; + import hudson.EnvVars; import hudson.Extension; import hudson.ExtensionPoint; @@ -67,6 +68,7 @@ import jenkins.security.HexStringConfidentialKey; import jenkins.util.io.OnMaster; import net.sf.json.JSONException; import net.sf.json.JSONObject; + import org.jfree.chart.ChartFactory; import org.jfree.chart.JFreeChart; import org.jfree.chart.axis.CategoryAxis; @@ -87,6 +89,7 @@ import org.kohsuke.stapler.export.Exported; import org.kohsuke.stapler.interceptor.RequirePOST; import javax.servlet.ServletException; + import java.awt.*; import java.io.*; import java.net.URLEncoder; @@ -898,8 +901,52 @@ public abstract class Job, RunT extends Run getEstimedDurationCandidates() { + List candidates = new ArrayList(3); + RunT lastSuccessful = (RunT) Permalink.LAST_SUCCESSFUL_BUILD.resolve(this); + int lastSuccessfulNumber = -1; + if (lastSuccessful != null) { + candidates.add(lastSuccessful); + lastSuccessfulNumber = lastSuccessful.getNumber(); + } + + int i = 0; + RunT r = (RunT) Permalink.LAST_BUILD.resolve(this); + List fallbackCandidates = new ArrayList(3); + while (r != null && candidates.size() < 3 && i < 6) { + if (!r.isBuilding() && r.getResult() != null && r.getNumber() != lastSuccessfulNumber) { + Result result = r.getResult(); + if (result.isBetterOrEqualTo(Result.UNSTABLE)) { + candidates.add(r); + } else if (result.isCompleteBuild()) { + fallbackCandidates.add(r); + } + } + i++; + r = r.getPreviousBuild(); + } + + while (candidates.size() < 3) { + if (fallbackCandidates.isEmpty()) + break; + RunT run = fallbackCandidates.remove(0); + candidates.add(run); + } + + return candidates; + } + public long getEstimatedDuration() { - List builds = getLastBuildsOverThreshold(3, Result.UNSTABLE); + List builds = getEstimedDurationCandidates(); if(builds.isEmpty()) return -1; diff --git a/core/src/main/java/hudson/model/Result.java b/core/src/main/java/hudson/model/Result.java index d7f4a5970c..b95001e07d 100644 --- a/core/src/main/java/hudson/model/Result.java +++ b/core/src/main/java/hudson/model/Result.java @@ -48,30 +48,30 @@ public final class Result implements Serializable, CustomExportedBean { /** * The build had no errors. */ - public static final Result SUCCESS = new Result("SUCCESS",BallColor.BLUE,0); + public static final Result SUCCESS = new Result("SUCCESS",BallColor.BLUE,0,true); /** * The build had some errors but they were not fatal. * For example, some tests failed. */ - public static final Result UNSTABLE = new Result("UNSTABLE",BallColor.YELLOW,1); + public static final Result UNSTABLE = new Result("UNSTABLE",BallColor.YELLOW,1,true); /** * The build had a fatal error. */ - public static final Result FAILURE = new Result("FAILURE",BallColor.RED,2); + public static final Result FAILURE = new Result("FAILURE",BallColor.RED,2,true); /** * The module was not built. *

* This status code is used in a multi-stage build (like maven2) * where a problem in earlier stage prevented later stages from building. */ - public static final Result NOT_BUILT = new Result("NOT_BUILT",BallColor.NOTBUILT,3); + public static final Result NOT_BUILT = new Result("NOT_BUILT",BallColor.NOTBUILT,3,false); /** * The build was manually aborted. * * If you are catching {@link InterruptedException} and interpreting it as {@link #ABORTED}, * you should check {@link Executor#abortResult()} instead (starting 1.417.) */ - public static final Result ABORTED = new Result("ABORTED",BallColor.ABORTED,4); + public static final Result ABORTED = new Result("ABORTED",BallColor.ABORTED,4,false); private final String name; @@ -84,11 +84,17 @@ public final class Result implements Serializable, CustomExportedBean { * Default ball color for this status. */ public final BallColor color; + + /** + * Is this a complete build - i.e. did it run to the end (not aborted)? + */ + public final boolean completeBuild; - private Result(String name, BallColor color, int ordinal) { + private Result(String name, BallColor color, int ordinal, boolean complete) { this.name = name; this.color = color; this.ordinal = ordinal; + this.completeBuild = complete; } /** @@ -116,6 +122,15 @@ public final class Result implements Serializable, CustomExportedBean { public boolean isBetterOrEqualTo(Result that) { return this.ordinal <= that.ordinal; } + + /** + * Is this a complete build - i.e. did it run to the end (not aborted)? + * + * @since TODO + */ + public boolean isCompleteBuild() { + return this.completeBuild; + } @Override diff --git a/maven-plugin/src/main/java/hudson/maven/MavenModuleSetBuild.java b/maven-plugin/src/main/java/hudson/maven/MavenModuleSetBuild.java index dab68b559d..3460f4e999 100644 --- a/maven-plugin/src/main/java/hudson/maven/MavenModuleSetBuild.java +++ b/maven-plugin/src/main/java/hudson/maven/MavenModuleSetBuild.java @@ -333,7 +333,7 @@ public class MavenModuleSetBuild extends AbstractMavenBuild moduleSetBuilds = getPreviousBuildsOverThreshold(numberOfBuilds, Result.UNSTABLE); + List moduleSetBuilds = getParent().getEstimedDurationCandidates(); if (moduleSetBuilds.isEmpty()) { return 0; diff --git a/test/src/test/java/hudson/model/SimpleJobTest.java b/test/src/test/java/hudson/model/SimpleJobTest.java index 6e36e2cfbd..43523b04ad 100644 --- a/test/src/test/java/hudson/model/SimpleJobTest.java +++ b/test/src/test/java/hudson/model/SimpleJobTest.java @@ -4,15 +4,21 @@ import java.io.IOException; import java.util.SortedMap; import java.util.TreeMap; -import junit.framework.Assert; -import org.jvnet.hudson.test.HudsonTestCase; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; /** * Unit test for {@link Job}. */ @SuppressWarnings("rawtypes") -public class SimpleJobTest extends HudsonTestCase { +public class SimpleJobTest { + @Rule + public JenkinsRule rule = new JenkinsRule(); + + @Test public void testGetEstimatedDuration() throws IOException { final SortedMap runs = new TreeMap(); @@ -28,12 +34,13 @@ public class SimpleJobTest extends HudsonTestCase { TestBuild lastBuild = new TestBuild(project, Result.SUCCESS, 42, previousBuild); runs.put(1, lastBuild); - // without assuming to know to much about the internal calculation + // without assuming to know too much about the internal calculation // we can only assume that the result is between the maximum and the minimum - Assert.assertTrue(project.getEstimatedDuration() < 42); - Assert.assertTrue(project.getEstimatedDuration() > 15); + Assert.assertTrue("Expected < 42, but was "+project.getEstimatedDuration(), project.getEstimatedDuration() < 42); + Assert.assertTrue("Expected > 15, but was "+project.getEstimatedDuration(), project.getEstimatedDuration() > 15); } + @Test public void testGetEstimatedDurationWithOneRun() throws IOException { final SortedMap runs = new TreeMap(); @@ -58,6 +65,7 @@ public class SimpleJobTest extends HudsonTestCase { Assert.assertEquals(-1, project.getEstimatedDuration()); } + @Test public void testGetEstimatedDurationWithNoRuns() throws IOException { final SortedMap runs = new TreeMap(); @@ -67,13 +75,17 @@ public class SimpleJobTest extends HudsonTestCase { Assert.assertEquals(-1, project.getEstimatedDuration()); } + @Test public void testGetEstimatedDurationIfPrevious3BuildsFailed() throws IOException { final SortedMap runs = new TreeMap(); Job project = createMockProject(runs); - TestBuild prev4Build = new TestBuild(project, Result.SUCCESS, 1, null); + TestBuild prev5Build = new TestBuild(project, Result.UNSTABLE, 1, null); + runs.put(6, prev5Build); + + TestBuild prev4Build = new TestBuild(project, Result.SUCCESS, 1, prev5Build); runs.put(5, prev4Build); TestBuild prev3Build = new TestBuild(project, Result.SUCCESS, 1, prev4Build); @@ -88,9 +100,21 @@ public class SimpleJobTest extends HudsonTestCase { TestBuild lastBuild = new TestBuild(project, Result.FAILURE, 50, previousBuild); runs.put(1, lastBuild); - // failed builds must not be used. Instead the last successful builds before them - // must be used - Assert.assertEquals(project.getEstimatedDuration(), 1); + // failed builds must not be used, if there are succesfulBuilds available. + Assert.assertEquals(1, project.getEstimatedDuration()); + } + + @Test + public void testGetEstimatedDurationIfNoSuccessfulBuildTakeDurationOfFailedBuild() throws IOException { + + final SortedMap runs = new TreeMap(); + + Job project = createMockProject(runs); + + TestBuild lastBuild = new TestBuild(project, Result.FAILURE, 50, null); + runs.put(1, lastBuild); + + Assert.assertEquals(50, project.getEstimatedDuration()); } private Job createMockProject(final SortedMap runs) { @@ -124,13 +148,14 @@ public class SimpleJobTest extends HudsonTestCase { } + @SuppressWarnings("unchecked") private class TestJob extends Job implements TopLevelItem { int i; private final SortedMap runs; public TestJob(SortedMap runs) { - super(SimpleJobTest.this.jenkins, "name"); + super(rule.jenkins, "name"); this.runs = runs; i = 1; } -- GitLab