Skip to content

Commit 8313e9f

Browse files
committed
Add check for incorrect rebase by checking for duplicate commits and skip PR rules check(mentions, reviewers) if any appears.
1 parent 050cedf commit 8313e9f

4 files changed

Lines changed: 97 additions & 1 deletion

File tree

src/main/java/org/wildfly/bot/PullRequestRuleProcessor.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ void pullRequestRuleCheck(
4646
return;
4747
}
4848

49+
if (githubProcessor.hasDuplicateCommitInBase(pullRequest, pullRequestPayload.getRepository())) {
50+
return;
51+
}
52+
4953
GHRepository repository = pullRequest.getRepository();
5054
SequencedMap<String, List<String>> ccMentionsWithRules = new LinkedHashMap<>();
5155
Set<String> reviewers = new HashSet<>();

src/main/java/org/wildfly/bot/util/GithubProcessor.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,14 @@
77
import jakarta.inject.Inject;
88
import org.eclipse.microprofile.config.inject.ConfigProperty;
99
import org.jboss.logging.Logger;
10+
import org.kohsuke.github.GHCommit;
11+
import org.kohsuke.github.GHCommitQueryBuilder;
1012
import org.kohsuke.github.GHCommitState;
1113
import org.kohsuke.github.GHIssueComment;
1214
import org.kohsuke.github.GHLabel;
1315
import org.kohsuke.github.GHPerson;
1416
import org.kohsuke.github.GHPullRequest;
17+
import org.kohsuke.github.GHPullRequestCommitDetail;
1518
import org.kohsuke.github.GHRepository;
1619
import org.kohsuke.github.GHUser;
1720
import org.kohsuke.github.GitHub;
@@ -389,4 +392,33 @@ private String createCCMentionComment(SequencedMap<String, List<String>> ccMenti
389392
)
390393
.toList());
391394
}
395+
396+
public boolean hasDuplicateCommitInBase(GHPullRequest pullRequest, GHRepository repository)
397+
throws IOException {
398+
String baseBranch = pullRequest.getBase().getRef();
399+
GHCommitQueryBuilder commitQuery = repository.queryCommits();
400+
if (commitQuery == null) {
401+
return false;
402+
}
403+
404+
Set<String> baseCommitSHAs = commitQuery
405+
.from(baseBranch)
406+
.pageSize(100)
407+
.list()
408+
.toSet()
409+
.stream()
410+
.map(GHCommit::getSHA1)
411+
.collect(Collectors.toSet());
412+
413+
for (GHPullRequestCommitDetail prCommit : pullRequest.listCommits()) {
414+
String prSha = prCommit.getSha();
415+
if (baseCommitSHAs.contains(prSha)) {
416+
LOG.infof("Skipping rules due to incorrect rebase detected: commit %s is already in the base branch %s",
417+
prSha,
418+
baseBranch);
419+
return true;
420+
}
421+
}
422+
return false;
423+
}
392424
}

src/test/java/org/wildfly/bot/PRSkipPullRequestTest.java

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,30 @@
44
import io.quarkus.test.junit.QuarkusTest;
55
import org.junit.jupiter.api.BeforeAll;
66
import org.junit.jupiter.api.Test;
7+
import org.kohsuke.github.GHCommit;
8+
import org.kohsuke.github.GHCommitQueryBuilder;
9+
import org.kohsuke.github.GHPullRequestCommitDetail;
10+
import org.kohsuke.github.PagedIterable;
11+
import org.kohsuke.github.PagedIterator;
712
import org.wildfly.bot.utils.TestConstants;
813
import org.wildfly.bot.utils.WildflyGitHubBotTesting;
914
import org.wildfly.bot.utils.mocking.Mockable;
1015
import org.wildfly.bot.utils.mocking.MockedGHPullRequest;
1116
import org.wildfly.bot.utils.testing.PullRequestJson;
1217
import org.wildfly.bot.utils.testing.internal.TestModel;
1318

19+
import java.util.Set;
20+
21+
import static org.mockito.ArgumentMatchers.nullable;
22+
import static org.mockito.Mockito.any;
23+
import static org.mockito.Mockito.anyInt;
24+
import static org.mockito.Mockito.eq;
25+
import static org.mockito.Mockito.mock;
26+
import static org.mockito.Mockito.never;
1427
import static org.mockito.Mockito.times;
1528
import static org.mockito.Mockito.verify;
1629
import static org.mockito.Mockito.verifyNoMoreInteractions;
30+
import static org.mockito.Mockito.when;
1731

