diff --git a/components/git/metadata.js b/components/git/metadata.js index 60da097f..7f974b53 100644 --- a/components/git/metadata.js +++ b/components/git/metadata.js @@ -36,6 +36,10 @@ const options = { describe: 'Check for \'LGTM\' in comments', type: 'boolean' }, + json: { + describe: 'Print metadata and PR readiness result as JSON', + type: 'boolean' + }, 'max-commits': { describe: 'Number of commits to warn', type: 'number', @@ -45,6 +49,29 @@ const options = { let yargsInstance; +function writeStdout(text) { + return new Promise((resolve, reject) => { + process.stdout.write(text, (error) => { + if (error) { + reject(error); + } else { + resolve(); + } + }); + }); +} + +export async function writeMetadataJsonResult(metadataPromise) { + try { + const { json } = await metadataPromise; + await writeStdout(`${JSON.stringify(json, null, 2)}\n`); + process.exitCode = json.exitCode; + } catch (error) { + console.error(error); + process.exitCode = 1; + } +} + export function builder(yargs) { yargsInstance = yargs; return yargs @@ -81,10 +108,16 @@ export function handler(argv) { return yargsInstance.showHelp(); } - const logStream = process.stdout.isTTY ? process.stdout : process.stderr; + const logStream = argv.json || !process.stdout.isTTY + ? process.stderr + : process.stdout; const cli = new CLI(logStream); const merged = Object.assign({}, argv, parsed, config); + if (argv.json) { + return writeMetadataJsonResult(getMetadata(merged, false, cli)); + } + return runPromise(getMetadata(merged, false, cli) .then(({ status }) => { if (status === false) { diff --git a/components/metadata.js b/components/metadata.js index 156341f8..136113ce 100644 --- a/components/metadata.js +++ b/components/metadata.js @@ -4,9 +4,71 @@ import Request from '../lib/request.js'; import auth from '../lib/auth.js'; import PRData from '../lib/pr_data.js'; import PRSummary from '../lib/pr_summary.js'; -import PRChecker from '../lib/pr_checker.js'; +import PRChecker, { + PR_CHECK_REASON_CODES +} from '../lib/pr_checker.js'; import MetadataGenerator from '../lib/metadata_gen.js'; +export const METADATA_READINESS = Object.freeze({ + READY: 'ready', + DEFERRABLE: 'deferrable', + FAILED: 'failed' +}); + +export const METADATA_EXIT_CODES = Object.freeze({ + READY: 0, + DEFERRABLE: 20, + FAILED: 40 +}); + +const DEFERRABLE_REASON_CODES = new Set([ + PR_CHECK_REASON_CODES.WAIT_TIME +]); + +export function classifyMetadataReadiness(ready, reasonCodes) { + if (ready) { + return METADATA_READINESS.READY; + } + + if (reasonCodes.length > 0 && + reasonCodes.every((code) => DEFERRABLE_REASON_CODES.has(code))) { + return METADATA_READINESS.DEFERRABLE; + } + + return METADATA_READINESS.FAILED; +} + +export function getMetadataExitCode(readiness) { + switch (readiness) { + case METADATA_READINESS.READY: + return METADATA_EXIT_CODES.READY; + case METADATA_READINESS.DEFERRABLE: + return METADATA_EXIT_CODES.DEFERRABLE; + default: + return METADATA_EXIT_CODES.FAILED; + } +} + +export function formatMetadataResult({ status, data, metadata, checker }) { + const reasonCodes = [...new Set(checker.reasons.map(({ code }) => code))]; + const readiness = classifyMetadataReadiness(status, reasonCodes); + const exitCode = getMetadataExitCode(readiness); + return { + ready: status, + readiness, + exitCode, + pullRequest: { + owner: data.owner, + repo: data.repo, + number: data.prid, + url: data.pr.url + }, + metadata, + reasonCodes, + reasons: checker.reasons + }; +} + export async function getMetadata(argv, skipRefs, cli) { const credentials = await auth({ github: true, @@ -22,7 +84,7 @@ export async function getMetadata(argv, skipRefs, cli) { summary.display(); const metadata = new MetadataGenerator({ skipRefs, ...data }).getMetadata(); - if (!process.stdout.isTTY) { + if (!argv.json && !process.stdout.isTTY) { process.stdout.write(metadata); } @@ -33,17 +95,23 @@ export async function getMetadata(argv, skipRefs, cli) { cli.stopSpinner(`Done writing metadata to ${argv.file}`); } - cli.separator('Generated metadata'); - cli.write(metadata); - cli.separator(); + if (!argv.json) { + cli.separator('Generated metadata'); + cli.write(metadata); + cli.separator(); + } const checker = new PRChecker(cli, data, request, argv); const status = await checker.checkAll(argv.checkComments, argv.checkCI); - return { + const result = { status, request, data, metadata, checker }; + return { + ...result, + json: formatMetadataResult(result) + }; }; diff --git a/docs/git-node.md b/docs/git-node.md index 7261f8b5..dc93303a 100644 --- a/docs/git-node.md +++ b/docs/git-node.md @@ -291,6 +291,7 @@ Options: --file, -f File to write the metadata in [string] --readme Path to file that contains collaborator contacts [string] --check-comments Check for 'LGTM' in comments [boolean] + --json Print metadata and PR readiness result as JSON [boolean] --max-commits Number of commits to warn [number] [default: 3] ``` @@ -309,6 +310,9 @@ $ git node metadata $PRID -o nodejs -r node # Or, redirect the metadata to a file while see the checks in stderr $ git node metadata $PRID > msg.txt +# Or, get machine-readable metadata and readiness reason codes +$ git node metadata $PRID --json + # Using it to amend commit messages: $ git node metadata $PRID -f msg.txt $ echo -e "$(git show -s --format=%B)\n\n$(cat msg.txt)" > msg.txt @@ -319,6 +323,26 @@ $ git commit --amend -F msg.txt git node metadata 167 --repo llnode --readme ../node/README.md ``` +When `--json` is used, stdout contains a JSON object and progress/check output +is written to stderr. The command still exits non-zero when the pull request is +not ready to land. The JSON includes `ready`, `readiness`, `exitCode`, +`metadata`, `reasonCodes`, and `reasons`. `reasonCodes` is a de-duplicated list +of stable machine-readable codes such as `missing-approval`, +`missing-tsc-approval`, `wait-time`, `missing-github-ci`, `pending-github-ci`, +`conflict`, `requested-changes`, and `stale-review`. + +The metadata JSON exit-code contract is: + +- `0`: the pull request is ready +- `20`: the pull request is not ready, but only for the deferrable metadata + reason currently owned by the lightweight queue selector: `wait-time` +- `40`: the pull request is not ready for a hard or mixed reason and should be + handled by the existing landing/failure path + +Exit codes `20`-`29` are reserved for future deferrable metadata states, and +`40`-`49` are reserved for future hard metadata failure states. Unexpected tool +or runtime failures continue to use exit code `1`. + ### Optional Settings diff --git a/lib/pr_checker.js b/lib/pr_checker.js index d722b607..99d9ea44 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -29,6 +29,30 @@ const FAST_TRACK_RE = /^Fast-track has been requested by @(.+?)\. Please 👍 to const FAST_TRACK_MIN_APPROVALS = 2; const GIT_CONFIG_GUIDE_URL = 'https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork'; +export const PR_CHECK_REASON_CODES = Object.freeze({ + CANCELLED_GITHUB_CI: 'cancelled-github-ci', + CLOSED: 'closed', + CONFLICT: 'conflict', + FAILED_GITHUB_CI: 'failed-github-ci', + FAILED_JENKINS_CI: 'failed-jenkins-ci', + INVALID_CI_TYPE: 'invalid-ci-type', + MERGED: 'merged', + MISSING_APPROVAL: 'missing-approval', + MISSING_FAST_TRACK_COMMENT: 'missing-fast-track-comment', + MISSING_FULL_JENKINS_CI: 'missing-full-jenkins-ci', + MISSING_GITHUB_CI: 'missing-github-ci', + MISSING_JENKINS_CI: 'missing-jenkins-ci', + MISSING_TSC_APPROVAL: 'missing-tsc-approval', + NEW_CONTRIBUTOR: 'new-contributor', + NO_COMMITS: 'no-commits', + PENDING_GITHUB_CI: 'pending-github-ci', + PENDING_JENKINS_CI: 'pending-jenkins-ci', + REQUESTED_CHANGES: 'requested-changes', + STALE_CI: 'stale-ci', + STALE_REVIEW: 'stale-review', + WAIT_TIME: 'wait-time' +}); + export default class PRChecker { /** * @param {{}} cli @@ -51,6 +75,22 @@ export default class PRChecker { this.reviews = reviews; this.commits = commits; this.argv = argv; + this.reasons = []; + } + + addReason(code, message, details = {}) { + this.reasons.push({ + code, + message, + ...details + }); + } + + getResult(status) { + return { + ready: status, + reasons: this.reasons + }; } get waitTimeSingleApproval() { @@ -68,6 +108,8 @@ export default class PRChecker { } async checkAll(checkComments = false, checkCI = true) { + this.reasons = []; + const status = [ this.checkCommitsAfterReview(), this.checkReviewsAndWait(new Date(), checkComments), @@ -107,14 +149,20 @@ export default class PRChecker { displayReviews(checkComments) { const { cli, reviewers: { requestedChanges, approved } } = this; if (requestedChanges.length > 0) { - cli.error(`Requested Changes: ${requestedChanges.length}`); + const message = `Requested Changes: ${requestedChanges.length}`; + cli.error(message); + this.addReason(PR_CHECK_REASON_CODES.REQUESTED_CHANGES, message, { + count: requestedChanges.length + }); for (const { reviewer, review } of requestedChanges) { cli.error(this.formatReview(reviewer, review)); } } if (approved.length === 0) { - cli.error('Approvals: 0'); + const message = 'Approvals: 0'; + cli.error(message); + this.addReason(PR_CHECK_REASON_CODES.MISSING_APPROVAL, message); return; } @@ -166,7 +214,12 @@ export default class PRChecker { .filter((p) => p.reviewer.isTSC()) .map((p) => p.reviewer.login); if (tscApproved.length < 2) { - cli.error('semver-major requires at least 2 TSC approvals'); + const message = 'semver-major requires at least 2 TSC approvals'; + cli.error(message); + this.addReason(PR_CHECK_REASON_CODES.MISSING_TSC_APPROVAL, message, { + approvals: tscApproved.length, + required: 2 + }); return false; // 7 day rule doesn't matter here } } @@ -176,7 +229,12 @@ export default class PRChecker { const comment = [...this.comments].reverse().find((c) => FAST_TRACK_RE.test(c.bodyText)); if (!comment) { - cli.error('Unable to find the fast-track request comment.'); + const message = 'Unable to find the fast-track request comment.'; + cli.error(message); + this.addReason( + PR_CHECK_REASON_CODES.MISSING_FAST_TRACK_COMMENT, + message + ); return false; } const [, requester] = comment.bodyText.match(FAST_TRACK_RE); @@ -221,9 +279,10 @@ export default class PRChecker { if (timeLeftMulti < 0) { return true; } - cli.error( - `This PR needs to wait ${timeToText(timeLeftMulti, 'more')} to land${fastTrackAppendix}` - ); + const message = + `This PR needs to wait ${timeToText(timeLeftMulti, 'more')} to land${fastTrackAppendix}`; + cli.error(message); + this.addReason(PR_CHECK_REASON_CODES.WAIT_TIME, message); return false; } @@ -231,9 +290,12 @@ export default class PRChecker { if (timeLeftSingle < 0) { return true; } - cli.error(`This PR needs to wait ${timeToText(timeLeftSingle, 'more')} to land (or ${ + const message = + `This PR needs to wait ${timeToText(timeLeftSingle, 'more')} to land (or ${ timeToText(timeLeftMulti < 0 || isFastTracked ? 0 : timeLeftMulti) - } if there is one more approval)${fastTrackAppendix}`); + } if there is one more approval)${fastTrackAppendix}`; + cli.error(message); + this.addReason(PR_CHECK_REASON_CODES.WAIT_TIME, message); return false; } } @@ -248,8 +310,12 @@ export default class PRChecker { const providers = Object.values(CI_PROVIDERS); if (!providers.includes(ciType)) { - this.cli.error( - `Invalid ciType ${ciType} - must be one of ${providers.join(', ')}`); + const message = + `Invalid ciType ${ciType} - must be one of ${providers.join(', ')}`; + this.cli.error(message); + this.addReason(PR_CHECK_REASON_CODES.INVALID_CI_TYPE, message, { + ciType + }); return false; } @@ -273,12 +339,20 @@ export default class PRChecker { let status = true; if (!ciMap.size) { - cli.error('No Jenkins CI runs detected'); + const message = 'No Jenkins CI runs detected'; + cli.error(message); + this.addReason(PR_CHECK_REASON_CODES.MISSING_JENKINS_CI, message, { + provider: 'jenkins' + }); this.CIStatus = false; return false; } else if (!this.hasFullCI(ciMap)) { status = false; - cli.error('No full Jenkins CI runs detected'); + const message = 'No full Jenkins CI runs detected'; + cli.error(message); + this.addReason(PR_CHECK_REASON_CODES.MISSING_FULL_JENKINS_CI, message, { + provider: 'jenkins' + }); } let lastCI; @@ -310,6 +384,10 @@ export default class PRChecker { `${lastCI.typeName} CI run:`; cli.warn(warnMsg); + this.addReason(PR_CHECK_REASON_CODES.STALE_CI, warnMsg, { + commits: totalCommits, + provider: 'jenkins' + }); const sliceLength = maxCommits === 0 ? totalCommits : -maxCommits; afterCommits.slice(sliceLength) .forEach(commit => { @@ -329,13 +407,21 @@ export default class PRChecker { const { result, failures } = await build.getResults(); if (result === 'FAILURE') { - cli.error( - `${failures.length} failure(s) on the last Jenkins CI run`); + const message = + `${failures.length} failure(s) on the last Jenkins CI run`; + cli.error(message); + this.addReason(PR_CHECK_REASON_CODES.FAILED_JENKINS_CI, message, { + failures: failures.length, + provider: 'jenkins' + }); status = false; // NOTE(mmarchini): not sure why PEDING returns null } else if (result === null) { - cli.error( - 'Last Jenkins CI still running'); + const message = 'Last Jenkins CI still running'; + cli.error(message); + this.addReason(PR_CHECK_REASON_CODES.PENDING_JENKINS_CI, message, { + provider: 'jenkins' + }); status = false; } else { cli.ok('Last Jenkins CI successful'); @@ -350,7 +436,9 @@ export default class PRChecker { const { cli, commits } = this; if (!commits || commits.length === 0) { - cli.error('No commits detected'); + const message = 'No commits detected'; + cli.error(message); + this.addReason(PR_CHECK_REASON_CODES.NO_COMMITS, message); return false; } @@ -361,7 +449,11 @@ export default class PRChecker { this.CIStatus = false; const checkSuites = commit.checkSuites || { nodes: [] }; if (!commit.status && checkSuites.nodes.length === 0) { - cli.error('No GitHub CI runs detected'); + const message = 'No GitHub CI runs detected'; + cli.error(message); + this.addReason(PR_CHECK_REASON_CODES.MISSING_GITHUB_CI, message, { + provider: 'github' + }); return false; } @@ -428,13 +520,23 @@ export default class PRChecker { // Report pending jobs if (pendingJobs.length > 0) { - cli.error('GitHub CI is still running'); + const message = 'GitHub CI is still running'; + cli.error(message); + this.addReason(PR_CHECK_REASON_CODES.PENDING_GITHUB_CI, message, { + jobs: pendingJobs.length, + provider: 'github' + }); return false; } // Report failed jobs if (failedJobs.length > 0) { - cli.error(`${failedJobs.length} GitHub CI job(s) failed:`); + const message = `${failedJobs.length} GitHub CI job(s) failed:`; + cli.error(message); + this.addReason(PR_CHECK_REASON_CODES.FAILED_GITHUB_CI, message, { + jobs: failedJobs.length, + provider: 'github' + }); for (const job of failedJobs) { const urlInfo = job.url ? ` (${job.url})` : ''; cli.error(` - ${job.name}: ${job.conclusion}${urlInfo}`); @@ -443,7 +545,12 @@ export default class PRChecker { // Report cancelled jobs if (cancelledJobs.length > 0) { - cli.error(`${cancelledJobs.length} GitHub CI job(s) cancelled:`); + const message = `${cancelledJobs.length} GitHub CI job(s) cancelled:`; + cli.error(message); + this.addReason(PR_CHECK_REASON_CODES.CANCELLED_GITHUB_CI, message, { + jobs: cancelledJobs.length, + provider: 'github' + }); for (const job of cancelledJobs) { const urlInfo = job.url ? ` (${job.url})` : ''; cli.error(` - ${job.name}: ${job.conclusion}${urlInfo}`); @@ -458,12 +565,21 @@ export default class PRChecker { if (commit.status) { const { state } = commit.status; if (state === 'PENDING') { - cli.error('GitHub CI is still running'); + const message = 'GitHub CI is still running'; + cli.error(message); + this.addReason(PR_CHECK_REASON_CODES.PENDING_GITHUB_CI, message, { + provider: 'github' + }); return false; } if (!['SUCCESS', 'EXPECTED'].includes(state)) { - cli.error(`GitHub CI failed with status: ${state}`); + const message = `GitHub CI failed with status: ${state}`; + cli.error(message); + this.addReason(PR_CHECK_REASON_CODES.FAILED_GITHUB_CI, message, { + provider: 'github', + state + }); return false; } } @@ -534,7 +650,9 @@ export default class PRChecker { } const prAuthor = `${pr.author.login}(${pr.author.email})`; - cli.warn(`PR author is a new contributor: @${prAuthor}`); + const message = `PR author is a new contributor: @${prAuthor}`; + cli.warn(message); + this.addReason(PR_CHECK_REASON_CODES.NEW_CONTRIBUTOR, message); for (const c of oddCommits) { const { oid, author } = c.commit; const hash = shortSha(oid); @@ -594,7 +712,9 @@ export default class PRChecker { const { maxCommits } = argv; if (commits.length === 0) { - cli.warn('No commits detected'); + const message = 'No commits detected'; + cli.warn(message); + this.addReason(PR_CHECK_REASON_CODES.NO_COMMITS, message); return false; } @@ -603,7 +723,9 @@ export default class PRChecker { ); if (reviewIndex === -1) { - cli.warn('No approving reviews found'); + const message = 'No approving reviews found'; + cli.warn(message); + this.addReason(PR_CHECK_REASON_CODES.MISSING_APPROVAL, message); return false; } @@ -611,7 +733,11 @@ export default class PRChecker { .findLastIndex(({ commit }) => commit.oid === reviews[reviewIndex].commit.oid); if (reviewedCommitIndex !== commits.length - 1) { - cli.warn('Commits were pushed since the last approving review:'); + const message = 'Commits were pushed since the last approving review:'; + cli.warn(message); + this.addReason(PR_CHECK_REASON_CODES.STALE_REVIEW, message, { + commits: commits.length - reviewedCommitIndex - 1 + }); commits.slice(Math.max(reviewedCommitIndex + 1, commits.length - maxCommits)) .forEach(({ commit }) => { cli.warn(`- ${commit.messageHeadline}`); @@ -641,7 +767,9 @@ export default class PRChecker { } = this; if (pr.mergeable && pr.mergeable === CONFLICTING) { - cli.warn('This PR has conflicts that must be resolved'); + const message = 'This PR has conflicts that must be resolved'; + cli.warn(message); + this.addReason(PR_CHECK_REASON_CODES.CONFLICT, message); return false; } @@ -656,13 +784,17 @@ export default class PRChecker { if (merged) { const dateStr = new Date(mergedAt).toUTCString(); - cli.warn(`This PR was merged on ${dateStr}`); + const message = `This PR was merged on ${dateStr}`; + cli.warn(message); + this.addReason(PR_CHECK_REASON_CODES.MERGED, message); return false; } if (closed) { const dateStr = new Date(closedAt).toUTCString(); - cli.warn(`This PR was closed on ${dateStr}`); + const message = `This PR was closed on ${dateStr}`; + cli.warn(message); + this.addReason(PR_CHECK_REASON_CODES.CLOSED, message); return false; } diff --git a/test/unit/metadata.test.js b/test/unit/metadata.test.js new file mode 100644 index 00000000..21fc9c06 --- /dev/null +++ b/test/unit/metadata.test.js @@ -0,0 +1,164 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert'; + +import { + formatMetadataResult, + METADATA_EXIT_CODES, + METADATA_READINESS +} from '../../components/metadata.js'; +import { + writeMetadataJsonResult +} from '../../components/git/metadata.js'; +import { + PR_CHECK_REASON_CODES +} from '../../lib/pr_checker.js'; + +describe('metadata command helpers', () => { + it('formats ready metadata JSON output with unique reason codes', () => { + const reason = { + code: PR_CHECK_REASON_CODES.WAIT_TIME, + message: 'This PR needs to wait 24 more hours to land' + }; + const result = formatMetadataResult({ + status: true, + data: { + owner: 'nodejs', + repo: 'node', + prid: 12345, + pr: { + url: 'https://github.com/nodejs/node/pull/12345' + } + }, + metadata: 'PR-URL: https://github.com/nodejs/node/pull/12345\n', + checker: { + reasons: [reason, reason] + } + }); + + assert.deepStrictEqual(result, { + ready: true, + readiness: METADATA_READINESS.READY, + exitCode: METADATA_EXIT_CODES.READY, + pullRequest: { + owner: 'nodejs', + repo: 'node', + number: 12345, + url: 'https://github.com/nodejs/node/pull/12345' + }, + metadata: 'PR-URL: https://github.com/nodejs/node/pull/12345\n', + reasonCodes: [PR_CHECK_REASON_CODES.WAIT_TIME], + reasons: [reason, reason] + }); + }); + + it('classifies metadata JSON output with deferrable reasons', () => { + const result = formatMetadataResult({ + status: false, + data: { + owner: 'nodejs', + repo: 'node', + prid: 12345, + pr: { + url: 'https://github.com/nodejs/node/pull/12345' + } + }, + metadata: 'PR-URL: https://github.com/nodejs/node/pull/12345\n', + checker: { + reasons: [ + { + code: PR_CHECK_REASON_CODES.WAIT_TIME, + message: 'This PR needs to wait 24 more hours to land' + } + ] + } + }); + + assert.strictEqual(result.readiness, METADATA_READINESS.DEFERRABLE); + assert.strictEqual(result.exitCode, METADATA_EXIT_CODES.DEFERRABLE); + }); + + it('classifies missing approvals as failed', () => { + const result = formatMetadataResult({ + status: false, + data: { + owner: 'nodejs', + repo: 'node', + prid: 12345, + pr: { + url: 'https://github.com/nodejs/node/pull/12345' + } + }, + metadata: 'PR-URL: https://github.com/nodejs/node/pull/12345\n', + checker: { + reasons: [ + { + code: PR_CHECK_REASON_CODES.MISSING_APPROVAL, + message: 'Approvals: 0' + } + ] + } + }); + + assert.strictEqual(result.readiness, METADATA_READINESS.FAILED); + assert.strictEqual(result.exitCode, METADATA_EXIT_CODES.FAILED); + }); + + it('classifies mixed metadata JSON output as failed', () => { + const result = formatMetadataResult({ + status: false, + data: { + owner: 'nodejs', + repo: 'node', + prid: 12345, + pr: { + url: 'https://github.com/nodejs/node/pull/12345' + } + }, + metadata: 'PR-URL: https://github.com/nodejs/node/pull/12345\n', + checker: { + reasons: [ + { + code: PR_CHECK_REASON_CODES.WAIT_TIME, + message: 'This PR needs to wait 24 more hours to land' + }, + { + code: PR_CHECK_REASON_CODES.CONFLICT, + message: 'This PR has conflicts that must be resolved' + } + ] + } + }); + + assert.strictEqual(result.readiness, METADATA_READINESS.FAILED); + assert.strictEqual(result.exitCode, METADATA_EXIT_CODES.FAILED); + }); + + it('writes metadata JSON output before setting the exit code', async(t) => { + const originalWrite = process.stdout.write; + const originalExitCode = process.exitCode; + const written = []; + + t.after(() => { + process.stdout.write = originalWrite; + process.exitCode = originalExitCode; + }); + + process.stdout.write = (chunk, encoding, callback) => { + written.push(chunk.toString()); + const cb = typeof encoding === 'function' ? encoding : callback; + cb?.(); + return true; + }; + + const json = { + ready: false, + readiness: METADATA_READINESS.DEFERRABLE, + exitCode: METADATA_EXIT_CODES.DEFERRABLE + }; + + await writeMetadataJsonResult(Promise.resolve({ json })); + + assert.deepStrictEqual(JSON.parse(written.join('')), json); + assert.strictEqual(process.exitCode, METADATA_EXIT_CODES.DEFERRABLE); + }); +}); diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js index 34827e2b..22fed0b1 100644 --- a/test/unit/pr_checker.test.js +++ b/test/unit/pr_checker.test.js @@ -4,7 +4,9 @@ import assert from 'node:assert'; import * as sinon from 'sinon'; import PRData from '../../lib/pr_data.js'; -import PRChecker from '../../lib/pr_checker.js'; +import PRChecker, { + PR_CHECK_REASON_CODES +} from '../../lib/pr_checker.js'; import { jobCache } from '../../lib/ci/build-types/job.js'; import TestCLI from '../fixtures/test_cli.js'; @@ -54,6 +56,16 @@ const LT_48H_GT_47_59 = '2018-11-29T17:51:43.477Z'; const NOW = '2018-11-31T17:50:44.477Z'; const argv = { maxCommits: 3 }; +const { + CONFLICT, + MISSING_APPROVAL, + MISSING_GITHUB_CI, + MISSING_JENKINS_CI, + MISSING_TSC_APPROVAL, + REQUESTED_CHANGES, + STALE_REVIEW, + WAIT_TIME +} = PR_CHECK_REASON_CODES; describe('PRChecker', () => { describe('checkAll', () => { @@ -155,6 +167,12 @@ describe('PRChecker', () => { const status = checker.checkReviewsAndWait(new Date(NOW), true); assert(!status); + assert.deepStrictEqual(checker.reasons, [{ + code: MISSING_TSC_APPROVAL, + message: 'semver-major requires at least 2 TSC approvals', + approvals: 1, + required: 2 + }]); cli.assertCalledWith(expectedLogs); }); @@ -190,6 +208,17 @@ describe('PRChecker', () => { const status = checker.checkReviewsAndWait(new Date(NOW)); assert(!status); + assert.deepStrictEqual(checker.reasons, [ + { + code: REQUESTED_CHANGES, + message: 'Requested Changes: 2', + count: 2 + }, + { + code: MISSING_APPROVAL, + message: 'Approvals: 0' + } + ]); cli.assertCalledWith(expectedLogs); }); @@ -227,6 +256,10 @@ describe('PRChecker', () => { const status = checker.checkReviewsAndWait(new Date(NOW)); assert(!status); + assert.deepStrictEqual(checker.reasons, [{ + code: WAIT_TIME, + message: 'This PR needs to wait 24 more hours to land' + }]); cli.assertCalledWith(expectedLogs); }); @@ -1475,6 +1508,18 @@ describe('PRChecker', () => { const status = await checker.checkCI(); assert(!status); + assert.deepStrictEqual(checker.reasons, [ + { + code: MISSING_GITHUB_CI, + message: 'No GitHub CI runs detected', + provider: 'github' + }, + { + code: MISSING_JENKINS_CI, + message: 'No Jenkins CI runs detected', + provider: 'jenkins' + } + ]); cli.assertCalledWith(expectedLogs); }); @@ -2417,6 +2462,11 @@ describe('PRChecker', () => { const status = checker.checkCommitsAfterReview(); assert.deepStrictEqual(status, false); + assert.deepStrictEqual(checker.reasons, [{ + code: STALE_REVIEW, + message: 'Commits were pushed since the last approving review:', + commits: 1 + }]); cli.assertCalledWith(expectedLogs); }); @@ -2610,6 +2660,10 @@ describe('PRChecker', () => { const status = checker.checkMergeableState(); assert.deepStrictEqual(status, false); + assert.deepStrictEqual(checker.reasons, [{ + code: CONFLICT, + message: 'This PR has conflicts that must be resolved' + }]); cli.assertCalledWith(expectedLogs); });