Skip to content
Permalink
Browse files
448: Rerun check after updating title
Reviewed-by: ehelin
  • Loading branch information
rwestberg committed Aug 31, 2020
1 parent cca94e0 commit 159f939a1b0946b11a698ab24dae9533f9d149a3
@@ -25,8 +25,7 @@
import org.openjdk.skara.email.EmailAddress;
import org.openjdk.skara.forge.*;
import org.openjdk.skara.host.HostUser;
import org.openjdk.skara.issuetracker.Comment;
import org.openjdk.skara.issuetracker.IssueProject;
import org.openjdk.skara.issuetracker.*;
import org.openjdk.skara.vcs.*;
import org.openjdk.skara.vcs.openjdk.Issue;

@@ -35,7 +34,6 @@
import java.util.*;
import java.util.logging.Logger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.*;

class CheckRun {
@@ -61,7 +59,6 @@ class CheckRun {
private static final String emptyPrBodyMarker = "<!--\nReplace this text with a description of your pull request (also remove the surrounding HTML comment markers).\n" +
"If in doubt, feel free to delete everything in this edit box first, the bot will restore the progress section as needed.\n-->";
private final Set<String> newLabels;
static final Pattern ISSUE_ID_PATTERN = Pattern.compile("^(?:[A-Za-z][A-Za-z0-9]+-)?([0-9]+)$");

private CheckRun(CheckWorkItem workItem, PullRequest pr, Repository localRepo, List<Comment> comments,
List<Review> allReviews, List<Review> activeReviews, Set<String> labels,
@@ -463,27 +460,6 @@ private String updateStatusMessage(String message) {
return newBody;
}

private String updateTitle() {
var title = pr.title();
var m = ISSUE_ID_PATTERN.matcher(title);
var project = issueProject();

var newTitle = title;
if (m.matches() && project != null) {
var id = m.group(1);
var issue = project.issue(id);
if (issue.isPresent()) {
newTitle = id + ": " + issue.get().title();
}
}

if (!title.equals(newTitle)) {
pr.setTitle(newTitle);
}

return newTitle;
}

private String verdictToString(Review.Verdict verdict) {
switch (verdict) {
case APPROVED:
@@ -760,7 +736,7 @@ private void checkStatus() {
// Calculate and update the status message if needed
var statusMessage = getStatusMessage(comments, activeReviews, visitor, additionalErrors);
var updatedBody = updateStatusMessage(statusMessage);
var title = updateTitle();
var title = pr.title();

// Post / update approval messages (only needed if the review itself can't contain a body)
if (!pr.repository().forge().supportsReviewBody()) {
@@ -42,6 +42,7 @@
class CheckWorkItem extends PullRequestWorkItem {
private final Pattern metadataComments = Pattern.compile("<!-- (?:(add|remove) (?:contributor|reviewer))|(?:summary: ')|(?:solves: ')|(?:additional required reviewers)");
private final Logger log = Logger.getLogger("org.openjdk.skara.bots.pr");
static final Pattern ISSUE_ID_PATTERN = Pattern.compile("^(?:[A-Za-z][A-Za-z0-9]+-)?([0-9]+)$");

CheckWorkItem(PullRequestBot bot, PullRequest pr, Consumer<RuntimeException> errorHandler) {
super(bot, pr, errorHandler);
@@ -127,6 +128,28 @@ private boolean currentCheckValid(CensusInstance censusInstance, List<Comment> c
return false;
}

private boolean updateTitle() {
var title = pr.title();
var m = ISSUE_ID_PATTERN.matcher(title);
var project = bot.issueProject();

var newTitle = title;
if (m.matches() && project != null) {
var id = m.group(1);
var issue = project.issue(id);
if (issue.isPresent()) {
newTitle = id + ": " + issue.get().title();
}
}

if (!title.equals(newTitle)) {
pr.setTitle(newTitle);
return true;
}

return false;
}

@Override
public String toString() {
return "CheckWorkItem@" + pr.repository().name() + "#" + pr.id();
@@ -149,6 +172,11 @@ public Collection<WorkItem> run(Path scratchPath) {
return List.of();
}

// If the title needs updating, we run the check again
if (updateTitle()) {
return List.of(new CheckWorkItem(bot, pr.repository().pullRequest(pr.id()), errorHandler));
}

try {
var seedPath = bot.seedStorage().orElse(scratchPath.resolve("seeds"));
var hostedRepositoryPool = new HostedRepositoryPool(seedPath);
@@ -1529,6 +1529,59 @@ void expandTitleWithIssueId(TestInfo testInfo) throws IOException {
}
}

@Test
void expandInvalidTitleWithNumericIssueId(TestInfo testInfo) throws IOException {
try (var credentials = new HostCredentials(testInfo);
var tempFolder = new TemporaryDirectory()) {
var author = credentials.getHostedRepository();
var reviewer = credentials.getHostedRepository();
var issues = credentials.getIssueProject();

var censusBuilder = credentials.getCensusBuilder()
.addAuthor(author.forge().currentUser().id())
.addReviewer(reviewer.forge().currentUser().id());
var checkBot = PullRequestBot.newBuilder()
.repo(author)
.censusRepo(censusBuilder.build())
.issueProject(issues)
.build();

var bug = issues.createIssue("My first bug", List.of("A bug"), Map.of());
var numericId = bug.id().split("-")[1];

// Populate the projects repository
var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType(),
Path.of("appendable.txt"), Set.of("issues"), "0.9");
var masterHash = localRepo.resolve("master").orElseThrow();
localRepo.push(masterHash, author.url(), "master", true);

// Make a change with a corresponding PR
var bugHash = CheckableRepository.appendAndCommit(localRepo);
localRepo.push(bugHash, author.url(), "bug", true);

var bugPR = credentials.createPullRequest(author, "master", "bug", "bad title", true);

// Check the status (should not expand title)
TestBotRunner.runPeriodicItems(checkBot);
assertEquals("bad title", bugPR.title());
assertEquals(CheckStatus.FAILURE, bugPR.checks(bugHash).get("jcheck").status());
assertTrue(bugPR.checks(bugHash).get("jcheck").summary().get().contains("The commit message does not reference any issue"));

// Now update it
bugPR.setTitle(numericId);
bugPR = author.pullRequest(bugPR.id());
assertEquals(numericId, bugPR.title());

// Check the status (should expand title)
TestBotRunner.runPeriodicItems(checkBot);
assertEquals(CheckStatus.SUCCESS, bugPR.checks(bugHash).get("jcheck").status());

// Verify that the title is expanded
bugPR = author.pullRequest(bugPR.id());
assertEquals(numericId + ": " + bug.title(), bugPR.title());
}
}

@Test
void overrideJcheckConf(TestInfo testInfo) throws IOException {
try (var credentials = new HostCredentials(testInfo);

1 comment on commit 159f939

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented on 159f939 Aug 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.