Skip to content

Commit 34ae997

Browse files
authored
chore: make the PR linter check CodeCov statuses (#33128)
This has the advantage that failing CodeCov checks can be ignored with a label. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 4eb87b6 commit 34ae997

File tree

10 files changed

+274
-94
lines changed

10 files changed

+274
-94
lines changed

.github/workflows/pr-linter.yml

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,19 @@ on:
1111
- opened
1212
- synchronize
1313
- reopened
14+
15+
# Triggered from a separate job when a review is added
1416
workflow_run:
1517
workflows: [PR Linter Trigger]
1618
types:
1719
- completed
18-
status:
20+
21+
# Trigger when a status is updated (CodeBuild leads to statuses)
22+
status: {}
23+
24+
# Trigger when a check suite is completed (GitHub actions and CodeCov create checks)
25+
check_suite:
26+
types: [completed]
1927

2028
jobs:
2129
download-if-workflow-run:
@@ -57,6 +65,7 @@ jobs:
5765
pull-requests: write
5866
statuses: read
5967
issues: read
68+
checks: read
6069
runs-on: ubuntu-latest
6170
needs: download-if-workflow-run
6271
steps:

tools/@aws-cdk/prlint/constants.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,14 @@ export enum Exemption {
1616
REQUEST_EXEMPTION = 'pr-linter/exemption-requested',
1717
CODECOV = "pr-linter/exempt-codecov",
1818
}
19+
20+
const CODECOV_PREFIX = 'codecov/';
21+
22+
export const CODECOV_CHECKS = [
23+
`${CODECOV_PREFIX}patch`,
24+
`${CODECOV_PREFIX}patch/packages/aws-cdk`,
25+
`${CODECOV_PREFIX}patch/packages/aws-cdk-lib/core`,
26+
`${CODECOV_PREFIX}project`,
27+
`${CODECOV_PREFIX}project/packages/aws-cdk`,
28+
`${CODECOV_PREFIX}project/packages/aws-cdk-lib/core`
29+
];

tools/@aws-cdk/prlint/github.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import { Endpoints } from '@octokit/types';
22
import { StatusEvent } from '@octokit/webhooks-definitions/schema';
3+
import type { components } from '@octokit/openapi-types';
34

45
export type GitHubPr =
56
Endpoints['GET /repos/{owner}/{repo}/pulls/{pull_number}']['response']['data'];
67

8+
export type CheckRun = components['schemas']['check-run'];
79

810
export interface GitHubComment {
911
id: number;
@@ -31,3 +33,33 @@ export interface GitHubLabel {
3133
export interface GitHubFile {
3234
readonly filename: string;
3335
}
36+
37+
38+
/**
39+
* Combine all check run conclusions
40+
*
41+
* Returns `success` if they all return a positive result, `failure` if
42+
* one of them failed for some reason, and `waiting` if the result isn't available
43+
* yet.
44+
*/
45+
export function summarizeRunConclusions(conclusions: Array<CheckRun['conclusion'] | undefined>): 'success' | 'failure' | 'waiting' {
46+
for (const concl of conclusions) {
47+
switch (concl) {
48+
case null:
49+
case undefined:
50+
case 'action_required':
51+
return 'waiting';
52+
53+
case 'failure':
54+
case 'cancelled':
55+
case 'timed_out':
56+
return 'failure';
57+
58+
case 'neutral':
59+
case 'skipped':
60+
case 'success':
61+
break;
62+
}
63+
}
64+
return 'success';
65+
}

tools/@aws-cdk/prlint/index.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as github from '@actions/github';
22
import { Octokit } from '@octokit/rest';
3-
import { StatusEvent, PullRequestEvent } from '@octokit/webhooks-definitions/schema';
3+
import { StatusEvent, PullRequestEvent, CheckSuiteEvent } from '@octokit/webhooks-definitions/schema';
44
import { PullRequestLinter } from './lint';
55
import { LinterActions } from './linter-base';
66
import { DEFAULT_LINTER_LOGIN } from './constants';
@@ -25,11 +25,12 @@ async function run() {
2525

2626
const number = await determinePrNumber(client);
2727
if (!number) {
28-
if (github.context.eventName === 'status') {
29-
console.error(`Could not find PR belonging to status event, but that's not unusual. Skipping.`);
28+
if (['check_suite', 'status'].includes(github.context.eventName)) {
29+
console.error(`Could not find PR belonging to event, but that's not unusual. Skipping.`);
3030
process.exit(0);
3131
}
32-
throw new Error(`Could not find PR number from event: ${github.context.eventName}`);
32+
33+
throw new Error(`Could not determine a PR number from either \$PR_NUMBER or \$PR_SHA or ${github.context.eventName}: ${JSON.stringify(github.context.payload)}`);
3334
}
3435

3536
const prLinter = new PullRequestLinter({
@@ -87,7 +88,12 @@ async function determinePrNumber(client: Octokit): Promise<number | undefined> {
8788
if (!sha && github.context.eventName === 'status') {
8889
sha = (github.context.payload as StatusEvent)?.sha;
8990
}
91+
if (!sha && github.context.eventName === 'check_suite') {
92+
// For a check_suite event, take the SHA and try to find a PR for it.
93+
sha = (github.context.payload as CheckSuiteEvent)?.check_suite.head_sha;
94+
}
9095
if (!sha) {
96+
return undefined;
9197
throw new Error(`Could not determine a SHA from either \$PR_SHA or ${JSON.stringify(github.context.payload)}`);
9298
}
9399

tools/@aws-cdk/prlint/lint.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import * as path from 'path';
22
import { findModulePath, moduleStability } from './module';
33
import { breakingModules } from './parser';
4-
import { GitHubFile, GitHubPr, Review } from "./github";
4+
import { CheckRun, GitHubFile, GitHubPr, Review, summarizeRunConclusions } from "./github";
55
import { TestResult, ValidationCollector } from './results';
6-
import { CODE_BUILD_CONTEXT, Exemption } from './constants';
6+
import { CODE_BUILD_CONTEXT, CODECOV_CHECKS, Exemption } from './constants';
77
import { StatusEvent } from '@octokit/webhooks-definitions/schema';
88
import { LinterActions, mergeLinterActions, PR_FROM_MAIN_ERROR, PullRequestLinterBase } from './linter-base';
99

@@ -253,6 +253,28 @@ export class PullRequestLinter extends PullRequestLinterBase {
253253
],
254254
});
255255

256+
if (pr.base.ref === 'main') {
257+
// Only check CodeCov for PRs targeting 'main'
258+
const runs = await this.checkRuns(sha);
259+
const codeCovRuns = CODECOV_CHECKS.map(c => runs[c] as CheckRun | undefined);
260+
261+
validationCollector.validateRuleSet({
262+
exemption: () => hasLabel(pr, Exemption.CODECOV),
263+
testRuleSet: [{
264+
test: () => {
265+
const summary = summarizeRunConclusions(codeCovRuns.map(r => r?.conclusion));
266+
console.log('CodeCov Summary:', summary);
267+
268+
switch (summary) {
269+
case 'failure': return TestResult.failure('CodeCov is indicating a drop in code coverage');
270+
case 'waiting': return TestResult.failure('Still waiting for CodeCov results (make sure the build is passing first)');
271+
case 'success': return TestResult.success();
272+
}
273+
},
274+
}],
275+
});
276+
}
277+
256278
// We always delete all comments; in the future we will just communicate via reviews.
257279
ret.deleteComments = await this.findExistingPRLinterComments();
258280

tools/@aws-cdk/prlint/linter-base.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Octokit } from '@octokit/rest';
2-
import { GitHubComment, GitHubPr, Review } from "./github";
2+
import { CheckRun, GitHubComment, GitHubPr, Review } from "./github";
33

44
export const PR_FROM_MAIN_ERROR = 'Pull requests from `main` branch of a fork cannot be accepted. Please reopen this contribution from another branch on your fork. For more information, see https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md#step-4-pull-request.';
55

@@ -263,6 +263,33 @@ export class PullRequestLinterBase {
263263
}
264264
}
265265

266+
/**
267+
* Return the check run results for the given SHA
268+
*
269+
* It *seems* like this only returns completed check runs. That is, in-progress
270+
* runs are not included.
271+
*/
272+
public async checkRuns(sha: string): Promise<{[name: string]: CheckRun}> {
273+
const response = await this.client.paginate(this.client.checks.listForRef, {
274+
owner: this.prParams.owner,
275+
repo: this.prParams.repo,
276+
ref: sha,
277+
});
278+
279+
const ret: {[name: string]: CheckRun} = {};
280+
for (const c of response) {
281+
if (!c.started_at) {
282+
continue;
283+
}
284+
285+
// Insert if this is the first time seeing this check, or the current DT is newer
286+
if (!ret[c.name] || ret[c.name].started_at!.localeCompare(c.started_at) > 1) {
287+
ret[c.name] = c;
288+
}
289+
}
290+
return ret;
291+
}
292+
266293
/**
267294
* Delete a comment
268295
*/

tools/@aws-cdk/prlint/results.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,25 @@ import { GitHubFile, GitHubPr } from "./github";
88
*/
99
export class TestResult {
1010
/**
11-
* Create a test result from a potential failure
11+
* Return a successful test result
12+
*/
13+
public static success() {
14+
return new TestResult();
15+
}
16+
17+
/**
18+
* Return an unconditionally failing test result
19+
*/
20+
public static failure(message: string) {
21+
const ret = new TestResult();
22+
ret.assessFailure(true, message);
23+
return ret;
24+
}
25+
26+
/**
27+
* Create a test result from a POTENTIAL failure
28+
*
29+
* If `failureCondition` is true, this will return a failure, otherwise it will return a success.
1230
*/
1331
public static fromFailure(failureCondition: boolean, failureMessage: string): TestResult {
1432
const ret = new TestResult();

0 commit comments

Comments
 (0)