opendev/gerrit
Code Issues Proposed changes

Document broken diff.context parameter with tests

Browse Source 
For a diff, requesting a context different from the whole file doesn't
seem to work. Fixing this might not be easy as it could easily break
syntax highlighting on the UI. Considering our diff code, this doesn't
seem like a recent regression but rather something we have lived with
for years.

As we don't have any tests which explicitly set the context parameter
to a lower value than the whole file, add some. Of course, they can
only show the current, broken state.

Change-Id: Iae2b098c31be405e2d720e00c0ae744efc64219d
This commit is contained in:
Alice Kober-Sotzek
2019-05-24 14:42:26 +02:00
parent d86ba0284d
commit 47e22a780b
1 changed files with 40 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

40
javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
View File

@@ -2406,6 +2406,46 @@ public class RevisionDiffIT extends AbstractDaemonTest {
assertThat(diffInfo).content().isEmpty();
}
// This behavior is likely a bug. A fix might not be easy as it might break syntax highlighting.
// TODO: Fix this issue or remove the broken parameter (at least in the documentation).
@Test
public void contextParameterIsIgnored() throws Exception {
addModifiedPatchSet(
changeId, FILE_NAME, content -> content.replace("Line 20\n", "Line twenty\n"));
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME)
.withBase(initialPatchSetId)
.withContext(5)
.get();
assertThat(diffInfo).content().element(0).commonLines().hasSize(19);
assertThat(diffInfo).content().element(1).linesOfA().containsExactly("Line 20");
assertThat(diffInfo).content().element(1).linesOfB().containsExactly("Line twenty");
assertThat(diffInfo).content().element(2).commonLines().hasSize(81);
}
// This behavior is likely a bug. A fix might not be easy as it might break syntax highlighting.
// TODO: Fix this issue or remove the broken parameter (at least in the documentation).
@Test
public void contextParameterIsIgnoredForUnmodifiedFileWithComment() throws Exception {
addModifiedPatchSet(
changeId, FILE_NAME, content -> content.replace("Line 20\n", "Line twenty\n"));
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
CommentInput comment = createCommentInput(20, 0, 21, 0, "Should be 'Line 20'.");
ReviewInput reviewInput = new ReviewInput();
reviewInput.comments = ImmutableMap.of(FILE_NAME, ImmutableList.of(comment));
gApi.changes().id(changeId).revision(previousPatchSetId).review(reviewInput);
addModifiedPatchSet(
changeId, FILE_NAME2, content -> content.replace("2nd line\n", "Second line\n"));
DiffInfo diffInfo =
getDiffRequest(changeId, CURRENT, FILE_NAME)
.withBase(previousPatchSetId)
.withContext(5)
.get();
assertThat(diffInfo).content().element(0).commonLines().hasSize(101);
}
@Test
public void requestingDiffForOldFileNameOfRenamedFileYieldsReasonableResult() throws Exception {
addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));