chore(prlint): add need-xxx-reviews when building with github action

.github/workflows/pr-linter.yml

@@ -18,9 +18,6 @@ on:
     types:
       - completed
 
-  # 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]

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

@@ -1,7 +1,7 @@
 
 export const DEFAULT_LINTER_LOGIN = 'aws-cdk-automation';
 
-export const CODE_BUILD_CONTEXT = 'AWS CodeBuild us-east-1 (AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv)';
+export const CODE_BUILD_WORKFLOW_FILE = 'codebuild-pr-build.yml';
 
 /**
  * Types of exemption labels in aws-cdk project.

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

@@ -2,7 +2,6 @@ import * as github from '@actions/github';
 import { Octokit } from '@octokit/rest';
 import { StatusEvent, PullRequestEvent, CheckSuiteEvent } from '@octokit/webhooks-definitions/schema';
 import { PullRequestLinter } from './lint';
-import { LinterActions } from './linter-base';
 import { DEFAULT_LINTER_LOGIN } from './constants';
 
 /**
@@ -42,23 +41,7 @@ async function run() {
     linterLogin: process.env.LINTER_LOGIN || DEFAULT_LINTER_LOGIN,
   });
 
-  let actions: LinterActions | undefined;
-
-  switch (github.context.eventName) {
-    case 'status':
-      // Triggering on a 'status' event is solely used to see if the CodeBuild
-      // job just turned green, and adding certain 'ready for review' labels
-      // if it does.
-      const statusPayload = github.context.payload as StatusEvent;
-      console.log('validating status event');
-      actions = await prLinter.validateStatusEvent(statusPayload);
-      break;
-
-    default:
-      // This is the main PR validator action.
-      actions = await prLinter.validatePullRequestTarget();
-      break;
-  }
+  const actions = await prLinter.validatePullRequestTarget();
 
   if (actions) {
     console.log(actions);

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

@@ -3,8 +3,7 @@ import { findModulePath, moduleStability } from './module';
 import { breakingModules } from './parser';
 import { CheckRun, GitHubFile, GitHubPr, Review, sumChanges, summarizeRunConclusions } from "./github";
 import { TestResult, ValidationCollector } from './results';
-import { CODE_BUILD_CONTEXT, CODECOV_CHECKS, Exemption } from './constants';
-import { StatusEvent } from '@octokit/webhooks-definitions/schema';
+import { CODE_BUILD_WORKFLOW_FILE, CODECOV_CHECKS, Exemption } from './constants';
 import { LinterActions, mergeLinterActions, PR_FROM_MAIN_ERROR, PullRequestLinterBase } from './linter-base';
 
 /**
@@ -20,21 +19,15 @@ export class PullRequestLinter extends PullRequestLinterBase {
    * @param sha the commit sha to evaluate
    */
   private async codeBuildJobSucceeded(sha: string): Promise<boolean> {
-    const statuses = await this.client.repos.listCommitStatusesForRef({
+    const statuses = await this.client.actions.listWorkflowRuns({
       owner: this.prParams.owner,
       repo: this.prParams.repo,
-      ref: sha,
+      head_sha: sha,
+      workflow_id: CODE_BUILD_WORKFLOW_FILE,
     });
-    let status = statuses.data.filter(status => status.context === CODE_BUILD_CONTEXT).map(status => status.state);
-    console.log("CodeBuild Commit Statuses: ", status);
-    return statuses.data.some(status => status.context === CODE_BUILD_CONTEXT && status.state === 'success');
-  }
-
-  public async validateStatusEvent(status: StatusEvent): Promise<LinterActions> {
-    if (status.context === CODE_BUILD_CONTEXT && status.state === 'success') {
-      return this.assessNeedsReview();
-    }
-    return {};
+    let conclusions = statuses.data.workflow_runs.filter(run => run.status === 'completed').map(run => run.conclusion);
+    console.log("CodeBuild Commit Conclusions: ", conclusions);
+    return conclusions.some(conclusion => conclusion === 'success');
   }
 
   /**

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

@@ -1,8 +1,7 @@
 import * as path from 'path';
 import { GitHubFile, GitHubLabel, GitHubPr } from '../github';
-import { CODE_BUILD_CONTEXT, CODECOV_CHECKS } from '../constants';
+import { CODECOV_CHECKS } from '../constants';
 import { PullRequestLinter } from '../lint';
-import { StatusEvent } from '@octokit/webhooks-definitions/schema';
 import { createOctomock, OctoMock } from './octomock';
 
 type GitHubFileName = Omit<GitHubFile, 'deletions' | 'additions'>;
@@ -589,13 +588,22 @@ describe('integration tests required on features', () => {
     });
   });
 
-  describe('assess needs review from status event', () => {
+  describe('assess needs review', () => {
     const pr = {
+      title: 'chore(s3): something',
       draft: false,
       mergeable_state: 'behind',
       number: 1234,
       labels: [{ name: 'p2' }],
     };
+    const files = [
+      {
+        filename: 'some-test.test.ts',
+      },
+      {
+        filename: 'readme.md',
+      },
+    ];
     beforeEach(() => {
       mockListReviews.mockImplementation(() => {
         return {
@@ -606,12 +614,8 @@ describe('integration tests required on features', () => {
 
     test('needs a review', async () => {
       // WHEN
-      const prLinter = configureMock(pr);
-      await legacyValidateStatusEvent(prLinter, {
-        sha: SHA,
-        context: CODE_BUILD_CONTEXT,
-        state: 'success',
-      } as any);
+      const prLinter = configureMock(pr, files);
+      await legacyValidatePullRequestTarget(prLinter);
 
       // THEN
       expect(mockAddLabel.mock.calls[0][0]).toEqual({
@@ -626,12 +630,8 @@ describe('integration tests required on features', () => {
     test('needs a review and is p1', async () => {
       // WHEN
       pr.labels = [{ name: 'p1' }];
-      const prLinter = configureMock(pr);
-      await legacyValidateStatusEvent(prLinter, {
-        sha: SHA,
-        context: CODE_BUILD_CONTEXT,
-        state: 'success',
-      } as any);
+      const prLinter = configureMock(pr, files);
+      await legacyValidatePullRequestTarget(prLinter);
 
       // THEN
       expect(mockAddLabel.mock.calls[0][0]).toEqual({
@@ -657,12 +657,8 @@ describe('integration tests required on features', () => {
       ];
 
       // WHEN
-      const prLinter = configureMock(pr);
-      await legacyValidateStatusEvent(prLinter, {
-        sha: SHA,
-        context: CODE_BUILD_CONTEXT,
-        state: 'success',
-      } as any);
+      const prLinter = configureMock(pr, files);
+      await legacyValidatePullRequestTarget(prLinter);
 
       // THEN
       expect(mockRemoveLabel.mock.calls[0][0]).toEqual({
@@ -688,12 +684,8 @@ describe('integration tests required on features', () => {
       ];
 
       // WHEN
-      const prLinter = configureMock(pr);
-      await legacyValidateStatusEvent(prLinter, {
-        sha: SHA,
-        context: CODE_BUILD_CONTEXT,
-        state: 'success',
-      } as any);
+      const prLinter = configureMock(pr, files);
+      await legacyValidatePullRequestTarget(prLinter);
 
       // THEN
       expect(mockAddLabel.mock.calls[0][0]).toEqual({
@@ -725,12 +717,8 @@ describe('integration tests required on features', () => {
       ];
 
       // WHEN
-      const prLinter = configureMock(pr);
-      await legacyValidateStatusEvent(prLinter, {
-        sha: SHA,
-        context: CODE_BUILD_CONTEXT,
-        state: 'success',
-      } as any);
+      const prLinter = configureMock(pr, files);
+      await legacyValidatePullRequestTarget(prLinter);
 
       // THEN
       expect(mockRemoveLabel.mock.calls[0][0]).toEqual({
@@ -758,12 +746,8 @@ describe('integration tests required on features', () => {
       ];
 
       // WHEN
-      const prLinter = configureMock(pr);
-      await legacyValidateStatusEvent(prLinter, {
-        sha: SHA,
-        context: CODE_BUILD_CONTEXT,
-        state: 'success',
-      } as any);
+      const prLinter = configureMock(pr, files);
+      await legacyValidatePullRequestTarget(prLinter);
 
       // THEN
       expect(mockRemoveLabel.mock.calls[0][0]).toEqual({
@@ -791,12 +775,8 @@ describe('integration tests required on features', () => {
       ];
 
       // WHEN
-      const prLinter = configureMock(pr);
-      await legacyValidateStatusEvent(prLinter, {
-        sha: SHA,
-        context: CODE_BUILD_CONTEXT,
-        state: 'success',
-      } as any);
+      const prLinter = configureMock(pr, files);
+      await legacyValidatePullRequestTarget(prLinter);
 
       // THEN
       expect(mockRemoveLabel.mock.calls[0][0]).toEqual({
@@ -831,12 +811,8 @@ describe('integration tests required on features', () => {
       ];
 
       // WHEN
-      const prLinter = configureMock(pr);
-      await legacyValidateStatusEvent(prLinter, {
-        sha: SHA,
-        context: CODE_BUILD_CONTEXT,
-        state: 'success',
-      } as any);
+      const prLinter = configureMock(pr, files);
+      await legacyValidatePullRequestTarget(prLinter);
 
       // THEN
       expect(mockRemoveLabel.mock.calls[0][0]).toEqual({
@@ -869,12 +845,8 @@ describe('integration tests required on features', () => {
       ];
 
       // WHEN
-      const prLinter = configureMock(pr);
-      await legacyValidateStatusEvent(prLinter, {
-        sha: SHA,
-        context: CODE_BUILD_CONTEXT,
-        state: 'success',
-      } as any);
+      const prLinter = configureMock(pr, files);
+      await legacyValidatePullRequestTarget(prLinter);
 
       // THEN
       expect(mockRemoveLabel.mock.calls[0][0]).toEqual({
@@ -899,12 +871,8 @@ describe('integration tests required on features', () => {
       (pr as any).labels = [];
 
       // WHEN
-      const prLinter = configureMock(pr);
-      await legacyValidateStatusEvent(prLinter, {
-        sha: SHA,
-        context: CODE_BUILD_CONTEXT,
-        state: 'success',
-      } as any);
+      const prLinter = configureMock(pr, files);
+      await legacyValidatePullRequestTarget(prLinter);
 
       // THEN
       expect(mockRemoveLabel.mock.calls).toEqual([]);
@@ -927,12 +895,8 @@ describe('integration tests required on features', () => {
       ];
 
       // WHEN
-      const prLinter = configureMock(pr);
-      await legacyValidateStatusEvent(prLinter, {
-        sha: SHA,
-        context: CODE_BUILD_CONTEXT,
-        state: 'success',
-      } as any);
+      const prLinter = configureMock(pr, files);
+      await legacyValidatePullRequestTarget(prLinter);
 
       // THEN
       expect(mockRemoveLabel.mock.calls).toEqual([]);
@@ -956,12 +920,8 @@ describe('integration tests required on features', () => {
       ];
 
       // WHEN
-      const prLinter = configureMock(pr);
-      await legacyValidateStatusEvent(prLinter, {
-        sha: SHA,
-        context: CODE_BUILD_CONTEXT,
-        state: 'success',
-      } as any);
+      const prLinter = configureMock(pr, files);
+      await legacyValidatePullRequestTarget(prLinter);
 
       // THEN
       expect(mockRemoveLabel.mock.calls[0][0]).toEqual({
@@ -986,12 +946,8 @@ describe('integration tests required on features', () => {
       (pr as any).labels = [];
 
       // WHEN
-      const prLinter = configureMock(pr);
-      await legacyValidateStatusEvent(prLinter, {
-        sha: SHA,
-        context: CODE_BUILD_CONTEXT,
-        state: 'success',
-      } as any);
+      const prLinter = configureMock(pr, files);
+      await legacyValidatePullRequestTarget(prLinter);
 
       // THEN
       expect(mockRemoveLabel.mock.calls).toEqual([]);
@@ -1019,12 +975,8 @@ describe('integration tests required on features', () => {
       ];
 
       // WHEN
-      const prLinter = configureMock(pr);
-      await legacyValidateStatusEvent(prLinter, {
-        sha: SHA,
-        context: CODE_BUILD_CONTEXT,
-        state: 'success',
-      } as any);
+      const prLinter = configureMock(pr, files);
+      await legacyValidatePullRequestTarget(prLinter);
 
       // THEN
       expect(mockRemoveLabel.mock.calls).toEqual([]);
@@ -1367,12 +1319,6 @@ function configureMock(pr: Subset<GitHubPr>, prFiles?: GitHubFileName[], existin
   });
   octomock.issues.addLabels = mockAddLabel;
   octomock.issues.removeLabel = mockRemoveLabel;
-  octomock.repos.listCommitStatusesForRef.mockImplementation(() => ({
-    data: [{
-      context: CODE_BUILD_CONTEXT,
-      state: 'success',
-    }],
-  }));
 
   // We need to pretend that all CodeCov checks are passing by default, otherwise
   // the linter will complain about these even in tests that aren't testing for this.
@@ -1384,6 +1330,19 @@ function configureMock(pr: Subset<GitHubPr>, prFiles?: GitHubFileName[], existin
     })),
   }));
 
+  // Mock workflow runs to simulate successful CodeBuild jobs
+  octomock.actions.listWorkflowRuns.mockImplementation(() => ({
+    data: {
+      workflow_runs: [
+        {
+          head_sha: SHA,
+          status: 'completed',
+          conclusion: 'success',
+        },
+      ],
+    },
+  }));
+
   const linter = new PullRequestLinter({
     owner: 'aws',
     repo: 'aws-cdk',
@@ -1420,13 +1379,3 @@ async function legacyValidatePullRequestTarget(prLinter: PullRequestLinter) {
   await prLinter.executeActions(actions);
   prLinter.actionsToException(actions);
 }
-
-/**
- * Same as for validatePullRequesTarget
- *
- * @deprecated Assert on the contents of `LinterActions` instead.
- */
-async function legacyValidateStatusEvent(prLinter: PullRequestLinter, statusEvent: StatusEvent) {
-  const actions = await prLinter.validateStatusEvent(statusEvent);
-  await prLinter.executeActions(actions);
-}

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

@@ -31,7 +31,7 @@ beforeEach(() => {
     },
   });
   octomock.pulls.listReviews.mockReturnValue({ data: [] });
-  octomock.repos.listCommitStatusesForRef.mockReturnValue({ data: [] });
+  octomock.actions.listWorkflowRuns.mockReturnValue({ data: [] });
 });
 
 test('ignore if dismissing reviews throws a specific "already dismissed" error', async () => {