Skip to content
Open
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
187 changes: 187 additions & 0 deletions .github/tests/auto-reviewers.test.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
const assert = require('node:assert/strict');
const { readFileSync } = require('node:fs');
const { resolve } = require('node:path');
const { test } = require('node:test');

const workflow = readFileSync(resolve(__dirname, '..', 'workflows', 'auto-reviewers.yml'), 'utf8');
const blocks = [...workflow.matchAll(/ script: \|\r?\n((?: [^\r\n]*(?:\r?\n|$)|\r?\n)+)/g)];
assert.equal(blocks.length, 2, 'Expected the team-reading and reviewer-requesting scripts.');
const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor;
const scripts = blocks.map(match => new AsyncFunction(
'github', 'context', 'core', 'process',
match[1].replace(/^ /gm, '')
));
const context = {
repo: { owner: 'mini-software', repo: 'MiniExcel' },
payload: { pull_request: { number: 1040 } }
};

function harness(options = {}) {
const logs = [];
const core = {
info: message => logs.push(message),
notice: message => logs.push(message)
};
const requested = (options.requested ?? []).map(login => ({ login }));
const reviews = (options.reviewed ?? []).map(login => ({ user: { login } }));
const batches = [];
const pull = {
state: 'open',
draft: false,
base: { ref: 'master' },
user: { login: 'author' },
...options.pull
};
const listReviews = async () => {};
const github = {
rest: {
pulls: {
get: async params => {
assert.ok(Number.isSafeInteger(params.pull_number));
return { data: pull };
},
listRequestedReviewers: async () => ({ data: { users: requested } }),
listReviews,
requestReviewers: async ({ reviewers }) => {
if (options.error) {
throw options.error;
}
batches.push(reviewers);
if (options.omit) {
return;
}
if (options.reviewImmediately) {
reviews.push(...reviewers.map(login => ({ user: { login } })));
} else {
requested.push(...reviewers.map(login => ({ login })));
}
}
}
},
paginate: async (method, params) => {
assert.equal(method, listReviews);
assert.equal(params.per_page, 100);
return [...reviews];
}
};
const run = (members, customContext = context, input = '') => scripts[1](
github, customContext, core,
{ env: { TEAM_REVIEWERS: JSON.stringify(members), INPUT_PR_NUMBER: input } }
);
return { run, logs, batches };
}

test('team reader requires an explicit organization credential', async () => {
await assert.rejects(scripts[0]({}, context, {}, { env: {} }), /CORE_REVIEWERS_TOKEN/);
});

test('team reader dynamically paginates active members and excludes bots', async () => {
const listMembersInOrg = async () => {};
const github = {
rest: { teams: { listMembersInOrg } },
paginate: async (method, params) => {
assert.equal(method, listMembersInOrg);
assert.deepEqual(params, {
org: 'mini-software', team_slug: 'core-reviewers', per_page: 100
});
return [{ login: 'new-member', type: 'User' }, { login: 'bot', type: 'Bot' }];
}
};
const result = await scripts[0](github, context, { info() {} }, {
env: { CORE_REVIEWERS_TOKEN: 'test-only' }
});
assert.deepEqual(JSON.parse(result), ['new-member']);
github.paginate = async () => [];
await assert.rejects(scripts[0](github, context, {}, {
env: { CORE_REVIEWERS_TOKEN: 'test-only' }
}), /no active human members/);
});

test('excludes the author, requested users, reviewed users and duplicate names case-insensitively', async () => {
const h = harness({ requested: ['requested'], reviewed: ['reviewed'] });
await h.run(['AUTHOR', 'REQUESTED', 'REVIEWED', 'new-member', 'NEW-MEMBER']);
assert.deepEqual(h.batches, [['new-member']]);
});

test('requests all twenty eligible members in batches of at most fifteen', async () => {
const members = Array.from({ length: 20 }, (_, index) => `member-${index}`);
const h = harness();
await h.run(members);
assert.deepEqual(h.batches.map(batch => batch.length), [15, 5]);
assert.deepEqual(h.batches.flat(), members);
});

test('repeated events do not send duplicate review requests', async () => {
const h = harness();
await h.run(['first', 'second']);
await h.run(['first', 'second', 'new-member']);
assert.deepEqual(h.batches, [['first', 'second'], ['new-member']]);
});

test('reports when no additional reviewer is eligible', async () => {
const h = harness();
await h.run(['author']);
assert.deepEqual(h.batches, []);
assert.match(h.logs[0], /already been requested or reviewed/);
});

test('skips closed, draft and non-master PRs explicitly', async () => {
for (const pull of [{ state: 'closed' }, { draft: true }, { base: { ref: 'other' } }]) {
const h = harness({ pull });
await h.run(['reviewer']);
assert.deepEqual(h.batches, []);
assert.match(h.logs[0], /Skipping/);
}
});

test('manual dispatch accepts a PR number and rejects invalid inputs', async () => {
const manualContext = { ...context, payload: {} };
const h = harness();
await h.run(['reviewer'], manualContext, '1040');
assert.deepEqual(h.batches, [['reviewer']]);
for (const input of ['', '-1', '0', '1.5', 'text', '9007199254740992']) {
await assert.rejects(h.run(['reviewer'], manualContext, input), /positive integer/);
}
});

