Return parsed objects from TestRepository.commit/tree/blob()

It is convenient for TestRepository to return fully parsed
objects from its commit()/tree()/blob() methods, so that test
code doesn't have to remember to parse them before making
assertions about them.

Update TestRepostiory to return fully parsed objects.
Adjust the tests that are affected by this change in behavior.

Change-Id: I09d03d0c80ad22cb7092f4a2eaed99d40a10af63
Signed-off-by: Terry Parker <tparker@google.com>

org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java

@@ -276,7 +276,7 @@ public class TestRepository<R extends Repository> {
 	 *
 	 * @param content
 	 *            binary file content.
-	 * @return reference to the blob.
+	 * @return the new, fully parsed blob.
 	 * @throws Exception
 	 */
 	public RevBlob blob(byte[] content) throws Exception {
@@ -285,7 +285,7 @@ public class TestRepository<R extends Repository> {
 			id = ins.insert(Constants.OBJ_BLOB, content);
 			ins.flush();
 		}
-		return pool.lookupBlob(id);
+		return (RevBlob) pool.parseAny(id);
 	}
 
 	/**
@@ -312,21 +312,22 @@ public class TestRepository<R extends Repository> {
 	 * @param entries
 	 *            the files to include in the tree. The collection does not need
 	 *            to be sorted properly and may be empty.
-	 * @return reference to the tree specified by the entry list.
+	 * @return the new, fully parsed tree specified by the entry list.
 	 * @throws Exception
 	 */
 	public RevTree tree(DirCacheEntry... entries) throws Exception {
 		final DirCache dc = DirCache.newInCore();
 		final DirCacheBuilder b = dc.builder();
-		for (DirCacheEntry e : entries)
+		for (DirCacheEntry e : entries) {
 			b.add(e);
+		}
 		b.finish();
 		ObjectId root;
 		try (ObjectInserter ins = inserter) {
 			root = dc.writeTree(ins);
 			ins.flush();
 		}
-		return pool.lookupTree(root);
+		return pool.parseTree(root);
 	}
 
 	/**
@@ -422,7 +423,7 @@ public class TestRepository<R extends Repository> {
 	 *            the root tree for the commit.
 	 * @param parents
 	 *            zero or more parents of the commit.
-	 * @return the new commit.
+	 * @return the new, fully parsed commit.
 	 * @throws Exception
 	 */
 	public RevCommit commit(final int secDelta, final RevTree tree,
@@ -442,7 +443,7 @@ public class TestRepository<R extends Repository> {
 			id = ins.insert(c);
 			ins.flush();
 		}
-		return pool.lookupCommit(id);
+		return pool.parseCommit(id);
 	}
 
 	/**
@@ -467,7 +468,7 @@ public class TestRepository<R extends Repository> {
 	 *            with {@code refs/tags/}.
 	 * @param dst
 	 *            object the tag should be pointed at.
-	 * @return the annotated tag object.
+	 * @return the new, fully parsed annotated tag object.
 	 * @throws Exception
 	 */
 	public RevTag tag(String name, RevObject dst) throws Exception {
@@ -481,7 +482,7 @@ public class TestRepository<R extends Repository> {
 			id = ins.insert(t);
 			ins.flush();
 		}
-		return (RevTag) pool.lookupAny(id, Constants.OBJ_TAG);
+		return pool.parseTag(id);
 	}
 
 	/**
@@ -705,8 +706,8 @@ public class TestRepository<R extends Repository> {
 	 *
 	 * @param id
 	 *            commit-ish to cherry-pick.
-	 * @return newly created commit, or null if no work was done due to the
+	 * @return the new, fully parsed commit, or null if no work was done due to
-	 *         resulting tree being identical.
+	 *         the resulting tree being identical.
 	 * @throws Exception
 	 */
 	public RevCommit cherryPick(AnyObjectId id) throws Exception {
@@ -1197,7 +1198,7 @@ public class TestRepository<R extends Repository> {
 					commitId = ins.insert(c);
 					ins.flush();
 				}
-				self = pool.lookupCommit(commitId);
+				self = pool.parseCommit(commitId);
 
 				if (branch != null)
 					branch.update(self);

org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java

@@ -360,7 +360,6 @@ public class GcCommitSelectionTest extends GcTestCase {
 			}
 		}
 		RevCommit c = commit.create();
-		tr.parseBody(c);
 		commits.add(c);
 		return c;
 	}

