2304: /backport jdk23 should provide a more helpful message

Reviewed-by: erikj

bots/pr/src/main/java/org/openjdk/skara/bots/pr/BackportCommand.java

@@ -22,6 +22,7 @@
  */
 package org.openjdk.skara.bots.pr;
 
+import org.openjdk.skara.forge.HostedBranch;
 import org.openjdk.skara.forge.HostedCommit;
 import org.openjdk.skara.forge.HostedRepository;
 import org.openjdk.skara.forge.PullRequest;
@@ -35,6 +36,7 @@
 import java.io.UncheckedIOException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Comparator;
 import java.util.List;
 import java.util.stream.Collectors;
 import java.time.format.DateTimeFormatter;
@@ -74,6 +76,8 @@ public boolean allowedInCommit() {
 
     private static final String INSUFFICIENT_ACCESS_WARNING = "The backport can not be created because you don't have access to the target repository.";
 
+    private static final int BRANCHES_LIMIT = 10;
+
     @Override
     public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInstance, ScratchArea scratchArea, CommandInvocation command,
                        List<Comment> allComments, PrintWriter reply, List<String> labelsToAdd, List<String> labelsToRemove) {
@@ -196,8 +200,18 @@ private HostedRepository getTargetRepo(PullRequestBot bot, String repoName, Prin
         if (potentialTargetRepo.isEmpty()) {
             reply.println("The target repository `" + repoNameArg + "` is not a valid target for backports. ");
             reply.print("List of valid target repositories: ");
-            reply.println(String.join(", ", bot.forks().keySet().stream().sorted().toList()) + ".");
+            reply.println(String.join(", ", bot.forks().keySet().stream()
+                    .sorted()
+                    .map(repo -> "`" + repo + "`")
+                    .toList()) + ".");
             reply.println("Supplying the organization/group prefix is optional.");
+            var branchNamesInCurrentRepo = bot.repo().branches().stream().map(HostedBranch::name).toList();
+            if (branchNamesInCurrentRepo.contains(repoName)) {
+                reply.println();
+                reply.println("There is a branch `" + repoName + "` in the current repository `" + bot.repo().name() + "`.");
+                reply.println("To target a backport to this branch in the current repository use:");
+                reply.println("`/backport :" + repoName + "`");
+            }
             return null;
         }
         return potentialTargetRepo.get();
@@ -208,6 +222,16 @@ private Branch getTargetBranch(String[] parts, int index, HostedRepository targe
         var targetBranchHash = targetRepo.branchHash(targetBranchName);
         if (targetBranchHash.isEmpty()) {
             reply.println("The target branch `" + targetBranchName + "` does not exist");
+            reply.print("List of valid branches: ");
+            var branches = targetRepo.branches().stream()
+                    .map(HostedBranch::name)
+                    .filter(name -> !name.startsWith("pr/"))
+                    .sorted(Comparator.reverseOrder())
+                    .toList();
+            reply.println(String.join(", ", branches.stream()
+                    .limit(BRANCHES_LIMIT)
+                    .map(branch -> "`" + branch + "`")
+                    .toList()) + (branches.size() > BRANCHES_LIMIT ? "..." : "."));
             return null;
         }
         return new Branch(targetBranchName);

bots/pr/src/test/java/org/openjdk/skara/bots/pr/BackportCommitCommandTests.java

@@ -63,7 +63,10 @@ void simple(TestInfo testInfo) throws IOException {
             // Make a change in another branch
             var editHash = CheckableRepository.appendAndCommit(localRepo);
             localRepo.push(editHash, author.authenticatedUrl(), "edit");
-
+            // Create more branches
+            for (int i = 1; i <= 20; i++) {
+                localRepo.push(editHash, author.authenticatedUrl(), "jdk" + i, true);
+            }
             // Add a backport command
             author.addCommitComment(editHash, "/backport " + author.name());
             TestBotRunner.runPeriodicItems(bot);
@@ -95,6 +98,20 @@ void simple(TestInfo testInfo) throws IOException {
             botReply = recentCommitComments.get(0);
             assertTrue(botReply.body().contains("To create a pull request with this backport targeting " +
                     "[" + author.name() + ":" + author.defaultBranchName() + "]"));
+
+            author.addCommitComment(editHash, "/backport jdk11");
+            TestBotRunner.runPeriodicItems(bot);
+            recentCommitComments = author.recentCommitComments();
+            assertEquals(8, recentCommitComments.size());
+            botReply = recentCommitComments.get(0);
+            assertTrue(botReply.body().contains("There is a branch `jdk11` in the current repository `test`."));
+
+            author.addCommitComment(editHash, "/backport :jdk31");
+            TestBotRunner.runPeriodicItems(bot);
+            recentCommitComments = author.recentCommitComments();
+            assertEquals(10, recentCommitComments.size());
+            botReply = recentCommitComments.get(0);
+            assertTrue(botReply.body().contains("List of valid branches:"));
         }
     }
 
@@ -135,7 +152,7 @@ void unknownTargetRepo(TestInfo testInfo) throws IOException {
             var botReply = recentCommitComments.get(0);
             assertTrue(botReply.body().contains("target repository"));
             assertTrue(botReply.body().contains("is not a valid target for backports"));
-            assertTrue(botReply.body().contains("List of valid target repositories: foobar/other-repo, test"));
+            assertTrue(botReply.body().contains("List of valid target repositories: `foobar/other-repo`, `test`"));
             assertEquals(List.of(), author.openPullRequests());
         }
     }
@@ -176,7 +193,7 @@ void unknownTargetRepoFork(TestInfo testInfo) throws IOException {
             assertEquals(2, recentCommitComments.size());
             var botReply = recentCommitComments.get(0);
             assertTrue(botReply.body().contains("is not a valid target for backports"));
-            assertTrue(botReply.body().contains("List of valid target repositories: foobar/other-repo"));
+            assertTrue(botReply.body().contains("List of valid target repositories: `foobar/other-repo`"));
             assertEquals(List.of(), author.openPullRequests());
         }
     }