From bc917f61986be956395d70e7a8dd27dc040d2316 Mon Sep 17 00:00:00 2001 From: Zhao Song Date: Tue, 9 Apr 2024 15:41:16 -0700 Subject: [PATCH] SKARA-1533 --- .../org/openjdk/skara/bots/pr/CheckRun.java | 2 +- .../openjdk/skara/bots/pr/CheckWorkItem.java | 21 +++++++++++++---- .../org/openjdk/skara/bots/pr/CheckTests.java | 12 ++++++++++ .../openjdk/skara/bots/pr/IssueBotTests.java | 23 +++++++++++++++---- 4 files changed, 48 insertions(+), 10 deletions(-) diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java index 60f678213..39e851e8d 100644 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java +++ b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java @@ -648,7 +648,7 @@ private Optional getContributorsList() { } } - private boolean relaxedEquals(String s1, String s2) { + static boolean relaxedEquals(String s1, String s2) { return s1.trim() .replaceAll("\\s+", " ") .equalsIgnoreCase(s2.trim() diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java index 89337e116..fdc14993b 100644 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java +++ b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java @@ -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("^(?:(?[A-Za-z][A-Za-z0-9]+)-)?(?[0-9]+)" - + "(?::(?[\\s\u00A0\u2007\u202F]+)(?.+))?$"); + + "(?::?(?<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: ')"); @@ -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(); @@ -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) { @@ -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 { @@ -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; + } } } diff --git a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/CheckTests.java b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/CheckTests.java index d0d4f5b6f..0f566ed63 100644 --- a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/CheckTests.java +++ b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/CheckTests.java @@ -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()); } } diff --git a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/IssueBotTests.java b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/IssueBotTests.java index 623494233..d8a1f5f56 100644 --- a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/IssueBotTests.java +++ b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/IssueBotTests.java @@ -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 @@ -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); @@ -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); } }