test('API permission failures propagate instead of reporting success', async () => {
const error = new Error('Resource not accessible by integration');
const h = harness({ error });
await assert.rejects(h.run(['reviewer']), error);
assert.deepEqual(h.logs, []);
});

test('silently omitted API requests fail verification', async () => {
const h = harness({ omit: true });
await assert.rejects(h.run(['reviewer']), /did not confirm review requests for: reviewer/);
assert.deepEqual(h.logs, []);
});

test('review completed during the request is counted as confirmed', async () => {
const h = harness({ reviewImmediately: true });
await h.run(['reviewer']);
assert.match(h.logs[0], /Requested 1/);
});

test('malformed team output fails instead of treating it as an empty list', async () => {
await assert.rejects(scripts[1](
{
rest: { pulls: {
get: async () => ({ data: {
state: 'open', draft: false, base: { ref: 'master' }, user: { login: 'author' }
} }),
listRequestedReviewers: async () => ({ data: { users: [] } }),
listReviews: async () => {}
} },
paginate: async () => []
},
context, {}, { env: { TEAM_REVIEWERS: 'not-json' } }
), SyntaxError);
});

test('fork-capable workflow never checks out or executes PR code', () => {
assert.match(workflow, /pull_request_target:/);
assert.match(workflow, /pull-requests: write/);
assert.doesNotMatch(workflow, /actions\/checkout|head\.sha|head\.ref|pull_request\.title|pull_request\.body/);
assert.match(workflow, /github-token: \$\{\{ secrets\.CORE_REVIEWERS_TOKEN \}\}/);
});
208 changes: 208 additions & 0 deletions .github/tests/review-approval-policy.test.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
const assert = require('node:assert/strict');
const { readFileSync } = require('node:fs');
const { resolve } = require('node:path');
const { test } = require('node:test');

const workflow = readFileSync(resolve(__dirname, '..', 'workflows', 'review-approval-policy.yml'), 'utf8');
const signal = readFileSync(resolve(__dirname, '..', 'workflows', 'review-changed.yml'), 'utf8');
const block = workflow.match(/ script: \|\r?\n((?: [^\r\n]*(?:\r?\n|$)|\r?\n)+)/);
assert.ok(block, 'Expected an embedded approval policy script.');
const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor;
const evaluate = new AsyncFunction('github', 'context', 'core', block[1].replace(/^ /gm, ''));

function pull(number = 1, labels = [], sha = `sha-${number}`) {
return {
number, draft: false, head: { sha },
user: { login: 'author' }, labels: labels.map(name => ({ name }))
};
}

function review(id, login, state = 'APPROVED', type = 'User') {
return { id, state, user: { login, type } };
}

function harness(pulls, reviews = {}, options = {}) {
const statuses = [];
const logs = [];
const permissionCalls = [];
const list = async () => {};
const listReviews = async () => {};
const github = {
rest: {
pulls: { list, listReviews },
repos: {
createCommitStatus: async status => {
statuses.push(status);
},
getCollaboratorPermissionLevel: async ({ username }) => {
permissionCalls.push(username);
if (options.permissionError) {
throw options.permissionError;
}
return { data: { permission: options.permissions?.[username] ?? 'write' } };
}
}
},
paginate: async (method, params) => {
assert.equal(params.per_page, 100);
if (method === list) {
assert.equal(params.base, 'master');
assert.equal(params.state, 'open');
return pulls;
}
assert.equal(method, listReviews);
if (options.reviewError) {
throw options.reviewError;
}
return [...(reviews[params.pull_number] ?? [])];
}
};
const context = {
repo: { owner: 'mini-software', repo: 'MiniExcel' },
serverUrl: 'https://github.com', runId: 123,
payload: { workflow_run: { pull_requests: [], head_repository: { full_name: 'untrusted/fork' } } }
};
const run = () => evaluate(github, context, { info: message => logs.push(message) });
return { run, statuses, logs, permissionCalls };
}

test('ordinary PR requires exactly one eligible approval', async () => {
for (const [reviews, expected] of [[[], 'pending'], [[review(1, 'reviewer')], 'success']]) {
const h = harness([pull()], { 1: reviews });
await h.run();
assert.equal(h.statuses[0].state, 'pending');
assert.equal(h.statuses.at(-1).state, expected);
assert.equal(h.statuses.at(-1).context, 'Review approval policy');
}
});

test('major-change PR remains blocked at two and passes at three approvals', async () => {
for (const count of [0, 1, 2, 3]) {
const h = harness([pull(1, ['MAJOR-CHANGE'])], {
1: Array.from({ length: count }, (_, index) => review(index + 1, `reviewer-${index}`))
});
await h.run();
assert.equal(h.statuses.at(-1).state, count === 3 ? 'success' : 'pending');
if (count < 3) {
assert.match(h.statuses.at(-1).description, new RegExp(`${count}/3`));
}
}
});

