Skip to content

Commit 46e17c7

Browse files
matt-fiddmatt-fidd
authored
refactor to recalculate labels from current PR state when triggered (#36)
* refactor to recalculate labels from current PR state when triggered * fix tests * expand tests to add checkSuite * feedback
1 parent 80c6341 commit 46e17c7

8 files changed

Lines changed: 286 additions & 70 deletions

File tree

‎src/classes/PullRequest.ts‎

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ export default class PullRequest {
2929
repo: string;
3030
branch: string;
3131
number: number;
32-
wip: boolean;
3332
data: TContext['payload']['pull_request'];
3433
octokit: ProbotOctokit;
3534

@@ -42,7 +41,10 @@ export default class PullRequest {
4241
this.data = context.payload.pull_request;
4342
this.number = this.data.number;
4443
this.branch = this.data.base.ref;
45-
this.wip = labels.wip.regex?.test(this.data.title) ?? false;
44+
}
45+
46+
get wip() {
47+
return labels.wip.regex?.test(this.data.title) ?? false;
4648
}
4749

4850
static async getFromNumber(context: BarebonesContext, number: number) {
@@ -75,17 +77,26 @@ export default class PullRequest {
7577
pull_number: this.number,
7678
title,
7779
});
80+
this.data.title = title;
7881
}
7982

80-
async addLabel<LKey extends keyof typeof labels>(name: LKey) {
83+
async setLabelsByKeys(keys: Array<keyof typeof labels>) {
84+
const uniqueKeys = [...new Set(keys)];
8185
await this.octokit.issues.setLabels({
8286
owner: this.owner,
8387
repo: this.repo,
8488
issue_number: this.number,
85-
labels: [...this.additionalLabels, labels[name].name],
89+
labels: [
90+
...this.additionalLabels,
91+
...uniqueKeys.map(key => labels[key].name),
92+
],
8693
});
8794
}
8895

96+
async addLabel<LKey extends keyof typeof labels>(name: LKey) {
97+
await this.setLabelsByKeys([name]);
98+
}
99+
89100
async clearLabels() {
90101
await this.octokit.issues.setLabels({
91102
owner: this.owner,
@@ -95,6 +106,31 @@ export default class PullRequest {
95106
});
96107
}
97108

109+
async hasFailingCI(ref?: string): Promise<boolean> {
110+
const suites = (
111+
await this.octokit.checks.listSuitesForRef({
112+
owner: this.owner,
113+
repo: this.repo,
114+
ref: ref ?? this.data.head.sha,
115+
})
116+
).data.check_suites;
117+
118+
const runs = [];
119+
for (const suite of suites) {
120+
const run = (
121+
await this.octokit.checks.listForSuite({
122+
owner: this.owner,
123+
repo: this.repo,
124+
check_suite_id: suite.id,
125+
})
126+
).data.check_runs;
127+
128+
runs.push(...run);
129+
}
130+
131+
return runs.some(r => r.conclusion === 'failure');
132+
}
133+
98134
async getRequiredReviews() {
99135
try {
100136
const { data } = await this.octokit.repos.getBranchProtection({

‎src/handlers/checkSuite.ts‎

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,14 @@
11
import { Probot } from 'probot';
22

3-
import { checkFeatureFlag } from '../config.js';
43
import PullRequest from '../classes/PullRequest.js';
4+
import { syncPullRequestLabels } from '../labels/labelCalculator.js';
55

66
export default (app: Probot) => {
77
app.on(['check_suite.completed'], async context => {
8-
if (!(await checkFeatureFlag(context, 'enableFailingCI'))) return;
9-
108
const commitSha = context.payload.check_suite.head_sha;
119
const prNum = context.payload.check_suite.pull_requests[0].number;
1210

1311
const pr = await PullRequest.getFromNumber(context, prNum);
14-
15-
if (pr.wip || pr.data.draft) return;
16-
17-
const suites = (
18-
await context.octokit.checks.listSuitesForRef({
19-
owner: pr.owner,
20-
repo: pr.repo,
21-
ref: commitSha,
22-
})
23-
).data.check_suites;
24-
25-
const runs = [];
26-
for (const suite of suites) {
27-
const run = (
28-
await context.octokit.checks.listForSuite({
29-
owner: pr.owner,
30-
repo: pr.repo,
31-
check_suite_id: suite.id,
32-
})
33-
).data.check_runs;
34-
35-
runs.push(...run);
36-
}
37-
38-
const failed = runs.some(r => r.conclusion === 'failure');
39-
40-
if (failed) {
41-
await pr.addLabel('failingCI');
42-
} else {
43-
await pr.addLabel('readyForReview');
44-
}
12+
await syncPullRequestLabels(context, pr, { failingCiSha: commitSha });
4513
});
4614
};

‎src/handlers/pullRequest.ts‎

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import { Probot } from 'probot';
22

3-
import { labels } from '../labels.js';
43
import PullRequest from '../classes/PullRequest.js';
4+
import { labels } from '../labels.js';
5+
import { syncPullRequestLabels } from '../labels/labelCalculator.js';
56

67
export default (app: Probot) => {
78
app.on(['pull_request.opened', 'pull_request.reopened'], async context => {
@@ -14,7 +15,7 @@ export default (app: Probot) => {
1415
await pr.setTitle(title);
1516
}
1617

17-
await pr.addLabel('wip');
18+
await syncPullRequestLabels(context, pr);
1819
});
1920

2021
app.on(['pull_request.edited'], async context => {
@@ -30,36 +31,23 @@ export default (app: Probot) => {
3031
return;
3132
}
3233

33-
if (pr.wip) {
34-
await pr.addLabel('wip');
35-
} else {
36-
await pr.addLabel('readyForReview');
37-
}
34+
await syncPullRequestLabels(context, pr);
3835
});
3936

4037
app.on(['pull_request.synchronize'], async context => {
4138
const pr = new PullRequest(context);
4239

43-
if (pr.wip || pr.data.draft) return;
44-
45-
const reviewStatus = await pr.getReviewStatus();
46-
47-
await pr.addLabel(reviewStatus);
40+
await syncPullRequestLabels(context, pr);
4841
});
4942

5043
app.on(['pull_request.closed'], async context => {
5144
const pr = new PullRequest(context);
52-
53-
if (pr.data.merged_at !== null) {
54-
await pr.addLabel('merged');
55-
} else {
56-
await pr.clearLabels();
57-
}
45+
await syncPullRequestLabels(context, pr);
5846
});
5947

6048
app.on(['pull_request.converted_to_draft'], async context => {
6149
const pr = new PullRequest(context);
62-
await pr.addLabel('wip');
50+
await syncPullRequestLabels(context, pr);
6351
});
6452

6553
app.on(['pull_request.ready_for_review'], async context => {
@@ -68,6 +56,6 @@ export default (app: Probot) => {
6856
const title = pr.data.title.replace(labels.wip.regex ?? '', '');
6957
await pr.setTitle(title);
7058

71-
await pr.addLabel('readyForReview');
59+
await syncPullRequestLabels(context, pr);
7260
});
7361
};

‎src/handlers/pullRequestReview.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { Probot } from 'probot';
22

33
import PullRequest from '../classes/PullRequest.js';
4+
import { syncPullRequestLabels } from '../labels/labelCalculator.js';
45

56
export default (app: Probot) => {
67
app.on(['pull_request_review'], async context => {
@@ -20,10 +21,6 @@ export default (app: Probot) => {
2021
}
2122
}
2223

23-
if (pr.wip || pr.data.draft) return;
24-
25-
const reviewStatus = await pr.getReviewStatus();
26-
27-
await pr.addLabel(reviewStatus);
24+
await syncPullRequestLabels(context, pr);
2825
});
2926
};

‎src/labels/labelCalculator.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import { Context } from 'probot';
2+
3+
import PullRequest from '../classes/PullRequest.js';
4+
import { checkFeatureFlag } from '../config.js';
5+
import { labels } from '../labels.js';
6+
7+
export type LabelKey = keyof typeof labels;
8+
9+
type LabelCalcOptions = {
10+
failingCiSha?: string;
11+
};
12+
13+
export async function calculateRequiredLabels(
14+
pr: PullRequest,
15+
options: { includeFailingCI: boolean; failingCiSha?: string },
16+
): Promise<LabelKey[]> {
17+
if (pr.data.state === 'closed') {
18+
if (pr.data.merged_at != null) return ['merged'];
19+
return [];
20+
}
21+
22+
if (pr.data.draft || pr.wip) {
23+
return ['wip'];
24+
}
25+
26+
const reviewStatus = await pr.getReviewStatus();
27+
28+
if (options.includeFailingCI) {
29+
const hasFailure = await pr.hasFailingCI(options.failingCiSha);
30+
if (hasFailure) {
31+
if (reviewStatus === 'approved') {
32+
return ['approved', 'failingCI'];
33+
}
34+
return ['failingCI'];
35+
}
36+
}
37+
38+
return [reviewStatus];
39+
}
40+
41+
export async function syncPullRequestLabels(
42+
context: Context,
43+
pr: PullRequest,
44+
options: LabelCalcOptions = {},
45+
) {
46+
const includeFailingCI = await checkFeatureFlag(context, 'enableFailingCI');
47+
const requiredLabels = await calculateRequiredLabels(pr, {
48+
includeFailingCI,
49+
failingCiSha: options.failingCiSha,
50+
});
51+
52+
if (requiredLabels.length === 0) {
53+
await pr.clearLabels();
54+
return;
55+
}
56+
57+
await pr.setLabelsByKeys(requiredLabels);
58+
}

‎test/checkSuite/checkSuite.test.ts‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import { describe, afterEach, test, expect } from 'vitest';
2+
import nock from 'nock';
3+
4+
import { setupProbot, teardownProbot } from '../testHelpers';
5+
import { labels } from '../../src/labels.js';
6+
7+
const checkSuitePayload = {
8+
action: 'completed',
9+
check_suite: {
10+
head_sha: 'abc123',
11+
pull_requests: [{ number: 1 }],
12+
},
13+
repository: {
14+
name: 'your-repo-name',
15+
owner: {
16+
login: 'your-repo',
17+
},
18+
},
19+
installation: {
20+
id: 2,
21+
},
22+
};
23+
24+
const pullRequestResponse = {
25+
number: 1,
26+
title: 'Test Pull Request',
27+
state: 'open',
28+
draft: false,
29+
user: {
30+
login: 'pr-author',
31+
id: 456,
32+
},
33+
head: {
34+
sha: 'abc123',
35+
},
36+
base: {
37+
ref: 'master',
38+
},
39+
labels: [],
40+
};
41+
42+
describe('Probot Check Suite Handler', () => {
43+
let probot: any;
44+
45+
afterEach(() => {
46+
teardownProbot();
47+
});
48+
49+
test('adds failing CI label when a check run fails', async () => {
50+
probot = setupProbot({
51+
checkSuites: [{ id: 101 }],
52+
checkRunsBySuite: { 101: [{ conclusion: 'failure' }] },
53+
});
54+
55+
const mock = nock('https://api.github.com')
56+
.post('/app/installations/2/access_tokens')
57+
.reply(200, {
58+
token: 'test',
59+
permissions: {
60+
pull_requests: 'write',
61+
},
62+
})
63+
.get('/repos/your-repo/your-repo-name/pulls/1')
64+
.reply(200, pullRequestResponse)
65+
.get('/repos/your-repo/your-repo-name/pulls/1/reviews')
66+
.reply(200, [])
67+
.put('/repos/your-repo/your-repo-name/issues/1/labels', (body: any) => {
68+
expect(body).toMatchObject({ labels: [labels.failingCI.name] });
69+
return true;
70+
})
71+
.reply(200);
72+
73+
await probot.receive({
74+
name: 'check_suite',
75+
payload: checkSuitePayload,
76+
});
77+
78+
expect(mock.pendingMocks()).toStrictEqual([]);
79+
});
80+
81+
test('sets ready for review when CI passes and no reviews', async () => {
82+
probot = setupProbot({
83+
checkSuites: [{ id: 101 }],
84+
checkRunsBySuite: { 101: [{ conclusion: 'success' }] },
85+
});
86+
87+
const mock = nock('https://api.github.com')
88+
.post('/app/installations/2/access_tokens')
89+
.reply(200, {
90+
token: 'test',
91+
permissions: {
92+
pull_requests: 'write',
93+
},
94+
})
95+
.get('/repos/your-repo/your-repo-name/pulls/1')
96+
.reply(200, pullRequestResponse)
97+
.get('/repos/your-repo/your-repo-name/pulls/1/reviews')
98+
.reply(200, [])
99+
.put('/repos/your-repo/your-repo-name/issues/1/labels', (body: any) => {
100+
expect(body).toMatchObject({ labels: [labels.readyForReview.name] });
101+
return true;
102+
})
103+
.reply(200);
104+
105+
await probot.receive({
106+
name: 'check_suite',
107+
payload: checkSuitePayload,
108+
});
109+
110+
expect(mock.pendingMocks()).toStrictEqual([]);
111+
});
112+
});

0 commit comments

Comments
 (0)