1832
@QuarkusTest
1933
@GitHubAppTest
@@ -81,4 +95,49 @@ void testSkippingFormatCheckOnDraft() throws Throwable {
8195
verifyNoMoreInteractions(mocks.pullRequest(pullRequestJson.id()));
8296
});
8397
}
84-
}
98+
99+
@Test
100+
void testSkippingRulesOnIncorrectRebase() throws Throwable {
101+
final String duplicateSHA = "sha1";
102+
final String baseBranch = "main";
103+
pullRequestJson = TestModel.setPullRequestJsonBuilder(pullRequestJsonBuilder -> pullRequestJsonBuilder);
104+
mockedContext = MockedGHPullRequest.builder(pullRequestJson.id());
105+
TestModel.given(
106+
mocks -> {
107+
WildflyGitHubBotTesting.mockRepo(mocks, wildflyConfigFile, pullRequestJson, mockedContext);
108+
GHCommit commit1 = mock(GHCommit.class);
109+
GHCommit commit2 = mock(GHCommit.class);
110+
111+
when(commit1.getSHA1()).thenReturn(duplicateSHA);
112+
when(commit2.getSHA1()).thenReturn("sha2");
113+
114+
// Create a fake PagedIterable for repository commits.
115+
Set<GHCommit> fakeRepoCommitSet = Set.of(commit1, commit2);
116+
PagedIterable<GHCommit> fakeRepoCommits = mock(PagedIterable.class);
117+
GHCommitQueryBuilder mockedQueryBuilder = mock(GHCommitQueryBuilder.class);
118+
119+
when(mocks.repository(TestConstants.TEST_REPO).queryCommits()).thenReturn(mockedQueryBuilder);
120+
when(mockedQueryBuilder.from(eq(baseBranch))).thenReturn(mockedQueryBuilder);
121+
when(mockedQueryBuilder.pageSize(anyInt())).thenReturn(mockedQueryBuilder);
122+
when(mockedQueryBuilder.list()).thenReturn(fakeRepoCommits);
123+
when(fakeRepoCommits.toSet()).thenReturn(fakeRepoCommitSet);
124+
125+
GHPullRequestCommitDetail fakePRCommit = mock(GHPullRequestCommitDetail.class);
126+
PagedIterator<GHPullRequestCommitDetail> mockIterator = mock(PagedIterator.class);
127+
PagedIterable<GHPullRequestCommitDetail> fakePRCommits = mock(PagedIterable.class);
128+
129+
when(fakePRCommit.getSha()).thenReturn(duplicateSHA);
130+
when(mockIterator.hasNext()).thenReturn(true, false);
131+
when(mockIterator.next()).thenReturn(fakePRCommit);
132+
when(mocks.pullRequest(pullRequestJson.id()).listCommits()).thenReturn(fakePRCommits);
133+
when(fakePRCommits._iterator(anyInt())).thenReturn(mockIterator);
134+
})
135+
.pullRequestEvent(pullRequestJson)
136+
.then(mocks -> {
137+
verify(mocks.pullRequest(pullRequestJson.id())).getBase();
138+
verify(mocks.repository(TestConstants.TEST_REPO)).queryCommits();
139+
verify(mocks.pullRequest(pullRequestJson.id()), never()).addLabels(nullable(String[].class));
140+
verify(mocks.pullRequest(pullRequestJson.id()), never()).requestReviewers(any());
141+
});
142+
}
143+
}

src/test/java/org/wildfly/bot/PRTriggerTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ private void checkValidRun(SsePullRequestPayload ssePullRequestPayload, Action a
185185
Mockito.verify(repo, Mockito.atMostOnce()).listLabels();
186186
Mockito.verify(repo, Mockito.atMostOnce()).createLabel(ArgumentMatchers.anyString(),
187187
ArgumentMatchers.anyString());
188+
Mockito.verify(repo, Mockito.atMostOnce()).queryCommits();
188189
Mockito.verifyNoMoreInteractions(repo);
189190
});
190191
}

0 commit comments

Comments
 (0)