Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

430: /summary command does not support multi-line summaries #678

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -29,6 +29,7 @@
import java.nio.file.Path;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

public class SummaryCommand implements CommandHandler {
@Override
@@ -39,9 +40,9 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst
}

var currentSummary = Summary.summary(pr.repository().forge().currentUser(), allComments);
var lines = Arrays.asList(comment.body().split("\n"));

if (args.isBlank()) {
var lines = Arrays.asList(comment.body().split("\n"));
if (lines.size() == 1) {
if (currentSummary.isPresent()) {
reply.println("Removing existing summary");
@@ -51,21 +52,42 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst
}
} else {
// A multi-line summary
var summary = String.join("\n", lines.subList(1, lines.size()));
var action = currentSummary.isPresent() ? "Updating existing" : "Setting";
reply.println(action + " summary to:\n" +
"\n" +
"```\n" +
summary +
"\n```");
reply.println(Summary.setSummaryMarker(summary));
var summaryLines = lines.subList(1, lines.size())
.stream()
.dropWhile(String::isEmpty)
.collect(Collectors.toList());
var lastIndexWithNonBlankLine = summaryLines.size() - 1;
while (lastIndexWithNonBlankLine >= 0 && summaryLines.get(lastIndexWithNonBlankLine).isEmpty()) {
lastIndexWithNonBlankLine--;
}
if (lastIndexWithNonBlankLine == 0) {
reply.println("To set a summary, use the syntax `/summary <summary text>`");
} else {
var summary = String.join("\n", summaryLines.subList(0, lastIndexWithNonBlankLine + 1));
var action = currentSummary.isPresent() ? "Updating existing" : "Setting";
reply.println(action + " summary to:\n" +
"\n" +
"```\n" +
summary +
"\n```");
reply.println(Summary.setSummaryMarker(summary));
}
}
} else {
// A single-line summary
edvbld marked this conversation as resolved.
Show resolved Hide resolved
var summary = args.strip();
var action = currentSummary.isPresent() ? "Updating existing" : "Setting";
reply.println(action + " summary to `" + summary + "`");
reply.println(Summary.setSummaryMarker(summary));
if (lines.size() > 1) {
reply.println("To set a multi-line summary, use the syntax:\n" +
"\n```\n" +
"/summary\n" +
"This is the first line\n" +
"This is the second line\n" +
"```");
} else {
var summary = args.strip();
var action = currentSummary.isPresent() ? "Updating existing" : "Setting";
reply.println(action + " summary to `" + summary + "`");
reply.println(Summary.setSummaryMarker(summary));
}
}
}

@@ -255,4 +255,129 @@ void multiline(TestInfo testInfo) throws IOException {
assertLastCommentContains(pr, "Updating existing summary to `single line`");
}
}

@Test
void precedingBlankLines(TestInfo testInfo) throws IOException {
try (var credentials = new HostCredentials(testInfo);
var tempFolder = new TemporaryDirectory()) {
var author = credentials.getHostedRepository();
var integrator = credentials.getHostedRepository();

var censusBuilder = credentials.getCensusBuilder()
.addReviewer(integrator.forge().currentUser().id())
.addCommitter(author.forge().currentUser().id());
var prBot = PullRequestBot.newBuilder().repo(integrator).censusRepo(censusBuilder.build()).build();

// Populate the projects repository
var localRepoFolder = tempFolder.path().resolve("localrepo");
var localRepo = CheckableRepository.init(localRepoFolder, author.repositoryType());
var masterHash = localRepo.resolve("master").orElseThrow();
assertFalse(CheckableRepository.hasBeenEdited(localRepo));
localRepo.push(masterHash, author.url(), "master", true);

// Make a change with a corresponding PR
var editHash = CheckableRepository.appendAndCommit(localRepo);
localRepo.push(editHash, author.url(), "edit", true);
var pr = credentials.createPullRequest(author, "master", "edit", "This is a pull request");

// Try setting a summary when none has been set yet
pr.addComment("/summary");
TestBotRunner.runPeriodicItems(prBot);

// The bot should reply with a help message
assertLastCommentContains(pr,"To set a summary");

// Add a multi-line summary with preceding blank lines
pr.addComment("/summary\n\n\nFirst line\nSecond line");
TestBotRunner.runPeriodicItems(prBot);

// The bot should reply with a success message
assertLastCommentContains(pr,
"Setting summary to:\n" +
"\n" +
"```\n" +
"First line\n" +
"Second line\n" +
"```");
}
}

@Test
void trailingBlankLines(TestInfo testInfo) throws IOException {
try (var credentials = new HostCredentials(testInfo);
var tempFolder = new TemporaryDirectory()) {
var author = credentials.getHostedRepository();
var integrator = credentials.getHostedRepository();

var censusBuilder = credentials.getCensusBuilder()
.addReviewer(integrator.forge().currentUser().id())
.addCommitter(author.forge().currentUser().id());
var prBot = PullRequestBot.newBuilder().repo(integrator).censusRepo(censusBuilder.build()).build();

// Populate the projects repository
var localRepoFolder = tempFolder.path().resolve("localrepo");
var localRepo = CheckableRepository.init(localRepoFolder, author.repositoryType());
var masterHash = localRepo.resolve("master").orElseThrow();
assertFalse(CheckableRepository.hasBeenEdited(localRepo));
localRepo.push(masterHash, author.url(), "master", true);

// Make a change with a corresponding PR
var editHash = CheckableRepository.appendAndCommit(localRepo);
localRepo.push(editHash, author.url(), "edit", true);
var pr = credentials.createPullRequest(author, "master", "edit", "This is a pull request");

// Try setting a summary when none has been set yet
pr.addComment("/summary");
TestBotRunner.runPeriodicItems(prBot);

// The bot should reply with a help message
assertLastCommentContains(pr,"To set a summary");

// Add a multi-line summary with preceding blank lines
pr.addComment("/summary\nFirst line\nSecond line\n\n\n");
TestBotRunner.runPeriodicItems(prBot);

// The bot should reply with a success message
assertLastCommentContains(pr,
"Setting summary to:\n" +
"\n" +
"```\n" +
"First line\n" +
"Second line\n" +
"```");
}
}

@Test
void singleAndMultiLineSummary(TestInfo testInfo) throws IOException {
try (var credentials = new HostCredentials(testInfo);
var tempFolder = new TemporaryDirectory()) {
var author = credentials.getHostedRepository();
var integrator = credentials.getHostedRepository();

var censusBuilder = credentials.getCensusBuilder()
.addReviewer(integrator.forge().currentUser().id())
.addCommitter(author.forge().currentUser().id());
var prBot = PullRequestBot.newBuilder().repo(integrator).censusRepo(censusBuilder.build()).build();

// Populate the projects repository
var localRepoFolder = tempFolder.path().resolve("localrepo");
var localRepo = CheckableRepository.init(localRepoFolder, author.repositoryType());
var masterHash = localRepo.resolve("master").orElseThrow();
assertFalse(CheckableRepository.hasBeenEdited(localRepo));
localRepo.push(masterHash, author.url(), "master", true);

// Make a change with a corresponding PR
var editHash = CheckableRepository.appendAndCommit(localRepo);
localRepo.push(editHash, author.url(), "edit", true);
var pr = credentials.createPullRequest(author, "master", "edit", "This is a pull request");

// Try setting a summary when none has been set yet
pr.addComment("/summary inline\nnext line");
TestBotRunner.runPeriodicItems(prBot);

// The bot should reply with a help message
assertLastCommentContains(pr,"To set a multi-line summary, use the syntax:");
}
}
}