From 396fe6da4593645e1b9bada4fe314f6169ab2d17 Mon Sep 17 00:00:00 2001 From: Jeff Schumacher Date: Thu, 22 Jul 2010 12:55:28 -0700 Subject: Break dissimilar file pairs during diff File pairs that are very dissimilar during a diff were not being broken apart into their constituent ADD/DELETE pairs. The leads to sub-optimal rename detection. Take, for example, this situation: A file exists at src/a.txt containing "foo". A user renames src/a.txt to src/b.txt, then adds a new src/a.txt containing "bar". Even though the old a.txt and the new b.txt are identical, the rename detection algorithm would not detect it as a rename since it was already paired in a MODIFY. I added code to split all MODIFYs below a certain score into their constituent ADD/DELETE pairs. This allows situations like the one I described above to be more correctly handled. Change-Id: I22c04b70581f206bbc68c4cd1ee87a1f663b418e Signed-off-by: Shawn O. Pearce --- .../org/eclipse/jgit/diff/RenameDetectorTest.java | 121 +++++++++++++++++++++ 1 file changed, 121 insertions(+) (limited to 'org.eclipse.jgit.test/tst/org/eclipse/jgit/diff') diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RenameDetectorTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RenameDetectorTest.java index bfc51b68f9..eb965cf601 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RenameDetectorTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RenameDetectorTest.java @@ -383,6 +383,116 @@ public class RenameDetectorTest extends RepositoryTestCase { assertSame(b, entries.get(1)); } + public void testBreakModify_BreakAll() throws Exception { + ObjectId aId = blob("foo"); + ObjectId bId = blob("bar"); + + DiffEntry m = DiffEntry.modify(PATH_A); + m.oldId = AbbreviatedObjectId.fromObjectId(aId); + m.newId = AbbreviatedObjectId.fromObjectId(bId); + + DiffEntry a = DiffEntry.add(PATH_B, aId); + + rd.add(a); + rd.add(m); + + rd.setBreakScore(101); + + List entries = rd.compute(); + assertEquals(2, entries.size()); + assertAdd(PATH_A, bId, FileMode.REGULAR_FILE, entries.get(0)); + assertRename(DiffEntry.breakModify(m).get(0), a, 100, entries.get(1)); + } + + public void testBreakModify_BreakNone() throws Exception { + ObjectId aId = blob("foo"); + ObjectId bId = blob("bar"); + + DiffEntry m = DiffEntry.modify(PATH_A); + m.oldId = AbbreviatedObjectId.fromObjectId(aId); + m.newId = AbbreviatedObjectId.fromObjectId(bId); + + DiffEntry a = DiffEntry.add(PATH_B, aId); + + rd.add(a); + rd.add(m); + + rd.setBreakScore(-1); + + List entries = rd.compute(); + assertEquals(2, entries.size()); + assertSame(m, entries.get(0)); + assertSame(a, entries.get(1)); + } + + public void testBreakModify_BreakBelowScore() throws Exception { + ObjectId aId = blob("foo"); + ObjectId bId = blob("bar"); + + DiffEntry m = DiffEntry.modify(PATH_A); + m.oldId = AbbreviatedObjectId.fromObjectId(aId); + m.newId = AbbreviatedObjectId.fromObjectId(bId); + + DiffEntry a = DiffEntry.add(PATH_B, aId); + + rd.add(a); + rd.add(m); + + rd.setBreakScore(20); // Should break the modify + + List entries = rd.compute(); + assertEquals(2, entries.size()); + assertAdd(PATH_A, bId, FileMode.REGULAR_FILE, entries.get(0)); + assertRename(DiffEntry.breakModify(m).get(0), a, 100, entries.get(1)); + } + + public void testBreakModify_DontBreakAboveScore() throws Exception { + ObjectId aId = blob("blah\nblah\nfoo"); + ObjectId bId = blob("blah\nblah\nbar"); + + DiffEntry m = DiffEntry.modify(PATH_A); + m.oldId = AbbreviatedObjectId.fromObjectId(aId); + m.newId = AbbreviatedObjectId.fromObjectId(bId); + + DiffEntry a = DiffEntry.add(PATH_B, aId); + + rd.add(a); + rd.add(m); + + rd.setBreakScore(20); // Should not break the modify + + List entries = rd.compute(); + assertEquals(2, entries.size()); + assertSame(m, entries.get(0)); + assertSame(a, entries.get(1)); + } + + public void testBreakModify_RejoinIfUnpaired() throws Exception { + ObjectId aId = blob("foo"); + ObjectId bId = blob("bar"); + + DiffEntry m = DiffEntry.modify(PATH_A); + m.oldId = AbbreviatedObjectId.fromObjectId(aId); + m.newId = AbbreviatedObjectId.fromObjectId(bId); + + rd.add(m); + + rd.setBreakScore(101); // Ensure m is broken apart + + List entries = rd.compute(); + assertEquals(1, entries.size()); + + DiffEntry modify = entries.get(0); + assertEquals(m.oldName, modify.oldName); + assertEquals(m.oldId, modify.oldId); + assertEquals(m.oldMode, modify.oldMode); + assertEquals(m.newName, modify.newName); + assertEquals(m.newId, modify.newId); + assertEquals(m.newMode, modify.newMode); + assertEquals(m.changeType, modify.changeType); + assertEquals(0, modify.score); + } + public void testSetRenameScore_IllegalArgs() throws Exception { try { rd.setRenameScore(-1); @@ -462,4 +572,15 @@ public class RenameDetectorTest extends RepositoryTestCase { assertEquals(score, copy.getScore()); } + + private static void assertAdd(String newName, ObjectId newId, + FileMode newMode, DiffEntry add) { + assertEquals(DiffEntry.DEV_NULL, add.oldName); + assertEquals(DiffEntry.A_ZERO, add.oldId); + assertEquals(FileMode.MISSING, add.oldMode); + assertEquals(ChangeType.ADD, add.changeType); + assertEquals(newName, add.newName); + assertEquals(AbbreviatedObjectId.fromObjectId(newId), add.newId); + assertEquals(newMode, add.newMode); + } } -- cgit v1.2.3