265: Mail bot shows wrong source line for inline review comments

Reviewed-by: ehelin
openjdk · Aug 31, 2020 · cca94e0 · cca94e0 · bridgekeeper · Aug 31, 2020
1 parent b007595
commit cca94e0
Show file tree

Hide file tree

Showing 2 changed files with 3 additions and 4 deletions.
diff --git a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveMessages.java b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveMessages.java
@@ -338,7 +338,7 @@ static String composeReviewComment(PullRequest pr, ReviewComment reviewComment)
             body.append(reviewComment.path()).append(" line ").append(reviewComment.line()).append(":\n\n");
             try {
                 var contents = pr.repository().fileContents(reviewComment.path(), reviewComment.hash().hex()).lines().collect(Collectors.toList());
-                for (int i = Math.max(0, reviewComment.line() - 2); i < Math.min(contents.size(), reviewComment.line() + 1); ++i) {
+                for (int i = Math.max(0, reviewComment.line() - 3); i < Math.min(contents.size(), reviewComment.line()); ++i) {
                     body.append("> ").append(i + 1).append(": ").append(contents.get(i)).append("\n");
                 }
                 body.append("\n");

diff --git a/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java b/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java
@@ -1337,11 +1337,10 @@ void reviewContext(TestInfo testInfo) throws IOException {
             TestBotRunner.runPeriodicItems(mlBot);
             listServer.processIncoming();
 
-            // The archive should only contain context around line 2
+            // The archive should only contain context up to and including Line 2
             Repository.materialize(archiveFolder.path(), archive.url(), "master");
             assertTrue(archiveContains(archiveFolder.path(), "^> 2: Line 1$"));
-            assertTrue(archiveContains(archiveFolder.path(), "^> 3: Line 2$"));
-            assertFalse(archiveContains(archiveFolder.path(), "^> 4: Line 3$"));
+            assertFalse(archiveContains(archiveFolder.path(), "^> 3: Line 2$"));
         }
     }