[FIXED JENKINS-22681] Fix of JENKINS-19418 should have used getById, …

…not load, to avoid reloading the last build gratuitously.
jenkinsci · Apr 18, 2014 · b6b7bfd · b6b7bfd
1 parent b55c157
commit b6b7bfd
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 1 deletion.
diff --git a/changelog.html b/changelog.html
@@ -66,6 +66,9 @@
   <li class=bug>
     Better correction of the anomalous condition that several builds of a job specify the same number.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-22631">issue 22631</a>)
+  <li class=bug>
+    Under certain conditions, a running build could mistakenly be shown as completed (and failed), while still producing output.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-22681">issue 22681</a>)
   <li class=bug>
     Fix a bug which only showed the first detail part for radio buttons.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-22583">issue 22583</a>)

diff --git a/core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java b/core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java
@@ -509,7 +509,7 @@ public R get(int n) {
             return getById(idOnDisk.get(lo-1));
         case EXACT:
             if (hi<=0)                 return null;
-            R r = load(idOnDisk.get(hi-1), null);
+            R r = getById(idOnDisk.get(hi-1));
             if (r==null)               return null;
 
             int found = getNumberOf(r);

diff --git a/core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java b/core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java
@@ -149,6 +149,7 @@ public void search() {
         assertNull(a.search(-99, Direction.DESC));
     }
 
+    @Bug(19418)
     @Test
     public void searchExactWhenIndexedButSoftReferenceExpired() throws IOException {
         final FakeMap m = localExpiredBuilder.add(1, "A").add(2, "B").make();
@@ -161,6 +162,24 @@ public void searchExactWhenIndexedButSoftReferenceExpired() throws IOException {
         assertNull(m.search(0, Direction.EXACT));
     }
 
+    @Bug(22681)
+    @Test public void exactSearchShouldNotReload() throws Exception {
+        FakeMap m = localBuilder.add(1, "A").add(2, "B").make();
+        assertNull(m.search(0, Direction.EXACT));
+        Build a = m.search(1, Direction.EXACT);
+        a.asserts(1, "A");
+        Build b = m.search(2, Direction.EXACT);
+        b.asserts(2, "B");
+        assertNull(m.search(0, Direction.EXACT));
+        assertSame(a, m.search(1, Direction.EXACT));
+        assertSame(b, m.search(2, Direction.EXACT));
+        assertNull(m.search(3, Direction.EXACT));
+        assertNull(m.search(0, Direction.EXACT));
+        assertSame(a, m.search(1, Direction.EXACT));
+        assertSame("#2 should not have been reloaded by searching for #3", b, m.search(2, Direction.EXACT));
+        assertNull(m.search(3, Direction.EXACT));
+    }
+
     /**
      * If load fails, search needs to gracefully handle it
      */

diff --git a/core/src/test/java/jenkins/model/lazy/FakeMap.java b/core/src/test/java/jenkins/model/lazy/FakeMap.java
@@ -82,4 +82,9 @@ public void asserts(int n, String id) {
         assert this.n==n;
         assert this.id.equals(id);
     }
+
+    @Override public String toString() {
+        return "Build #" + n + " [" + id + "] @" + hashCode();
+    }
+
 }