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
Changes from all commits
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
@@ -27,32 +27,67 @@

import java.io.PrintWriter;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

public class SummaryCommand implements CommandHandler {
@Override
public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInstance, Path scratchPath, String args, Comment comment, List<Comment> allComments, PrintWriter reply) {
if (!comment.author().equals(pr.author())) {
reply.println("Only the author (@" + pr.author().userName() + ") is allowed to issue the `summary` command.");
reply.println("Only the author (@" + pr.author().userName() + ") is allowed to issue the `/summary` command.");
return;
}

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

if (args.isBlank()) {
if (currentSummary.isPresent()) {
reply.println("Removing existing summary");
reply.println(Summary.setSummaryMarker(""));
if (lines.size() == 1) {
if (currentSummary.isPresent()) {
reply.println("Removing existing summary");
reply.println(Summary.setSummaryMarker(""));
} else {
reply.println("To set a summary, use the syntax `/summary <summary text>`");
}
} else {
reply.println("To set a summary, use the syntax `/summary <summary text>`");
// A multi-line 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 {
if (currentSummary.isPresent()) {
reply.println("Updating existing summary to `" + args.strip() + "`");
// A single-line summary
edvbld marked this conversation as resolved.
Show resolved Hide resolved
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 {
reply.println("Setting summary to `" + args.strip() + "`");
var summary = args.strip();
var action = currentSummary.isPresent() ? "Updating existing" : "Setting";
reply.println(action + " summary to `" + summary + "`");
reply.println(Summary.setSummaryMarker(summary));
}
reply.println(Summary.setSummaryMarker(args.strip()));
}
}

@@ -133,6 +133,7 @@ void simple(TestInfo testInfo) throws IOException {
assertEquals("Third time is surely the charm", summaryLine);
}
}

@Test
void invalidCommandAuthor(TestInfo testInfo) throws IOException {
try (var credentials = new HostCredentials(testInfo);
@@ -168,4 +169,215 @@ void invalidCommandAuthor(TestInfo testInfo) throws IOException {
assertEquals(1, error);
}
}

@Test
void multiline(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
pr.addComment("/summary\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" +
"```");

// Remove it again
pr.addComment("/summary");
TestBotRunner.runPeriodicItems(prBot);

// The bot should reply with a success message
assertLastCommentContains(pr,"Removing existing");

// Now add one again
pr.addComment("/summary\nL1\nL2");
TestBotRunner.runPeriodicItems(prBot);

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

// Now update it
pr.addComment("/summary\n1L\n2L");
TestBotRunner.runPeriodicItems(prBot);

// The bot should reply with a success message
assertLastCommentContains(pr,
"Updating existing summary to:\n" +
"\n" +
"```\n" +
"1L\n" +
"2L\n" +
"```");

// Finally update it to a single line summary
pr.addComment("/summary single line");
TestBotRunner.runPeriodicItems(prBot);

// The bot should reply with a success message
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:");
}
}
}