Skip to content

Commit

Permalink
SKARA-1533
Browse files Browse the repository at this point in the history
  • Loading branch information
zhaosongzs committed Apr 9, 2024
1 parent 7b71ff0 commit bc917f6
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ private Optional<String> getContributorsList() {
}
}

private boolean relaxedEquals(String s1, String s2) {
static boolean relaxedEquals(String s1, String s2) {
return s1.trim()
.replaceAll("\\s+", " ")
.equalsIgnoreCase(s2.trim()
Expand Down
21 changes: 16 additions & 5 deletions bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
class CheckWorkItem extends PullRequestWorkItem {
private final Logger log = Logger.getLogger("org.openjdk.skara.bots.pr");
static final Pattern ISSUE_ID_PATTERN = Pattern.compile("^(?:(?<prefix>[A-Za-z][A-Za-z0-9]+)-)?(?<id>[0-9]+)"
+ "(?::(?<space>[\\s\u00A0\u2007\u202F]+)(?<title>.+))?$");
+ "(?::?(?<space>[\\s\u00A0\u2007\u202F]+)(?<title>.+))?$");
private static final Pattern BACKPORT_HASH_TITLE_PATTERN = Pattern.compile("^Backport\\s*([0-9a-z]{40})\\s*$", Pattern.CASE_INSENSITIVE);
private static final Pattern BACKPORT_ISSUE_TITLE_PATTERN = Pattern.compile("^Backport\\s*(?:(?<prefix>[A-Za-z][A-Za-z0-9]+)-)?(?<id>[0-9]+)\\s*$", Pattern.CASE_INSENSITIVE);
private static final Pattern METADATA_COMMENTS_PATTERN = Pattern.compile("<!-- (?:backport)|(?:(add|remove) (?:contributor|reviewer))|(?:summary: ')|(?:solves: ')|(?:additional required reviewers)|(?:jep: ')|(?:csr: ')");
Expand Down Expand Up @@ -213,6 +213,7 @@ String getIssueMetadata(String prBody) {
.map(issue -> {
var issueData = new StringBuilder();
issueData.append(issue.id());
issueData.append(issue.title());
issueData.append(issue.status());
issue.resolution().ifPresent(issueData::append);
var properties = issue.properties();
Expand Down Expand Up @@ -339,7 +340,8 @@ private String getMatchGroup(java.util.regex.Matcher m, String group) {
* @return true if the PR was modified
*/
private boolean updateTitle() {
var m = ISSUE_ID_PATTERN.matcher(pr.title());
var oldPrTitle = pr.title();
var m = ISSUE_ID_PATTERN.matcher(oldPrTitle.trim());
var project = bot.issueProject();

if (m.matches() && project != null) {
Expand All @@ -357,10 +359,10 @@ private boolean updateTitle() {
var issue = issueTrackerIssue(id);
if (issue.isPresent()) {
var issueTitle = issue.get().title();
var newPrTitle = id + ": " + issueTitle;
if (title.isEmpty()) {
// If the title is in the form of "[project-]<bugid>" only
// we add the title from JBS
var newPrTitle = id + ": " + issueTitle;
pr.setTitle(newPrTitle);
return true;
} else {
Expand All @@ -370,16 +372,25 @@ private boolean updateTitle() {
title = title.substring(0, title.length() - 1);
}
if (issueTitle.startsWith(title) && issueTitle.length() > title.length()) {
var newPrTitle = id + ": " + issueTitle;
pr.setTitle(newPrTitle);
var remainingTitle = issueTitle.substring(title.length());
if (pr.body().startsWith(ELLIPSIS + remainingTitle + "\n\n")) {
// Remove remaning title, plus decorations
// Remove remaining title, plus decorations
var newPrBody = pr.body().substring(remainingTitle.length() + 3);
pr.setBody(newPrBody);
}
return true;
}
// Automatically update PR title if pr title is not in canonical form
if (title.equals(issueTitle) && !oldPrTitle.equals(newPrTitle)) {
pr.setTitle(newPrTitle);
return true;
}
// Automatically update PR title if titles are relaxed match but not exactly match
if (!title.equals(issueTitle) && CheckRun.relaxedEquals(title, issueTitle)) {
pr.setTitle(newPrTitle);
return true;
}
}
}

Expand Down
12 changes: 12 additions & 0 deletions bots/pr/src/test/java/org/openjdk/skara/bots/pr/CheckTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -1849,6 +1849,18 @@ void expandInvalidTitleWithNumericIssueId(TestInfo testInfo) throws IOException

// Verify that the title is expanded
assertEquals(numericId + ": " + bug.title(), bugPR.store().title());

// Now update pr title to non-canonical form
bugPR.setTitle(bug.id() + " " + bug.title());
TestBotRunner.runPeriodicItems(checkBot);
// Verify that the title is in canonical form
assertEquals(numericId + ": " + bug.title(), bugPR.store().title());

// Now update pr title to another non-canonical form
bugPR.setTitle(bug.id() + ": " + bug.title());
TestBotRunner.runPeriodicItems(checkBot);
// Verify that the title is in canonical form
assertEquals(numericId + ": " + bug.title(), bugPR.store().title());
}
}

Expand Down
23 changes: 19 additions & 4 deletions bots/pr/src/test/java/org/openjdk/skara/bots/pr/IssueBotTests.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -76,7 +76,7 @@ void simple(TestInfo testInfo) throws IOException {
// Make a change with a corresponding PR
var editHash = CheckableRepository.appendAndCommit(localRepo);
localRepo.push(editHash, author.authenticatedUrl(), "edit", true);
var pr = credentials.createPullRequest(author, "master", "edit", issue.id() + ": This is an issue");
var pr = credentials.createPullRequest(author, "master", "edit", "TEST-1: This is an issue");

TestBotRunner.runPeriodicItems(prBot);
var checks = pr.checks(editHash);
Expand Down Expand Up @@ -137,12 +137,27 @@ void simple(TestInfo testInfo) throws IOException {
// issue metadata should not be updated because no update in the issue
assertEquals(issueMetadata2, issueMetadata3);

// Extra run of prBot and issueBot
// Update issue title and run prBot first
// There should be no update in the check
issue.setTitle("This is an Issue");
TestBotRunner.runPeriodicItems(prBot);
TestBotRunner.runPeriodicItems(issueBot);
check = pr.checks(editHash).get("jcheck");
var completedTime7 = check.completedAt().get();
assertEquals(completedTime6, completedTime7);

// Run issueBot
TestBotRunner.runPeriodicItems(issueBot);
check = pr.checks(editHash).get("jcheck");
var completedTime8 = check.completedAt().get();
assertNotEquals(completedTime7, completedTime8);
assertEquals("1: This is an Issue", pr.store().title());

// Extra run of prBot and issueBot
TestBotRunner.runPeriodicItems(prBot);
TestBotRunner.runPeriodicItems(issueBot);
check = pr.checks(editHash).get("jcheck");
var completedTime9 = check.completedAt().get();
assertEquals(completedTime8, completedTime9);
}
}

Expand Down

0 comments on commit bc917f6

Please sign in to comment.