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
14 changes: 12 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand All @@ -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",
Expand Down
20 changes: 20 additions & 0 deletions src/check-pr-reviews/check-pr-reviews.spec.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
13 changes: 10 additions & 3 deletions src/check-pr-reviews/check-pr-reviews.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -41,10 +44,14 @@ export default async function (): Promise<void> {
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');
}
24 changes: 21 additions & 3 deletions src/github-api/index-reviews.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this test, or maybe I don't understand the code. But the implementation throws an error, shouldn't we be checking for that somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's how the tests were originally put together which does seem odd, I can't recall the history here.
I've created another test file that exercises the larger action.

const daysSinceOldestApprovedReview = differenceInCalendarDays(
new Date().toISOString(),
result.oldestApprovedReviewDate,
);
assert.ok(daysSinceOldestApprovedReview > 90);
});
});
18 changes: 17 additions & 1 deletion src/github-api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export interface GithubConfigurationResponse {
export interface GithubReviewStatus {
approvedReviews: number;
totalReviewers: number;
oldestApprovedReviewDate: string;
}

export interface RequestedReviewers {
Expand All @@ -24,6 +25,7 @@ export interface RequestedReviewers {
login?: string;
};
state: string;
submitted_at: string;
}

export interface PullRequestState {
Expand Down Expand Up @@ -242,6 +244,8 @@ export async function approvedReviews(): Promise<GithubReviewStatus> {

const reviewState: Record<string, string[]> = {};

const dateOfApprovedReviews: string[] = [];

for (const review of requestedReviewers) {
if (review.user?.login === undefined || review.user.login === '') {
throw new Error(THROW_ACTION_ERROR_MESSAGE);
Expand All @@ -262,12 +266,24 @@ export async function approvedReviews(): Promise<GithubReviewStatus> {
} 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) => {
const reviews = reviewState[user];
return reviews && reviews.includes('APPROVED');
});

return { approvedReviews: approvedReviewsList.length, totalReviewers: Object.keys(reviewState).length };
return {
approvedReviews: approvedReviewsList.length,
totalReviewers: Object.keys(reviewState).length,
oldestApprovedReviewDate,
};
}
78 changes: 78 additions & 0 deletions src/nocks/github.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ 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: [],
}));

nock('https://api.github.com/')
.persist()
.get(`/repos/checkdigit/preview/pulls/${PR_NUMBER_DEFAULT}/reviews`)
Expand All @@ -89,6 +96,8 @@ export default function (options?: GithubNock): void {
},
body: 'body string',
state: 'COMMENTED',
// eslint-disable-next-line camelcase
submitted_at: new Date().toISOString(),
},
{
id: '1234prReviewPull',
Expand All @@ -97,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',
Expand All @@ -105,6 +116,8 @@ export default function (options?: GithubNock): void {
},
body: 'body string',
state: 'APPROVED',
// eslint-disable-next-line camelcase
submitted_at: new Date().toISOString(),
},
{
id: '1234prReviewPull',
Expand All @@ -113,14 +126,64 @@ export default function (options?: GithubNock): void {
},
body: 'body string',
state: 'CHANGES_REQUESTED',
// eslint-disable-next-line camelcase
submitted_at: new Date().toISOString(),
},
{
id: '1234prReviewPull',
user: {
login: 'commituser4',
},
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',
},
]);

Expand All @@ -136,6 +199,21 @@ 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',
},
}));

// 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()
Expand Down
Loading