diff --git a/java/com/google/gerrit/extensions/common/testing/ContentEntrySubject.java b/java/com/google/gerrit/extensions/common/testing/ContentEntrySubject.java index 48f7f45f86..9030a1cdd3 100644 --- a/java/com/google/gerrit/extensions/common/testing/ContentEntrySubject.java +++ b/java/com/google/gerrit/extensions/common/testing/ContentEntrySubject.java @@ -17,6 +17,7 @@ package com.google.gerrit.extensions.common.testing; import static com.google.common.truth.Truth.assertAbout; import com.google.common.truth.FailureMetadata; +import com.google.common.truth.IterableSubject; import com.google.common.truth.StringSubject; import com.google.common.truth.Subject; import com.google.common.truth.Truth; @@ -68,4 +69,16 @@ public class ContentEntrySubject extends Subject { if (editsDueToRebase.contains(c) || editsDueToRebase.contains(n)) { // Don't combine any edits which were identified as being introduced by a rebase as we would // lose that information because of the combination. + j++; continue; } diff --git a/java/com/google/gerrit/server/patch/PatchListKey.java b/java/com/google/gerrit/server/patch/PatchListKey.java index 7475fec829..083c142d48 100644 --- a/java/com/google/gerrit/server/patch/PatchListKey.java +++ b/java/com/google/gerrit/server/patch/PatchListKey.java @@ -32,7 +32,7 @@ import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.ObjectId; public class PatchListKey implements Serializable { - public static final long serialVersionUID = 30L; + public static final long serialVersionUID = 31L; public static final ImmutableBiMap WHITESPACE_TYPES = ImmutableBiMap.of( diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java index 6a9b542c17..4c8f53f56b 100644 --- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java +++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java @@ -23,6 +23,7 @@ import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toMap; import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GitUtil; @@ -82,6 +83,11 @@ public class RevisionDiffIT extends AbstractDaemonTest { @Before public void setUp() throws Exception { + // Reduce flakiness of tests. (If tests aren't fast enough, we would use a fall-back + // computation, which might yield different results.) + baseConfig.setString("cache", "diff", "timeout", "1 minute"); + baseConfig.setString("cache", "diff_intraline", "timeout", "1 minute"); + intraline = baseConfig.getBoolean(TEST_PARAMETER_MARKER, "intraline", false); ObjectId headCommit = testRepo.getRepository().resolve("HEAD"); @@ -1253,6 +1259,66 @@ public class RevisionDiffIT extends AbstractDaemonTest { assertThat(changedFiles.get(FILE_NAME)).linesDeleted().isEqualTo(1); } + @Test + public void intralineEditsInNonRebaseHunksAreIdentified() throws Exception { + assume().that(intraline).isTrue(); + + Function contentModification = + fileContent -> fileContent.replace("Line 1\n", "Line one\n"); + addModifiedPatchSet(changeId, FILE_NAME, contentModification); + + DiffInfo diffInfo = + getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(initialPatchSetId).get(); + assertThat(diffInfo).content().element(0).linesOfA().containsExactly("Line 1"); + assertThat(diffInfo).content().element(0).linesOfB().containsExactly("Line one"); + assertThat(diffInfo) + .content() + .element(0) + .intralineEditsOfA() + .containsExactly(ImmutableList.of(5, 1)); + assertThat(diffInfo) + .content() + .element(0) + .intralineEditsOfB() + .containsExactly(ImmutableList.of(5, 3)); + assertThat(diffInfo).content().element(0).isNotDueToRebase(); + assertThat(diffInfo).content().element(1).commonLines().hasSize(99); + } + + @Test + public void intralineEditsInRebaseHunksAreIdentified() throws Exception { + assume().that(intraline).isTrue(); + + String newFileContent = FILE_CONTENT.replace("Line 1\n", "Line one\n"); + ObjectId commit2 = addCommit(commit1, FILE_NAME, newFileContent); + + rebaseChangeOn(changeId, commit2); + Function contentModification = + fileContent -> fileContent.replace("Line 50\n", "Line fifty\n"); + addModifiedPatchSet(changeId, FILE_NAME, contentModification); + + DiffInfo diffInfo = + getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(initialPatchSetId).get(); + assertThat(diffInfo).content().element(0).linesOfA().containsExactly("Line 1"); + assertThat(diffInfo).content().element(0).linesOfB().containsExactly("Line one"); + assertThat(diffInfo) + .content() + .element(0) + .intralineEditsOfA() + .containsExactly(ImmutableList.of(5, 1)); + assertThat(diffInfo) + .content() + .element(0) + .intralineEditsOfB() + .containsExactly(ImmutableList.of(5, 3)); + assertThat(diffInfo).content().element(0).isDueToRebase(); + assertThat(diffInfo).content().element(1).commonLines().hasSize(48); + assertThat(diffInfo).content().element(2).linesOfA().containsExactly("Line 50"); + assertThat(diffInfo).content().element(2).linesOfB().containsExactly("Line fifty"); + assertThat(diffInfo).content().element(2).isNotDueToRebase(); + assertThat(diffInfo).content().element(3).commonLines().hasSize(50); + } + @Test public void closeNonRebaseHunksAreCombinedForIntralineOptimizations() throws Exception { assume().that(intraline).isTrue(); @@ -1365,6 +1431,47 @@ public class RevisionDiffIT extends AbstractDaemonTest { assertThat(changedFiles.get(FILE_NAME)).linesDeleted().isEqualTo(1); } + @Test + public void closeNonRebaseHunksNextToRebaseHunksAreCombinedForIntralineOptimizations() + throws Exception { + assume().that(intraline).isTrue(); + + String fileContent = FILE_CONTENT.replace("Line 5\n", "{\n").replace("Line 7\n", "{\n"); + ObjectId commit2 = addCommit(commit1, FILE_NAME, fileContent); + rebaseChangeOn(changeId, commit2); + String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision; + + String newFileContent = fileContent.replace("Line 8\n", "Line eight!\n"); + ObjectId commit3 = addCommit(commit1, FILE_NAME, newFileContent); + rebaseChangeOn(changeId, commit3); + + addModifiedPatchSet( + changeId, + FILE_NAME, + content -> content.replace("Line 4\n", "Line four\n").replace("Line 6\n", "Line six\n")); + + DiffInfo diffInfo = + getDiffRequest(changeId, CURRENT, FILE_NAME).withBase(previousPatchSetId).get(); + assertThat(diffInfo).content().element(0).commonLines().hasSize(3); + assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 4", "{", "Line 6"); + assertThat(diffInfo) + .content() + .element(1) + .linesOfB() + .containsExactly("Line four", "{", "Line six"); + assertThat(diffInfo).content().element(1).isNotDueToRebase(); + assertThat(diffInfo).content().element(2).commonLines().hasSize(1); + assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 8"); + assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line eight!"); + assertThat(diffInfo).content().element(3).isDueToRebase(); + assertThat(diffInfo).content().element(4).commonLines().hasSize(92); + + Map changedFiles = + gApi.changes().id(changeId).current().files(previousPatchSetId); + assertThat(changedFiles.get(FILE_NAME)).linesInserted().isEqualTo(2); + assertThat(changedFiles.get(FILE_NAME)).linesDeleted().isEqualTo(2); + } + private void assertDiffForNewFile( PushOneCommit.Result pushResult, String path, String expectedContentSideB) throws Exception { DiffInfo diff =