diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java index 3775ee14..a39ff981 100644 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java +++ b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java @@ -89,13 +89,12 @@ private boolean checkCommitAuthor(List commits) throws IOException { return ((names.size() == 1) && emails.size() == 1); } - private Optional mergeSourceRepository() { + private Optional mergeSourceRepository() { var repoMatcher = mergeSourcePattern.matcher(pr.getTitle()); if (!repoMatcher.matches()) { return Optional.empty(); } - var mergeSourceRepo = pr.repository().host().getRepository(repoMatcher.group(1)); - return Optional.of(mergeSourceRepo); + return Optional.of(repoMatcher.group(1)); } private Optional mergeSourceBranch() { @@ -133,17 +132,20 @@ private boolean checkCommitAuthor(List commits) throws IOException { var sourceRepo = mergeSourceRepository(); var sourceBranch = mergeSourceBranch(); if (sourceBranch.isPresent() && sourceRepo.isPresent()) { - Hash sourceHash = null; try { - sourceHash = prInstance.localRepo().fetch(sourceRepo.get().getUrl(), sourceBranch.get()); - } catch (IOException e) { - ret.add("Could not fetch branch `" + sourceBranch.get() + "` from project `" + - sourceRepo.get().getName() + "` - check that they are correct."); - } - if (sourceHash != null) { - if (!prInstance.localRepo().isAncestor(commits.get(1).hash(), sourceHash)) { - ret.add("The merge contains commits that are not ancestors of the source"); + var mergeSourceRepo = pr.repository().host().getRepository(sourceRepo.get()); + try { + var sourceHash = prInstance.localRepo().fetch(mergeSourceRepo.getUrl(), sourceBranch.get()); + if (!prInstance.localRepo().isAncestor(commits.get(1).hash(), sourceHash)) { + ret.add("The merge contains commits that are not ancestors of the source"); + } + } catch (IOException e) { + ret.add("Could not fetch branch `" + sourceBranch.get() + "` from project `" + + sourceRepo.get() + "` - check that they are correct."); } + } catch (RuntimeException e) { + ret.add("Could not find project `" + + sourceRepo.get() + "` - check that it is correct."); } } else { ret.add("Could not determine the source for this merge. A Merge PR title must be specified on the format: " + diff --git a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/MergeTests.java b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/MergeTests.java index c4126c19..c61ed08d 100644 --- a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/MergeTests.java +++ b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/MergeTests.java @@ -75,7 +75,7 @@ void branchMerge(TestInfo testInfo) throws IOException { localRepo.merge(otherHash2); var mergeHash = localRepo.commit("Merge commit", "some", "some@one"); localRepo.push(mergeHash, author.getUrl(), "edit", true); - var pr = credentials.createPullRequest(author, "master", "edit", "Merge " + credentials.getHostedRepository().getName() + ":other"); + var pr = credentials.createPullRequest(author, "master", "edit", "Merge " + author.getName() + ":other"); // Approve it as another user var approvalPr = integrator.getPullRequest(pr.getId()); @@ -154,7 +154,7 @@ void branchMergeRebase(TestInfo testInfo) throws IOException { localRepo.merge(otherHash2); var mergeHash = localRepo.commit("Merge commit", "some", "some@one"); localRepo.push(mergeHash, author.getUrl(), "edit", true); - var pr = credentials.createPullRequest(author, "master", "edit", "Merge " + credentials.getHostedRepository().getName() + ":other"); + var pr = credentials.createPullRequest(author, "master", "edit", "Merge " + author.getName() + ":other"); // Approve it as another user var approvalPr = integrator.getPullRequest(pr.getId()); @@ -240,7 +240,7 @@ void mergedCommitsFailingJCheck(TestInfo testInfo) throws IOException { localRepo.merge(otherHash); var mergeHash = localRepo.commit("Merge commit", "some", "some@one"); localRepo.push(mergeHash, author.getUrl(), "edit", true); - var pr = credentials.createPullRequest(author, "master", "edit", "Merge " + credentials.getHostedRepository().getName() + ":other"); + var pr = credentials.createPullRequest(author, "master", "edit", "Merge " + author.getName() + ":other"); // Approve it as another user var approvalPr = integrator.getPullRequest(pr.getId()); @@ -295,7 +295,7 @@ void invalidSourceRepo(TestInfo testInfo) throws IOException { localRepo.merge(otherHash); var mergeHash = localRepo.commit("Merge commit", "some", "some@one"); localRepo.push(mergeHash, author.getUrl(), "edit", true); - var pr = credentials.createPullRequest(author, "master", "edit", "Merge " + credentials.getHostedRepository().getName() + "xyz" + ":other"); + var pr = credentials.createPullRequest(author, "master", "edit", "Merge " + author.getName() + "xyz" + ":other"); // Approve it as another user var approvalPr = integrator.getPullRequest(pr.getId()); @@ -315,7 +315,7 @@ void invalidSourceRepo(TestInfo testInfo) throws IOException { assertEquals(1, error, () -> pr.getComments().stream().map(Comment::body).collect(Collectors.joining("\n\n"))); var check = pr.getChecks(mergeHash).get("jcheck"); - assertEquals("- Could not fetch branch `other` from project `" + credentials.getHostedRepository().getName() + "xyz` - check that they are correct.", check.summary().orElseThrow()); + assertEquals("- Could not find project `" + author.getName() + "xyz` - check that it is correct.", check.summary().orElseThrow()); } } @@ -353,7 +353,7 @@ void invalidSourceBranch(TestInfo testInfo) throws IOException { localRepo.merge(otherHash); var mergeHash = localRepo.commit("Merge commit", "some", "some@one"); localRepo.push(mergeHash, author.getUrl(), "edit", true); - var pr = credentials.createPullRequest(author, "master", "edit", "Merge " + credentials.getHostedRepository().getName() + ":otherxyz"); + var pr = credentials.createPullRequest(author, "master", "edit", "Merge " + author.getName() + ":otherxyz"); // Approve it as another user var approvalPr = integrator.getPullRequest(pr.getId()); @@ -373,7 +373,7 @@ void invalidSourceBranch(TestInfo testInfo) throws IOException { assertEquals(1, error, () -> pr.getComments().stream().map(Comment::body).collect(Collectors.joining("\n\n"))); var check = pr.getChecks(mergeHash).get("jcheck"); - assertEquals("- Could not fetch branch `otherxyz` from project `" + credentials.getHostedRepository().getName() + "` - check that they are correct.", check.summary().orElseThrow()); + assertEquals("- Could not fetch branch `otherxyz` from project `" + author.getName() + "` - check that they are correct.", check.summary().orElseThrow()); } } @@ -416,7 +416,7 @@ void wrongSourceBranch(TestInfo testInfo) throws IOException { localRepo.merge(other1Hash, "ours"); var mergeHash = localRepo.commit("Merge commit", "some", "some@one"); localRepo.push(mergeHash, author.getUrl(), "edit", true); - var pr = credentials.createPullRequest(author, "master", "edit", "Merge " + credentials.getHostedRepository().getName() + ":other2"); + var pr = credentials.createPullRequest(author, "master", "edit", "Merge " + author.getName() + ":other2"); // Approve it as another user var approvalPr = integrator.getPullRequest(pr.getId()); @@ -474,7 +474,7 @@ void invalidAuthor(TestInfo testInfo) throws IOException { localRepo.merge(otherHash); var mergeHash = localRepo.commit("Merge commit", "some", "some@one"); localRepo.push(mergeHash, author.getUrl(), "edit", true); - var pr = credentials.createPullRequest(author, "master", "edit", "Merge " + credentials.getHostedRepository().getName() + ":other"); + var pr = credentials.createPullRequest(author, "master", "edit", "Merge " + author.getName() + ":other"); // Approve it as another user var approvalPr = integrator.getPullRequest(pr.getId()); @@ -546,7 +546,7 @@ void unrelatedHistory(TestInfo testInfo) throws IOException { //localRepo.merge(otherHash); var mergeHash = localRepo.commit("Merge commit", "some", "some@one"); localRepo.push(mergeHash, author.getUrl(), "edit", true); - var pr = credentials.createPullRequest(author, "master", "edit", "Merge " + credentials.getHostedRepository().getName() + ":other"); + var pr = credentials.createPullRequest(author, "master", "edit", "Merge " + author.getName() + ":other"); // Approve it as another user var approvalPr = integrator.getPullRequest(pr.getId()); diff --git a/build.gradle b/build.gradle index a1591839..bc877ca6 100644 --- a/build.gradle +++ b/build.gradle @@ -53,7 +53,6 @@ configure(subprojects.findAll() { it.name != 'bots' }) { if (findProperty('credentials')) { systemProperty "credentials", findProperty('credentials') - System.getProperties().findAll { it.key.toString().toLowerCase().contains('proxy') }.each { systemProperty it.key, it.value } } testLogging { diff --git a/host/src/main/java/org/openjdk/skara/host/gitlab/GitLabMergeRequest.java b/host/src/main/java/org/openjdk/skara/host/gitlab/GitLabMergeRequest.java index 59dc2e4f..7524b134 100644 --- a/host/src/main/java/org/openjdk/skara/host/gitlab/GitLabMergeRequest.java +++ b/host/src/main/java/org/openjdk/skara/host/gitlab/GitLabMergeRequest.java @@ -335,6 +335,9 @@ private String encodeMarkdown(String message) { return message.replaceAll("\n", " \n"); } + private final Pattern checkBodyPattern = Pattern.compile("^##### ([^\\n\\r]*)\\R(.*)", + Pattern.DOTALL | Pattern.MULTILINE); + @Override public Map getChecks(Hash hash) { var pattern = Pattern.compile(String.format(checkResultPattern, hash.hex())); @@ -354,7 +357,11 @@ private String encodeMarkdown(String message) { if (!entry.getValue().group(3).equals("NONE")) { checkBuilder.metadata(new String(Base64.getDecoder().decode(entry.getValue().group(3)), StandardCharsets.UTF_8)); } - checkBuilder.summary(entry.getKey().body()); + var checkBodyMatcher = checkBodyPattern.matcher(entry.getKey().body()); + if (checkBodyMatcher.find()) { + checkBuilder.title(checkBodyMatcher.group(1)); + checkBuilder.summary(checkBodyMatcher.group(2)); + } return checkBuilder.build(); })); } diff --git a/test/build.gradle b/test/build.gradle index f0420948..49274608 100644 --- a/test/build.gradle +++ b/test/build.gradle @@ -33,6 +33,7 @@ dependencies { implementation project(':host') implementation project(':email') implementation project(':mailinglist') + implementation project(':proxy') implementation 'org.junit.jupiter:junit-jupiter-api:5.3.1' implementation 'org.junit.jupiter:junit-jupiter-params:5.3.1' diff --git a/test/src/main/java/module-info.java b/test/src/main/java/module-info.java index 6033032c..921795c4 100644 --- a/test/src/main/java/module-info.java +++ b/test/src/main/java/module-info.java @@ -31,6 +31,7 @@ requires org.openjdk.skara.host; requires org.openjdk.skara.email; requires org.openjdk.skara.mailinglist; + requires org.openjdk.skara.proxy; requires org.junit.jupiter.api; diff --git a/test/src/main/java/org/openjdk/skara/test/HostCredentials.java b/test/src/main/java/org/openjdk/skara/test/HostCredentials.java index b9745edb..046d530f 100644 --- a/test/src/main/java/org/openjdk/skara/test/HostCredentials.java +++ b/test/src/main/java/org/openjdk/skara/test/HostCredentials.java @@ -25,6 +25,7 @@ import org.openjdk.skara.host.*; import org.openjdk.skara.host.network.URIBuilder; import org.openjdk.skara.json.*; +import org.openjdk.skara.proxy.HttpProxy; import org.openjdk.skara.vcs.*; import org.junit.jupiter.api.TestInfo; @@ -170,6 +171,8 @@ private Host getHost() { } public HostCredentials(TestInfo testInfo) throws IOException { + HttpProxy.setup(); + var credentialsFile = System.getProperty("credentials"); testName = testInfo.getDisplayName(); diff --git a/test/src/main/java/org/openjdk/skara/test/TestHost.java b/test/src/main/java/org/openjdk/skara/test/TestHost.java index 8103eb60..de559c27 100644 --- a/test/src/main/java/org/openjdk/skara/test/TestHost.java +++ b/test/src/main/java/org/openjdk/skara/test/TestHost.java @@ -86,6 +86,9 @@ public HostedRepository getRepository(String name) { if (data.repositories.containsKey(name)) { localRepository = data.repositories.get(name); } else { + if (data.repositories.size() > 0) { + throw new RuntimeException("A test host can only manage a single repository"); + } localRepository = createLocalRepository(); data.repositories.put(name, localRepository); }