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

383: More fuzzy matching of bug ids in PR titles #623

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
@@ -83,12 +83,12 @@ public void run(Path scratchPath) {
continue;
}

var issue = org.openjdk.skara.vcs.openjdk.Issue.fromString(pr.title());
var issue = org.openjdk.skara.vcs.openjdk.Issue.fromStringRelaxed(pr.title());
if (issue.isEmpty()) {
log.info("No issue found in title for " + describe(pr));
continue;
}
var jbsIssue = project.issue(issue.get().id());
var jbsIssue = project.issue(issue.get().shortId());
if (jbsIssue.isEmpty()) {
log.info("No issue found in JBS for " + describe(pr));
continue;
@@ -137,8 +137,8 @@ private static String describeCommits(List<CommitMetadata> commits, String adjec
}

private static Optional<String> issueUrl(PullRequest pr, URI issueTracker, String projectPrefix) {
var issue = Issue.fromString(pr.title());
return issue.map(value -> URIBuilder.base(issueTracker).appendPath(projectPrefix + "-" + value.id()).build().toString());
var issue = Issue.fromStringRelaxed(pr.title());
return issue.map(value -> URIBuilder.base(issueTracker).appendPath(projectPrefix + "-" + value.shortId()).build().toString());
}

private static String stats(Repository localRepo, Hash base, Hash head) {
@@ -71,12 +71,12 @@ private void generate(PullRequest pr, Repository localRepository, Path folder, D
.pullRequest(pr.webUrl().toString())
.username(fullName);

var issue = Issue.fromString(pr.title());
var issue = Issue.fromStringRelaxed(pr.title());
if (issue.isPresent()) {
var conf = JCheckConfiguration.from(localRepository, head);
if (!conf.isEmpty()) {
var project = conf.get().general().jbs() != null ? conf.get().general().jbs() : conf.get().general().project();
var id = issue.get().id();
var id = issue.get().shortId();
IssueTracker issueTracker = null;
try {
issueTracker = IssueTracker.from("jira", URI.create("https://bugs.openjdk.java.net"));
@@ -248,7 +248,7 @@ public void handleCommits(HostedRepository repository, Repository localRepositor
var commitNotification = CommitFormatters.toTextBrief(repository, commit);
var commitMessage = CommitMessageParsers.v1.parse(commit);
for (var commitIssue : commitMessage.issues()) {
var optionalIssue = issueProject.issue(commitIssue.id());
var optionalIssue = issueProject.issue(commitIssue.shortId());
if (optionalIssue.isEmpty()) {
log.severe("Cannot update issue " + commitIssue.id() + " with commit " + commit.hash().abbreviate()
+ " - issue not found in issue project");
@@ -347,7 +347,7 @@ public String name() {

@Override
public void handleNewIssue(PullRequest pr, org.openjdk.skara.vcs.openjdk.Issue issue) {
var realIssue = issueProject.issue(issue.id());
var realIssue = issueProject.issue(issue.shortId());
if (realIssue.isEmpty()) {
log.warning("Pull request " + pr + " added unknown issue: " + issue.id());
return;
@@ -367,7 +367,7 @@ public void handleNewIssue(PullRequest pr, org.openjdk.skara.vcs.openjdk.Issue i

@Override
public void handleRemovedIssue(PullRequest pr, org.openjdk.skara.vcs.openjdk.Issue issue) {
var realIssue = issueProject.issue(issue.id());
var realIssue = issueProject.issue(issue.shortId());
if (realIssue.isEmpty()) {
log.warning("Pull request " + pr + " removed unknown issue: " + issue.id());
return;
@@ -51,7 +51,7 @@ private JSONObject commitToChanges(HostedRepository repository, Repository local
var parsedMessage = CommitMessageParsers.v1.parse(commit);
var issueIds = JSON.array();
for (var issue : parsedMessage.issues()) {
issueIds.add(JSON.of(issue.id()));
issueIds.add(JSON.of(issue.shortId()));
}
ret.put("issue", issueIds);
ret.put("user", commit.author().name());
@@ -67,7 +67,7 @@ private JSONObject issuesToChanges(HostedRepository repository, Repository local

var issueIds = JSON.array();
for (var issue : issues) {
issueIds.add(JSON.of(issue.id()));
issueIds.add(JSON.of(issue.shortId()));
}

ret.put("issue", issueIds);
@@ -22,7 +22,7 @@
*/
package org.openjdk.skara.bots.pr;

import org.openjdk.skara.forge.PullRequest;
import org.openjdk.skara.forge.*;
import org.openjdk.skara.issuetracker.*;
import org.openjdk.skara.json.JSON;

@@ -83,15 +83,15 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst
}

var issueProject = bot.issueProject();
var issue = org.openjdk.skara.vcs.openjdk.Issue.fromString(pr.title());
var issue = org.openjdk.skara.vcs.openjdk.Issue.fromStringRelaxed(pr.title());
if (issue.isEmpty()) {
csrReply(reply);
jbsReply(pr, reply);
pr.addLabel(CSR_LABEL);
return;
}

var jbsIssue = issueProject.issue(issue.get().id());
var jbsIssue = issueProject.issue(issue.get().shortId());
if (jbsIssue.isEmpty()) {
csrReply(reply);
jbsReply(pr, reply);
@@ -270,6 +270,14 @@ private String formatContributor(EmailAddress contributor) {
}
}

private boolean relaxedEquals(String s1, String s2) {
return s1.trim()
.replaceAll("\\s+", " ")
.equalsIgnoreCase(s2.trim()
.replaceAll("\\s+", " "));
}


private String getStatusMessage(List<Comment> comments, List<Review> reviews, PullRequestCheckIssueVisitor visitor, List<String> additionalErrors) {
var progressBody = new StringBuilder();
progressBody.append("---------\n");
@@ -288,7 +296,7 @@ private String getStatusMessage(List<Comment> comments, List<Review> reviews, Pu
progressBody.append(getAdditionalErrorsList(allAdditionalErrors));
}

var issue = Issue.fromString(pr.title());
var issue = Issue.fromStringRelaxed(pr.title());
var issueProject = workItem.bot.issueProject();
if (issueProject != null && issue.isPresent()) {
var allIssues = new ArrayList<Issue>();
@@ -301,25 +309,36 @@ private String getStatusMessage(List<Comment> comments, List<Review> reviews, Pu
progressBody.append("\n");
for (var currentIssue : allIssues) {
progressBody.append(" * ");
try {
var iss = issueProject.issue(currentIssue.id());
if (iss.isPresent()) {
progressBody.append("[");
progressBody.append(iss.get().id());
progressBody.append("](");
progressBody.append(iss.get().webUrl());
progressBody.append("): ");
progressBody.append(iss.get().title());
progressBody.append("\n");
} else {
progressBody.append("⚠️ Failed to retrieve information on issue `");
if (currentIssue.project().isPresent() && !currentIssue.project().get().equals(issueProject.name())) {
progressBody.append("⚠️ Issue `");
progressBody.append(currentIssue.id());
progressBody.append("` does not belong to the `");
progressBody.append(issueProject.name());
progressBody.append("` project.");
} else {
try {
var iss = issueProject.issue(currentIssue.shortId());
if (iss.isPresent()) {
progressBody.append("[");
progressBody.append(iss.get().id());
progressBody.append("](");
progressBody.append(iss.get().webUrl());
progressBody.append("): ");
progressBody.append(iss.get().title());
if (!relaxedEquals(iss.get().title(), currentIssue.description())) {
progressBody.append(" ⚠️ Title mismatch between PR and JBS.");
}
progressBody.append("\n");
} else {
progressBody.append("⚠️ Failed to retrieve information on issue `");
progressBody.append(currentIssue.id());
progressBody.append("`.\n");
}
} catch (RuntimeException e) {
progressBody.append("⚠️ Temporary failure when trying to retrieve information on issue `");
progressBody.append(currentIssue.id());
progressBody.append("`.\n");
}
} catch (RuntimeException e) {
progressBody.append("⚠️ Temporary failure when trying to retrieve information on issue `");
progressBody.append(currentIssue.id());
progressBody.append("`.\n");
}
}
}
@@ -63,7 +63,7 @@ private String commitMessage(List<Review> activeReviews, Namespace namespace) th

var additionalIssues = SolvesTracker.currentSolved(currentUser, comments);
var summary = Summary.summary(currentUser, comments);
var issue = Issue.fromString(pr.title());
var issue = Issue.fromStringRelaxed(pr.title());
var commitMessageBuilder = issue.map(CommitMessage::title).orElseGet(() -> CommitMessage.title(pr.title()));
if (issue.isPresent()) {
commitMessageBuilder.issues(additionalIssues);
@@ -82,15 +82,12 @@ private void showHelp(PrintWriter reply) {
.collect(Collectors.toList());
}
for (var issue : ret) {
if (issue.id().contains("-") && !issue.id().startsWith(allowedPrefix)) {
if (issue.project().isPresent() && !issue.project().get().equals(allowedPrefix)) {
throw new InvalidIssue(issue.id(), "This PR can only solve issues in the " + allowedPrefix + " project");
}
}

// Drop the validated project prefixes
return ret.stream()
.map(issue -> issue.id().contains("-") ? new Issue(issue.id().split("-", 2)[1], issue.description()) : issue)
.collect(Collectors.toList());
return ret;
}

IssueCommand(String name) {
@@ -119,7 +116,7 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst

var currentSolved = SolvesTracker.currentSolved(pr.repository().forge().currentUser(), allComments)
.stream()
.map(Issue::id)
.map(Issue::shortId)
.collect(Collectors.toSet());
try {
if (args.startsWith("remove") || args.startsWith("delete")) {
@@ -134,11 +131,11 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst
}
var issuesToRemove = parseIssueList(bot.issueProject() == null ? "" : bot.issueProject().name(), args.substring(issueListStart));
for (var issue : issuesToRemove) {
if (currentSolved.contains(issue.id())) {
if (currentSolved.contains(issue.shortId())) {
reply.println(SolvesTracker.removeSolvesMarker(issue));
reply.println("Removing additional issue from " + name + " list: `" + issue.id() + "`.");
reply.println("Removing additional issue from " + name + " list: `" + issue.shortId() + "`.");
} else {
reply.print("The issue `" + issue.id() + "` was not found in the list of additional solved issues. The list currently contains these issues: ");
reply.print("The issue `" + issue.shortId() + "` was not found in the list of additional solved issues. The list currently contains these issues: ");
var currentList = currentSolved.stream()
.map(id -> "`" + id + "`")
.collect(Collectors.joining(", "));
@@ -165,16 +162,16 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst
if (bot.issueProject() == null) {
if (issue.description() == null) {
reply.print("This repository does not have an issue project configured - you will need to input the issue title manually ");
reply.println("using the syntax `/" + name + " " + issue.id() + ": title of the issue`.");
reply.println("using the syntax `/" + name + " " + issue.shortId() + ": title of the issue`.");
return;
} else {
validatedIssues.add(issue);
continue;
}
}
var validatedIssue = bot.issueProject().issue(issue.id());
var validatedIssue = bot.issueProject().issue(issue.shortId());
if (validatedIssue.isEmpty()) {
reply.println("The issue `" + issue.id() + "` was not found in the `" + bot.issueProject().name() + "` project - make sure you have entered it correctly.");
reply.println("The issue `" + issue.shortId() + "` was not found in the `" + bot.issueProject().name() + "` project - make sure you have entered it correctly.");
continue;
}
if (validatedIssue.get().state() != org.openjdk.skara.issuetracker.Issue.State.OPEN) {
@@ -189,8 +186,8 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst

} catch (RuntimeException e) {
if (issue.description() == null) {
reply.print("Temporary failure when trying to look up issue `" + issue.id() + "` - you will need to input the issue title manually ");
reply.println("using the syntax `/" + name + " " + issue.id() + ": title of the issue`.");
reply.print("Temporary failure when trying to look up issue `" + issue.shortId() + "` - you will need to input the issue title manually ");
reply.println("using the syntax `/" + name + " " + issue.shortId() + ": title of the issue`.");
return;
} else {
validatedIssues.add(issue);
@@ -202,29 +199,25 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst
return;
}

// Drop the validated project prefixes
var strippedValidatedIssues = validatedIssues.stream()
.map(issue -> issue.id().contains("-") ? new Issue(issue.id().split("-", 2)[1], issue.description()) : issue)
.collect(Collectors.toList());
var titleIssue = Issue.fromString(pr.title());
for (var issue : strippedValidatedIssues) {
var titleIssue = Issue.fromStringRelaxed(pr.title());
for (var issue : validatedIssues) {
if (titleIssue.isEmpty()) {
reply.print("The primary solved issue for a PR is set through the PR title. Since the current title does ");
reply.println("not contain an issue reference, it will now be updated.");
pr.setTitle(issue.toString());
pr.setTitle(issue.toShortString());
titleIssue = Optional.of(issue);
continue;
}
if (titleIssue.get().id().equals(issue.id())) {
if (titleIssue.get().shortId().equals(issue.shortId())) {
reply.println("This issue is referenced in the PR title - it will now be updated.");
pr.setTitle(issue.toString());
pr.setTitle(issue.toShortString());
continue;
}
reply.println(SolvesTracker.setSolvesMarker(issue));
if (currentSolved.contains(issue.id())) {
reply.println("Updating description of additional solved issue: `" + issue.toString() + "`.");
if (currentSolved.contains(issue.shortId())) {
reply.println("Updating description of additional solved issue: `" + issue.toShortString() + "`.");
} else {
reply.println("Adding additional issue to " + name + " list: `" + issue.toString() + "`.");
reply.println("Adding additional issue to " + name + " list: `" + issue.toShortString() + "`.");
}
}
}
@@ -37,11 +37,11 @@

static String setSolvesMarker(Issue issue) {
var encodedDescription = Base64.getEncoder().encodeToString(issue.description().getBytes(StandardCharsets.UTF_8));
return String.format(solvesMarker, issue.id(), encodedDescription);
return String.format(solvesMarker, issue.shortId(), encodedDescription);
}

static String removeSolvesMarker(Issue issue) {
return String.format(solvesMarker, issue.id(), "");
return String.format(solvesMarker, issue.shortId(), "");
}

static List<Issue> currentSolved(HostUser botUser, List<Comment> comments) {
@@ -974,6 +974,16 @@ void issueInSummary(TestInfo testInfo) throws IOException {
assertFalse(pr.body().contains("My first issue"));
assertTrue(pr.body().contains("My second issue"));

// The PR title does not match the issue title
assertTrue(pr.body().contains("Title mismatch"));

// Correct it
pr.setTitle(issue2.id() + " - " + issue2.title());

// Check the status again - it should now match
TestBotRunner.runPeriodicItems(checkBot);
assertFalse(pr.body().contains("Title mismatch"));

// Use an invalid issue key
var issueKey = issue1.id().replace("TEST", "BADPROJECT");
pr.setTitle(issueKey + ": This is a pull request");
@@ -982,7 +992,7 @@ void issueInSummary(TestInfo testInfo) throws IOException {
TestBotRunner.runPeriodicItems(checkBot);
assertFalse(pr.body().contains("My first issue"));
assertFalse(pr.body().contains("My second issue"));
assertTrue(pr.body().contains("Failed to retrieve"));
assertTrue(pr.body().contains("does not belong to the `TEST` project"));

// Now drop the issue key
issueKey = issue1.id().replace("TEST-", "");