Skip to content
Permalink
Browse files
Avoid quoting an empty review body
Reviewed-by: ehelin
  • Loading branch information
rwestberg committed Jan 20, 2020
1 parent 9f56fca commit 3d62e289ebd05a4991cce46320a789377a8a1d72
Showing 2 changed files with 58 additions and 2 deletions.
@@ -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");
}
@@ -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"));
}
}
}

0 comments on commit 3d62e28

Please sign in to comment.