Skip to content

Commit 3ff9d93

Browse files
jpongejponge
committed
Post comment advising squash merge if merge commits are found
This still reports a failing check when a PR branch contains merge commits, but it reminds maintainers about the possibility to use a squashed commit merge if the repository and branch protection rules allow it.
1 parent 33fdff9 commit 3ff9d93

3 files changed

Lines changed: 137 additions & 0 deletions

File tree

‎src/main/java/io/quarkus/bot/CheckPullRequestContributionRules.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import java.util.ArrayList;
66
import java.util.Date;
77
import java.util.List;
8+
import java.util.Optional;
89

910
import jakarta.inject.Inject;
1011

@@ -17,6 +18,7 @@
1718
import org.kohsuke.github.GHCheckRunBuilder.Output;
1819
import org.kohsuke.github.GHCommit;
1920
import org.kohsuke.github.GHEventPayload;
21+
import org.kohsuke.github.GHIssueComment;
2022
import org.kohsuke.github.GHPullRequest;
2123
import org.kohsuke.github.GHPullRequestCommitDetail;
2224
import org.kohsuke.github.GHRepository;
@@ -26,6 +28,7 @@
2628
import io.quarkus.bot.config.Feature;
2729
import io.quarkus.bot.config.QuarkusGitHubBotConfig;
2830
import io.quarkus.bot.config.QuarkusGitHubBotConfigFile;
31+
import io.quarkus.bot.util.Strings;
2932

