From b5278c1604f0fed31914a9a1c7506e74e435ffed Mon Sep 17 00:00:00 2001 From: epolon Date: Sun, 12 Jan 2025 23:33:10 +0200 Subject: [PATCH 1/4] mid work --- tools/@aws-cdk/prlint/lint.ts | 51 +++++++++++++++++++++++++ tools/@aws-cdk/prlint/test/lint.test.ts | 8 ++++ 2 files changed, 59 insertions(+) diff --git a/tools/@aws-cdk/prlint/lint.ts b/tools/@aws-cdk/prlint/lint.ts index 48c5a23819459..06f267be643dd 100644 --- a/tools/@aws-cdk/prlint/lint.ts +++ b/tools/@aws-cdk/prlint/lint.ts @@ -10,6 +10,16 @@ export type GitHubPr = Endpoints['GET /repos/{owner}/{repo}/pulls/{pull_number}']['response']['data']; export const CODE_BUILD_CONTEXT = 'AWS CodeBuild us-east-1 (AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv)'; +export const CODECOV_PREFIX = 'codecov/'; + +export const CODECOV_CHECKS = [ + 'patch', + 'patch/packages/aws-cdk', + 'patch/packages/aws-cdk-lib/core', + 'project', + 'project/packages/aws-cdk', + 'project/packages/aws-cdk-lib/core' +]; 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.'; @@ -24,6 +34,7 @@ enum Exemption { CLI_INTEG_TESTED = 'pr-linter/cli-integ-tested', REQUEST_CLARIFICATION = 'pr/reviewer-clarification-requested', REQUEST_EXEMPTION = 'pr-linter/exemption-requested', + CODECOV = "pr-linter/exempt-codecov", } export interface GithubStatusEvent { @@ -352,6 +363,24 @@ export class PullRequestLinter { } } + private async checkRunSucceeded(sha: string, checkName: string) { + const response = await this.client.paginate(this.client.checks.listForRef, { + owner: this.prParams.owner, + repo: this.prParams.repo, + ref: sha, + }); + + // grab the last check run that was started + const status = response + .filter(c => c.started_at !== undefined) + .sort((c1, c2) => c2.started_at!.localeCompare(c1.started_at!)) + .filter(c => c.name === checkName) + .map(s => s.conclusion)[0]; + + console.log(`${checkName} status: ${status}`) + return status === 'success'; + } + /** * Assess whether or not a PR is ready for review. * This is needed because some things that we need to evaluate are not filterable on @@ -575,6 +604,24 @@ export class PullRequestLinter { ], }); + const codecovTests: Test[] = []; + for (const c of CODECOV_CHECKS) { + const checkName = `${CODECOV_PREFIX}${c}`; + const succeeded = await this.checkRunSucceeded(sha, checkName); + codecovTests.push({ + test: () => { + const result = new TestResult(); + result.assessFailure(!succeeded, `${checkName} job is not succeeding`); + return result; + } + }) + } + + validationCollector.validateRuleSet({ + exemption: shouldExemptCodecov, + testRuleSet: codecovTests, + }); + console.log("Deleting PR Linter Comment now"); await this.deletePRLinterComment(); try { @@ -656,6 +703,10 @@ function fixContainsIntegTest(pr: GitHubPr, files: GitHubFile[]): TestResult { return result; } +function shouldExemptCodecov(pr: GitHubPr): boolean { + return hasLabel(pr, Exemption.CODECOV); +} + function shouldExemptReadme(pr: GitHubPr): boolean { return hasLabel(pr, Exemption.README); } diff --git a/tools/@aws-cdk/prlint/test/lint.test.ts b/tools/@aws-cdk/prlint/test/lint.test.ts index 1de1f3c334ee0..3cc25584b3818 100644 --- a/tools/@aws-cdk/prlint/test/lint.test.ts +++ b/tools/@aws-cdk/prlint/test/lint.test.ts @@ -1190,6 +1190,14 @@ function configureMock(pr: Subset, prFiles?: linter.GitHubFile[ }, }; + const checksClient = { + listForRef() { + return { + data: linter.CODECOV_CHECKS.map(c => ({ name: `${linter.CODECOV_PREFIX}${c}`, conclusion: 'success' })), + } + } + } + const searchClient = { issuesAndPullRequests() {}, }; From d8d84277d0fa197725e9fc363875935160b4f735 Mon Sep 17 00:00:00 2001 From: epolon Date: Mon, 13 Jan 2025 09:09:28 +0200 Subject: [PATCH 2/4] mid work --- tools/@aws-cdk/prlint/lint.ts | 20 ++++++++++++-------- tools/@aws-cdk/prlint/test/lint.test.ts | 1 + 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/tools/@aws-cdk/prlint/lint.ts b/tools/@aws-cdk/prlint/lint.ts index 06f267be643dd..5ad0a97a0ac14 100644 --- a/tools/@aws-cdk/prlint/lint.ts +++ b/tools/@aws-cdk/prlint/lint.ts @@ -3,6 +3,7 @@ import * as path from 'path'; import { Octokit } from '@octokit/rest'; import { Endpoints } from '@octokit/types'; import { StatusEvent } from '@octokit/webhooks-definitions/schema'; +import type { components } from '@octokit/openapi-types'; import { findModulePath, moduleStability } from './module'; import { breakingModules } from './parser'; @@ -21,6 +22,8 @@ export const CODECOV_CHECKS = [ 'project/packages/aws-cdk-lib/core' ]; +type CheckRunConclusion = components['schemas']['check-run']['conclusion'] + 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.'; /** @@ -363,7 +366,7 @@ export class PullRequestLinter { } } - private async checkRunSucceeded(sha: string, checkName: string) { + private async checkRunConclusion(sha: string, checkName: string): Promise { const response = await this.client.paginate(this.client.checks.listForRef, { owner: this.prParams.owner, repo: this.prParams.repo, @@ -371,14 +374,14 @@ export class PullRequestLinter { }); // grab the last check run that was started - const status = response - .filter(c => c.started_at !== undefined) - .sort((c1, c2) => c2.started_at!.localeCompare(c1.started_at!)) + const conclusion = response .filter(c => c.name === checkName) + .filter(c => c.started_at != null) + .sort((c1, c2) => c2.started_at!.localeCompare(c1.started_at!)) .map(s => s.conclusion)[0]; - console.log(`${checkName} status: ${status}`) - return status === 'success'; + console.log(`${checkName} conclusion: ${conclusion}`) + return conclusion; } /** @@ -607,11 +610,12 @@ export class PullRequestLinter { const codecovTests: Test[] = []; for (const c of CODECOV_CHECKS) { const checkName = `${CODECOV_PREFIX}${c}`; - const succeeded = await this.checkRunSucceeded(sha, checkName); + const status = await this.checkRunConclusion(sha, checkName); codecovTests.push({ test: () => { const result = new TestResult(); - result.assessFailure(!succeeded, `${checkName} job is not succeeding`); + const message = status == null ? `${checkName} has not started yet` : `${checkName} job is in status: ${status}`; + result.assessFailure(status !== 'success', message); return result; } }) diff --git a/tools/@aws-cdk/prlint/test/lint.test.ts b/tools/@aws-cdk/prlint/test/lint.test.ts index 3cc25584b3818..1eb2fee558309 100644 --- a/tools/@aws-cdk/prlint/test/lint.test.ts +++ b/tools/@aws-cdk/prlint/test/lint.test.ts @@ -1212,6 +1212,7 @@ function configureMock(pr: Subset, prFiles?: linter.GitHubFile[ issues: issuesClient as any, search: searchClient as any, repos: reposClient as any, + checks: checksClient as any, paginate: (method: any, args: any) => { return method(args).data; }, } as any, }); From 493539a89280f183bc80de5fdf1bc0b6b5fe3d07 Mon Sep 17 00:00:00 2001 From: epolon Date: Mon, 13 Jan 2025 09:41:44 +0200 Subject: [PATCH 3/4] fix tests --- tools/@aws-cdk/prlint/lint.ts | 4 ++-- tools/@aws-cdk/prlint/test/lint.test.ts | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/tools/@aws-cdk/prlint/lint.ts b/tools/@aws-cdk/prlint/lint.ts index 5ad0a97a0ac14..39cd4db00919c 100644 --- a/tools/@aws-cdk/prlint/lint.ts +++ b/tools/@aws-cdk/prlint/lint.ts @@ -366,7 +366,7 @@ export class PullRequestLinter { } } - private async checkRunConclusion(sha: string, checkName: string): Promise { + private async checkRunConclusion(sha: string, checkName: string): Promise { const response = await this.client.paginate(this.client.checks.listForRef, { owner: this.prParams.owner, repo: this.prParams.repo, @@ -374,7 +374,7 @@ export class PullRequestLinter { }); // grab the last check run that was started - const conclusion = response + const conclusion = response.check_runs .filter(c => c.name === checkName) .filter(c => c.started_at != null) .sort((c1, c2) => c2.started_at!.localeCompare(c1.started_at!)) diff --git a/tools/@aws-cdk/prlint/test/lint.test.ts b/tools/@aws-cdk/prlint/test/lint.test.ts index 1eb2fee558309..fe64c74d026f6 100644 --- a/tools/@aws-cdk/prlint/test/lint.test.ts +++ b/tools/@aws-cdk/prlint/test/lint.test.ts @@ -1193,7 +1193,11 @@ function configureMock(pr: Subset, prFiles?: linter.GitHubFile[ const checksClient = { listForRef() { return { - data: linter.CODECOV_CHECKS.map(c => ({ name: `${linter.CODECOV_PREFIX}${c}`, conclusion: 'success' })), + data: { check_runs: linter.CODECOV_CHECKS.map(c => ({ + name: `${linter.CODECOV_PREFIX}${c}`, + conclusion: 'success', + started_at: '1' + }))}, } } } From c5da0d3aedcb7e8a5cd361ed021f2a3684961b50 Mon Sep 17 00:00:00 2001 From: epolon Date: Wed, 15 Jan 2025 10:37:51 +0200 Subject: [PATCH 4/4] mid work --- tools/@aws-cdk/prlint/lint.ts | 47 +++++++++++++------------ tools/@aws-cdk/prlint/test/lint.test.ts | 8 ++--- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/tools/@aws-cdk/prlint/lint.ts b/tools/@aws-cdk/prlint/lint.ts index 39cd4db00919c..547184e722fb2 100644 --- a/tools/@aws-cdk/prlint/lint.ts +++ b/tools/@aws-cdk/prlint/lint.ts @@ -373,11 +373,10 @@ export class PullRequestLinter { ref: sha, }); - // grab the last check run that was started - const conclusion = response.check_runs - .filter(c => c.name === checkName) - .filter(c => c.started_at != null) - .sort((c1, c2) => c2.started_at!.localeCompare(c1.started_at!)) + // grab the last check run that was completed + const conclusion = response + .filter(c => c.name === checkName && c.completed_at != null) + .sort((c1, c2) => c2.completed_at!.localeCompare(c1.completed_at!)) .map(s => s.conclusion)[0]; console.log(`${checkName} conclusion: ${conclusion}`) @@ -555,6 +554,7 @@ export class PullRequestLinter { console.log(`⌛ Fetching PR number ${number}`); const pr = (await this.client.pulls.get(this.prParams)).data as GitHubPr; + console.log(`PR base ref is: ${pr.base.ref}`) console.log(`⌛ Fetching files for PR number ${number}`); const files = await this.client.paginate(this.client.pulls.listFiles, this.prParams); @@ -607,25 +607,28 @@ export class PullRequestLinter { ], }); - const codecovTests: Test[] = []; - for (const c of CODECOV_CHECKS) { - const checkName = `${CODECOV_PREFIX}${c}`; - const status = await this.checkRunConclusion(sha, checkName); - codecovTests.push({ - test: () => { - const result = new TestResult(); - const message = status == null ? `${checkName} has not started yet` : `${checkName} job is in status: ${status}`; - result.assessFailure(status !== 'success', message); - return result; - } - }) + if (pr.base.ref === 'main') { + // we don't enforce codecov on release branches + const codecovTests: Test[] = []; + for (const c of CODECOV_CHECKS) { + const checkName = `${CODECOV_PREFIX}${c}`; + const conclusion = await this.checkRunConclusion(sha, checkName); + codecovTests.push({ + test: () => { + const result = new TestResult(); + const message = conclusion == null ? `${checkName} has not reported a status yet` : `${checkName} job is in status: ${conclusion}`; + result.assessFailure(conclusion !== 'success', message); + return result; + } + }) + } + + validationCollector.validateRuleSet({ + exemption: shouldExemptCodecov, + testRuleSet: codecovTests, + }); } - validationCollector.validateRuleSet({ - exemption: shouldExemptCodecov, - testRuleSet: codecovTests, - }); - console.log("Deleting PR Linter Comment now"); await this.deletePRLinterComment(); try { diff --git a/tools/@aws-cdk/prlint/test/lint.test.ts b/tools/@aws-cdk/prlint/test/lint.test.ts index fe64c74d026f6..d287f27186c74 100644 --- a/tools/@aws-cdk/prlint/test/lint.test.ts +++ b/tools/@aws-cdk/prlint/test/lint.test.ts @@ -1142,7 +1142,7 @@ describe('integration tests required on features', () => { function configureMock(pr: Subset, prFiles?: linter.GitHubFile[], existingComments?: string[]): linter.PullRequestLinter { const pullsClient = { get(_props: { _owner: string, _repo: string, _pull_number: number, _user: { _login: string} }) { - return { data: pr }; + return { data: { ...pr, base: { ref: 'main'}} }; }, listFiles(_props: { _owner: string, _repo: string, _pull_number: number }) { @@ -1193,11 +1193,11 @@ function configureMock(pr: Subset, prFiles?: linter.GitHubFile[ const checksClient = { listForRef() { return { - data: { check_runs: linter.CODECOV_CHECKS.map(c => ({ + data: linter.CODECOV_CHECKS.map(c => ({ name: `${linter.CODECOV_PREFIX}${c}`, conclusion: 'success', - started_at: '1' - }))}, + completed_at: '1' + })), } } }