From 00e51e35eeb0c3f15af9a03a600fa669d79d5beb Mon Sep 17 00:00:00 2001 From: Terry Parker Date: Tue, 26 Jun 2018 17:22:07 -0700 Subject: [PATCH] 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 --- .../eclipse/jgit/junit/TestRepository.java | 25 +-- .../storage/pack/GcCommitSelectionTest.java | 1 - .../eclipse/jgit/revwalk/RevObjectTest.java | 18 +- .../eclipse/jgit/revwalk/RevWalkCullTest.java | 10 +- .../jgit/revwalk/RevWalkShallowTest.java | 197 +++++++++--------- 5 files changed, 129 insertions(+), 122 deletions(-) diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java index 29a410019..49715e559 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java @@ -276,7 +276,7 @@ public class TestRepository { * * @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 { 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 { * @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 { * 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 { id = ins.insert(c); ins.flush(); } - return pool.lookupCommit(id); + return pool.parseCommit(id); } /** @@ -467,7 +468,7 @@ public class TestRepository { * 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 { 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 { * * @param id * commit-ish to cherry-pick. - * @return newly created commit, or null if no work was done due to the - * resulting tree being identical. + * @return the new, fully parsed commit, or null if no work was done due to + * the resulting tree being identical. * @throws Exception */ public RevCommit cherryPick(AnyObjectId id) throws Exception { @@ -1197,7 +1198,7 @@ public class TestRepository { commitId = ins.insert(c); ins.flush(); } - self = pool.lookupCommit(commitId); + self = pool.parseCommit(commitId); if (branch != null) branch.update(self); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java index c50e88f97..ffb6f4ee7 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java +++ b/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; } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevObjectTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevObjectTest.java index 8e389ae25..4969305de 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevObjectTest.java +++ b/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); } } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkCullTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkCullTest.java index fb52828c5..7984a3719 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkCullTest.java +++ b/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); } } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkShallowTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkShallowTest.java index 37243e1fa..7554d7a47 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkShallowTest.java +++ b/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(); - final RevCommit b = commit(a); - final RevCommit c = commit(b); - final RevCommit d = commit(c); + RevCommit[] commits = setupLinearChain(); - createShallowFile(d); + createShallowFile(commits[3]); + updateCommits(commits); - rw.reset(); - markStart(d); - assertCommit(d, rw.next()); + rw.markStart(commits[3]); + assertCommit(commits[3], rw.next()); assertNull(rw.next()); } @Test public void testDepth2() throws Exception { - final RevCommit a = commit(); - final RevCommit b = commit(a); - final RevCommit c = commit(b); - final RevCommit d = commit(c); + RevCommit[] commits = setupLinearChain(); - createShallowFile(c); + createShallowFile(commits[2]); + updateCommits(commits); - rw.reset(); - markStart(d); - assertCommit(d, rw.next()); - assertCommit(c, rw.next()); + rw.markStart(commits[3]); + assertCommit(commits[3], rw.next()); + assertCommit(commits[2], rw.next()); assertNull(rw.next()); } @Test public void testDepth3() throws Exception { - final RevCommit a = commit(); - final RevCommit b = commit(a); - final RevCommit c = commit(b); - final RevCommit d = commit(c); + RevCommit[] commits = setupLinearChain(); - createShallowFile(b); + createShallowFile(commits[1]); + updateCommits(commits); - rw.reset(); - markStart(d); - assertCommit(d, rw.next()); - assertCommit(c, rw.next()); - assertCommit(b, rw.next()); + rw.markStart(commits[3]); + assertCommit(commits[3], rw.next()); + assertCommit(commits[2], rw.next()); + assertCommit(commits[1], rw.next()); assertNull(rw.next()); } @Test - public void testMergeCommitOneParentShallow() throws Exception { - final RevCommit a = commit(); - final RevCommit b = commit(a); - final RevCommit c = commit(b); - final RevCommit d = commit(b); - final RevCommit e = commit(d); - final RevCommit merge = commit(c, e); - - 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()); + public void testObjectDirectorySnapshot() throws Exception { + RevCommit[] commits = setupLinearChain(); + + createShallowFile(commits[3]); + updateCommits(commits); + + markStart(commits[3]); + assertCommit(commits[3], 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 { - final RevCommit a = commit(); - final RevCommit b = commit(a); - final RevCommit c = commit(b); - final RevCommit d = commit(b); - final RevCommit e = commit(d); - final RevCommit merge = commit(c, e); - - createShallowFile(c, e); - - rw.reset(); - markStart(merge); - assertCommit(merge, rw.next()); - assertCommit(e, rw.next()); - assertCommit(c, rw.next()); + public void testMergeCommitOneParentShallow() throws Exception { + RevCommit[] commits = setupMergeChain(); + + createShallowFile(commits[4]); + updateCommits(commits); + + markStart(commits[5]); + assertCommit(commits[5], rw.next()); + assertCommit(commits[4], rw.next()); + assertCommit(commits[2], rw.next()); + assertCommit(commits[1], rw.next()); + assertCommit(commits[0], rw.next()); assertNull(rw.next()); } @Test - public void testObjectDirectorySnapshot() throws Exception { - RevCommit a = commit(); - RevCommit b = commit(a); - RevCommit c = commit(b); - RevCommit d = commit(c); + public void testMergeCommitEntirelyShallow() throws Exception { + RevCommit[] commits = setupMergeChain(); - createShallowFile(d); + createShallowFile(commits[2], commits[4]); + updateCommits(commits); - rw.reset(); - markStart(d); - assertCommit(d, rw.next()); + markStart(commits[5]); + assertCommit(commits[5], rw.next()); + assertCommit(commits[4], rw.next()); + assertCommit(commits[2], rw.next()); assertNull(rw.next()); + } - rw = createRevWalk(); - a = rw.lookupCommit(a); - b = rw.lookupCommit(b); - c = rw.lookupCommit(c); - d = rw.lookupCommit(d); - - rw.reset(); - markStart(d); - assertCommit(d, rw.next()); - assertNull(rw.next()); + private RevCommit[] setupMergeChain() throws Exception { + /*- + * Create a history like this, diverging at 1 and merging at 5: + * + * ---o--o commits 3,4 + * / \ + * o--o--o------o commits 0,1,2,5 + */ + RevCommit[] commits = new RevCommit[6]; + 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); - b = rw.lookupCommit(b); - c = rw.lookupCommit(c); - d = rw.lookupCommit(d); - - rw.reset(); - markStart(d); - assertCommit(d, rw.next()); - assertCommit(c, rw.next()); - assertNull(rw.next()); + StringBuilder builder = new StringBuilder(); + for (ObjectId commit : shallowCommits) { + builder.append(commit.getName() + "\n"); + } + JGitTestUtil.write(new File(db.getDirectory(), "shallow"), + builder.toString()); } @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()); - } }