org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevObjectTest.java

@@ -144,13 +144,13 @@ public class RevObjectTest extends RevWalkTestCase {
 		final RevCommit a = commit();
 		final RevFlag flag1 = rw.newFlag("flag1");
 		final RevFlag flag2 = rw.newFlag("flag2");
-		assertEquals(0, a.flags);
+		assertEquals(RevWalk.PARSED, a.flags);
 
 		a.add(flag1);
-		assertEquals(flag1.mask, a.flags);
+		assertEquals(RevWalk.PARSED | flag1.mask, a.flags);
 
 		a.add(flag2);
-		assertEquals(flag1.mask | flag2.mask, a.flags);
+		assertEquals(RevWalk.PARSED | flag1.mask | flag2.mask, a.flags);
 	}
 
 	@Test
@@ -162,10 +162,10 @@ public class RevObjectTest extends RevWalkTestCase {
 		s.add(flag1);
 		s.add(flag2);
 
-		assertEquals(0, a.flags);
+		assertEquals(RevWalk.PARSED, a.flags);
 
 		a.add(s);
-		assertEquals(flag1.mask | flag2.mask, a.flags);
+		assertEquals(RevWalk.PARSED | flag1.mask | flag2.mask, a.flags);
 	}
 
 	@Test
@@ -175,9 +175,9 @@ public class RevObjectTest extends RevWalkTestCase {
 		final RevFlag flag2 = rw.newFlag("flag2");
 		a.add(flag1);
 		a.add(flag2);
-		assertEquals(flag1.mask | flag2.mask, a.flags);
+		assertEquals(RevWalk.PARSED | flag1.mask | flag2.mask, a.flags);
 		a.remove(flag2);
-		assertEquals(flag1.mask, a.flags);
+		assertEquals(RevWalk.PARSED | flag1.mask, a.flags);
 	}
 
 	@Test
@@ -191,8 +191,8 @@ public class RevObjectTest extends RevWalkTestCase {
 		s.add(flag2);
 		a.add(flag3);
 		a.add(s);
-		assertEquals(flag1.mask | flag2.mask | flag3.mask, a.flags);
+		assertEquals(RevWalk.PARSED | flag1.mask | flag2.mask | flag3.mask, a.flags);
 		a.remove(s);
-		assertEquals(flag3.mask, a.flags);
+		assertEquals(RevWalk.PARSED | flag3.mask, a.flags);
 	}
 }

org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkCullTest.java

