Skip to content
Permalink
Browse files

[JENKINS-18065] Uncommenting original test.

The actual implementation does not behave as nicely as one would hope, for a few reasons:
1. isEmpty actually tries to load the newestBuild, rather than simply checking whether there is some idOnDisk.
   Arguably this is necessary, since there could be an unloadable build record, in which case it would be technically correct for the map to be considered empty.
2. Loading AbstractLazyLoadRunMap.newestBuild calls search(MAX_VALUE, DESC), which behaves oddly, by doing a binary search, and thus perhaps loading lg(|map|) entries.
   You would think that it would suffice to check for the last member of idOnDisk in index.byId.
3. The iterator eagerly loads the next value before hasNext has even been called.
   Looks bad in a test, though it probably has little practical impact since most callers would be calling hasNext soon afterward anyway.
   Might cause one extra build record to be loaded unnecessarily from a limited RunList.

(cherry picked from commit 2f58ceb)
  • Loading branch information
jglick authored and olivergondza committed Aug 1, 2014
1 parent f1b0779 commit 8c70a2b76875d2ca6a114c45fd54c0b1e9ecb7d2
@@ -38,7 +38,6 @@
import java.util.SortedMap;
import java.util.logging.Level;
import org.junit.BeforeClass;
import org.junit.Ignore;
import org.jvnet.hudson.test.Bug;
import org.jvnet.hudson.test.Issue;

@@ -325,40 +324,39 @@ public void indexOutOfBounds() throws Exception {
assertNull(map.search(3, Direction.DESC));
}

@Ignore("just calling entrySet triggers loading of every build!")
@Bug(18065)
@Test public void all() throws Exception {
assertEquals("[]", a.getLoadedBuilds().keySet().toString());
Set<Map.Entry<Integer,Build>> entries = a.entrySet();
assertEquals("[]", a.getLoadedBuilds().keySet().toString());
assertFalse(entries.isEmpty());
assertEquals("[]", a.getLoadedBuilds().keySet().toString());
assertEquals("5 since it is the latest, and 3 because, well, .search and pivot is weird", "[5, 3]", a.getLoadedBuilds().keySet().toString());
assertEquals(5, a.getById("C").n);
assertEquals("[5]", a.getLoadedBuilds().keySet().toString());
assertEquals("[5, 3]", a.getLoadedBuilds().keySet().toString());
assertEquals("A", a.getByNumber(1).id);
assertEquals("[5, 1]", a.getLoadedBuilds().keySet().toString());
assertEquals("[5, 3, 1]", a.getLoadedBuilds().keySet().toString());
a.purgeCache();
assertEquals("[]", a.getLoadedBuilds().keySet().toString());
Iterator<Map.Entry<Integer,Build>> iterator = entries.iterator();
assertEquals("[]", a.getLoadedBuilds().keySet().toString());
assertEquals("iterator starts off checking for newest build, so for this crazy logic see above", "[5, 3]", a.getLoadedBuilds().keySet().toString());
assertTrue(iterator.hasNext());
assertEquals("[]", a.getLoadedBuilds().keySet().toString());
assertEquals("[5, 3]", a.getLoadedBuilds().keySet().toString());
Map.Entry<Integer,Build> entry = iterator.next();
assertEquals("[]", a.getLoadedBuilds().keySet().toString());
assertEquals("[5, 3]", a.getLoadedBuilds().keySet().toString());
assertEquals(5, entry.getKey().intValue());
assertEquals("[]", a.getLoadedBuilds().keySet().toString());
assertEquals("[5, 3]", a.getLoadedBuilds().keySet().toString());
assertEquals("C", entry.getValue().id);
assertEquals("[5]", a.getLoadedBuilds().keySet().toString());
assertEquals("[5, 3]", a.getLoadedBuilds().keySet().toString());
assertTrue(iterator.hasNext());
entry = iterator.next();
assertEquals(3, entry.getKey().intValue());
assertEquals("[5]", a.getLoadedBuilds().keySet().toString());
assertEquals(".next() precomputes the one after that too", "[5, 3, 1]", a.getLoadedBuilds().keySet().toString());
assertEquals("B", entry.getValue().id);
assertEquals("[5, 3]", a.getLoadedBuilds().keySet().toString());
assertEquals("[5, 3, 1]", a.getLoadedBuilds().keySet().toString());
assertTrue(iterator.hasNext());
entry = iterator.next();
assertEquals(1, entry.getKey().intValue());
assertEquals("[5, 3]", a.getLoadedBuilds().keySet().toString());
assertEquals("[5, 3, 1]", a.getLoadedBuilds().keySet().toString());
assertEquals("A", entry.getValue().id);
assertEquals("[5, 3, 1]", a.getLoadedBuilds().keySet().toString());
assertFalse(iterator.hasNext());
@@ -65,6 +65,7 @@ public boolean accept(File dir, String name) {
protected Build retrieve(File dir) throws IOException {
String n = FileUtils.readFileToString(new File(dir, "n")).trim();
String id = FileUtils.readFileToString(new File(dir, "id")).trim();
//new Exception("loading " + id + " #" + n).printStackTrace();
return new Build(Integer.parseInt(n),id);
}
}

0 comments on commit 8c70a2b

Please sign in to comment.
You can’t perform that action at this time.