Skip to content
Permalink
Browse files
merge-bot: specs can have prerequisites
Reviewed-by: rwestberg
  • Loading branch information
edvbld committed Apr 27, 2020
1 parent db5254e commit 0c2920e38e46e4dd3ec5dd8196a7d5f0b946a4c4
Show file tree
Hide file tree
Showing 3 changed files with 198 additions and 27 deletions.
@@ -191,34 +191,34 @@ int minute() {
private final Frequency frequency;
private final String name;
private final List<String> dependencies;
private final List<HostedRepository> prerequisites;

Spec(HostedRepository fromRepo, Branch fromBranch, Branch toBranch) {
this(fromRepo, fromBranch, toBranch, null, null, List.of());
this(fromRepo, fromBranch, toBranch, null, null, List.of(), List.of());
}

Spec(HostedRepository fromRepo, Branch fromBranch, Branch toBranch, String name) {
this(fromRepo, fromBranch, toBranch, null, name, List.of());
}

Spec(HostedRepository fromRepo, Branch fromBranch, Branch toBranch, String name, List<String> dependencies) {
this(fromRepo, fromBranch, toBranch, null, name, dependencies);
this(fromRepo, fromBranch, toBranch, null, name, List.of(), List.of());
}

Spec(HostedRepository fromRepo, Branch fromBranch, Branch toBranch, Frequency frequency) {
this(fromRepo, fromBranch, toBranch, frequency, null, List.of());
}

Spec(HostedRepository fromRepo, Branch fromBranch, Branch toBranch, Frequency frequency, String name) {
this(fromRepo, fromBranch, toBranch, frequency, name, List.of());
this(fromRepo, fromBranch, toBranch, frequency, null, List.of(), List.of());
}

Spec(HostedRepository fromRepo, Branch fromBranch, Branch toBranch, Frequency frequency, String name, List<String> dependencies) {
Spec(HostedRepository fromRepo,
Branch fromBranch,
Branch toBranch,
Frequency frequency,
String name,
List<String> dependencies,
List<HostedRepository> prerequisites) {
this.fromRepo = fromRepo;
this.fromBranch = fromBranch;
this.toBranch = toBranch;
this.frequency = frequency;
this.name = name;
this.dependencies = dependencies;
this.prerequisites = prerequisites;
}

HostedRepository fromRepo() {
@@ -244,6 +244,10 @@ Optional<String> name() {
List<String> dependencies() {
return dependencies;
}

List<HostedRepository> prerequisites() {
return prerequisites;
}
}

private static void deleteDirectory(Path dir) throws IOException {
@@ -303,8 +307,9 @@ public void run(Path scratchPath) {
var fromName = Path.of(fromRepo.name()).getFileName();
var fromDesc = targetName.equals(fromName) ? fromBranch.name() : fromName + ":" + fromBranch.name();

// Check if merge conflict pull request is present
var shouldMerge = true;

// Check if merge conflict pull request is present
var title = "Merge " + fromDesc;
var marker = "<!-- AUTOMATIC MERGE PR -->";
for (var pr : prs) {
@@ -354,6 +359,7 @@ public void run(Path scratchPath) {
}
}

// Check if merge should happen at this time
if (spec.frequency().isPresent()) {
var now = clock.now();
var desc = toBranch.name() + "->" + fromRepo.name() + ":" + fromBranch.name();
@@ -426,14 +432,35 @@ public void run(Path scratchPath) {
}
}

if (spec.dependencies().stream().anyMatch(unmerged::contains)) {
var failed = spec.dependencies()
.stream()
.filter(unmerged::contains)
.collect(Collectors.toList());
log.info("Will not merge because the following dependencies did not merge successfully: " +
String.join(", ", failed));
shouldMerge = false;
// Check if any prerequisite repository has a conflict pull request open
if (shouldMerge) {
for (var prereq : spec.prerequisites()) {
var openMergeConflictPRs = prereq.pullRequests()
.stream()
.filter(pr -> pr.title().startsWith("Merge "))
.filter(pr -> pr.body().startsWith(marker))
.map(PullRequest::id)
.collect(Collectors.toList());
if (!openMergeConflictPRs.isEmpty()) {
log.info("Will not merge because the prerequisite " + prereq.name() +
" has open merge conflicts PRs: " +
String.join(", ", openMergeConflictPRs));
shouldMerge = false;
}
}
}

// Check if any dependencies failed
if (shouldMerge) {
if (spec.dependencies().stream().anyMatch(unmerged::contains)) {
var failed = spec.dependencies()
.stream()
.filter(unmerged::contains)
.collect(Collectors.toList());
log.info("Will not merge because the following dependencies did not merge successfully: " +
String.join(", ", failed));
shouldMerge = false;
}
}

if (!shouldMerge) {
@@ -186,8 +186,19 @@ public List<Bot> create(BotConfiguration configuration) {
.stream()
.map(e -> e.asString())
.collect(Collectors.toList());

specs.add(new MergeBot.Spec(fromRepo, fromBranch, toBranch, frequency, name, dependencies));
var prerequisites = spec.get("prerequisites")
.stream()
.map(e -> e.asString())
.map(configuration::repository)
.collect(Collectors.toList());

specs.add(new MergeBot.Spec(fromRepo,
fromBranch,
toBranch,
frequency,
name,
dependencies,
prerequisites));
}

bots.add(new MergeBot(storage, targetRepo, forkRepo, specs));
@@ -25,6 +25,7 @@
import org.openjdk.skara.host.*;
import org.openjdk.skara.test.*;
import org.openjdk.skara.vcs.*;
import org.openjdk.skara.issuetracker.Issue;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInfo;
@@ -186,8 +187,8 @@ void successfulDependency(TestInfo testInfo) throws IOException {
var storage = temp.path().resolve("storage");
var master = new Branch("master");
var feature = new Branch("feature");
var specs = List.of(new MergeBot.Spec(fromHostedRepo, master, master, "master"),
new MergeBot.Spec(fromHostedRepo, feature, feature, "feature", List.of("master")));
var specs = List.of(new MergeBot.Spec(fromHostedRepo, master, master, null, "master", List.of(), List.of()),
new MergeBot.Spec(fromHostedRepo, feature, feature, null, "feature", List.of("master"), List.of()));
var bot = new MergeBot(storage, toHostedRepo, toFork, specs);
TestBotRunner.runPeriodicItems(bot);

@@ -283,8 +284,8 @@ void failedDependency(TestInfo testInfo) throws IOException {
var storage = temp.path().resolve("storage");
var master = new Branch("master");
var feature = new Branch("feature");
var specs = List.of(new MergeBot.Spec(fromHostedRepo, master, master, "master"),
new MergeBot.Spec(fromHostedRepo, feature, feature, "feature", List.of("master")));
var specs = List.of(new MergeBot.Spec(fromHostedRepo, master, master, null, "master", List.of(), List.of()),
new MergeBot.Spec(fromHostedRepo, feature, feature, null, "feature", List.of("master"), List.of()));
var bot = new MergeBot(storage, toHostedRepo, toFork, specs);
TestBotRunner.runPeriodicItems(bot);

@@ -369,6 +370,138 @@ void failingMergeTest(TestInfo testInfo) throws IOException {
}
}

@Test
void failingPrerequisite(TestInfo testInfo) throws IOException {
try (var temp = new TemporaryDirectory()) {
var host = TestHost.createNew(List.of(new HostUser(0, "duke", "J. Duke")));

var fromDir = temp.path().resolve("from.git");
var fromLocalRepo = Repository.init(fromDir, VCS.GIT);
var fromHostedRepo = new TestHostedRepository(host, "test", fromLocalRepo);

var toDir = temp.path().resolve("to.git");
var toLocalRepo = Repository.init(toDir, VCS.GIT);
var toGitConfig = toDir.resolve(".git").resolve("config");
Files.write(toGitConfig, List.of("[receive]", "denyCurrentBranch = ignore"),
StandardOpenOption.APPEND);
var toHostedRepo = new TestHostedRepository(host, "test-mirror", toLocalRepo);

var forkDir = temp.path().resolve("fork.git");
var forkLocalRepo = Repository.init(forkDir, VCS.GIT);
var forkGitConfig = forkDir.resolve(".git").resolve("config");
Files.write(forkGitConfig, List.of("[receive]", "denyCurrentBranch = ignore"),
StandardOpenOption.APPEND);
var toFork = new TestHostedRepository(host, "test-mirror-fork", forkLocalRepo);

var now = ZonedDateTime.now();
var fromFileA = fromDir.resolve("a.txt");
Files.writeString(fromFileA, "Hello A\n");
fromLocalRepo.add(fromFileA);
var fromHashA = fromLocalRepo.commit("Adding a.txt", "duke", "duke@openjdk.org", now);
var fromCommits = fromLocalRepo.commits().asList();
assertEquals(1, fromCommits.size());
assertEquals(fromHashA, fromCommits.get(0).hash());

var toFileA = toDir.resolve("a.txt");
Files.writeString(toFileA, "Hello A\n");
toLocalRepo.add(toFileA);
var toHashA = toLocalRepo.commit("Adding a.txt", "duke", "duke@openjdk.org", now);
var toCommits = toLocalRepo.commits().asList();
assertEquals(1, toCommits.size());
assertEquals(toHashA, toCommits.get(0).hash());
assertEquals(fromHashA, toHashA);

var fromFileB = fromDir.resolve("b.txt");
Files.writeString(fromFileB, "Hello B1\n");
fromLocalRepo.add(fromFileB);
var fromHashB = fromLocalRepo.commit("Adding b1.txt", "duke", "duke@openjdk.org");

var toFileB = toDir.resolve("b.txt");
Files.writeString(toFileB, "Hello B2\n");
toLocalRepo.add(toFileB);
var toHashB = toLocalRepo.commit("Adding b2.txt", "duke", "duke@openjdk.org");

var storage = temp.path().resolve("storage");
var master = new Branch("master");
var specs = List.of(new MergeBot.Spec(fromHostedRepo, master, master));
var bot = new MergeBot(storage, toHostedRepo, toFork, specs);
TestBotRunner.runPeriodicItems(bot);

toCommits = toLocalRepo.commits().asList();
assertEquals(2, toCommits.size());
var toHashes = toCommits.stream().map(Commit::hash).collect(Collectors.toSet());
assertTrue(toHashes.contains(toHashA));
assertTrue(toHashes.contains(toHashB));

var pullRequests = toHostedRepo.pullRequests();
assertEquals(1, pullRequests.size());
var pr = pullRequests.get(0);
assertEquals("Merge test:master", pr.title());

var fromDir2 = temp.path().resolve("from2.git");
var fromLocalRepo2 = Repository.init(fromDir2, VCS.GIT);
var fromHostedRepo2 = new TestHostedRepository(host, "test-2", fromLocalRepo2);

var host2 = TestHost.createNew(List.of(new HostUser(0, "duke", "J. Duke")));
var toDir2 = temp.path().resolve("to2.git");
var toLocalRepo2 = Repository.init(toDir2, VCS.GIT);
var toGitConfig2 = toDir2.resolve(".git").resolve("config");
Files.write(toGitConfig2, List.of("[receive]", "denyCurrentBranch = ignore"),
StandardOpenOption.APPEND);
var toHostedRepo2 = new TestHostedRepository(host2, "test-mirror-2", toLocalRepo2);

var forkDir2 = temp.path().resolve("fork2.git");
var forkLocalRepo2 = Repository.init(forkDir2, VCS.GIT);
var forkGitConfig2 = forkDir2.resolve(".git").resolve("config");
Files.write(forkGitConfig2, List.of("[receive]", "denyCurrentBranch = ignore"),
StandardOpenOption.APPEND);
var toFork2 = new TestHostedRepository(host2, "test-mirror-fork-2", forkLocalRepo2);

var now2 = ZonedDateTime.now();
var fromFileA2 = fromDir2.resolve("a2.txt");
Files.writeString(fromFileA2, "Hello A2\n");
fromLocalRepo2.add(fromFileA2);
var fromHashA2 = fromLocalRepo2.commit("Adding a2.txt", "duke", "duke@openjdk.org", now2);

var toFileA2 = toDir2.resolve("a2.txt");
Files.writeString(toFileA2, "Hello A2\n");
toLocalRepo2.add(toFileA2);
var toHashA2 = toLocalRepo2.commit("Adding a2.txt", "duke", "duke@openjdk.org", now2);
var toCommits2 = toLocalRepo2.commits().asList();
assertEquals(1, toCommits2.size());
assertEquals(toHashA2, toCommits2.get(0).hash());
assertEquals(fromHashA2, toHashA2);

var fromFileB2 = fromDir2.resolve("b2.txt");
Files.writeString(fromFileB2, "Hello B2\n");
fromLocalRepo2.add(fromFileB2);
var fromHashB2 = fromLocalRepo2.commit("Adding b2.txt", "duke", "duke@openjdk.org");
var fromCommits2 = fromLocalRepo2.commits().asList();
assertEquals(2, fromCommits2.size());
assertEquals(fromHashB2, fromCommits2.get(0).hash());
assertEquals(fromHashA2, fromCommits2.get(1).hash());

var storage2 = temp.path().resolve("storage-2");
var master2 = new Branch("master");
var specs2 = List.of(new MergeBot.Spec(fromHostedRepo2, master2, master2, null, "master", List.of(), List.of(toHostedRepo)));
var bot2 = new MergeBot(storage2, toHostedRepo2, toFork2, specs2);
TestBotRunner.runPeriodicItems(bot2);

var toCommitsAfterMerge2 = toLocalRepo2.commits().asList();
assertEquals(1, toCommitsAfterMerge2.size());
assertEquals(toHashA2, toCommitsAfterMerge2.get(0).hash());
assertEquals(toHashA2, toLocalRepo2.resolve("master").get());

pr.setState(Issue.State.CLOSED);
TestBotRunner.runPeriodicItems(bot2);
toCommitsAfterMerge2 = toLocalRepo2.commits().asList();
assertEquals(2, toCommitsAfterMerge2.size());
assertEquals(fromHashB2, toCommitsAfterMerge2.get(0).hash());
assertEquals(toHashA2, toCommitsAfterMerge2.get(1).hash());
assertEquals(fromHashB2, toLocalRepo2.resolve("master").get());
}
}

@Test
void failingMergeShouldResultInOnlyOnePR(TestInfo testInfo) throws IOException {
try (var temp = new TemporaryDirectory()) {

0 comments on commit 0c2920e

Please sign in to comment.