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

pr: filter external commands in CommandWorkItem #695

Closed
wants to merge 1 commit 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
@@ -59,16 +59,14 @@ public class CommandWorkItem extends PullRequestWorkItem {
);

static class HelpCommand implements CommandHandler {
static private Map<String, String> external = null;

@Override
public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInstance, Path scratchPath, CommandInvocation command, List<Comment> allComments, PrintWriter reply) {
reply.println("Available commands:");
Stream.concat(
commandHandlers.entrySet().stream()
.map(entry -> entry.getKey() + " - " + entry.getValue().description()),
external.entrySet().stream()
.map(entry -> entry.getKey() + " - " + entry.getValue())
bot.externalCommands().entrySet().stream()
.map(entry -> entry.getKey() + " - " + entry.getValue())
).sorted().forEachOrdered(c -> reply.println(" * " + c));
}

@@ -177,6 +175,7 @@ private Optional<CommandInvocation> nextCommand(PullRequest pr, List<Comment> co

return allCommands.stream()
.filter(ci -> !handled.contains(ci.id()))
.filter(ci -> !bot.externalCommands().containsKey(ci.name()))
.findFirst();
}

@@ -193,14 +192,9 @@ private void processCommand(PullRequest pr, CensusInstance censusInstance, Path
if (handler.isPresent()) {
handler.get().handle(bot, pr, censusInstance, scratchPath, command, allComments, printer);
} else {
if (!bot.externalCommands().containsKey(command.name())) {
printer.print("Unknown command `");
printer.print(command.name());
printer.println("` - for a list of valid commands use `/help`.");
} else {
// Do not reply to external commands
return;
}
printer.print("Unknown command `");
printer.print(command.name());
printer.println("` - for a list of valid commands use `/help`.");
}

pr.addComment(writer.toString());
@@ -218,16 +212,14 @@ public Collection<WorkItem> run(Path scratchPath) {
var comments = pr.comments();
var nextCommand = nextCommand(pr, comments);
if (nextCommand.isEmpty()) {
log.fine("No new PR commands found, stopping further processing");
log.info("No new non-external PR commands found, stopping further processing");
return List.of();
}

if (HelpCommand.external == null) {
HelpCommand.external = bot.externalCommands();
}

var census = CensusInstance.create(bot.censusRepo(), bot.censusRef(), scratchPath.resolve("census"), pr);
processCommand(pr, census, scratchPath.resolve("pr").resolve("command"), nextCommand.get(), comments);
var command = nextCommand.get();
log.info("Processing command: " + command.id() + " - " + command.name());
processCommand(pr, census, scratchPath.resolve("pr").resolve("command"), command, comments);

// Run another check to reflect potential changes from commands
return List.of(new CheckWorkItem(bot, pr, errorHandler));
@@ -26,6 +26,7 @@
import org.openjdk.skara.test.*;

import java.io.IOException;
import java.util.Map;

import static org.junit.jupiter.api.Assertions.*;
import static org.openjdk.skara.bots.pr.PullRequestAsserts.assertLastCommentContains;
@@ -260,4 +261,50 @@ void disallowedInBody(TestInfo testInfo) throws IOException {
assertLastCommentContains(pr, "The command `help` cannot be used in the pull request body");
}
}

@Test
void externalCommandFollowedByNonExternalCommand(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()
.addAuthor(author.forge().currentUser().id());
var mergeBot = PullRequestBot.newBuilder()
.repo(integrator)
.censusRepo(censusBuilder.build())
.externalCommands(Map.of("external", "Help for external command"))
.build();

// Populate the projects repository
var localRepo = CheckableRepository.init(tempFolder.path(), 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(), "refs/heads/edit", true);
var pr = credentials.createPullRequest(author, "master", "edit", "This is a pull request");

// Issue an external command
var externalCommandComment = pr.addComment("/external");
TestBotRunner.runPeriodicItems(mergeBot);

// The bot should not reply since the external command will be handled by another bot
var lastComment = pr.comments().get(pr.comments().size() - 1);
assertEquals(externalCommandComment, lastComment);

// Issue the help command
pr.addComment("/help");
TestBotRunner.runPeriodicItems(mergeBot);

// The bot should reply with help
assertLastCommentContains(pr, "@user1 Available commands:");
assertLastCommentContains(pr, " * help - shows this text");
assertLastCommentContains(pr, " * external - Help for external command");
}
}
}