[FIXED JENKINS-22681] Fix of JENKINS-19418 should have used getById, not load,...

[FIXED JENKINS-22681] Fix of JENKINS-19418 should have used getById, not load, to avoid reloading the last build gratuitously.

changelog.html

@@ -66,6 +66,9 @@ Upcoming changes</a>
   <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>)

core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java

@@ -509,7 +509,7 @@ public abstract class AbstractLazyLoadRunMap<R> extends AbstractMap<Integer,R> i
             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);

core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java

@@ -149,6 +149,7 @@ public class AbstractLazyLoadRunMapTest extends Assert {
         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 class AbstractLazyLoadRunMapTest extends Assert {
         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
      */

core/src/test/java/jenkins/model/lazy/FakeMap.java

@@ -82,4 +82,9 @@ class Build {
         assert this.n==n;
         assert this.id.equals(id);
     }
+
+    @Override public String toString() {
+        return "Build #" + n + " [" + id + "] @" + hashCode();
+    }
+
 }
\ No newline at end of file