From 5cf9d46c6a6faa6e25dfe756ccba3671ed67d6b7 Mon Sep 17 00:00:00 2001 From: Erik Joelsson Date: Tue, 15 Jun 2021 13:38:55 +0000 Subject: [PATCH] 1080: /backport command reports conflict even though there isn't any Reviewed-by: kcr --- .../skara/bots/pr/BackportCommand.java | 2 +- .../bots/pr/BackportCommitCommandTests.java | 93 +++++++++++++++++-- 2 files changed, 88 insertions(+), 7 deletions(-) diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/BackportCommand.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/BackportCommand.java index 79c06cd91..180b9ffdd 100644 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/BackportCommand.java +++ b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/BackportCommand.java @@ -122,7 +122,7 @@ public void handle(PullRequestBot bot, HostedCommit commit, CensusInstance censu .resolve("fork"); var localRepo = bot.hostedRepositoryPool() .orElseThrow(() -> new IllegalStateException("Missing repository pool for PR bot")) - .materialize(fork, localRepoDir); + .materialize(targetRepo, localRepoDir); var fetchHead = localRepo.fetch(bot.repo().url(), hash.hex(), false); localRepo.checkout(targetBranch); var head = localRepo.head(); diff --git a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/BackportCommitCommandTests.java b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/BackportCommitCommandTests.java index 3258dc84e..5000ab531 100644 --- a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/BackportCommitCommandTests.java +++ b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/BackportCommitCommandTests.java @@ -26,6 +26,7 @@ import org.openjdk.skara.test.*; import java.io.IOException; +import java.nio.file.Files; import java.util.*; import static org.junit.jupiter.api.Assertions.*; @@ -33,9 +34,11 @@ public class BackportCommitCommandTests { @Test void simple(TestInfo testInfo) throws IOException { try (var credentials = new HostCredentials(testInfo); + var forkCredentials = new HostCredentials(testInfo); var tempFolder = new TemporaryDirectory()) { var author = credentials.getHostedRepository(); var reviewer = credentials.getHostedRepository(); + var fork = forkCredentials.getHostedRepository(); var censusBuilder = credentials.getCensusBuilder() .addAuthor(author.forge().currentUser().id()) @@ -46,7 +49,7 @@ void simple(TestInfo testInfo) throws IOException { .censusRepo(censusBuilder.build()) .censusLink("https://census.com/{{contributor}}-profile") .seedStorage(seedFolder) - .forks(Map.of(author.name(), author)) + .forks(Map.of(author.name(), fork)) .build(); // Populate the projects repository @@ -54,6 +57,9 @@ void simple(TestInfo testInfo) throws IOException { var masterHash = CheckableRepository.appendAndCommit(localRepo); localRepo.push(masterHash, author.url(), "master", true); + // Initiate the fork repo + localRepo.push(masterHash, fork.url(), "master", true); + // Make a change in another branch var editHash = CheckableRepository.appendAndCommit(localRepo); localRepo.push(editHash, author.url(), "edit"); @@ -199,9 +205,11 @@ void unknownTargetBranch(TestInfo testInfo) throws IOException { @Test void backportDoesNotApply(TestInfo testInfo) throws IOException { try (var credentials = new HostCredentials(testInfo); + var forkCredentials = new HostCredentials(testInfo); var tempFolder = new TemporaryDirectory()) { var author = credentials.getHostedRepository(); var reviewer = credentials.getHostedRepository(); + var fork = forkCredentials.getHostedRepository(); var censusBuilder = credentials.getCensusBuilder() .addAuthor(author.forge().currentUser().id()) @@ -212,19 +220,23 @@ void backportDoesNotApply(TestInfo testInfo) throws IOException { .censusRepo(censusBuilder.build()) .censusLink("https://census.com/{{contributor}}-profile") .seedStorage(seedFolder) - .forks(Map.of(author.name(), author)) + .forks(Map.of(author.name(), fork)) .build(); // Populate the projects repository var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType()); - var masterHash = CheckableRepository.appendAndCommit(localRepo); - localRepo.push(masterHash, author.url(), "master", true); + var initialHash = localRepo.head(); + + // Initiate the fork repository + localRepo.push(initialHash, fork.url(), "master", true); - // Make a change push it to edit branch + // Make a change and push it to edit branch var editHash = CheckableRepository.appendAndCommit(localRepo); localRepo.push(editHash, author.url(), "edit", true); - var masterHash2 = CheckableRepository.appendAndCommit(localRepo); + // Add another conflicting change in the master branch + localRepo.checkout(initialHash); + var masterHash2 = CheckableRepository.appendAndCommit(localRepo, "a different line"); localRepo.push(masterHash2, author.url(), "master", true); // Add a backport command @@ -292,4 +304,73 @@ void backportTwice(TestInfo testInfo) throws IOException { assertTrue(botReply.body().contains("with this backport")); } } + + /** + * Tests backport with unrelated change in target and a common dependency + * change in both target and source + */ + @Test + void complex(TestInfo testInfo) throws IOException { + try (var credentials = new HostCredentials(testInfo); + var forkCredentials = new HostCredentials(testInfo); + var tempFolder = new TemporaryDirectory()) { + var author = credentials.getHostedRepository(); + var reviewer = credentials.getHostedRepository(); + var fork = forkCredentials.getHostedRepository(); + + var censusBuilder = credentials.getCensusBuilder() + .addAuthor(author.forge().currentUser().id()) + .addReviewer(reviewer.forge().currentUser().id()); + var seedFolder = tempFolder.path().resolve("seed"); + var bot = PullRequestBot.newBuilder() + .repo(author) + .censusRepo(censusBuilder.build()) + .censusLink("https://census.com/{{contributor}}-profile") + .seedStorage(seedFolder) + .forks(Map.of(author.name(), fork)) + .build(); + + // Populate the projects repository + var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType()); + var masterHash = CheckableRepository.appendAndCommit(localRepo); + localRepo.push(masterHash, author.url(), "master", true); + + // Initiate the fork repo + localRepo.push(masterHash, fork.url(), "master", true); + + // Make an unrelated change in master + var unrelated = localRepo.root().resolve("unrelated.txt"); + Files.writeString(unrelated, "Hello"); + localRepo.add(unrelated); + var unrelatedHash = localRepo.commit("Unrelated", "X", "x@y.z"); + localRepo.push(unrelatedHash, author.url(), "master"); + + // Make a change in edit branch, without the unrelated change + localRepo.checkout(masterHash); + var editHashCommon = CheckableRepository.appendAndCommit(localRepo); + localRepo.push(editHashCommon, author.url(), "edit"); + + // Make the same change in master + localRepo.checkout(unrelatedHash); + var masterHashCommon = CheckableRepository.appendAndCommit(localRepo); + localRepo.push(masterHashCommon, author.url(), "master"); + + // Make another change in edit branch that will be the source of the backport + localRepo.checkout(editHashCommon); + var editHash = CheckableRepository.appendAndCommit(localRepo); + localRepo.push(editHash, author.url(), "edit"); + + // Add a backport command + author.addCommitComment(editHash, "/backport " + author.name()); + TestBotRunner.runPeriodicItems(bot); + + var recentCommitComments = author.recentCommitComments(); + assertEquals(2, recentCommitComments.size()); + var botReply = recentCommitComments.get(0); + assertTrue(botReply.body().contains("backport")); + assertTrue(botReply.body().contains("was successfully created")); + assertTrue(botReply.body().contains("To create a pull request")); + assertTrue(botReply.body().contains("with this backport")); + } + } }