@@ -85,7 +85,7 @@ public class RevWalkCullTest extends RevWalkTestCase {
 
 	@Test
 	public void testProperlyCullAllAncestors_LongHistory() throws Exception {
-		final RevCommit a = commit();
+		RevCommit a = commit();
 		RevCommit b = commit(a);
 		for (int i = 0; i < 24; i++) {
 			b = commit(b);
@@ -94,6 +94,12 @@ public class RevWalkCullTest extends RevWalkTestCase {
 		}
 		final RevCommit c = commit(b);
 
+		// TestRepository eagerly parses newly created objects. The current rw
+		// is caching that parsed state. To verify that RevWalk itself is lazy,
+		// set up a new one.
+		rw.close();
+		rw = createRevWalk();
+		RevCommit a2 = rw.lookupCommit(a);
 		markStart(c);
 		markUninteresting(b);
 		assertCommit(c, rw.next());
@@ -102,6 +108,6 @@ public class RevWalkCullTest extends RevWalkTestCase {
 		// We should have aborted before we got back so far that "a"
 		// would be parsed. Thus, its parents shouldn't be allocated.
 		//
-		assertNull(a.parents);
+		assertNull(a2.parents);
 	}
 }

org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkShallowTest.java

@@ -58,130 +58,140 @@ public class RevWalkShallowTest extends RevWalkTestCase {
 
 	@Test
 	public void testDepth1() throws Exception {
-		final RevCommit a = commit();
+		RevCommit[] commits = setupLinearChain();
-		final RevCommit b = commit(a);
-		final RevCommit c = commit(b);
-		final RevCommit d = commit(c);
 
-		createShallowFile(d);
+		createShallowFile(commits[3]);
+		updateCommits(commits);
 
-		rw.reset();
+		rw.markStart(commits[3]);
-		markStart(d);
+		assertCommit(commits[3], rw.next());
-		assertCommit(d, rw.next());
 		assertNull(rw.next());
 	}
 
 	@Test
 	public void testDepth2() throws Exception {
-		final RevCommit a = commit();
+		RevCommit[] commits = setupLinearChain();
-		final RevCommit b = commit(a);
-		final RevCommit c = commit(b);
-		final RevCommit d = commit(c);
 
-		createShallowFile(c);
+		createShallowFile(commits[2]);
+		updateCommits(commits);
 
-		rw.reset();
+		rw.markStart(commits[3]);
-		markStart(d);
+		assertCommit(commits[3], rw.next());
-		assertCommit(d, rw.next());
+		assertCommit(commits[2], rw.next());
-		assertCommit(c, rw.next());
 		assertNull(rw.next());
 	}
 
 	@Test
 	public void testDepth3() throws Exception {
-		final RevCommit a = commit();
+		RevCommit[] commits = setupLinearChain();
-		final RevCommit b = commit(a);
-		final RevCommit c = commit(b);
-		final RevCommit d = commit(c);
 
-		createShallowFile(b);
+		createShallowFile(commits[1]);
+		updateCommits(commits);
 
-		rw.reset();
+		rw.markStart(commits[3]);
-		markStart(d);
+		assertCommit(commits[3], rw.next());
-		assertCommit(d, rw.next());
+		assertCommit(commits[2], rw.next());
-		assertCommit(c, rw.next());
+		assertCommit(commits[1], rw.next());
-		assertCommit(b, rw.next());
 		assertNull(rw.next());
 	}
 
 	@Test
-	public void testMergeCommitOneParentShallow() throws Exception {
+	public void testObjectDirectorySnapshot() throws Exception {
-		final RevCommit a = commit();
+		RevCommit[] commits = setupLinearChain();
-		final RevCommit b = commit(a);
+
-		final RevCommit c = commit(b);
+		createShallowFile(commits[3]);
-		final RevCommit d = commit(b);
+		updateCommits(commits);
-		final RevCommit e = commit(d);
+
-		final RevCommit merge = commit(c, e);
+		markStart(commits[3]);
-
+		assertCommit(commits[3], rw.next());
-		createShallowFile(e);
-
-		rw.reset();
-		markStart(merge);
-		assertCommit(merge, rw.next());
-		assertCommit(e, rw.next());
-		assertCommit(c, rw.next());
-		assertCommit(b, rw.next());
-		assertCommit(a, rw.next());
 		assertNull(rw.next());
+
+		createShallowFile(commits[2]);
+		updateCommits(commits);
+
+		markStart(commits[3]);
+		assertCommit(commits[3], rw.next());
+		assertCommit(commits[2], rw.next());
+		assertNull(rw.next());
+	}
+
+	private RevCommit[] setupLinearChain() throws Exception {
+		RevCommit[] commits = new RevCommit[4];
+		RevCommit parent = null;
+		for (int i = 0; i < commits.length; i++) {
+			commits[i] = parent != null ? commit(parent) : commit();
+			parent = commits[i];
+		}
+		return commits;
 	}
 
 	@Test
-	public void testMergeCommitEntirelyShallow() throws Exception {
+	public void testMergeCommitOneParentShallow() throws Exception {
-		final RevCommit a = commit();
+		RevCommit[] commits = setupMergeChain();
-		final RevCommit b = commit(a);
+
-		final RevCommit c = commit(b);
+		createShallowFile(commits[4]);
-		final RevCommit d = commit(b);
+		updateCommits(commits);
-		final RevCommit e = commit(d);
+
-		final RevCommit merge = commit(c, e);
+		markStart(commits[5]);
-
+		assertCommit(commits[5], rw.next());
-		createShallowFile(c, e);
+		assertCommit(commits[4], rw.next());
-
+		assertCommit(commits[2], rw.next());
-		rw.reset();
+		assertCommit(commits[1], rw.next());
-		markStart(merge);
+		assertCommit(commits[0], rw.next());
-		assertCommit(merge, rw.next());
-		assertCommit(e, rw.next());
-		assertCommit(c, rw.next());
 		assertNull(rw.next());
 	}
 
 	@Test
-	public void testObjectDirectorySnapshot() throws Exception {
+	public void testMergeCommitEntirelyShallow() throws Exception {
-		RevCommit a = commit();
+		RevCommit[] commits = setupMergeChain();
-		RevCommit b = commit(a);
-		RevCommit c = commit(b);
-		RevCommit d = commit(c);
 
-		createShallowFile(d);
+		createShallowFile(commits[2], commits[4]);
+		updateCommits(commits);
 
-		rw.reset();
+		markStart(commits[5]);
-		markStart(d);
+		assertCommit(commits[5], rw.next());
-		assertCommit(d, rw.next());
+		assertCommit(commits[4], rw.next());
+		assertCommit(commits[2], rw.next());
 		assertNull(rw.next());
+	}
 
-		rw = createRevWalk();
+	private RevCommit[] setupMergeChain() throws Exception {
-		a = rw.lookupCommit(a);
+		/*-
-		b = rw.lookupCommit(b);
+		 * Create a history like this, diverging at 1 and merging at 5:
-		c = rw.lookupCommit(c);
+		 *
-		d = rw.lookupCommit(d);
+		 *      ---o--o       commits 3,4
-
+		 *     /       \
-		rw.reset();
+		 * o--o--o------o   commits 0,1,2,5
-		markStart(d);
+		 */
-		assertCommit(d, rw.next());
+		RevCommit[] commits = new RevCommit[6];
-		assertNull(rw.next());
+		commits[0] = commit();
+		commits[1] = commit(commits[0]);
+		commits[2] = commit(commits[1]);
+		commits[3] = commit(commits[1]);
+		commits[4] = commit(commits[3]);
+		commits[5] = commit(commits[2], commits[4]);
+		return commits;
+	}
 
-		createShallowFile(c);
+	private void updateCommits(RevCommit[] commits) {
+		// Relookup commits using the new RevWalk
+		for (int i = 0; i < commits.length; i++) {
+			commits[i] = rw.lookupCommit(commits[i].getId());
+		}
+	}
 
+	private void createShallowFile(ObjectId... shallowCommits)
+			throws IOException {
+		// Reset the RevWalk since the new shallow file invalidates the existing
+		// RevWalk's shallow state.
+		rw.close();
 		rw = createRevWalk();
-		a = rw.lookupCommit(a);
+		StringBuilder builder = new StringBuilder();
-		b = rw.lookupCommit(b);
+		for (ObjectId commit : shallowCommits) {
-		c = rw.lookupCommit(c);
+			builder.append(commit.getName() + "\n");
-		d = rw.lookupCommit(d);
+		}
-
+		JGitTestUtil.write(new File(db.getDirectory(), "shallow"),
-		rw.reset();
+				builder.toString());
-		markStart(d);
-		assertCommit(d, rw.next());
-		assertCommit(c, rw.next());
-		assertNull(rw.next());
 	}
 
 	@Test
@@ -199,13 +209,4 @@ public class RevWalkShallowTest extends RevWalkTestCase {
 		assertCommit(b, rw.next());
 		assertNull(rw.next());
 	}
-
-	private void createShallowFile(ObjectId... shallowCommits)
-			throws IOException {
-		final StringBuilder builder = new StringBuilder();
-		for (ObjectId commit : shallowCommits)
-			builder.append(commit.getName() + "\n");
-		JGitTestUtil.write(new File(db.getDirectory(), "shallow"),
-				builder.toString());
-	}
 }