Skip to content

Commit

Permalink
1080: /backport command reports conflict even though there isn't any
Browse files Browse the repository at this point in the history 
Reviewed-by: kcr
  • Loading branch information
@erikj79
erikj79 committed Jun 15, 2021
1 parent ada4e37 commit 5cf9d46
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 7 deletions.
2 changes: 1 addition & 1 deletion bots/pr/src/main/java/org/openjdk/skara/bots/pr/BackportCommand.java
View file
Expand Up @@ -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();
Expand Down
93 changes: 87 additions & 6 deletions bots/pr/src/test/java/org/openjdk/skara/bots/pr/BackportCommitCommandTests.java
View file
Expand Up @@ -26,16 +26,19 @@
import org.openjdk.skara.test.*;

import java.io.IOException;
import java.nio.file.Files;
import java.util.*;
import static org.junit.jupiter.api.Assertions.*;

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())
Expand All @@ -46,14 +49,17 @@ 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
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 a change in another branch
var editHash = CheckableRepository.appendAndCommit(localRepo);
localRepo.push(editHash, author.url(), "edit");
Expand Down Expand Up @@ -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())
Expand All @@ -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
Expand Down Expand Up @@ -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"));
}
}
}

1 comment on commit 5cf9d46

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

Please sign in to comment.