Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merge-bot: specs can have prerequisites #604

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -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()) {