Avoid quoting an empty review body

Reviewed-by: ehelin
openjdk · Jan 20, 2020 · 3d62e28 · 3d62e28
1 parent 9f56fca
commit 3d62e28
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 2 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
@@ -226,7 +226,7 @@ private static String composeReviewVerdict(Review review, HostUserToUserName hos
     }
 
     static String composeReview(PullRequest pr, Review review, HostUserToUserName hostUserToUserName, HostUserToRole hostUserToRole) {
-        if (review.body().isPresent()) {
+        if (review.body().isPresent() && !review.body().get().isBlank()) {
             return filterComments(review.body().get());
         } else {
             return composeReviewVerdict(review, hostUserToUserName, hostUserToRole);
@@ -235,7 +235,7 @@ static String composeReview(PullRequest pr, Review review, HostUserToUserName ho
 
     static String composeReviewFooter(PullRequest pr, Review review, HostUserToUserName hostUserToUserName, HostUserToRole hostUserToRole) {
         var result = new StringBuilder();
-        if (review.body().isPresent()) {
+        if (review.body().isPresent() && !review.body().get().isBlank()) {
             result.append(composeReviewVerdict(review, hostUserToUserName, hostUserToRole));
             result.append("\n\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
@@ -1327,4 +1327,60 @@ void ignoreComments(TestInfo testInfo) throws IOException {
             assertFalse(archiveContains(archiveFolder.path(), "Review ignore"));
         }
     }
+
+    @Test
+    void replyToEmptyReview(TestInfo testInfo) throws IOException {
+        try (var credentials = new HostCredentials(testInfo);
+             var tempFolder = new TemporaryDirectory();
+             var archiveFolder = new TemporaryDirectory();
+             var listServer = new TestMailmanServer();
+             var webrevServer = new TestWebrevServer()) {
+            var author = credentials.getHostedRepository();
+            var reviewer = credentials.getHostedRepository();
+            var archive = credentials.getHostedRepository();
+            var listAddress = EmailAddress.parse(listServer.createList("test"));
+            var censusBuilder = credentials.getCensusBuilder()
+                                           .addReviewer(reviewer.forge().currentUser().id())
+                                           .addAuthor(author.forge().currentUser().id());
+            var from = EmailAddress.from("test", "test@test.mail");
+            var mlBot = new MailingListBridgeBot(from, author, archive, "master", censusBuilder.build(), "master",
+                                                 listAddress, Set.of(), Set.of(),
+                                                 listServer.getArchive(),
+                                                 listServer.getSMTP(),
+                                                 archive, "webrev", Path.of("test"),
+                                                 webrevServer.uri(),
+                                                 Set.of(), Map.of(),
+                                                 URIBuilder.base("http://issues.test/browse/").build(),
+                                                 Map.of(), Duration.ZERO);
+
+            // Populate the projects repository
+            var reviewFile = Path.of("reviewfile.txt");
+            var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType(), reviewFile);
+            var masterHash = localRepo.resolve("master").orElseThrow();
+            localRepo.push(masterHash, author.url(), "master", true);
+            localRepo.push(masterHash, archive.url(), "webrev", true);
+
+            // Make a change with a corresponding PR
+            var editHash = CheckableRepository.appendAndCommit(localRepo);
+            localRepo.push(editHash, author.url(), "edit", true);
+            var pr = credentials.createPullRequest(archive, "master", "edit", "This is a pull request");
+            pr.setBody("This is now ready");
+            TestBotRunner.runPeriodicItems(mlBot);
+            listServer.processIncoming();
+
+            // Make an empty approval
+            var reviewPr = reviewer.pullRequest(pr.id());
+            reviewPr.addReview(Review.Verdict.APPROVED, "");
+            TestBotRunner.runPeriodicItems(mlBot);
+            listServer.processIncoming();
+
+            pr.addComment("Thanks for the review!");
+            TestBotRunner.runPeriodicItems(mlBot);
+            listServer.processIncoming();
+
+            // The approval text should be included in the quote
+            Repository.materialize(archiveFolder.path(), archive.url(), "master");
+            assertEquals(1, archiveContainsCount(archiveFolder.path(), "^> Marked as reviewed"));
+        }
+    }
 }