test('invited users, comments, author approvals, bots and Read users do not count', async () => {
const h = harness([pull(1, ['major-change'])], {
1: [
review(1, 'AUTHOR'), review(2, 'bot', 'APPROVED', 'Bot'),
review(3, 'read-user'), review(4, 'commenter', 'COMMENTED'),
review(5, 'valid')
]
}, { permissions: { 'read-user': 'read' } });
await h.run();
assert.match(h.statuses.at(-1).description, /1\/3/);
assert.deepEqual(h.permissionCalls, ['read-user', 'valid']);
});

test('counts Write, Maintain and Admin approvals', async () => {
const h = harness([pull(1, ['major-change'])], {
1: [review(1, 'writer'), review(2, 'maintainer'), review(3, 'admin')]
}, { permissions: { writer: 'write', maintainer: 'maintain', admin: 'admin' } });
await h.run();
assert.equal(h.statuses.at(-1).state, 'success');
});

test('later dismissed or changes-requested reviews supersede earlier approvals', async () => {
for (const state of ['DISMISSED', 'CHANGES_REQUESTED']) {
const h = harness([pull()], {
1: [review(2, 'REVIEWER', state), review(1, 'reviewer')]
});
await h.run();
assert.match(h.statuses.at(-1).description, /0\/1/);
}
});

test('comments do not cancel approvals and duplicate approvals count only once', async () => {
const h = harness([pull(1, ['major-change'])], {
1: [review(1, 'reviewer'), review(2, 'REVIEWER'), review(3, 'Reviewer', 'COMMENTED')]
});
await h.run();
assert.match(h.statuses.at(-1).description, /1\/3/);
});

test('a new approval after a change request is counted', async () => {
const h = harness([pull()], {
1: [review(1, 'reviewer', 'CHANGES_REQUESTED'), review(2, 'reviewer')]
});
await h.run();
assert.equal(h.statuses.at(-1).state, 'success');
});

test('a label change raises the requirement without trusting old success', async () => {
const current = pull();
const h = harness([current], { 1: [review(1, 'reviewer')] });
await h.run();
assert.equal(h.statuses.at(-1).state, 'success');
current.labels.push({ name: 'major-change' });
await h.run();
assert.equal(h.statuses.at(-2).state, 'pending');
assert.match(h.statuses.at(-1).description, /1\/3/);
});

test('PRs sharing a SHA cannot overwrite a stricter PR status with ordinary success', async () => {
const h = harness([pull(1, [], 'shared'), pull(2, ['major-change'], 'shared')], {
1: [review(1, 'one')],
2: [review(2, 'one'), review(3, 'two')]
});
await h.run();
assert.equal(h.statuses.length, 2);
assert.equal(h.statuses.at(-1).sha, 'shared');
assert.match(h.statuses.at(-1).description, /#2: 2\/3/);
assert.deepEqual(h.permissionCalls, ['one', 'two']);
});

test('all PRs sharing a SHA must satisfy their own approval requirements', async () => {
const h = harness([pull(1, [], 'shared'), pull(2, ['major-change'], 'shared')], {
1: [review(1, 'one')],
2: [review(2, 'one'), review(3, 'two'), review(4, 'three')]
});
await h.run();
assert.equal(h.statuses.at(-1).state, 'success');
});

test('review and permission API failures leave checks pending and propagate', async () => {
for (const option of ['reviewError', 'permissionError']) {
const error = new Error('GitHub API access failed');
const h = harness([pull()], { 1: [review(1, 'reviewer')] }, { [option]: error });
await assert.rejects(h.run(), error);
assert.deepEqual(h.statuses.map(status => status.state), ['pending']);
}
});

test('draft PRs are skipped and no open PRs are reported explicitly', async () => {
const draft = pull();
draft.draft = true;
for (const pulls of [[], [draft]]) {
const h = harness(pulls);
await h.run();
assert.deepEqual(h.statuses, []);
assert.match(h.logs[0], /No open/);
}
});

test('commit status descriptions obey the GitHub length limit', async () => {
const h = harness(Array.from({ length: 20 }, (_, index) => pull(index + 1, ['major-change'], 'shared')));
await h.run();
assert.equal(h.statuses.at(-1).description.length, 140);
});

test('trusted workflow uses no PR code, artifact content, team credential or write-capable review trigger', () => {
assert.match(workflow, /pull_request_target:/);
assert.match(workflow, /workflow_run:/);
assert.match(workflow, /workflows: \[PR review changed\]/);
assert.match(workflow, /statuses: write/);
assert.match(workflow, /pull-requests: read/);
assert.doesNotMatch(workflow, /actions\/checkout|\$\{\{[^}]*head\.(?:sha|ref)|download-artifact|secrets\.|pull_request_review:/);
assert.match(signal, /pull_request_review:/);
assert.match(signal, /types: \[submitted, dismissed\]/);
assert.match(signal, /permissions: \{\}/);
assert.doesNotMatch(signal, /actions\/checkout|\$\{\{|secrets\.|write/);
});
Loading
Loading