Skip to content
Permalink
Browse files
394: Merge bot conflict resolution interaction requires separate cred…
…entials

Reviewed-by: jvernee
  • Loading branch information
rwestberg committed May 11, 2020
1 parent eb9b6b1 commit 9573e7981f0dd16c308067fb192cb3c5dd4fa2c6
@@ -43,6 +43,7 @@
import java.util.logging.Logger;

class MergeBot implements Bot, WorkItem {
private final String integrationCommand = "/integrate\n<!-- Valid self-command -->";
private final Logger log = Logger.getLogger("org.openjdk.skara.bots");;
private final Path storage;

@@ -326,10 +327,10 @@ public void run(Path scratchPath) {
var integrateComments =
comments.stream()
.filter(c -> c.author().equals(currentUser))
.filter(c -> c.body().equals("/integrate"))
.filter(c -> c.body().equals(integrationCommand))
.collect(Collectors.toList());
if (integrateComments.isEmpty()) {
pr.addComment("/integrate");
pr.addComment(integrationCommand);
} else {
var lastIntegrateComment = integrateComments.get(integrateComments.size() - 1);
var id = lastIntegrateComment.id();
@@ -349,7 +350,7 @@ public void run(Path scratchPath) {
var errorPrefix = "@openjdk-bot Your merge request cannot be fulfilled at this time";
if (lines.length > 1 && lines[1].startsWith(errorPrefix)) {
// Try again
pr.addComment("/integrate");
pr.addComment(integrationCommand);
}
// Other reply, potentially due to rebase issue, just
// wait for the labeler to add appropriate labels.
@@ -660,7 +660,7 @@ void resolvedMergeConflictShouldResultInIntegrateCommand(TestInfo testInfo) thro
pr = pullRequests.get(0);
var numComments = pr.comments().size();
var lastComment = pr.comments().get(pr.comments().size() - 1);
assertEquals("/integrate", lastComment.body());
assertEquals("/integrate\n<!-- Valid self-command -->", lastComment.body());

// Running the bot again should not result in another comment
TestBotRunner.runPeriodicItems(bot);
@@ -34,10 +34,12 @@
import java.util.stream.*;

public class CommandWorkItem extends PullRequestWorkItem {
private final Logger log = Logger.getLogger("org.openjdk.skara.bots.pr");
private static final Logger log = Logger.getLogger("org.openjdk.skara.bots.pr");

private final String commandReplyMarker = "<!-- Jmerge command reply message (%s) -->";
private final Pattern commandReplyPattern = Pattern.compile("<!-- Jmerge command reply message \\((\\S+)\\) -->");
private static final Pattern commandPattern = Pattern.compile("^/(.*)");
private static final String commandReplyMarker = "<!-- Jmerge command reply message (%s) -->";
private static final Pattern commandReplyPattern = Pattern.compile("<!-- Jmerge command reply message \\((\\S+)\\) -->");
private static final String selfCommandMarker = "<!-- Valid self-command -->";

private final static Map<String, CommandHandler> commandHandlers = Map.of(
"help", new HelpCommand(),
@@ -84,10 +86,8 @@ public String description() {
.map(matcher -> matcher.group(1))
.collect(Collectors.toSet());

var commandPattern = Pattern.compile("^/(.*)");

return comments.stream()
.filter(comment -> !comment.author().equals(self))
.filter(comment -> !comment.author().equals(self) || comment.body().endsWith(selfCommandMarker))
.map(comment -> new AbstractMap.SimpleEntry<>(comment, commandPattern.matcher(comment.body())))
.filter(entry -> entry.getValue().find())
.filter(entry -> !handled.contains(entry.getKey().id()))
@@ -103,4 +103,50 @@ void helpCommand(TestInfo testInfo) throws IOException {
assertEquals(1, error);
}
}

@Test
void selfCommand(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()).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 command using the bot account
var botPr = integrator.pullRequest(pr.id());
botPr.addComment("/help");

// The bot should not reply
assertEquals(1, pr.comments().size());
TestBotRunner.runPeriodicItems(mergeBot);
assertEquals(1, pr.comments().size());

// But if we add an overriding marker, it should
botPr.addComment("/help\n<!-- Valid self-command -->");

assertEquals(2, pr.comments().size());
TestBotRunner.runPeriodicItems(mergeBot);
assertEquals(3, pr.comments().size());

var help = pr.comments().stream()
.filter(comment -> comment.body().contains("Available commands"))
.filter(comment -> comment.body().contains("help"))
.count();
assertEquals(1, help);
}
}
}

0 comments on commit 9573e79

Please sign in to comment.