Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion .github/workflows/pr-linter.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,19 @@ on:
- opened
- synchronize
- reopened

# Triggered from a separate job when a review is added
workflow_run:
workflows: [PR Linter Trigger]
types:
- completed
status:

# Trigger when a status is updated (CodeBuild leads to statuses)
status: {}

# Trigger when a check suite is completed (GitHub actions and CodeCov create checks)
check_suite:
types: [completed]

jobs:
download-if-workflow-run:
Expand Down Expand Up @@ -57,6 +65,7 @@ jobs:
pull-requests: write
statuses: read
issues: read
checks: read
runs-on: ubuntu-latest
needs: download-if-workflow-run
steps:
Expand Down
11 changes: 11 additions & 0 deletions tools/@aws-cdk/prlint/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,14 @@ export enum Exemption {
REQUEST_EXEMPTION = 'pr-linter/exemption-requested',
CODECOV = "pr-linter/exempt-codecov",
}

const CODECOV_PREFIX = 'codecov/';

export const CODECOV_CHECKS = [
`${CODECOV_PREFIX}patch`,
`${CODECOV_PREFIX}patch/packages/aws-cdk`,
`${CODECOV_PREFIX}patch/packages/aws-cdk-lib/core`,
`${CODECOV_PREFIX}project`,
`${CODECOV_PREFIX}project/packages/aws-cdk`,
`${CODECOV_PREFIX}project/packages/aws-cdk-lib/core`
];
32 changes: 32 additions & 0 deletions tools/@aws-cdk/prlint/github.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { Endpoints } from '@octokit/types';
import { StatusEvent } from '@octokit/webhooks-definitions/schema';
import type { components } from '@octokit/openapi-types';

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

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

export interface GitHubComment {
id: number;
Expand Down Expand Up @@ -31,3 +33,33 @@ export interface GitHubLabel {
export interface GitHubFile {
readonly filename: string;
}


/**
* Combine all check run conclusions
*
* Returns `success` if they all return a positive result, `failure` if
* one of them failed for some reason, and `waiting` if the result isn't available
* yet.
*/
export function summarizeRunConclusions(conclusions: Array<CheckRun['conclusion'] | undefined>): 'success' | 'failure' | 'waiting' {
for (const concl of conclusions) {
switch (concl) {
case null:
case undefined:
case 'action_required':
return 'waiting';

case 'failure':
case 'cancelled':
case 'timed_out':
return 'failure';

case 'neutral':
case 'skipped':
case 'success':
break;
}
}
return 'success';
}
14 changes: 10 additions & 4 deletions tools/@aws-cdk/prlint/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as github from '@actions/github';
import { Octokit } from '@octokit/rest';
import { StatusEvent, PullRequestEvent } from '@octokit/webhooks-definitions/schema';
import { StatusEvent, PullRequestEvent, CheckSuiteEvent } from '@octokit/webhooks-definitions/schema';
import { PullRequestLinter } from './lint';
import { LinterActions } from './linter-base';
import { DEFAULT_LINTER_LOGIN } from './constants';
Expand All @@ -25,11 +25,12 @@ async function run() {

const number = await determinePrNumber(client);
if (!number) {
if (github.context.eventName === 'status') {
console.error(`Could not find PR belonging to status event, but that's not unusual. Skipping.`);
if (['check_suite', 'status'].includes(github.context.eventName)) {
console.error(`Could not find PR belonging to event, but that's not unusual. Skipping.`);
process.exit(0);
}
throw new Error(`Could not find PR number from event: ${github.context.eventName}`);

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)}`);
}

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

Expand Down
26 changes: 24 additions & 2 deletions tools/@aws-cdk/prlint/lint.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import * as path from 'path';
import { findModulePath, moduleStability } from './module';
import { breakingModules } from './parser';
import { GitHubFile, GitHubPr, Review } from "./github";
import { CheckRun, GitHubFile, GitHubPr, Review, summarizeRunConclusions } from "./github";
import { TestResult, ValidationCollector } from './results';
import { CODE_BUILD_CONTEXT, Exemption } from './constants';
import { CODE_BUILD_CONTEXT, CODECOV_CHECKS, Exemption } from './constants';
import { StatusEvent } from '@octokit/webhooks-definitions/schema';
import { LinterActions, mergeLinterActions, PR_FROM_MAIN_ERROR, PullRequestLinterBase } from './linter-base';

Expand Down Expand Up @@ -253,6 +253,28 @@ export class PullRequestLinter extends PullRequestLinterBase {
],
});

if (pr.base.ref === 'main') {
// Only check CodeCov for PRs targeting 'main'
const runs = await this.checkRuns(sha);
const codeCovRuns = CODECOV_CHECKS.map(c => runs[c] as CheckRun | undefined);

validationCollector.validateRuleSet({
exemption: () => hasLabel(pr, Exemption.CODECOV),
testRuleSet: [{
test: () => {
const summary = summarizeRunConclusions(codeCovRuns.map(r => r?.conclusion));
console.log('CodeCov Summary:', summary);

switch (summary) {
case 'failure': return TestResult.failure('CodeCov is indicating a drop in code coverage');
case 'waiting': return TestResult.failure('Still waiting for CodeCov results (make sure the build is passing first)');
case 'success': return TestResult.success();
}
},
}],
});
}

// We always delete all comments; in the future we will just communicate via reviews.
ret.deleteComments = await this.findExistingPRLinterComments();

Expand Down
29 changes: 28 additions & 1 deletion tools/@aws-cdk/prlint/linter-base.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Octokit } from '@octokit/rest';
import { GitHubComment, GitHubPr, Review } from "./github";
import { CheckRun, GitHubComment, GitHubPr, Review } from "./github";

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.';

Expand Down Expand Up @@ -263,6 +263,33 @@ export class PullRequestLinterBase {
}
}

/**
* Return the check run results for the given SHA
*
* It *seems* like this only returns completed check runs. That is, in-progress
* runs are not included.
*/
public async checkRuns(sha: string): Promise<{[name: string]: CheckRun}> {
const response = await this.client.paginate(this.client.checks.listForRef, {
owner: this.prParams.owner,
repo: this.prParams.repo,
ref: sha,
});

const ret: {[name: string]: CheckRun} = {};
for (const c of response) {
if (!c.started_at) {
continue;
}

// Insert if this is the first time seeing this check, or the current DT is newer
if (!ret[c.name] || ret[c.name].started_at!.localeCompare(c.started_at) > 1) {
ret[c.name] = c;
}
}
return ret;
}

/**
* Delete a comment
*/
Expand Down
20 changes: 19 additions & 1 deletion tools/@aws-cdk/prlint/results.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,25 @@ import { GitHubFile, GitHubPr } from "./github";
*/
export class TestResult {
/**
* Create a test result from a potential failure
* Return a successful test result
*/
public static success() {
return new TestResult();
}

/**
* Return an unconditionally failing test result
*/
public static failure(message: string) {
const ret = new TestResult();
ret.assessFailure(true, message);
return ret;
}

/**
* Create a test result from a POTENTIAL failure
*
* If `failureCondition` is true, this will return a failure, otherwise it will return a success.
*/
public static fromFailure(failureCondition: boolean, failureMessage: string): TestResult {
const ret = new TestResult();
Expand Down
Loading
Loading