Skip to content

Commit dedb4cf

Browse files
yrodiereyrodiere
committed
Forbid fixup/squash commits in pull requests
Add a second rule to the existing merge commits check that detects commits starting with "fixup! " or "squash! ", which should be squashed via interactive rebase before merging. Closes #9 Assisted-By: Claude Code <noreply@anthropic.com>
1 parent 4276150 commit dedb4cf

2 files changed

Lines changed: 212 additions & 3 deletions

File tree

‎src/main/java/org/hibernate/infra/bot/CheckPullRequestContributionRules.java‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -204,19 +204,25 @@ static class MergeCommitsCheck extends PullRequestCheck {
204204
@Override
205205
public void perform(PullRequestCheckRunContext context, PullRequestCheckRunOutput output) throws IOException {
206206
List<String> mergeCommitShas = new ArrayList<>();
207+
List<String> fixupSquashCommitShas = new ArrayList<>();
207208
for ( GHPullRequestCommitDetail commitDetail : context.pullRequest.listCommits() ) {
208209
if ( commitDetail.getParents().length > 1 ) {
209210
mergeCommitShas.add( commitDetail.getSha() );
210211
}
212+
String message = commitDetail.getCommit().getMessage();
213+
if ( message.startsWith( "fixup! " ) || message.startsWith( "squash! " ) ) {
214+
fixupSquashCommitShas.add( commitDetail.getSha() );
215+
}
211216
}
212217

213-
PullRequestCheckRunRule rule = output.rule( "The pull request should not contain merge commits" );
218+
PullRequestCheckRunRule mergeCommitRule = output.rule(
219+
"The pull request should not contain merge commits" );
214220
if ( mergeCommitShas.isEmpty() ) {
215-
rule.passed();
221+
mergeCommitRule.passed();
216222
}
217223
else {
218224
String targetBranch = context.pullRequest.getBase().getRef();
219-
rule.failed(
225+
mergeCommitRule.failed(
220226
"Offending commits: "
221227
+ String.join( ", ", mergeCommitShas.stream().map( sha -> "`" + sha + "`" ).toList() )
222228
+ ".\n\nPlease [rebase](https://docs.github.com/en/get-started/using-git/about-git-rebase)"
@@ -225,6 +231,23 @@ public void perform(PullRequestCheckRunContext context, PullRequestCheckRunOutpu
225231
+ " rather than merging the target branch into your pull request branch."
226232
);
227233
}
234+
235+
PullRequestCheckRunRule fixupSquashRule = output.rule(
236+
"The pull request should not contain fixup! or squash! commits" );
237+
if ( fixupSquashCommitShas.isEmpty() ) {
238+
fixupSquashRule.passed();
239+
}
240+
else {
241+
fixupSquashRule.failed(
242+
"Offending commits: "
243+
+ String.join( ", ",
244+
fixupSquashCommitShas.stream().map( sha -> "`" + sha + "`" ).toList() )
245+
+ ".\n\nPlease squash your fixup/squash commits using"
246+
+ " [interactive rebase](https://git-scm.com/docs/git-rebase#_interactive_mode)"
247+
+ " and [force-push](https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/changing-a-commit-message#amending-older-or-multiple-commit-messages)"
248+
+ " before merging."
249+
);
250+
}
228251
}
229252
}
230253

‎src/test/java/org/hibernate/infra/bot/tests/CheckPullRequestContributionRulesFixupSquashTest.java‎

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
package org.hibernate.infra.bot.tests;
2+
3+
import static io.quarkiverse.githubapp.testing.GitHubAppTesting.given;
4+
import static org.assertj.core.api.Assertions.assertThat;
5+
import static org.mockito.Mockito.verify;
6+
import static org.mockito.Mockito.verifyNoMoreInteractions;
7+
import static org.mockito.Mockito.when;
8+
9+
import java.io.IOException;
10+
11+
import org.junit.jupiter.api.Test;
12+
import org.junit.jupiter.api.extension.ExtendWith;
13+
14+
import io.quarkiverse.githubapp.testing.GitHubAppTest;
15+
import io.quarkus.test.junit.QuarkusTest;
16+
import org.assertj.core.api.InstanceOfAssertFactories;
17+
import org.kohsuke.github.GHCheckRun;
18+
import org.kohsuke.github.GHCheckRunBuilder;
19+
import org.kohsuke.github.GHEvent;
20+
import org.kohsuke.github.GHPullRequest;
21+
import org.kohsuke.github.GHRepository;
22+
import org.mockito.ArgumentCaptor;
23+
import org.mockito.junit.jupiter.MockitoExtension;
24+
25+
@QuarkusTest
26+
@GitHubAppTest
27+
@ExtendWith(MockitoExtension.class)
28+
public class CheckPullRequestContributionRulesFixupSquashTest extends AbstractPullRequestTest {
29+
@Test
30+
void noFixupSquashCommits() throws IOException {
31+
long repoId = 344815557L;
32+
long prId = 585627026L;
33+
given()
34+
.github( mocks -> {
35+
mocks.configFile( "hibernate-github-bot.yml" )
36+
.fromString( """
37+
jira:
38+
projectKey: "HSEARCH"
39+
""" );
40+
41+
GHRepository repoMock = mocks.repository( "yrodiere/hibernate-github-bot-playground" );
42+
when( repoMock.getId() ).thenReturn( repoId );
43+
44+
PullRequestMockHelper.start( mocks, prId, repoMock )
45+
.commit( "HSEARCH-1111 Correct message" )
46+
.comment( "Some comment" )
47+
.comment( "Some other comment" );
48+
49+
mockCheckRuns( repoMock, "6e9f11a1e2946b207c6eb245ec942f2b5a3ea156" );
50+
} )
51+
.when()
52+
.payloadFromClasspath( "/pullrequest-opened-hsearch-1111.json" )
53+
.event( GHEvent.PULL_REQUEST )
54+
.then()
55+
.github( mocks -> {
56+
verify( mergeCommitsCheckRunUpdateBuilderMock ).withConclusion( GHCheckRun.Conclusion.SUCCESS );
57+
58+
var outputCaptor = ArgumentCaptor.forClass( GHCheckRunBuilder.Output.class );
59+
verify( mergeCommitsCheckRunUpdateBuilderMock ).add( outputCaptor.capture() );
60+
var output = outputCaptor.getValue();
61+
assertThat( output )
62+
.extracting( "title", InstanceOfAssertFactories.STRING )
63+
.contains( "All rules passed" );
64+
65+
verifyNoMoreInteractions( mocks.ghObjects() );
66+
} );
67+
}
68+
69+
@Test
70+
void hasFixupCommit() throws IOException {
71+
long repoId = 344815557L;
72+
long prId = 585627026L;
73+
given()
74+
.github( mocks -> {
75+
mocks.configFile( "hibernate-github-bot.yml" )
76+
.fromString( """
77+
jira:
78+
projectKey: "HSEARCH"
79+
""" );
80+
81+
GHRepository repoMock = mocks.repository( "yrodiere/hibernate-github-bot-playground" );
82+
when( repoMock.getId() ).thenReturn( repoId );
83+
84+
PullRequestMockHelper.start( mocks, prId, repoMock )
85+
.commit( "HSEARCH-1111 Some work" )
86+
.commit( "fixup! HSEARCH-1111 Some work", "abc123fixup" )
87+
.comment( "Some comment" )
88+
.comment( "Some other comment" );
89+
90+
mockCheckRuns( repoMock, "6e9f11a1e2946b207c6eb245ec942f2b5a3ea156" );
91+
} )
92+
.when()
93+
.payloadFromClasspath( "/pullrequest-opened-hsearch-1111.json" )
94+
.event( GHEvent.PULL_REQUEST )
95+
.then()
96+
.github( mocks -> {
97+
verify( mergeCommitsCheckRunUpdateBuilderMock ).withConclusion( GHCheckRun.Conclusion.FAILURE );
98+
99+
var outputCaptor = ArgumentCaptor.forClass( GHCheckRunBuilder.Output.class );
100+
verify( mergeCommitsCheckRunUpdateBuilderMock ).add( outputCaptor.capture() );
101+
var output = outputCaptor.getValue();
102+
assertThat( output )
103+
.extracting( "title", InstanceOfAssertFactories.STRING )
104+
.isEqualTo( "The pull request should not contain fixup! or squash! commits" );
105+
assertThat( output )
106+
.extracting( "summary", InstanceOfAssertFactories.STRING )
107+
.contains(
108+
"The pull request should not contain fixup! or squash! commits",
109+
"abc123fixup",
110+
"interactive rebase",
111+
"https://git-scm.com/docs/git-rebase#_interactive_mode"
112+
);
113+
114+
GHPullRequest prMock = mocks.pullRequest( prId );
115+
ArgumentCaptor<String> messageCaptor = ArgumentCaptor.forClass( String.class );
116+
verify( prMock ).comment( messageCaptor.capture() );
117+
assertThat( messageCaptor.getValue() )
118+
.contains(
119+
"The pull request should not contain fixup! or squash! commits",
120+
"abc123fixup",
121+
"interactive rebase",
122+
"https://git-scm.com/docs/git-rebase#_interactive_mode"
123+
);
124+
verifyNoMoreInteractions( mocks.ghObjects() );
125+
} );
126+
}
127+
128+
@Test
129+
void hasSquashCommit() throws IOException {
130+
long repoId = 344815557L;
131+
long prId = 585627026L;
132+
given()
133+
.github( mocks -> {
134+
mocks.configFile( "hibernate-github-bot.yml" )
135+
.fromString( """
136+
jira:
137+
projectKey: "HSEARCH"
138+
""" );
139+
140+
GHRepository repoMock = mocks.repository( "yrodiere/hibernate-github-bot-playground" );
141+
when( repoMock.getId() ).thenReturn( repoId );
142+
143+
PullRequestMockHelper.start( mocks, prId, repoMock )
144+
.commit( "HSEARCH-1111 Some work" )
145+
.commit( "squash! HSEARCH-1111 Some work", "def456squash" )
146+
.comment( "Some comment" )
147+
.comment( "Some other comment" );
148+
149+
mockCheckRuns( repoMock, "6e9f11a1e2946b207c6eb245ec942f2b5a3ea156" );
150+
} )
151+
.when()
152+
.payloadFromClasspath( "/pullrequest-opened-hsearch-1111.json" )
153+
.event( GHEvent.PULL_REQUEST )
154+
.then()
155+
.github( mocks -> {
156+
verify( mergeCommitsCheckRunUpdateBuilderMock ).withConclusion( GHCheckRun.Conclusion.FAILURE );
157+
158+
var outputCaptor = ArgumentCaptor.forClass( GHCheckRunBuilder.Output.class );
159+
verify( mergeCommitsCheckRunUpdateBuilderMock ).add( outputCaptor.capture() );
160+
var output = outputCaptor.getValue();
161+
assertThat( output )
162+
.extracting( "title", InstanceOfAssertFactories.STRING )
163+
.isEqualTo( "The pull request should not contain fixup! or squash! commits" );
164+
assertThat( output )
165+
.extracting( "summary", InstanceOfAssertFactories.STRING )
166+
.contains(
167+
"The pull request should not contain fixup! or squash! commits",
168+
"def456squash",
169+
"interactive rebase",
170+
"https://git-scm.com/docs/git-rebase#_interactive_mode"
171+
);
172+
173+
GHPullRequest prMock = mocks.pullRequest( prId );
174+
ArgumentCaptor<String> messageCaptor = ArgumentCaptor.forClass( String.class );
175+
verify( prMock ).comment( messageCaptor.capture() );
176+
assertThat( messageCaptor.getValue() )
177+
.contains(
178+
"The pull request should not contain fixup! or squash! commits",
179+
"def456squash",
180+
"interactive rebase",
181+
"https://git-scm.com/docs/git-rebase#_interactive_mode"
182+
);
183+
verifyNoMoreInteractions( mocks.ghObjects() );
184+
} );
185+
}
186+
}

0 commit comments

Comments
 (0)