From 7f7c780905c12bde57266e93063a4fb50937ad5c Mon Sep 17 00:00:00 2001 From: Max Haslbeck Date: Tue, 12 Mar 2024 13:23:20 +0000 Subject: [PATCH] Add more tests on rewriting parents in a RevWalk Change I4e4ff67fb279edbcc3461496b132cea774fb742f introduced new behaviour that also rewrote parents in a RevWalk if no TreeFilter was set. This led to unexpected behaviour when users were fetching from chromium.googlesource.com. I added a few more tests to better describe the behaviour of RevWalk in regards to rewriting parents. Change-Id: I1a5e5f8a1de9d8dd0e3664918ac010644b3ef87b Signed-off-by: Max Haslbeck --- .../jgit/revwalk/TreeRevFilterTest.java | 132 ++++++++++++++++-- 1 file changed, 123 insertions(+), 9 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/TreeRevFilterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/TreeRevFilterTest.java index 298facfd15..ddbb19cb8f 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/TreeRevFilterTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/TreeRevFilterTest.java @@ -20,17 +20,33 @@ import org.eclipse.jgit.treewalk.filter.TreeFilter; import org.junit.Test; public class TreeRevFilterTest extends RevWalkTestCase { - private RevFilter treeRevFilter() { - return new TreeRevFilter(rw, TreeFilter.ANY_DIFF); - } + @Test + public void testStringOfPearls_FilePath1_treeRevFilter() + throws Exception { + RevCommit a = commit(tree(file("d/f", blob("a")))); + RevCommit b = commit(tree(file("d/f", blob("a"))), a); + RevCommit c = commit(tree(file("d/f", blob("b"))), b); + rw.setRevFilter(new TreeRevFilter(rw, TreeFilter.ANY_DIFF)); + markStart(c); + + assertCommit(c, rw.next()); + assertEquals(1, c.getParentCount()); + assertCommit(b, c.getParent(0)); + + assertCommit(a, rw.next()); // b was skipped + assertEquals(0, a.getParentCount()); + assertNull(rw.next()); + } @Test - public void testStringOfPearls_FilePath1() + public void testStringOfPearls_FilePath1_noRewriteParents() throws Exception { RevCommit a = commit(tree(file("d/f", blob("a")))); RevCommit b = commit(tree(file("d/f", blob("a"))), a); RevCommit c = commit(tree(file("d/f", blob("b"))), b); - rw.setRevFilter(treeRevFilter()); + + rw.setRewriteParents(false); + rw.setTreeFilter(TreeFilter.ANY_DIFF); markStart(c); assertCommit(c, rw.next()); @@ -42,13 +58,75 @@ public class TreeRevFilterTest extends RevWalkTestCase { assertNull(rw.next()); } + @Test + public void testStringOfPearls_FilePath1_RewriteParents() + throws Exception { + RevCommit a = commit(tree(file("d/f", blob("a")))); + RevCommit b = commit(tree(file("d/f", blob("a"))), a); + RevCommit c = commit(tree(file("d/f", blob("b"))), b); + + rw.setRevFilter(new TreeRevFilter(rw, TreeFilter.ANY_DIFF)); + rw.setTreeFilter(TreeFilter.ANY_DIFF); + markStart(c); + + assertCommit(c, rw.next()); + assertEquals(1, c.getParentCount()); + assertCommit(a, c.getParent(0)); + + assertCommit(a, rw.next()); // b was skipped + assertEquals(0, a.getParentCount()); + assertNull(rw.next()); + } + @Test public void testStringOfPearls_FilePath2() throws Exception { RevCommit a = commit(tree(file("d/f", blob("a")))); RevCommit b = commit(tree(file("d/f", blob("a"))), a); RevCommit c = commit(tree(file("d/f", blob("b"))), b); RevCommit d = commit(tree(file("d/f", blob("b"))), c); - rw.setRevFilter(treeRevFilter()); + rw.setRevFilter(new TreeRevFilter(rw, TreeFilter.ANY_DIFF)); + markStart(d); + + // d was skipped + assertCommit(c, rw.next()); + assertEquals(1, c.getParentCount()); + assertCommit(b, c.getParent(0)); + + // b was skipped + assertCommit(a, rw.next()); + assertEquals(0, a.getParentCount()); + assertNull(rw.next()); + } + + @Test + public void testStringOfPearls_FilePath2_RewriteParents() throws Exception { + RevCommit a = commit(tree(file("d/f", blob("a")))); + RevCommit b = commit(tree(file("d/f", blob("a"))), a); + RevCommit c = commit(tree(file("d/f", blob("b"))), b); + RevCommit d = commit(tree(file("d/f", blob("b"))), c); + rw.setTreeFilter(TreeFilter.ANY_DIFF); + markStart(d); + + // d was skipped + assertCommit(c, rw.next()); + assertEquals(1, c.getParentCount()); + assertCommit(a, c.getParent(0)); + + // b was skipped + assertCommit(a, rw.next()); + assertEquals(0, a.getParentCount()); + assertNull(rw.next()); + } + + @Test + public void testStringOfPearls_FilePath2_RewriteParents_False() throws Exception { + RevCommit a = commit(tree(file("d/f", blob("a")))); + RevCommit b = commit(tree(file("d/f", blob("a"))), a); + RevCommit c = commit(tree(file("d/f", blob("b"))), b); + RevCommit d = commit(tree(file("d/f", blob("b"))), c); + rw.setRewriteParents(false); + rw.setRevFilter(new TreeRevFilter(rw, TreeFilter.ANY_DIFF)); + rw.setTreeFilter(TreeFilter.ANY_DIFF); markStart(d); // d was skipped @@ -68,7 +146,7 @@ public class TreeRevFilterTest extends RevWalkTestCase { RevCommit b = commit(tree(file("d/f", blob("a"))), a); RevCommit c = commit(tree(file("d/f", blob("b"))), b); RevCommit d = commit(tree(file("d/f", blob("b"))), c); - rw.setRevFilter(treeRevFilter()); + rw.setRevFilter(new TreeRevFilter(rw, TreeFilter.ANY_DIFF)); markStart(d); // d was skipped @@ -93,7 +171,9 @@ public class TreeRevFilterTest extends RevWalkTestCase { RevCommit g = commit(tree(file("d/f", blob("b"))), f); RevCommit h = commit(tree(file("d/f", blob("b"))), g); RevCommit i = commit(tree(file("d/f", blob("c"))), h); - rw.setRevFilter(treeRevFilter()); + + // Doesn't rewrite parents since no TreeFilter is set + rw.setRevFilter(new TreeRevFilter(rw, TreeFilter.ANY_DIFF)); markStart(i); assertCommit(i, rw.next()); @@ -111,9 +191,40 @@ public class TreeRevFilterTest extends RevWalkTestCase { assertNull(rw.next()); } + @Test + public void testStringOfPearls_FilePath3_RewriteParents() throws Exception { + RevCommit a = commit(tree(file("d/f", blob("a")))); + RevCommit b = commit(tree(file("d/f", blob("a"))), a); + RevCommit c = commit(tree(file("d/f", blob("b"))), b); + RevCommit d = commit(tree(file("d/f", blob("b"))), c); + RevCommit e = commit(tree(file("d/f", blob("b"))), d); + RevCommit f = commit(tree(file("d/f", blob("b"))), e); + RevCommit g = commit(tree(file("d/f", blob("b"))), f); + RevCommit h = commit(tree(file("d/f", blob("b"))), g); + RevCommit i = commit(tree(file("d/f", blob("c"))), h); + + rw.setRevFilter(new TreeRevFilter(rw, TreeFilter.ANY_DIFF)); + rw.setTreeFilter(TreeFilter.ANY_DIFF); + markStart(i); + + assertCommit(i, rw.next()); + assertEquals(1, i.getParentCount()); + assertCommit(c, i.getParent(0)); + + // h..d was skipped + assertCommit(c, rw.next()); + assertEquals(1, c.getParentCount()); + assertCommit(a, c.getParent(0)); + + // b was skipped + assertCommit(a, rw.next()); + assertEquals(0, a.getParentCount()); + assertNull(rw.next()); + } + @Test public void testPathFilterOrOtherFilter() throws Exception { - RevFilter pathFilter = treeRevFilter(); + RevFilter pathFilter = new TreeRevFilter(rw, TreeFilter.ANY_DIFF); RevFilter skipFilter = SkipRevFilter.create(1); RevFilter orFilter = OrRevFilter.create(skipFilter, pathFilter); @@ -125,6 +236,9 @@ public class TreeRevFilterTest extends RevWalkTestCase { rw.setRevFilter(pathFilter); markStart(c); assertCommit(c, rw.next()); + assertEquals(1, c.getParentCount()); + assertCommit(b, c.getParent(0)); + assertCommit(a, rw.next()); // Skip filter matches b, a. -- 2.39.5