Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
merge-bot: allow specs to define dependencies
Reviewed-by: rwestberg
  • Loading branch information
edvbld committed Apr 24, 2020
1 parent 8d9b6e9 commit 2dca7b7
Show file tree
Hide file tree
Showing 3 changed files with 240 additions and 7 deletions.
Expand Up @@ -189,16 +189,36 @@ int minute() {
private final Branch fromBranch;
private final Branch toBranch;
private final Frequency frequency;
private final String name;
private final List<String> dependencies;

Spec(HostedRepository fromRepo, Branch fromBranch, Branch toBranch) {
this(fromRepo, fromBranch, toBranch, null);
this(fromRepo, fromBranch, toBranch, null, null, 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);
}

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());
}

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

HostedRepository fromRepo() {
Expand All @@ -216,6 +236,14 @@ Branch toBranch() {
Optional<Frequency> frequency() {
return Optional.ofNullable(frequency);
}

Optional<String> name() {
return Optional.ofNullable(name);
}

List<String> dependencies() {
return dependencies;
}
}

private static void deleteDirectory(Path dir) throws IOException {
Expand Down Expand Up @@ -265,6 +293,7 @@ public void run(Path scratchPath) {
var prs = prTarget.pullRequests();
var currentUser = prTarget.forge().currentUser();

var unmerged = new HashSet<String>();
for (var spec : specs) {
var toBranch = spec.toBranch();
var fromRepo = spec.fromRepo();
Expand Down Expand Up @@ -397,8 +426,21 @@ 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;
}

if (!shouldMerge) {
log.info("Will not merge " + fromRepo.name() + ":" + fromBranch.name() + " to " + toBranch.name());
if (spec.name().isPresent()) {
unmerged.add(spec.name().get());
}
continue;
}

Expand Down Expand Up @@ -447,6 +489,9 @@ public void run(Path scratchPath) {
repo = cloneAndSyncFork(dir);
}
} else {
if (spec.name().isPresent()) {
unmerged.add(spec.name().get());
}
log.info("Got error: " + error.getMessage());
log.info("Aborting unsuccesful merge");
var status = repo.status();
Expand Down Expand Up @@ -477,16 +522,16 @@ public void run(Path scratchPath) {
"that can **not** be merged into the branch `" + toBranch.name() + "`:");

message.add("");
var unmerged = status.stream().filter(entry -> entry.status().isUnmerged()).collect(Collectors.toList());
if (unmerged.size() <= 10) {
var files = unmerged.size() > 1 ? "files" : "file";
var unmergedFiles = status.stream().filter(entry -> entry.status().isUnmerged()).collect(Collectors.toList());
if (unmergedFiles.size() <= 10) {
var files = unmergedFiles.size() > 1 ? "files" : "file";
message.add("The following " + files + " contains merge conflicts:");
message.add("");
for (var fileStatus : unmerged) {
for (var fileStatus : unmergedFiles) {
message.add("- " + fileStatus.source().path().orElseThrow());
}
} else {
message.add("Over " + unmerged.size() + " files contains merge conflicts.");
message.add("Over " + unmergedFiles.size() + " files contains merge conflicts.");
}
message.add("");

Expand Down
Expand Up @@ -30,6 +30,7 @@
import java.time.DayOfWeek;
import java.time.Month;
import java.util.*;
import java.util.stream.Collectors;
import java.util.logging.Logger;

public class MergeBotFactory implements BotFactory {
Expand Down Expand Up @@ -180,7 +181,13 @@ public List<Bot> create(BotConfiguration configuration) {
}
}

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

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

bots.add(new MergeBot(storage, targetRepo, forkRepo, specs));
Expand Down
Expand Up @@ -117,6 +117,187 @@ void mergeMasterBranch(TestInfo testInfo) throws IOException {
}
}

@Test
void successfulDependency(TestInfo testInfo) throws IOException {
try (var temp = new TemporaryDirectory(false)) {
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);
toLocalRepo.branch(toHashA, "feature");
assertEquals(2, toLocalRepo.branches().size());

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

var featureBranch = fromLocalRepo.branch(fromHashB, "feature");
fromLocalRepo.checkout(featureBranch);
var fromFileD = fromDir.resolve("d.txt");
Files.writeString(fromFileD, "Hello D\n");
fromLocalRepo.add(fromFileD);
var fromHashD = fromLocalRepo.commit("Adding d.txt", "duke", "duke@openjdk.org");

var toFileC = toDir.resolve("c.txt");
Files.writeString(toFileC, "Hello C\n");
toLocalRepo.add(toFileC);
var toHashC = toLocalRepo.commit("Adding c.txt", "duke", "duke@openjdk.org");

toLocalRepo.checkout(featureBranch);
var toFileE = toDir.resolve("e.txt");
Files.writeString(toFileE, "Hello E\n");
toLocalRepo.add(toFileE);
var toHashE = toLocalRepo.commit("Adding e.txt", "duke", "duke@openjdk.org");

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 bot = new MergeBot(storage, toHostedRepo, toFork, specs);
TestBotRunner.runPeriodicItems(bot);

toCommits = toLocalRepo.commits().asList();
assertEquals(7, toCommits.size());
var hashes = toCommits.stream().map(Commit::hash).collect(Collectors.toSet());
assertTrue(hashes.contains(toHashA));
assertTrue(hashes.contains(fromHashB));
assertTrue(hashes.contains(toHashC));

var merges = toCommits.stream().filter(c -> c.isMerge()).collect(Collectors.toList());
assertEquals(2, merges.size());

assertTrue(merges.stream().anyMatch(c -> c.message().get(0).equals("Automatic merge of test:master into master")));
assertTrue(merges.stream().anyMatch(c -> c.message().get(0).equals("Automatic merge of test:feature into feature")));
}
}

@Test
void failedDependency(TestInfo testInfo) throws IOException {
try (var temp = new TemporaryDirectory(false)) {
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);
toLocalRepo.branch(toHashA, "feature");
assertEquals(2, toLocalRepo.branches().size());

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

var featureBranch = fromLocalRepo.branch(fromHashB, "feature");
fromLocalRepo.checkout(featureBranch);
var fromFileD = fromDir.resolve("d.txt");
Files.writeString(fromFileD, "Hello D\n");
fromLocalRepo.add(fromFileD);
var fromHashD = fromLocalRepo.commit("Adding d.txt", "duke", "duke@openjdk.org");

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

toLocalRepo.checkout(featureBranch);
var toFileE = toDir.resolve("e.txt");
Files.writeString(toFileE, "Hello E\n");
toLocalRepo.add(toFileE);
var toHashE = toLocalRepo.commit("Adding e.txt", "duke", "duke@openjdk.org");

var toCommitsBeforeMerge = toLocalRepo.commits().asList();
assertEquals(3, toCommitsBeforeMerge.size());
assertEquals(toHashE, toCommitsBeforeMerge.get(0).hash());
assertEquals(toHashB, toCommitsBeforeMerge.get(1).hash());
assertEquals(toHashA, toCommitsBeforeMerge.get(2).hash());
assertEquals(toHashB, toLocalRepo.resolve("master").get());
assertEquals(toHashE, toLocalRepo.resolve("feature").get());

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 bot = new MergeBot(storage, toHostedRepo, toFork, specs);
TestBotRunner.runPeriodicItems(bot);

toCommits = toLocalRepo.commits().asList();
assertEquals(toCommitsBeforeMerge.size(), toCommits.size());
assertEquals(toCommitsBeforeMerge.get(0).hash(), toCommits.get(0).hash());
assertEquals(toCommitsBeforeMerge.get(1).hash(), toCommits.get(1).hash());
assertEquals(toCommitsBeforeMerge.get(2).hash(), toCommits.get(2).hash());
assertEquals(toHashB, toLocalRepo.resolve("master").get());
assertEquals(toHashE, toLocalRepo.resolve("feature").get());
}
}

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

0 comments on commit 2dca7b7

Please sign in to comment.