From abbdbc188fe14f8e3a769cb0073f561145dee498 Mon Sep 17 00:00:00 2001 From: David Date: Thu, 29 May 2025 13:08:00 +1000 Subject: [PATCH 1/3] Check stale date on approval --- package-lock.json | 14 +++- package.json | 3 +- src/check-pr-reviews/check-pr-reviews.ts | 13 +++- src/github-api/index-reviews.spec.ts | 24 ++++++- src/github-api/index.ts | 18 ++++- src/nocks/github.test.ts | 85 ++++++++++++++++++++++++ 6 files changed, 147 insertions(+), 10 deletions(-) diff --git a/package-lock.json b/package-lock.json index 896379d..7a2befd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,16 +1,17 @@ { "name": "@checkdigit/github-actions", - "version": "2.4.0", + "version": "2.4.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@checkdigit/github-actions", - "version": "2.4.0", + "version": "2.4.1", "license": "MIT", "dependencies": { "@actions/core": "^1.10.1", "@actions/github": "^6.0.0", + "@checkdigit/time": "^4.0.0", "@octokit/rest": "^20.1.1", "debug": "^4.3.5", "semver": "^7.6.2", @@ -796,6 +797,15 @@ "prettier": "3.3.0" } }, + "node_modules/@checkdigit/time": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/@checkdigit/time/-/time-4.0.0.tgz", + "integrity": "sha512-abj47K88UWtJWJUtBkayi5PVRenrlX3dFrAK9IWT69N7tIaYIx3w9CQLV3FpCpdJIGYV9CKfstmt7bNrRyGZLg==", + "license": "MIT", + "engines": { + "node": ">=20.11" + } + }, "node_modules/@checkdigit/typescript-config": { "version": "7.0.1", "resolved": "https://registry.npmjs.org/@checkdigit/typescript-config/-/typescript-config-7.0.1.tgz", diff --git a/package.json b/package.json index f063ed9..3d0536e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@checkdigit/github-actions", - "version": "2.4.0", + "version": "2.4.1", "description": " Provides supporting operations for github action builds.", "author": "Check Digit, LLC", "license": "MIT", @@ -20,6 +20,7 @@ "dependencies": { "@actions/core": "^1.10.1", "@actions/github": "^6.0.0", + "@checkdigit/time": "^4.0.0", "@octokit/rest": "^20.1.1", "debug": "^4.3.5", "semver": "^7.6.2", diff --git a/src/check-pr-reviews/check-pr-reviews.ts b/src/check-pr-reviews/check-pr-reviews.ts index 5a19581..cbbdf80 100644 --- a/src/check-pr-reviews/check-pr-reviews.ts +++ b/src/check-pr-reviews/check-pr-reviews.ts @@ -3,8 +3,11 @@ import debug from 'debug'; import { setFailed } from '@actions/core'; +import { differenceInCalendarDays } from '@checkdigit/time'; + import { approvedReviews, haveAllReviewersReviewed, publishCommentAndRemovePrevious } from '../github-api'; +const MAXIMUM_DAYS_SINCE_REVIEW = 90; const PULL_REQUEST_MESSAGE_KEYWORD = 'PR review status '; const log = debug('github-actions:check-pr-reviews'); @@ -41,10 +44,14 @@ export default async function (): Promise { throw new Error('PR has not been reviewed correctly - not all reviewers have approved'); } - await publishCommentAndRemovePrevious( - ':white_check_mark: PR review status - All reviews completed and approved!', - PULL_REQUEST_MESSAGE_KEYWORD, + const daysSinceOldestApprovedReview = differenceInCalendarDays( + new Date().toISOString(), + reviews.oldestApprovedReviewDate, ); + if (Math.abs(daysSinceOldestApprovedReview) > MAXIMUM_DAYS_SINCE_REVIEW) { + setFailed(`PR has stale review`); + throw new Error('PR has stale review'); + } log('Action end'); } diff --git a/src/github-api/index-reviews.spec.ts b/src/github-api/index-reviews.spec.ts index 3656ffb..c83c29e 100644 --- a/src/github-api/index-reviews.spec.ts +++ b/src/github-api/index-reviews.spec.ts @@ -3,13 +3,13 @@ import { strict as assert } from 'node:assert'; import { describe, it } from '@jest/globals'; +import { differenceInCalendarDays } from '@checkdigit/time'; import gitHubNock, { createGithubEventFile } from '../nocks/github.test'; import { approvedReviews, haveAllReviewersReviewed } from './index'; describe('github review', () => { it('review two outstanding reviewers', async () => { - // setGlobalDispatcher(gitHubNock); gitHubNock(); process.env['GITHUB_REPOSITORY'] = 'checkdigit/previewOutstanding'; process.env['GITHUB_TOKEN'] = 'token 0000000000000000000000000000000000000001'; @@ -19,12 +19,30 @@ describe('github review', () => { }); it('No outstanding reviewers and all approved reviews', async () => { - // setGlobalDispatcher(gitHubNock); gitHubNock(); process.env['GITHUB_REPOSITORY'] = 'checkdigit/preview'; process.env['GITHUB_TOKEN'] = 'token 0000000000000000000000000000000000000001'; process.env['GITHUB_EVENT_PATH'] = await createGithubEventFile(); const result = await approvedReviews(); - assert.deepEqual(result, { approvedReviews: 2, totalReviewers: 2 }); + assert.equal(result.approvedReviews, 2); + assert.equal(result.totalReviewers, 2); + }); + + it('Ensure reviews arent stale', async () => { + gitHubNock(); + process.env['GITHUB_REPOSITORY'] = 'checkdigit/previewOldReviews'; + process.env['GITHUB_TOKEN'] = 'token 0000000000000000000000000000000000000001'; + process.env['GITHUB_EVENT_PATH'] = await createGithubEventFile(); + const result = await approvedReviews(); + assert.equal(result.approvedReviews, 3); + assert.equal(result.totalReviewers, 3); + assert.equal(result.oldestApprovedReviewDate, '2023-10-01T01:03:30Z'); + + // Ensure the oldest review is more than 90 days old + const daysSinceOldestApprovedReview = differenceInCalendarDays( + new Date().toISOString(), + result.oldestApprovedReviewDate, + ); + assert.ok(daysSinceOldestApprovedReview > 90); }); }); diff --git a/src/github-api/index.ts b/src/github-api/index.ts index caf2c7c..18d888c 100644 --- a/src/github-api/index.ts +++ b/src/github-api/index.ts @@ -16,6 +16,7 @@ export interface GithubConfigurationResponse { export interface GithubReviewStatus { approvedReviews: number; totalReviewers: number; + oldestApprovedReviewDate: string; } export interface RequestedReviewers { @@ -24,6 +25,7 @@ export interface RequestedReviewers { login?: string; }; state: string; + submitted_at: string; } export interface PullRequestState { @@ -242,6 +244,8 @@ export async function approvedReviews(): Promise { const reviewState: Record = {}; + const dateOfApprovedReviews: string[] = []; + for (const review of requestedReviewers) { if (review.user?.login === undefined || review.user.login === '') { throw new Error(THROW_ACTION_ERROR_MESSAGE); @@ -262,6 +266,14 @@ export async function approvedReviews(): Promise { } else { reviewState[review.user.login] = [review.state]; } + if (review.state === 'APPROVED') { + dateOfApprovedReviews.push(review.submitted_at); + } + } + + const oldestApprovedReviewDate = dateOfApprovedReviews.sort()[0]; + if (oldestApprovedReviewDate === undefined) { + throw new Error('Invalid date received from approved reviews'); } const approvedReviewsList = Object.keys(reviewState).filter((user) => { @@ -269,5 +281,9 @@ export async function approvedReviews(): Promise { return reviews && reviews.includes('APPROVED'); }); - return { approvedReviews: approvedReviewsList.length, totalReviewers: Object.keys(reviewState).length }; + return { + approvedReviews: approvedReviewsList.length, + totalReviewers: Object.keys(reviewState).length, + oldestApprovedReviewDate, + }; } diff --git a/src/nocks/github.test.ts b/src/nocks/github.test.ts index c6f3477..8e76de2 100644 --- a/src/nocks/github.test.ts +++ b/src/nocks/github.test.ts @@ -78,6 +78,23 @@ export default function (options?: GithubNock): void { ], })); + nock('https://api.github.com/') + .persist() + .get(`/repos/checkdigit/previewOldReviews/pulls/${PR_NUMBER_DEFAULT}/requested_reviewers`) + .reply(200, () => ({ + users: [ + { + login: 'commituser4', + }, + { + login: 'commituser5', + }, + { + login: 'commituser6', + }, + ], + })); + nock('https://api.github.com/') .persist() .get(`/repos/checkdigit/preview/pulls/${PR_NUMBER_DEFAULT}/reviews`) @@ -89,6 +106,8 @@ export default function (options?: GithubNock): void { }, body: 'body string', state: 'COMMENTED', + // eslint-disable-next-line camelcase + submitted_at: new Date().toISOString(), }, { id: '1234prReviewPull', @@ -97,6 +116,8 @@ export default function (options?: GithubNock): void { }, body: 'body string', state: 'COMMENTED', + // eslint-disable-next-line camelcase + submitted_at: new Date().toISOString(), }, { id: '1234prReviewPull', @@ -105,6 +126,8 @@ export default function (options?: GithubNock): void { }, body: 'body string', state: 'APPROVED', + // eslint-disable-next-line camelcase + submitted_at: new Date().toISOString(), }, { id: '1234prReviewPull', @@ -113,6 +136,8 @@ export default function (options?: GithubNock): void { }, body: 'body string', state: 'CHANGES_REQUESTED', + // eslint-disable-next-line camelcase + submitted_at: new Date().toISOString(), }, { id: '1234prReviewPull', @@ -121,6 +146,54 @@ export default function (options?: GithubNock): void { }, body: 'body string', state: 'APPROVED', + // eslint-disable-next-line camelcase + submitted_at: new Date().toISOString(), + }, + ]); + + nock('https://api.github.com/') + .persist() + .get(`/repos/checkdigit/previewOldReviews/pulls/${PR_NUMBER_DEFAULT}/reviews`) + .reply(200, () => [ + { + id: '1234prReviewPull', + user: { + login: 'commituser4', + }, + body: 'body string', + state: 'APPROVED', + // eslint-disable-next-line camelcase + submitted_at: new Date().toISOString(), + }, + { + id: '1234prReviewPull', + user: { + login: 'commituser5', + }, + body: 'body string', + state: 'CHANGES_REQUESTED', + // eslint-disable-next-line camelcase + submitted_at: '2023-09-03T01:03:30Z', + }, + { + id: '1234prReviewPull', + user: { + login: 'commituser5', + }, + body: 'body string', + state: 'APPROVED', + // eslint-disable-next-line camelcase + submitted_at: new Date().toISOString(), + }, + { + id: '1234prReviewPull', + user: { + login: 'commituser6', + }, + body: 'body string', + state: 'APPROVED', + // eslint-disable-next-line camelcase + submitted_at: '2023-10-01T01:03:30Z', }, ]); @@ -136,6 +209,18 @@ export default function (options?: GithubNock): void { }, })); + nock('https://api.github.com/') + .persist() + .get(`/repos/checkdigit/previewOldReviews/pulls/${PR_NUMBER_DEFAULT}`) + .reply(200, () => ({ + head: { + sha: '1234', + }, + user: { + login: 'commituser1', + }, + })); + // return label nock('https://api.github.com/') .persist() From b8197545fc0d1daeeb5d23a79815dee031ab60d6 Mon Sep 17 00:00:00 2001 From: David Date: Tue, 3 Jun 2025 20:29:42 +1000 Subject: [PATCH 2/3] Created test to exercise all of check-pr-reviews --- src/check-pr-reviews/check-pr-reviews.spec.ts | 20 +++++++++++++++++++ src/nocks/github.test.ts | 15 ++++---------- 2 files changed, 24 insertions(+), 11 deletions(-) create mode 100644 src/check-pr-reviews/check-pr-reviews.spec.ts diff --git a/src/check-pr-reviews/check-pr-reviews.spec.ts b/src/check-pr-reviews/check-pr-reviews.spec.ts new file mode 100644 index 0000000..12ea2a4 --- /dev/null +++ b/src/check-pr-reviews/check-pr-reviews.spec.ts @@ -0,0 +1,20 @@ +// check-pr-reviews/check-pr-reviews.spec.ts + +import { strict as assert } from 'node:assert'; + +import { describe, it } from '@jest/globals'; + +import gitHubNock, { createGithubEventFile } from '../nocks/github.test'; +import checkPRReviews from './check-pr-reviews'; + +describe('check pr reviews', () => { + gitHubNock(); + it('Test basic patch', async () => { + process.env['GITHUB_REPOSITORY'] = 'checkdigit/previewOldReviews'; + process.env['GITHUB_TOKEN'] = 'token 0000000000000000000000000000000000000001'; + process.env['GITHUB_EVENT_PATH'] = await createGithubEventFile(); + + // ensure it throws with the correct error + await assert.rejects(checkPRReviews(), /PR has stale review/u); + }); +}); diff --git a/src/nocks/github.test.ts b/src/nocks/github.test.ts index 8e76de2..9f1d29b 100644 --- a/src/nocks/github.test.ts +++ b/src/nocks/github.test.ts @@ -82,17 +82,7 @@ export default function (options?: GithubNock): void { .persist() .get(`/repos/checkdigit/previewOldReviews/pulls/${PR_NUMBER_DEFAULT}/requested_reviewers`) .reply(200, () => ({ - users: [ - { - login: 'commituser4', - }, - { - login: 'commituser5', - }, - { - login: 'commituser6', - }, - ], + users: [], })); nock('https://api.github.com/') @@ -221,6 +211,9 @@ export default function (options?: GithubNock): void { }, })); + // allow POST of comments to the PRs + nock('https://api.github.com/').persist().post('/repos/checkdigit/previewOldReviews/issues/1/comments').reply(200); + // return label nock('https://api.github.com/') .persist() From f837470445f154ca546dbbfeb6535d578573db6e Mon Sep 17 00:00:00 2001 From: David Date: Tue, 3 Jun 2025 20:31:18 +1000 Subject: [PATCH 3/3] Update version --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 7a2befd..51e0ac5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@checkdigit/github-actions", - "version": "2.4.1", + "version": "2.4.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@checkdigit/github-actions", - "version": "2.4.1", + "version": "2.4.2", "license": "MIT", "dependencies": { "@actions/core": "^1.10.1", diff --git a/package.json b/package.json index 3d0536e..c0411cf 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@checkdigit/github-actions", - "version": "2.4.1", + "version": "2.4.2", "description": " Provides supporting operations for github action builds.", "author": "Check Digit, LLC", "license": "MIT",