diff --git a/bots/notify/src/main/java/org/openjdk/skara/bots/notify/prbranch/PullRequestBranchNotifier.java b/bots/notify/src/main/java/org/openjdk/skara/bots/notify/prbranch/PullRequestBranchNotifier.java index f20d5d70b..062a8e2b3 100644 --- a/bots/notify/src/main/java/org/openjdk/skara/bots/notify/prbranch/PullRequestBranchNotifier.java +++ b/bots/notify/src/main/java/org/openjdk/skara/bots/notify/prbranch/PullRequestBranchNotifier.java @@ -83,6 +83,7 @@ public void onNewPullRequest(PullRequest pr) { @Override public void onStateChange(PullRequest pr, Issue.State oldState) { if (pr.state() == Issue.State.CLOSED) { + PreIntegrations.retargetDependencies(pr); deleteBranch(pr); } else { pushBranch(pr); diff --git a/bots/notify/src/test/java/org/openjdk/skara/bots/notify/prbranch/PullRequestBranchNotifierTests.java b/bots/notify/src/test/java/org/openjdk/skara/bots/notify/prbranch/PullRequestBranchNotifierTests.java index 2ceeb6f9c..33daeaa0f 100644 --- a/bots/notify/src/test/java/org/openjdk/skara/bots/notify/prbranch/PullRequestBranchNotifierTests.java +++ b/bots/notify/src/test/java/org/openjdk/skara/bots/notify/prbranch/PullRequestBranchNotifierTests.java @@ -176,4 +176,51 @@ void branchMissing(TestInfo testInfo) throws IOException { TestBotRunner.runPeriodicItems(notifyBot); } } + + @Test + void retarget(TestInfo testInfo) throws IOException { + try (var credentials = new HostCredentials(testInfo); + var tempFolder = new TemporaryDirectory()) { + + var repo = credentials.getHostedRepository(); + var repoFolder = tempFolder.path().resolve("repo"); + var localRepo = CheckableRepository.init(repoFolder, repo.repositoryType()); + credentials.commitLock(localRepo); + localRepo.pushAll(repo.url()); + + var storageFolder = tempFolder.path().resolve("storage"); + var notifyBot = testBotBuilder(repo, storageFolder).create("notify", JSON.object()); + + // Initialize history + TestBotRunner.runPeriodicItems(notifyBot); + + // Create a PR + var editHash = CheckableRepository.appendAndCommit(localRepo, "Another line"); + localRepo.push(editHash, repo.url(), "source", true); + var pr = credentials.createPullRequest(repo, "master", "source", "This is a PR", false); + pr.addLabel("rfr"); + TestBotRunner.runPeriodicItems(notifyBot); + + // The target repo should now contain the new branch + var hash = localRepo.fetch(repo.url(), PreIntegrations.preIntegrateBranch(pr)); + assertEquals(editHash, hash); + + // Create follow-up work + var followUp = CheckableRepository.appendAndCommit(localRepo, "Follow-up work", "Follow-up change"); + localRepo.push(followUp, repo.url(), "followup", true); + var followUpPr = credentials.createPullRequest(repo, PreIntegrations.preIntegrateBranch(pr), "followup", "This is another pull request"); + assertEquals(PreIntegrations.preIntegrateBranch(pr), followUpPr.targetRef()); + + // Close the PR + pr.setState(Issue.State.CLOSED); + TestBotRunner.runPeriodicItems(notifyBot); + + // The target repo should no longer contain the branch + assertThrows(IOException.class, () -> localRepo.fetch(repo.url(), PreIntegrations.preIntegrateBranch(pr))); + + // The follow-up PR should have been retargeted + followUpPr = repo.pullRequest(followUpPr.id()); + assertEquals("master", followUpPr.targetRef()); + } + } } diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/IntegrateCommand.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/IntegrateCommand.java index 8650d578b..9121ac194 100644 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/IntegrateCommand.java +++ b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/IntegrateCommand.java @@ -220,11 +220,6 @@ public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInst "The error has been logged and will be investigated. It is possible that this error " + "is caused by a transient issue; feel free to retry the operation."); } - - // Additional cleanup outside of the integration lock - if (success) { - PreIntegrations.retargetDependencies(pr); - } } @Override diff --git a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/PreIntegrateTests.java b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/PreIntegrateTests.java index af221af3c..f460e3083 100644 --- a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/PreIntegrateTests.java +++ b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/PreIntegrateTests.java @@ -109,6 +109,9 @@ void integrateFollowup(TestInfo testInfo) throws IOException { // The bot should reply with an ok message assertLastCommentContains(pr, "Pushed as commit"); + // The notifier will now retarget the follow up PR, simulate this + followUpPr.setTargetRef("master"); + // The second should now become ready TestBotRunner.runPeriodicItems(mergeBot); followUpPr = author.pullRequest(followUpPr.id());