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

394: Merge bot conflict resolution interaction requires separate credentials #618

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -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);
}
}
}