LazyBuildMixIn.getEstimatedDurationCandidates duplication (#8233)

jenkinsci · Jul 11, 2023 · 8de949b · 8de949b
1 parent 616716d
commit 8de949b
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 145 deletions.
diff --git a/core/src/main/java/jenkins/model/lazy/LazyBuildMixIn.java b/core/src/main/java/jenkins/model/lazy/LazyBuildMixIn.java
@@ -277,17 +277,22 @@ public List<RunT> getEstimatedDurationCandidates() {
         List<RunT> candidates = new ArrayList<>(3);
         for (Result threshold : List.of(Result.UNSTABLE, Result.FAILURE)) {
             for (RunT build : loadedBuilds) {
+                if (candidates.contains(build)) {
+                    continue;
+                }
                 if (!build.isBuilding()) {
                     Result result = build.getResult();
                     if (result != null && result.isBetterOrEqualTo(threshold)) {
                         candidates.add(build);
                         if (candidates.size() == 3) {
+                            LOGGER.fine(() -> "Candidates: " + candidates);
                             return candidates;
                         }
                     }
                 }
             }
         }
+        LOGGER.fine(() -> "Candidates: " + candidates);
         return candidates;
     }
 

diff --git a/test/src/test/java/hudson/model/SimpleJobTest.java b/test/src/test/java/hudson/model/SimpleJobTest.java
@@ -1,188 +1,111 @@
 package hudson.model;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
 
-import java.io.IOException;
-import java.util.SortedMap;
-import java.util.TreeMap;
+import java.util.logging.Level;
+import jenkins.model.lazy.LazyBuildMixIn;
+import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.LoggerRule;
 
 /**
  * Unit test for {@link Job}.
  */
 @SuppressWarnings("rawtypes")
 public class SimpleJobTest {
 
+    @ClassRule
+    public static JenkinsRule r = new JenkinsRule();
+
     @Rule
-    public JenkinsRule rule = new JenkinsRule();
+    public LoggerRule logging = new LoggerRule().record(LazyBuildMixIn.class, Level.FINE);
 
     @Test
-    public void testGetEstimatedDuration() throws IOException {
-
-        final SortedMap<Integer, TestBuild> runs = new TreeMap<>();
-
-        Job project = createMockProject(runs);
+    public void testGetEstimatedDuration() throws Exception {
+        var project = r.createFreeStyleProject("testGetEstimatedDuration");
 
-        TestBuild previousPreviousBuild = new TestBuild(project, Result.SUCCESS, 20, null);
-        runs.put(3, previousPreviousBuild);
+        var b1 = r.buildAndAssertSuccess(project);
+        b1.duration = 200;
+        assertEquals(200, project.getEstimatedDuration());
 
-        TestBuild previousBuild = new TestBuild(project, Result.SUCCESS, 15, previousPreviousBuild);
-        runs.put(2, previousBuild);
+        var b2 = r.buildAndAssertSuccess(project);
+        b2.duration = 150;
+        assertEquals(175, project.getEstimatedDuration());
 
-        TestBuild lastBuild = new TestBuild(project, Result.SUCCESS, 42, previousBuild);
-        runs.put(1, lastBuild);
-
-        // without assuming to know too much about the internal calculation
-        // we can only assume that the result is between the maximum and the minimum
-        assertTrue("Expected < 42, but was " + project.getEstimatedDuration(), project.getEstimatedDuration() < 42);
-        assertTrue("Expected > 15, but was " + project.getEstimatedDuration(), project.getEstimatedDuration() > 15);
+        var b3 = r.buildAndAssertSuccess(project);
+        b3.duration = 400;
+        assertEquals(250, project.getEstimatedDuration());
     }
 
     @Test
-    public void testGetEstimatedDurationWithOneRun() throws IOException {
-
-        final SortedMap<Integer, TestBuild> runs = new TreeMap<>();
-
-        Job project = createMockProject(runs);
+    public void testGetEstimatedDurationWithOneRun() throws Exception {
+        var project = r.createFreeStyleProject("testGetEstimatedDurationWithOneRun");
 
-        TestBuild lastBuild = new TestBuild(project, Result.SUCCESS, 42, null);
-        runs.put(1, lastBuild);
-
-        assertEquals(42, project.getEstimatedDuration());
+        var b1 = r.buildAndAssertSuccess(project);
+        b1.duration = 420;
+        assertEquals(420, project.getEstimatedDuration());
     }
 
     @Test
-    public void testGetEstimatedDurationWithFailedRun() throws IOException {
-
-        final SortedMap<Integer, TestBuild> runs = new TreeMap<>();
-
-        Job project = createMockProject(runs);
+    public void testGetEstimatedDurationWithFailedRun() throws Exception {
+        var project = r.createFreeStyleProject("testGetEstimatedDurationWithFailedRun");
 
-        TestBuild lastBuild = new TestBuild(project, Result.FAILURE, 42, null);
-        runs.put(1, lastBuild);
-
-        assertEquals(42, project.getEstimatedDuration());
+        var b1 = r.buildAndAssertSuccess(project);
+        b1.result = Result.FAILURE;
+        b1.duration = 420;
+        assertEquals(420, project.getEstimatedDuration());
     }
 
     @Test
-    public void testGetEstimatedDurationWithNoRuns() {
-
-        final SortedMap<Integer, TestBuild> runs = new TreeMap<>();
-
-        Job project = createMockProject(runs);
+    public void testGetEstimatedDurationWithNoRuns() throws Exception {
+        var project = r.createFreeStyleProject("testGetEstimatedDurationWithNoRuns");
 
         assertEquals(-1, project.getEstimatedDuration());
     }
 
     @Test
-    public void testGetEstimatedDurationIfPrevious3BuildsFailed() throws IOException {
-
-        final SortedMap<Integer, TestBuild> runs = new TreeMap<>();
-
-        Job project = createMockProject(runs);
-
-        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);
-        runs.put(4, prev3Build);
-
-        TestBuild previous2Build = new TestBuild(project, Result.FAILURE, 50, prev3Build);
-        runs.put(3, previous2Build);
-
-        TestBuild previousBuild = new TestBuild(project, Result.FAILURE, 50, previous2Build);
-        runs.put(2, previousBuild);
-
-        TestBuild lastBuild = new TestBuild(project, Result.FAILURE, 50, previousBuild);
-        runs.put(1, lastBuild);
-
-        // failed builds must not be used, if there are successfulBuilds available.
-        assertEquals(1, project.getEstimatedDuration());
+    public void testGetEstimatedDurationIfPrevious3BuildsFailed() throws Exception {
+        var project = r.createFreeStyleProject("testGetEstimatedDurationIfPrevious3BuildsFailed");
+
+        var b1 = r.buildAndAssertSuccess(project);
+        b1.result = Result.UNSTABLE;
+        b1.duration = 100;
+        assertEquals(100, project.getEstimatedDuration());
+
+        var b2 = r.buildAndAssertSuccess(project);
+        b2.duration = 200;
+        assertEquals(150, project.getEstimatedDuration());
+
+        var b3 = r.buildAndAssertSuccess(project);
+        b3.duration = 300;
+        assertEquals(200, project.getEstimatedDuration());
+
+        var b4 = r.buildAndAssertSuccess(project);
+        b4.result = Result.FAILURE;
+        b4.duration = 500;
+        assertEquals(200, project.getEstimatedDuration());
+
+        var b5 = r.buildAndAssertSuccess(project);
+        b5.result = Result.FAILURE;
+        b5.duration = 500;
+        assertEquals(200, project.getEstimatedDuration());
+
+        var b6 = r.buildAndAssertSuccess(project);
+        b6.result = Result.FAILURE;
+        b6.duration = 500;
+        assertEquals(200, project.getEstimatedDuration());
     }
 
     @Test
-    public void testGetEstimatedDurationIfNoSuccessfulBuildTakeDurationOfFailedBuild() throws IOException {
-
-        final SortedMap<Integer, TestBuild> runs = new TreeMap<>();
-
-        Job project = createMockProject(runs);
-
-        TestBuild lastBuild = new TestBuild(project, Result.FAILURE, 50, null);
-        runs.put(1, lastBuild);
-
-        assertEquals(50, project.getEstimatedDuration());
-    }
-
-    private Job createMockProject(final SortedMap<Integer, TestBuild> runs) {
-        return new TestJob(runs);
-    }
-
-    @SuppressWarnings("unchecked")
-    private static class TestBuild extends Run {
-
-        TestBuild(Job project, Result result, long duration, TestBuild previousBuild) throws IOException {
-            super(project);
-            this.result = result;
-            this.duration = duration;
-            this.previousBuild = previousBuild;
-        }
-
-        @Override
-        public int compareTo(Run o) {
-            return 0;
-        }
-
-        @Override
-        public Result getResult() {
-            return result;
-        }
-
-        @Override
-        public boolean isBuilding() {
-            return false;
-        }
+    public void testGetEstimatedDurationIfNoSuccessfulBuildTakeDurationOfFailedBuild() throws Exception {
+        var project = r.createFreeStyleProject("testGetEstimatedDurationIfNoSuccessfulBuildTakeDurationOfFailedBuild");
 
+        var b1 = r.buildAndAssertSuccess(project);
+        b1.result = Result.FAILURE;
+        b1.duration = 500;
+        assertEquals(500, project.getEstimatedDuration());
     }
 
-    private class TestJob extends Job implements TopLevelItem {
-
-        int i;
-        private final SortedMap<Integer, TestBuild> runs;
-
-        TestJob(SortedMap<Integer, TestBuild> runs) {
-            super(rule.jenkins, "name");
-            this.runs = runs;
-            i = 1;
-        }
-
-        @Override
-        public int assignBuildNumber() {
-            return i++;
-        }
-
-        @Override
-        public SortedMap<Integer, ? extends Run> _getRuns() {
-            return runs;
-        }
-
-        @Override
-        public boolean isBuildable() {
-            return true;
-        }
-
-        @Override
-        protected void removeRun(Run run) {
-        }
-
-        @Override
-        public TopLevelItemDescriptor getDescriptor() {
-            throw new AssertionError();
-        }
-    }
 }