3033
public class CheckPullRequestContributionRules {
3134

@@ -36,6 +39,12 @@ public class CheckPullRequestContributionRules {
3639
public static final String MERGE_COMMIT_CHECK_RUN_NAME = "Check Pull Request - Merge commits";
3740
public static final String MERGE_COMMIT_ERROR_OUTPUT_TITLE = "PR contains merge commits";
3841
public static final String MERGE_COMMIT_ERROR_OUTPUT_SUMMARY = "Pull request that contains merge commits can not be merged";
42+
public static final String MERGE_COMMIT_COMMENT_MESSAGE = """
43+
\u26a0\ufe0f **This pull request contains merge commits.**
44+
45+
This is acceptable provided that it is **squash merged**.
46+
47+
Maintainers: please use the **Squash and merge** option when merging this pull request.""";
3948

4049
public static final String FIXUP_COMMIT_CHECK_RUN_NAME = "Check Pull Request - Fixup commits";
4150
public static final String FIXUP_COMMIT_ERROR_OUTPUT_TITLE = "PR contains fixup commits";
@@ -47,6 +56,9 @@ public class CheckPullRequestContributionRules {
4756
@Inject
4857
QuarkusGitHubBotConfig quarkusBotConfig;
4958

59+
@Inject
60+
GHIssueCommentService commentService;
61+
5062
void checkPullRequestContributionRules(
5163
@PullRequest.Opened @PullRequest.Reopened @PullRequest.Synchronize GHEventPayload.PullRequest pullRequestPayload,
5264
@ConfigFile("quarkus-github-bot.yml") QuarkusGitHubBotConfigFile quarkusBotConfigFile) throws IOException {
@@ -67,6 +79,9 @@ void checkPullRequestContributionRules(
6779
buildCheckRun(checkCommitData.getMergeCommitDetails(), repostitory, headCommit,
6880
MERGE_COMMIT_CHECK_RUN_NAME, MERGE_COMMIT_ERROR_OUTPUT_TITLE, MERGE_COMMIT_ERROR_OUTPUT_SUMMARY);
6981

82+
// Post comment advising squash merge if merge commits are found
83+
handleMergeCommitComment(pullRequest, checkCommitData.getMergeCommitDetails());
84+
7085
// Fixup commits
7186
buildCheckRun(checkCommitData.getFixupCommitDetails(), repostitory, headCommit,
7287
FIXUP_COMMIT_CHECK_RUN_NAME, FIXUP_COMMIT_ERROR_OUTPUT_TITLE, FIXUP_COMMIT_ERROR_OUTPUT_SUMMARY);
@@ -164,6 +179,23 @@ public static void buildCheckRun(List<GHPullRequestCommitDetail> commitDetails,
164179
}
165180
}
166181

182+
private void handleMergeCommitComment(GHPullRequest pullRequest,
183+
List<GHPullRequestCommitDetail> mergeCommitDetails) throws IOException {
184+
Optional<GHIssueComment> existingComment = commentService.findBotCommentInIssue(
185+
pullRequest, Strings.MERGE_COMMIT_COMMENT_MARKER);
186+
187+
if (!mergeCommitDetails.isEmpty()) {
188+
if (existingComment.isEmpty()) {
189+
String formattedComment = Strings.commentByBot(MERGE_COMMIT_COMMENT_MESSAGE)
190+
+ Strings.MERGE_COMMIT_COMMENT_MARKER;
191+
pullRequest.comment(formattedComment);
192+
}
193+
} else {
194+
existingComment.ifPresent(comment -> commentService.deleteComment(
195+
comment, pullRequest.getNumber(), false));
196+
}
197+
}
198+
167199
private static String buildDryRunLogMessage(List<GHPullRequestCommitDetail> commitDetails, String checkRunName,
168200
String errorOutputSummary) {
169201
StringBuilder comment = new StringBuilder();

‎src/main/java/io/quarkus/bot/util/Strings.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
public class Strings {
44
public static final String EDITORIAL_RULES_COMMENT_MARKER = "\n<!-- Quarkus-Bot-Editor-Rules -->";
5+
public static final String MERGE_COMMIT_COMMENT_MARKER = "\n<!-- Quarkus-Bot-Merge-Commits -->";
56

67
public static boolean isNotBlank(String string) {
78
return string != null && !string.trim().isEmpty();

‎src/test/java/io/quarkus/bot/it/CheckPullRequestContributionRulesTest.java‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static io.quarkiverse.githubapp.testing.GitHubAppTesting.given;
44
import static org.mockito.ArgumentMatchers.any;
5+
import static org.mockito.ArgumentMatchers.contains;
56
import static org.mockito.ArgumentMatchers.eq;
67
import static org.mockito.Mockito.mock;
78
import static org.mockito.Mockito.times;
@@ -10,6 +11,7 @@
1011
import static org.mockito.Mockito.when;
1112

1213
import java.io.IOException;
14+
import java.util.List;
1315

1416
import org.junit.jupiter.api.Test;
1517
import org.junit.jupiter.api.extension.ExtendWith;
@@ -19,6 +21,7 @@
1921
import org.kohsuke.github.GHCommit;
2022
import org.kohsuke.github.GHCommitPointer;
2123
import org.kohsuke.github.GHEvent;
24+
import org.kohsuke.github.GHIssueComment;
2225
import org.kohsuke.github.GHPullRequest;
2326
import org.kohsuke.github.GHPullRequestCommitDetail;
2427
import org.kohsuke.github.GHRepository;
@@ -80,6 +83,7 @@ void pullRequestHasTwoCheckRuns() throws IOException {
8083

8184
mockPR = mocks.pullRequest(samplePullRequestId);
8285
when(mockPR.getRepository()).thenReturn(mockRepo);
86+
when(mockPR.getComments()).thenReturn(List.of());
8387

8488
setupMockHeadCommit();
8589

@@ -101,6 +105,8 @@ void pullRequestHasTwoCheckRuns() throws IOException {
101105
verify(mockCheckRunBuilder, times(2)).withConclusion(eq(GHCheckRun.Conclusion.SUCCESS));
102106
verify(mockCheckRunBuilder, times(2)).create();
103107

108+
verify(mockPR, times(1)).getComments();
109+
104110
verifyNoMoreInteractions(mocks.ghObjects());
105111
});
106112
}
@@ -119,6 +125,7 @@ void pullRequestHasOneCheckRunSucessAndOneCheckRunFailIfMergeCommit() throws IOE
119125

120126
mockPR = mocks.pullRequest(samplePullRequestId);
121127
when(mockPR.getRepository()).thenReturn(mockRepo);
128+
when(mockPR.getComments()).thenReturn(List.of());
122129

123130
setupMockHeadCommit();
124131

@@ -143,10 +150,41 @@ void pullRequestHasOneCheckRunSucessAndOneCheckRunFailIfMergeCommit() throws IOE
143150
verify(mockCheckRunBuilder, times(1)).add(any(Output.class));
144151
verify(mockCheckRunBuilder, times(2)).create();
145152

153+
verify(mockPR, times(1)).getComments();
154+
verify(mockPR, times(1)).comment(contains("squash merged"));
155+
146156
verifyNoMoreInteractions(mocks.ghObjects());
147157
});
148158
}
149159

160+
/**
161+
* If PR contains merge commits and no existing bot comment,
162+
* a comment advising squash merge is posted.
163+
*/
164+
@Test
165+
void pullRequestWithMergeCommitPostsSquashMergeComment() throws IOException {
166+
167+
setupMock();
168+
169+
given().github(mocks -> {
170+
mocks.configFile("quarkus-github-bot.yml").fromString("features: [ CHECK_CONTRIBUTION_RULES ]\n");
171+
172+
mockPR = mocks.pullRequest(samplePullRequestId);
173+
when(mockPR.getRepository()).thenReturn(mockRepo);
174+
when(mockPR.getComments()).thenReturn(List.of());
175+
176+
setupMockHeadCommit();
177+
178+
GHPullRequestCommitDetail mockMergeCommitDetail = setupMockMergeCommit();
179+
PagedIterable<GHPullRequestCommitDetail> iterableMock = MockHelper.mockPagedIterable(mockMergeCommitDetail);
180+
when(mockPR.listCommits()).thenReturn(iterableMock);
181+
})
182+
.when().payloadFromString(getSamplePullRequestPayload())
183+
.event(GHEvent.PULL_REQUEST)
184+
.then().github(mocks -> verify(mockPR, times(1))
185+
.comment(contains("squash merged")));
186+
}
187+
150188
/**
151189
* If PR contain 1 fixup commit and not any merge commit,
152190
* then 2 checks run are created in PR: 1 in FAILURE and 1 in SUCESS
@@ -161,6 +199,7 @@ void pullRequestHasOneCheckRunSucessAndOneCheckRunFailIfFixupCommit() throws IOE
161199

162200
mockPR = mocks.pullRequest(samplePullRequestId);
163201
when(mockPR.getRepository()).thenReturn(mockRepo);
202+
when(mockPR.getComments()).thenReturn(List.of());
164203

165204
setupMockHeadCommit();
166205

@@ -185,10 +224,75 @@ void pullRequestHasOneCheckRunSucessAndOneCheckRunFailIfFixupCommit() throws IOE
185224
verify(mockCheckRunBuilder, times(1)).add(any(Output.class));
186225
verify(mockCheckRunBuilder, times(2)).create();
187226

227+
verify(mockPR, times(1)).getComments();
228+
188229
verifyNoMoreInteractions(mocks.ghObjects());
189230
});
190231
}
191232

233+
/**
234+
* If PR no longer contains merge commits but a bot comment exists,
235+
* the comment is deleted.
236+
*/
237+
@Test
238+
void pullRequestWithoutMergeCommitDeletesExistingComment() throws IOException {
239+
240+
setupMock();
241+
242+
GHIssueComment mockComment = mock(GHIssueComment.class);
243+
when(mockComment.getBody()).thenReturn(
244+
"old comment" + io.quarkus.bot.util.Strings.MERGE_COMMIT_COMMENT_MARKER);
245+
246+
given().github(mocks -> {
247+
mocks.configFile("quarkus-github-bot.yml").fromString("features: [ CHECK_CONTRIBUTION_RULES ]\n");
248+
249+
mockPR = mocks.pullRequest(samplePullRequestId);
250+
when(mockPR.getRepository()).thenReturn(mockRepo);
251+
when(mockPR.getComments()).thenReturn(List.of(mockComment));
252+
253+
setupMockHeadCommit();
254+
255+
PagedIterable<GHPullRequestCommitDetail> iterableMock = MockHelper.mockPagedIterable();
256+
when(mockPR.listCommits()).thenReturn(iterableMock);
257+
})
258+
.when().payloadFromString(getSamplePullRequestPayload())
259+
.event(GHEvent.PULL_REQUEST)
260+
.then().github(mocks -> verify(mockComment, times(1))
261+
.delete());
262+
}
263+
264+
/**
265+
* If PR contains merge commits and a bot comment already exists,
266+
* no new comment is posted (idempotent).
267+
*/
268+
@Test
269+
void pullRequestWithMergeCommitDoesNotDuplicateComment() throws IOException {
270+
271+
setupMock();
272+
273+
GHIssueComment mockComment = mock(GHIssueComment.class);
274+
when(mockComment.getBody()).thenReturn(
275+
"existing comment" + io.quarkus.bot.util.Strings.MERGE_COMMIT_COMMENT_MARKER);
276+
277+
given().github(mocks -> {
278+
mocks.configFile("quarkus-github-bot.yml").fromString("features: [ CHECK_CONTRIBUTION_RULES ]\n");
279+
280+
mockPR = mocks.pullRequest(samplePullRequestId);
281+
when(mockPR.getRepository()).thenReturn(mockRepo);
282+
when(mockPR.getComments()).thenReturn(List.of(mockComment));
283+
284+
setupMockHeadCommit();
285+
286+
GHPullRequestCommitDetail mockMergeCommitDetail = setupMockMergeCommit();
287+
PagedIterable<GHPullRequestCommitDetail> iterableMock = MockHelper.mockPagedIterable(mockMergeCommitDetail);
288+
when(mockPR.listCommits()).thenReturn(iterableMock);
289+
})
290+
.when().payloadFromString(getSamplePullRequestPayload())
291+
.event(GHEvent.PULL_REQUEST)
292+
.then().github(mocks -> verify(mockPR, times(0))
293+
.comment(any(String.class)));
294+
}
295+
192296
private static long samplePullRequestId = 1091703530;
193297

194298
private static String sampleHeadCommitSha = "7277231f08d6641edbdc07ee327dca1cc11e754d";

0 commit comments

Comments
 (0)