From d2815f8e494961e04feed7b0b4f5e6ba6deb4e9b Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Fri, 25 Sep 2026 11:38:08 +0200 Subject: [PATCH 1/6] feat: recover CI resumes through eligible ancestors Check Jenkins' context menu before scanning failures or posting a resume. When the action is missing, follow recorded resume ancestors for the same PR and approved commit, checking failures from every run along the way. Report unavailable actions and request failures with recovery guidance. Signed-off-by: Filip Skokan Assisted-by: Codex --- docs/ncu-ci.md | 18 +- lib/ci/resumable_build.js | 86 +++++ lib/ci/resume_ci.js | 74 +++- test/unit/ci_resume.test.js | 657 +++++++++++++++++++++++++++++++++--- 4 files changed, 769 insertions(+), 66 deletions(-) create mode 100644 lib/ci/resumable_build.js diff --git a/docs/ncu-ci.md b/docs/ncu-ci.md index 8f5d5c21..46e1667c 100644 --- a/docs/ncu-ci.md +++ b/docs/ncu-ci.md @@ -181,14 +181,26 @@ in the PR description, comments, or reviews. The job must have finished with If no PR CI run is found, or Jenkins rejects the resume request, the command reports the failure and exits unsuccessfully. +Before downloading failure logs or submitting a resume request, the command checks +that Jenkins offers the **Resume build** action to your account. A failed or +aborted result alone does not guarantee that this action is available. If it is +missing, the command follows Jenkins' recorded resume ancestry to the nearest +ancestor that offers the action, and reports which run it will resume. Each +ancestor must have finished with `FAILURE` or `ABORTED` and match the PR, +repository, and approved commit. It does not search unrelated older CI runs or +skip past a mismatched ancestor. If no eligible ancestor is available, the command +reports the build URL and a command to start a new CI run. Errors checking +availability stop the command without attempting to resume. + The CI-approved commit (`COMMIT_SHA_CHECK`) must match the PR's current HEAD. The command refuses to resume if they differ or the approved commit cannot be determined. Before resuming, the command streams failed-job console output and compares -failure diagnostics with the PR's changed files. It refuses to resume if a failed -test or a file referenced in a failure diagnostic is changed by the PR. Logs are -scanned one at a time with bounded memory. HTTP compression is decoded as the +failure diagnostics with the PR's changed files. When recovering through resume +ancestry, it checks the latest run and every ancestor visited. It refuses to resume +if a failed test or a file referenced in a failure diagnostic is changed by the PR. +Logs are scanned one at a time with bounded memory. HTTP compression is decoded as the response arrives. A match cancels the download and skips remaining logs. Unknown or unavailable failure details do not prevent resuming; the check uses the available diagnostics. Failure to retrieve the PR's changed-file list prevents diff --git a/lib/ci/resumable_build.js b/lib/ci/resumable_build.js new file mode 100644 index 00000000..24a9b3a4 --- /dev/null +++ b/lib/ci/resumable_build.js @@ -0,0 +1,86 @@ +import { PRBuild } from './build-types/pr_build.js'; + +export const RESUME_TREE = 'result,building,' + + 'actions[parameters[name,value],causes[_class,upstreamProject,upstreamBuild,upstreamUrl]]'; +const RESUME_CAUSE = 'com.tikal.jenkins.plugins.multijob.ResumeCause'; +const PR_JOB = 'node-test-pull-request'; + +function getParameter(data, name) { + const values = new Set((data.actions ?? []).flatMap(action => action.parameters ?? []) + .filter(parameter => parameter.name === name) + .map(parameter => parameter.value)); + return values.size === 1 ? [...values][0] : undefined; +} + +export function getApprovedSHA(data) { + const value = getParameter(data, 'COMMIT_SHA_CHECK'); + return typeof value === 'string' && value ? value : undefined; +} + +export async function hasResumeAction(request, build) { + // The build API omits unexported actions. The context menu exposes the + // same actions Jenkins offers to the authenticated user on the build page. + const response = await request.fetch(`${build.jobUrl}contextMenu`, { + method: 'GET', + redirect: 'error' + }); + if (response.status !== 200) { + await response.body?.cancel(); + throw new Error( + `Jenkins returned HTTP ${response.status} ${response.statusText ?? ''}`.trim()); + } + const menu = await response.json(); + if (!Array.isArray(menu?.items)) { + throw new Error('Jenkins returned an invalid build context menu'); + } + const resumeURL = `${build.jobUrl}resume`; + return menu.items.some(item => { + // Jenkins' new build page exposes action URLs through a LinkEvent. + const actionURL = item?.url ?? item?.event?.url; + if (typeof actionURL !== 'string') return false; + let url; + try { + url = new URL(actionURL, build.jobUrl).href; + } catch { + return false; + } + return url === resumeURL || url === `${resumeURL}/`; + }); +} + +export async function findResumableBuild(cli, request, { jobid, owner, repo, prid }, data) { + const approvedSHA = getApprovedSHA(data); + const checkedBuilds = []; + if (!approvedSHA) return { build: undefined, checkedBuilds, approvedSHA }; + let build = new PRBuild(cli, request, jobid, undefined, RESUME_TREE); + while (data.building === false && + (data.result === 'FAILURE' || data.result === 'ABORTED')) { + if (getApprovedSHA(data) !== approvedSHA) break; + const buildOwner = getParameter(data, 'TARGET_GITHUB_ORG'); + const buildRepo = getParameter(data, 'TARGET_REPO_NAME'); + if (typeof buildOwner !== 'string' || buildOwner.toLowerCase() !== owner.toLowerCase() || + typeof buildRepo !== 'string' || buildRepo.toLowerCase() !== repo.toLowerCase() || + String(getParameter(data, 'PR_ID')) !== String(prid)) { + throw new Error(`CI job ${jobid} does not match pull request ${owner}/${repo}#${prid}`); + } + checkedBuilds.push({ jobid, build }); + if (await hasResumeAction(request, build)) { + return { jobid, build, checkedBuilds, approvedSHA }; + } + + // Jenkins copies earlier causes when resuming. Follow the nearest parent, + // not the first cause, and only within this PR job's actual resume lineage. + const parent = (data.actions ?? []).flatMap(action => action.causes ?? []) + .filter(cause => cause?._class === RESUME_CAUSE && + cause.upstreamProject === PR_JOB && cause.upstreamUrl === `job/${PR_JOB}/` && + Number.isSafeInteger(cause.upstreamBuild) && + cause.upstreamBuild > 0 && cause.upstreamBuild < jobid) + .sort((a, b) => b.upstreamBuild - a.upstreamBuild)[0]; + if (!parent) break; + + jobid = parent.upstreamBuild; + build = new PRBuild(cli, request, jobid, undefined, RESUME_TREE); + data = await build.getBuildData(); + } + return { build: undefined, checkedBuilds, approvedSHA }; +} diff --git a/lib/ci/resume_ci.js b/lib/ci/resume_ci.js index e3bff2e7..3faedabc 100644 --- a/lib/ci/resume_ci.js +++ b/lib/ci/resume_ci.js @@ -4,6 +4,7 @@ import { PRBuild } from './build-types/pr_build.js'; import { getPrURL } from '../links.js'; import { debuglog } from '../verbosity.js'; import { FailureFileScanner } from './failure_file_scanner.js'; +import { findResumableBuild, getApprovedSHA, RESUME_TREE } from './resumable_build.js'; export class ResumePRJob { constructor(cli, request, owner, repo, prid) { @@ -14,6 +15,15 @@ export class ResumePRJob { this.prid = prid; } + reportUnavailable(jobid, build) { + const { cli } = this; + cli.stopSpinner( + `Cannot resume PR CI job ${jobid}: Jenkins does not offer a "Resume build" action. ` + + `Start a new CI run with: ncu-ci run ${getPrURL(this)}`, + cli.SPINNER_STATUS.FAILED); + cli.error(build.jobUrl); + } + async checkFailures(jobid) { const { cli, request, owner, repo, prid } = this; const filenames = new Set(); @@ -71,11 +81,13 @@ export class ResumePRJob { } } catch (err) { debuglog(err); - cli.stopSpinner('Jenkins credentials invalid', cli.SPINNER_STATUS.FAILED); + cli.stopSpinner(`Unable to validate Jenkins credentials: ${err.message}`, + cli.SPINNER_STATUS.FAILED); return false; } cli.stopSpinner('Jenkins credentials valid'); + let failureMessage = `Failed to find CI runs for pull request ${prid}`; try { cli.startSpinner(`Looking for CI runs for pull request ${prid}`); const parser = await JobParser.fromPR(getPrURL(this), cli, request); @@ -87,32 +99,48 @@ export class ResumePRJob { } cli.stopSpinner(`Found PR CI job ${job.jobid}`); - const build = new PRBuild(cli, request, job.jobid, undefined, - 'result,building,actions[parameters[name,value]]'); - const { result, building, actions = [] } = await build.getBuildData(); + failureMessage = `Failed to load PR CI job ${job.jobid}`; + const latestBuild = new PRBuild(cli, request, job.jobid, undefined, RESUME_TREE); + const data = await latestBuild.getBuildData(); + const { result, building } = data; if (building || (result !== 'FAILURE' && result !== 'ABORTED')) { const status = building ? 'RUNNING' : result ?? 'RUNNING'; cli.error(`CI job ${job.jobid} is in status ${status}, skipping resume`); return false; } - const approvedSHAs = new Set(actions.flatMap(action => action.parameters ?? []) - .filter(parameter => parameter.name === 'COMMIT_SHA_CHECK') - .map(parameter => parameter.value)); - const [approvedSHA] = approvedSHAs; - if (approvedSHAs.size !== 1 || typeof approvedSHA !== 'string' || !approvedSHA) { + const approvedSHA = getApprovedSHA(data); + if (!approvedSHA) { cli.error(`Refusing to resume CI job ${job.jobid}: cannot determine its approved commit`); return false; } - cli.startSpinner('Checking failures against changed PR files'); - if (!(await this.checkFailures(job.jobid))) { - cli.stopSpinner('Refusing to resume CI: failures reference files changed by this PR', - cli.SPINNER_STATUS.FAILED); + failureMessage = `Failed to check resume availability for PR CI job ${job.jobid}`; + cli.startSpinner(`Checking whether PR CI job ${job.jobid} can be resumed`); + const { build, jobid: resumeJobid, checkedBuilds } = await findResumableBuild(cli, request, { + jobid: job.jobid, owner: this.owner, repo: this.repo, prid + }, data); + if (!build) { + this.reportUnavailable(job.jobid, latestBuild); return false; } + cli.stopSpinner('Jenkins offers a Resume build action'); + if (resumeJobid !== job.jobid) { + cli.info(`Using resumable ancestor PR CI job ${resumeJobid} for latest job ${job.jobid}`); + } + + failureMessage = `Failed to check failures for PR CI job ${job.jobid}`; + cli.startSpinner('Checking failures against changed PR files'); + for (const { jobid } of checkedBuilds) { + if (!(await this.checkFailures(jobid))) { + cli.stopSpinner('Refusing to resume CI: failures reference files changed by this PR', + cli.SPINNER_STATUS.FAILED); + return false; + } + } cli.stopSpinner('No changed PR files found in available failure details'); // Read HEAD after inspecting failures so the comparison is fresh when resuming. + failureMessage = `Failed to read the current HEAD for pull request ${prid}`; const pr = await request.getPullRequest(getPrURL(this)); if (pr.head?.sha !== approvedSHA) { cli.error(`Refusing to resume CI job ${job.jobid}: ` + @@ -120,25 +148,35 @@ export class ResumePRJob { return false; } - cli.startSpinner(`Resuming PR CI job ${job.jobid}`); - // Jenkins does not export its resume action in the build API. Let the - // resume endpoint determine whether the action is available. + failureMessage = `Failed to resume PR CI job ${resumeJobid}`; + cli.startSpinner(`Resuming PR CI job ${resumeJobid}`); const response = await request.fetch(`${build.jobUrl}resume/`, { method: 'POST', headers: { 'Jenkins-Crumb': crumb } }); + await response.body?.cancel(); + // The action may disappear after the preflight check. + if (response.status === 404) { + this.reportUnavailable(resumeJobid, build); + return false; + } if (response.status !== 200) { + const status = `HTTP ${response.status} ${response.statusText ?? ''}`.trim(); + const reason = response.status === 401 || response.status === 403 + ? `Jenkins denied the request (${status}). ` + + 'Check your Jenkins credentials and build permissions.' + : `Jenkins returned ${status}. Check the build page for details: ${build.jobUrl}`; cli.stopSpinner( - `Failed to resume PR CI: ${response.status} ${response.statusText}`, + `${failureMessage}: ${reason}`, cli.SPINNER_STATUS.FAILED); return false; } cli.stopSpinner('PR CI job successfully resumed'); } catch (err) { debuglog(err); - cli.stopSpinner('Failed to resume CI', cli.SPINNER_STATUS.FAILED); + cli.stopSpinner(`${failureMessage}: ${err.message}`, cli.SPINNER_STATUS.FAILED); return false; } return true; diff --git a/test/unit/ci_resume.test.js b/test/unit/ci_resume.test.js index 084fe3be..375beff0 100644 --- a/test/unit/ci_resume.test.js +++ b/test/unit/ci_resume.test.js @@ -15,16 +15,27 @@ import Request from '../../lib/request.js'; import { PRBuild } from '../../lib/ci/build-types/pr_build.js'; const approvedSHA = 'a'.repeat(40); -const resumeTree = 'result,building,actions[parameters[name,value]]'; -const resumeBuildData = { - result: 'FAILURE', - building: false, - actions: [ - // Jenkins does not export MultiJobResumeBuild, so the action serializes as {}. - {}, - { parameters: [{ name: 'COMMIT_SHA_CHECK', value: approvedSHA }] } - ] -}; +const resumeTree = 'result,building,actions[parameters[name,value],' + + 'causes[_class,upstreamProject,upstreamBuild,upstreamUrl]]'; +function getResumeBuildData(owner, repo, prid) { + return { + result: 'FAILURE', + building: false, + actions: [ + // Jenkins does not export MultiJobResumeBuild, so the action serializes as {}. + {}, + { + parameters: [ + { name: 'COMMIT_SHA_CHECK', value: approvedSHA }, + { name: 'TARGET_GITHUB_ORG', value: owner }, + { name: 'TARGET_REPO_NAME', value: repo }, + { name: 'PR_ID', value: String(prid) } + ] + } + ] + }; +} +const resumeBuildData = getResumeBuildData('nodejs', 'node', 123456); const failureLog = 'not ok 1 parallel/test-example\n' + ' ---\n severity: fail\n stack: |-\n AssertionError\n ...\n'; const failureBuildData = { @@ -72,9 +83,14 @@ describe('Jenkins resume', () => { const owner = 'nodejs'; const repo = 'node-auto-test'; const prid = 123456; + const resumeBuildData = getResumeBuildData(owner, repo, prid); const jobid = 654321; const crumb = 'asdf1234'; const jobURL = `https://ci.nodejs.org/job/node-test-pull-request/${jobid}/`; + const menuURL = `${jobURL}contextMenu`; + const unavailableMessage = `Cannot resume PR CI job ${jobid}: Jenkins does not offer a ` + + '"Resume build" action. Start a new CI run with: ' + + `ncu-ci run https://github.com/${owner}/${repo}/pull/${prid}`; const apiURL = `${jobURL}api/json?tree=${encodeURIComponent(resumeTree)}`; const fullAPIURL = new PRBuild(null, null, jobid).apiUrl; const filesURL = `/repos/${owner}/${repo}/pulls/${prid}/files?per_page=100&page=1`; @@ -84,6 +100,8 @@ describe('Jenkins resume', () => { let cli; let request; let jobRunner; + let menuRequest; + let resumeRequest; beforeEach(() => { cli = new TestCLI(); @@ -94,8 +112,13 @@ describe('Jenkins resume', () => { async * stream(url) { yield Buffer.from(await request.text(url)); }, getPullRequestFiles: Request.prototype.getPullRequestFiles, getPullRequest: Request.prototype.getPullRequest, - fetch: sinon.stub().resolves({ status: 200 }) + fetch: sinon.stub().rejects(new Error('Unexpected fetch request')) }; + menuRequest = request.fetch.withArgs(menuURL).resolves({ + status: 200, + json: async() => ({ items: [{ url: new URL(`${jobURL}resume`).pathname }] }) + }); + resumeRequest = request.fetch.withArgs(`${jobURL}resume/`).resolves({ status: 200 }); request.json.withArgs(CI_CRUMB_URL).resolves({ crumb }); request.json.withArgs(apiURL).resolves(resumeBuildData); request.json.withArgs(prURL).resolves({ head: { sha: approvedSHA } }); @@ -113,7 +136,10 @@ describe('Jenkins resume', () => { it('resumes the PR job with a Jenkins crumb and an unexported resume action', async() => { assert.equal(await jobRunner.resume(), true); - sinon.assert.calledOnceWithExactly(request.fetch, `${jobURL}resume/`, { + sinon.assert.calledOnceWithExactly(menuRequest, menuURL, { + method: 'GET', redirect: 'error' + }); + sinon.assert.calledOnceWithExactly(resumeRequest, `${jobURL}resume/`, { method: 'POST', headers: { 'Jenkins-Crumb': crumb } }); @@ -124,6 +150,426 @@ describe('Jenkins resume', () => { assert.deepEqual(cli._calls.stopSpinner.at(-1), ['PR CI job successfully resumed']); }); + for (const result of ['FAILURE', 'ABORTED']) { + it(`does not scan failures or POST when a ${result} job has no resume action`, async() => { + request.json.withArgs(apiURL).resolves({ ...resumeBuildData, result }); + menuRequest.resolves({ status: 200, json: async() => ({ items: [] }) }); + assert.equal(await jobRunner.resume(), false); + sinon.assert.notCalled(resumeRequest); + sinon.assert.neverCalledWith(request.json, filesURL); + sinon.assert.neverCalledWith(request.json, fullAPIURL); + sinon.assert.neverCalledWith(request.json, prURL); + sinon.assert.notCalled(request.text); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + unavailableMessage, cli.SPINNER_STATUS.FAILED + ]); + assert.deepEqual(cli._calls.error, [[jobURL]]); + }); + } + + for (const [name, value] of [ + ['TARGET_GITHUB_ORG', 'another-owner'], ['TARGET_GITHUB_ORG', undefined], + ['TARGET_REPO_NAME', 'another-repo'], ['TARGET_REPO_NAME', undefined], + ['PR_ID', String(prid + 1)], ['PR_ID', undefined] + ]) { + it(`rejects an initial build with mismatched ${name}: ${value}`, async() => { + const data = structuredClone(resumeBuildData); + data.actions[1].parameters.find(parameter => parameter.name === name).value = value; + request.json.withArgs(apiURL).resolves(data); + assert.equal(await jobRunner.resume(), false); + sinon.assert.notCalled(request.fetch); + sinon.assert.neverCalledWith(request.json, filesURL); + assert.match(cli._calls.stopSpinner.at(-1)[0], + /CI job 654321 does not match pull request nodejs\/node-auto-test#123456/); + }); + } + + it('rejects conflicting identity parameters on the initial build', async() => { + const data = structuredClone(resumeBuildData); + data.actions.push({ parameters: [{ name: 'PR_ID', value: String(prid + 1) }] }); + request.json.withArgs(apiURL).resolves(data); + assert.equal(await jobRunner.resume(), false); + sinon.assert.notCalled(request.fetch); + }); + + it('accepts an initial build whose PR_ID parameter is a number', async() => { + const data = structuredClone(resumeBuildData); + data.actions[1].parameters.find(parameter => parameter.name === 'PR_ID').value = prid; + request.json.withArgs(apiURL).resolves(data); + assert.equal(await jobRunner.resume(), true); + sinon.assert.calledOnce(resumeRequest); + }); + + it('matches the initial build owner and repository case-insensitively', async() => { + const data = getResumeBuildData(owner.toUpperCase(), repo.toUpperCase(), prid); + request.json.withArgs(apiURL).resolves(data); + assert.equal(await jobRunner.resume(), true); + sinon.assert.calledOnce(resumeRequest); + }); + + describe('resume ancestry', () => { + const ancestorId = jobid - 1; + const resumeCause = (id, overrides = {}) => ({ + _class: 'com.tikal.jenkins.plugins.multijob.ResumeCause', + upstreamProject: 'node-test-pull-request', + upstreamBuild: id, + upstreamUrl: 'job/node-test-pull-request/', + ...overrides + }); + + function latestWithoutAction(causes) { + request.json.withArgs(apiURL).resolves({ + ...resumeBuildData, + actions: [...resumeBuildData.actions, { causes }] + }); + menuRequest.resolves({ status: 200, json: async() => ({ items: [] }) }); + } + + function ancestor(id = ancestorId, { resumable = true, causes = [], overrides = {} } = {}) { + const build = new PRBuild(cli, request, id, undefined, resumeTree); + const data = { + ...resumeBuildData, + actions: [{ + parameters: Object.entries({ + COMMIT_SHA_CHECK: approvedSHA, + TARGET_GITHUB_ORG: owner, + TARGET_REPO_NAME: repo, + PR_ID: String(prid), + ...overrides + }).map(([name, value]) => ({ name, value })) + }, { causes }] + }; + request.json.withArgs(build.apiUrl).resolves(data); + const menu = request.fetch.withArgs(`${build.jobUrl}contextMenu`).resolves({ + status: 200, + json: async() => ({ items: resumable ? [{ url: `${build.jobUrl}resume/` }] : [] }) + }); + const post = request.fetch.withArgs(`${build.jobUrl}resume/`).resolves({ status: 200 }); + const failures = structuredClone(failureBuildData); + failures.subBuilds[0].buildNumber = id; + failures.subBuilds[0].build.subBuilds[0].url = + `https://ci.nodejs.org/job/node-test-commit-linux-freestyle/${id}/`; + request.json.withArgs(new PRBuild(null, null, id).apiUrl).resolves(failures); + const consoleURL = `${failures.subBuilds[0].build.subBuilds[0].url}consoleText`; + request.text.withArgs(consoleURL) + .resolves(failureLog.replace('test-example', 'test-ancestor')); + return { build, data, menu, post, consoleURL }; + } + + it('resumes the nearest ancestor and checks both its failures and the latest run', async() => { + latestWithoutAction([resumeCause(ancestorId - 10), resumeCause(ancestorId)]); + const { build, post, consoleURL } = ancestor(); + assert.equal(await jobRunner.resume(), true); + sinon.assert.notCalled(resumeRequest); + sinon.assert.calledOnceWithExactly(post, `${build.jobUrl}resume/`, { + method: 'POST', headers: { 'Jenkins-Crumb': crumb } + }); + sinon.assert.calledWithExactly(request.text, + 'https://ci.nodejs.org/job/node-test-commit-linux-freestyle/1/consoleText'); + sinon.assert.calledWithExactly(request.text, consoleURL); + assert.deepEqual(cli._calls.info, [ + [`Using resumable ancestor PR CI job ${ancestorId} for latest job ${jobid}`] + ]); + }); + + it('ignores malformed causes while following a valid resume cause', async() => { + latestWithoutAction([null, {}, resumeCause(ancestorId)]); + const { post } = ancestor(); + assert.equal(await jobRunner.resume(), true); + sinon.assert.calledOnce(post); + }); + + it('follows multiple resume links until an ancestor offers the action', async() => { + latestWithoutAction([resumeCause(ancestorId)]); + const intermediate = ancestor(ancestorId, { + resumable: false, causes: [resumeCause(ancestorId - 1)] + }); + const earlier = ancestor(ancestorId - 1); + assert.equal(await jobRunner.resume(), true); + sinon.assert.calledOnce(earlier.post); + sinon.assert.notCalled(intermediate.post); + sinon.assert.notCalled(resumeRequest); + sinon.assert.calledWithExactly(request.text, intermediate.consoleURL); + sinon.assert.calledWithExactly(request.text, earlier.consoleURL); + }); + + for (const filename of ['test/parallel/test-example.js', 'test/parallel/test-ancestor.js']) { + it(`refuses ancestor recovery when failures reference changed file ${filename}`, async() => { + latestWithoutAction([resumeCause(ancestorId)]); + const { post } = ancestor(); + request.json.withArgs(filesURL).resolves([{ filename }]); + assert.equal(await jobRunner.resume(), false); + sinon.assert.notCalled(post); + sinon.assert.notCalled(resumeRequest); + assert.deepEqual(cli._calls.error, [[filename]]); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + 'Refusing to resume CI: failures reference files changed by this PR', + cli.SPINNER_STATUS.FAILED + ]); + }); + } + + for (const overrides of [ + { COMMIT_SHA_CHECK: 'b'.repeat(40) }, + { COMMIT_SHA_CHECK: undefined }, + { TARGET_GITHUB_ORG: 'another-owner' }, + { TARGET_REPO_NAME: 'another-repo' }, + { PR_ID: prid + 1 }, + { PR_ID: undefined } + ]) { + it(`stops at an ancestor with mismatched identity: ${JSON.stringify(overrides)}`, async() => { + latestWithoutAction([resumeCause(ancestorId), resumeCause(ancestorId - 1)]); + const candidate = ancestor(ancestorId, { + overrides, causes: [resumeCause(ancestorId - 1)] + }); + const older = ancestor(ancestorId - 1); + assert.equal(await jobRunner.resume(), false); + sinon.assert.notCalled(candidate.menu); + sinon.assert.notCalled(candidate.post); + sinon.assert.notCalled(older.menu); + sinon.assert.notCalled(older.post); + sinon.assert.neverCalledWith(request.json, older.build.apiUrl); + }); + } + + it('accepts an ancestor whose PR_ID parameter is a number', async() => { + latestWithoutAction([resumeCause(ancestorId)]); + const { post } = ancestor(ancestorId, { overrides: { PR_ID: prid } }); + assert.equal(await jobRunner.resume(), true); + sinon.assert.calledOnce(post); + }); + + it('rejects conflicting identity parameters on an ancestor', async() => { + latestWithoutAction([resumeCause(ancestorId)]); + const { data, menu, post } = ancestor(); + data.actions.push({ parameters: [{ name: 'PR_ID', value: String(prid + 1) }] }); + assert.equal(await jobRunner.resume(), false); + sinon.assert.notCalled(menu); + sinon.assert.notCalled(post); + }); + + for (const cause of [ + null, {}, resumeCause('654320'), resumeCause(-1), resumeCause(1.5), + resumeCause(jobid), resumeCause(jobid + 1), + resumeCause(ancestorId, { _class: 'hudson.model.Cause$UpstreamCause' }), + resumeCause(ancestorId, { upstreamProject: 'node-test-commit' }), + resumeCause(ancestorId, { upstreamUrl: 'job/node-test-commit/' }), + resumeCause(ancestorId, { upstreamUrl: 'https://example.org/job/node-test-pull-request/' }) + ]) { + it(`does not follow an invalid resume cause: ${JSON.stringify(cause)}`, async() => { + latestWithoutAction([cause]); + const { build, menu, post } = ancestor(); + assert.equal(await jobRunner.resume(), false); + sinon.assert.neverCalledWith(request.json, build.apiUrl); + sinon.assert.notCalled(menu); + sinon.assert.notCalled(post); + sinon.assert.notCalled(resumeRequest); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + unavailableMessage, cli.SPINNER_STATUS.FAILED + ]); + }); + } + + for (const state of [ + { result: 'SUCCESS', building: false }, + { result: 'UNSTABLE', building: false }, + { result: 'FAILURE', building: true } + ]) { + it(`stops at an ancestor in state ${JSON.stringify(state)}`, async() => { + latestWithoutAction([resumeCause(ancestorId)]); + const { data, menu, post } = ancestor(ancestorId, { + causes: [resumeCause(ancestorId - 1)] + }); + Object.assign(data, state); + const older = ancestor(ancestorId - 1); + assert.equal(await jobRunner.resume(), false); + sinon.assert.notCalled(menu); + sinon.assert.notCalled(post); + sinon.assert.neverCalledWith(request.json, older.build.apiUrl); + }); + } + + it('stops when the resume lineage points back to a newer build', async() => { + latestWithoutAction([resumeCause(ancestorId)]); + const { post } = ancestor(ancestorId, { resumable: false, causes: [resumeCause(jobid)] }); + assert.equal(await jobRunner.resume(), false); + sinon.assert.notCalled(post); + sinon.assert.notCalled(resumeRequest); + sinon.assert.calledOnce(menuRequest); + }); + + it('does not recover through ancestry when the latest menu lookup fails', async() => { + latestWithoutAction([resumeCause(ancestorId)]); + menuRequest.rejects(new Error('Connection reset')); + const { build, menu, post } = ancestor(); + assert.equal(await jobRunner.resume(), false); + sinon.assert.neverCalledWith(request.json, build.apiUrl); + sinon.assert.notCalled(menu); + sinon.assert.notCalled(post); + assert.match(cli._calls.stopSpinner.at(-1)[0], /Connection reset/); + }); + + for (const operation of ['metadata', 'menu']) { + it(`does not skip an ancestor after a failed ${operation} lookup`, async() => { + latestWithoutAction([resumeCause(ancestorId), resumeCause(ancestorId - 1)]); + const { build, menu, post } = ancestor(); + if (operation === 'metadata') { + request.json.withArgs(build.apiUrl).rejects(new Error('Unavailable ancestor metadata')); + } else { + menu.rejects(new Error('Unavailable ancestor menu')); + } + const older = ancestor(ancestorId - 1); + assert.equal(await jobRunner.resume(), false); + sinon.assert.notCalled(post); + sinon.assert.notCalled(older.menu); + sinon.assert.neverCalledWith(request.json, older.build.apiUrl); + assert.match(cli._calls.stopSpinner.at(-1)[0], /Unavailable ancestor/); + }); + } + + it('rechecks current PR HEAD before resuming an ancestor', async() => { + latestWithoutAction([resumeCause(ancestorId)]); + const { post } = ancestor(); + request.json.withArgs(prURL).resolves({ head: { sha: 'b'.repeat(40) } }); + assert.equal(await jobRunner.resume(), false); + sinon.assert.notCalled(post); + sinon.assert.notCalled(resumeRequest); + assert.match(cli._calls.error.at(-1)[0], /does not match the current PR HEAD/); + }); + }); + + for (const url of [ + 'resume', 'resume/', `${jobURL}resume`, `${jobURL}resume/`, + new URL(`${jobURL}resume`).pathname, new URL(`${jobURL}resume/`).pathname + ]) { + it(`recognizes the build's resume action URL: ${url}`, async() => { + menuRequest.resolves({ status: 200, json: async() => ({ items: [{ url }] }) }); + assert.equal(await jobRunner.resume(), true); + sinon.assert.calledOnce(resumeRequest); + }); + + it(`recognizes the new build page's resume event URL: ${url}`, async() => { + // Jenkins' new build page exports Action.getEvent(), rather than the + // plugin's POST sidebar task. Its default event type is GET. + menuRequest.resolves({ + status: 200, + json: async() => ({ items: [{ url: null, event: { url, type: 'GET' } }] }) + }); + assert.equal(await jobRunner.resume(), true); + sinon.assert.calledOnceWithExactly(resumeRequest, `${jobURL}resume/`, { + method: 'POST', headers: { 'Jenkins-Crumb': crumb } + }); + }); + } + + it('recognizes a resume event when the top-level URL is omitted', async() => { + menuRequest.resolves({ + status: 200, + json: async() => ({ items: [{ event: { url: 'resume', type: 'GET' } }] }) + }); + assert.equal(await jobRunner.resume(), true); + sinon.assert.calledOnceWithExactly(resumeRequest, `${jobURL}resume/`, { + method: 'POST', headers: { 'Jenkins-Crumb': crumb } + }); + }); + + it('rejects malformed resume events and event URLs for other builds or servers', async() => { + menuRequest.resolves({ + status: 200, + json: async() => ({ + items: [ + { event: null }, { event: {} }, { event: { url: false } }, + { event: { url: 'http://[' } }, + { event: { url: `${jobURL}console` }, displayName: 'Resume build' }, + { url: null, event: { url: '/job/node-test-pull-request/654320/resume' } }, + { event: { url: `${jobURL}resume?other=build` } }, + { event: { url: `${jobURL}resume#other` } }, + { event: { url: 'https://example.org/job/node-test-pull-request/654321/resume' } } + ] + }) + }); + assert.equal(await jobRunner.resume(), false); + sinon.assert.notCalled(resumeRequest); + sinon.assert.neverCalledWith(request.json, filesURL); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + unavailableMessage, cli.SPINNER_STATUS.FAILED + ]); + }); + + it('ignores malformed menu entries and resume links for other builds or servers', async() => { + menuRequest.resolves({ + status: 200, + json: async() => ({ + items: [ + null, {}, { url: false }, { url: 'http://[' }, + { url: `${jobURL}console`, displayName: 'Resume build' }, + { url: '/job/node-test-pull-request/654320/resume' }, + { url: `${jobURL}resume?other=build` }, + { url: `${jobURL}resume#other` }, + { url: 'https://example.org/job/node-test-pull-request/654321/resume' } + ] + }) + }); + assert.equal(await jobRunner.resume(), false); + sinon.assert.notCalled(resumeRequest); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + unavailableMessage, cli.SPINNER_STATUS.FAILED + ]); + }); + + for (const [status, statusText] of [ + [401, 'Unauthorized'], [403, 'Forbidden'], [404, 'Not Found'], [500, 'Internal Server Error'] + ]) { + it(`reports HTTP ${status} while checking resume availability`, async() => { + const cancel = sinon.stub().resolves(); + const json = sinon.stub(); + menuRequest.resolves({ status, statusText, body: { cancel }, json }); + assert.equal(await jobRunner.resume(), false); + sinon.assert.calledOnce(cancel); + sinon.assert.notCalled(json); + sinon.assert.notCalled(resumeRequest); + sinon.assert.neverCalledWith(request.json, filesURL); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + `Failed to check resume availability for PR CI job ${jobid}: ` + + `Jenkins returned HTTP ${status} ${statusText}`, + cli.SPINNER_STATUS.FAILED + ]); + }); + } + + for (const menu of [null, {}, { items: null }, { items: {} }]) { + it(`reports an invalid build menu: ${JSON.stringify(menu)}`, async() => { + menuRequest.resolves({ status: 200, json: async() => menu }); + assert.equal(await jobRunner.resume(), false); + sinon.assert.notCalled(resumeRequest); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + `Failed to check resume availability for PR CI job ${jobid}: ` + + 'Jenkins returned an invalid build context menu', + cli.SPINNER_STATUS.FAILED + ]); + }); + } + + it('reports malformed JSON when checking resume availability', async() => { + menuRequest.resolves({ status: 200, json: sinon.stub().rejects(new Error('Invalid JSON')) }); + assert.equal(await jobRunner.resume(), false); + sinon.assert.notCalled(resumeRequest); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + `Failed to check resume availability for PR CI job ${jobid}: Invalid JSON`, + cli.SPINNER_STATUS.FAILED + ]); + }); + + it('reports a network error when checking resume availability', async() => { + menuRequest.rejects(new Error('Connection reset')); + assert.equal(await jobRunner.resume(), false); + sinon.assert.notCalled(resumeRequest); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + `Failed to check resume availability for PR CI job ${jobid}: Connection reset`, + cli.SPINNER_STATUS.FAILED + ]); + }); + it('posts to the resume handler without redirecting the POST to a GET', async(t) => { const agent = new MockAgent(); agent.disableNetConnect(); @@ -131,6 +577,8 @@ describe('Jenkins resume', () => { const pool = agent.get('https://ci.nodejs.org'); const jobPath = new URL(jobURL).pathname; const projectPath = '/job/node-test-pull-request/'; + pool.intercept({ path: `${jobPath}contextMenu`, method: 'GET' }) + .reply(200, { items: [{ url: `${jobPath}resume` }] }); // Stapler redirects a slashless action URL before invoking its POST handler. pool.intercept({ path: `${jobPath}resume`, method: 'POST' }) .reply(302, '', { headers: { location: `${jobPath}resume/` } }); @@ -141,6 +589,7 @@ describe('Jenkins resume', () => { return { statusCode: 302, data: '', responseOptions: { headers: { location: projectPath } } }; }); pool.intercept({ path: projectPath, method: 'GET' }).reply(200, ''); + request.fetch.resetBehavior(); request.fetch.callsFake((url, options) => fetch(url, { ...options, dispatcher: agent })); assert.equal(await jobRunner.resume(), true); @@ -156,7 +605,7 @@ describe('Jenkins resume', () => { ]); request.gql.withArgs('Reviews').resolves([comment(jobURL)]); assert.equal(await jobRunner.resume(), true); - assert.equal(request.fetch.firstCall.args[0], `${jobURL}resume/`); + assert.equal(resumeRequest.firstCall.args[0], `${jobURL}resume/`); }); it('finds CI links in the PR description', async() => { @@ -167,7 +616,7 @@ describe('Jenkins resume', () => { } }); assert.equal(await jobRunner.resume(), true); - assert.equal(request.fetch.firstCall.args[0], `${jobURL}resume/`); + assert.equal(resumeRequest.firstCall.args[0], `${jobURL}resume/`); }); for (const comments of [[], [comment('https://ci.nodejs.org/job/node-test-commit/123456/')]]) { @@ -204,7 +653,7 @@ describe('Jenkins resume', () => { request.json.withArgs(apiURL).resolves({ ...resumeBuildData, result: 'ABORTED' }); request.json.withArgs(fullAPIURL).resolves({ ...failureBuildData, result: 'ABORTED' }); assert.equal(await jobRunner.resume(), true); - sinon.assert.calledOnce(request.fetch); + sinon.assert.calledOnce(resumeRequest); }); it('checks failed tests inside an aborted job before resuming', async() => { @@ -212,21 +661,21 @@ describe('Jenkins resume', () => { request.json.withArgs(fullAPIURL).resolves({ ...failureBuildData, result: 'ABORTED' }); request.json.withArgs(filesURL).resolves([{ filename: 'test/parallel/test-example.js' }]); assert.equal(await jobRunner.resume(), false); - sinon.assert.notCalled(request.fetch); + sinon.assert.notCalled(resumeRequest); assert.deepEqual(cli._calls.error, [['test/parallel/test-example.js']]); }); for (const result of ['FAILURE', 'ABORTED']) { it(`reports an unavailable resume endpoint for a ${result} job`, async() => { request.json.withArgs(apiURL).resolves({ ...resumeBuildData, result }); - request.fetch.resolves({ status: 404, statusText: 'Not Found' }); + resumeRequest.resolves({ status: 404, statusText: 'Not Found' }); assert.equal(await jobRunner.resume(), false); - sinon.assert.calledOnceWithExactly(request.fetch, `${jobURL}resume/`, { + sinon.assert.calledOnceWithExactly(resumeRequest, `${jobURL}resume/`, { method: 'POST', headers: { 'Jenkins-Crumb': crumb } }); assert.deepEqual(cli._calls.stopSpinner.at(-1), [ - 'Failed to resume PR CI: 404 Not Found', cli.SPINNER_STATUS.FAILED + unavailableMessage, cli.SPINNER_STATUS.FAILED ]); }); } @@ -244,7 +693,7 @@ describe('Jenkins resume', () => { request.json.withArgs(apiURL).resolves({ ...resumeBuildData, result }); request.json.withArgs(prURL).resolves({ head: { sha: 'b'.repeat(40) } }); assert.equal(await jobRunner.resume(), false); - sinon.assert.notCalled(request.fetch); + sinon.assert.notCalled(resumeRequest); assert.match(cli._calls.error.at(-1)[0], /does not match the current PR HEAD/); }); } @@ -285,7 +734,11 @@ describe('Jenkins resume', () => { it('refuses when the current PR HEAD cannot be retrieved', async() => { request.json.withArgs(prURL).rejects(new Error('Unavailable')); assert.equal(await jobRunner.resume(), false); - sinon.assert.notCalled(request.fetch); + sinon.assert.notCalled(resumeRequest); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + `Failed to read the current HEAD for pull request ${prid}: Unavailable`, + cli.SPINNER_STATUS.FAILED + ]); }); it('does not fall back to an older failed job when the latest job is successful', async() => { @@ -307,6 +760,9 @@ describe('Jenkins resume', () => { assert.equal(await jobRunner.resume(), false); sinon.assert.notCalled(request.gql); sinon.assert.notCalled(request.fetch); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + 'Unable to validate Jenkins credentials: Missing Jenkins crumb', cli.SPINNER_STATUS.FAILED + ]); }); } @@ -315,33 +771,56 @@ describe('Jenkins resume', () => { assert.equal(await jobRunner.resume(), false); sinon.assert.notCalled(request.gql); sinon.assert.notCalled(request.fetch); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + 'Unable to validate Jenkins credentials: Unauthorized', cli.SPINNER_STATUS.FAILED + ]); }); it('fails if the PR cannot be loaded', async() => { request.gql.withArgs('PR').rejects(new Error('Not found')); assert.equal(await jobRunner.resume(), false); sinon.assert.notCalled(request.fetch); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + `Failed to find CI runs for pull request ${prid}: Not found`, cli.SPINNER_STATUS.FAILED + ]); }); it('fails if build data cannot be loaded', async() => { request.json.withArgs(apiURL).rejects(new Error('Not found')); assert.equal(await jobRunner.resume(), false); sinon.assert.notCalled(request.fetch); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + `Failed to load PR CI job ${jobid}: Not found`, cli.SPINNER_STATUS.FAILED + ]); }); it('fails if the resume request throws', async() => { - request.fetch.rejects(new Error('Connection reset')); + resumeRequest.rejects(new Error('Connection reset')); assert.equal(await jobRunner.resume(), false); assert.deepEqual(cli._calls.stopSpinner.at(-1), [ - 'Failed to resume CI', cli.SPINNER_STATUS.FAILED + `Failed to resume PR CI job ${jobid}: Connection reset`, cli.SPINNER_STATUS.FAILED ]); }); - it('reports a failed resume request', async() => { - request.fetch.resolves({ status: 403, statusText: 'Forbidden' }); + for (const [status, statusText] of [[401, 'Unauthorized'], [403, 'Forbidden']]) { + it(`reports HTTP ${status} with Jenkins credential and permission guidance`, async() => { + resumeRequest.resolves({ status, statusText }); + assert.equal(await jobRunner.resume(), false); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + `Failed to resume PR CI job ${jobid}: Jenkins denied the request ` + + `(HTTP ${status} ${statusText}). Check your Jenkins credentials and build permissions.`, + cli.SPINNER_STATUS.FAILED + ]); + }); + } + + it('reports a Jenkins failure with the build page URL', async() => { + resumeRequest.resolves({ status: 500, statusText: 'Internal Server Error' }); assert.equal(await jobRunner.resume(), false); assert.deepEqual(cli._calls.stopSpinner.at(-1), [ - 'Failed to resume PR CI: 403 Forbidden', cli.SPINNER_STATUS.FAILED + `Failed to resume PR CI job ${jobid}: Jenkins returned HTTP 500 Internal Server Error. ` + + `Check the build page for details: ${jobURL}`, + cli.SPINNER_STATUS.FAILED ]); }); @@ -349,7 +828,7 @@ describe('Jenkins resume', () => { it(`refuses to resume a failed test changed by the PR: ${filename}`, async() => { request.json.withArgs(filesURL).resolves([{ filename }]); assert.equal(await jobRunner.resume(), false); - sinon.assert.notCalled(request.fetch); + sinon.assert.notCalled(resumeRequest); assert.deepEqual(cli._calls.error, [[filename]]); }); } @@ -362,7 +841,7 @@ describe('Jenkins resume', () => { previous_filename: 'test/fixtures/old-name.js' }]); assert.equal(await jobRunner.resume(), false); - sinon.assert.notCalled(request.fetch); + sinon.assert.notCalled(resumeRequest); assert.deepEqual(cli._calls.error, [['test/fixtures/old-name.js']]); }); @@ -371,7 +850,7 @@ describe('Jenkins resume', () => { request.text.resolves(failureLog.replace('AssertionError', `${path}: AssertionError`)); request.json.withArgs(filesURL).resolves([{ filename: 'src/node.cc' }]); assert.equal(await jobRunner.resume(), false); - sinon.assert.notCalled(request.fetch); + sinon.assert.notCalled(resumeRequest); }); } @@ -379,27 +858,27 @@ describe('Jenkins resume', () => { request.text.resolves('../src/node.cc:42:5: error: no matching function\n'); request.json.withArgs(filesURL).resolves([{ filename: 'src/node.cc' }]); assert.equal(await jobRunner.resume(), false); - sinon.assert.notCalled(request.fetch); + sinon.assert.notCalled(resumeRequest); }); it('does not match test names that only share a prefix', async() => { request.json.withArgs(filesURL).resolves([{ filename: 'test/parallel/test-exam.js' }]); assert.equal(await jobRunner.resume(), true); - sinon.assert.calledOnce(request.fetch); + sinon.assert.calledOnce(resumeRequest); }); it('checks all failed tests', async() => { request.text.resolves(failureLog + failureLog.replace('test-example', 'test-second')); request.json.withArgs(filesURL).resolves([{ filename: 'test/parallel/test-second.js' }]); assert.equal(await jobRunner.resume(), false); - sinon.assert.notCalled(request.fetch); + sinon.assert.notCalled(resumeRequest); }); it('checks failed tests even when an infrastructure error takes precedence', async() => { request.text.resolves('Read-only file system\n' + failureLog); request.json.withArgs(filesURL).resolves([{ filename: 'test/parallel/test-example.js' }]); assert.equal(await jobRunner.resume(), false); - sinon.assert.notCalled(request.fetch); + sinon.assert.notCalled(resumeRequest); }); it('checks available failures even when another build log cannot be downloaded', async() => { @@ -412,7 +891,7 @@ describe('Jenkins resume', () => { request.text.withArgs(`${url}consoleText`).rejects(new Error('Unavailable')); request.json.withArgs(filesURL).resolves([{ filename: 'test/parallel/test-example.js' }]); assert.equal(await jobRunner.resume(), false); - sinon.assert.notCalled(request.fetch); + sinon.assert.notCalled(resumeRequest); }); it('cancels a matching log and never opens queued logs', async() => { @@ -437,7 +916,7 @@ describe('Jenkins resume', () => { assert.equal(await jobRunner.resume(), false); assert.equal(opened, 1); assert.equal(cancelled, true); - sinon.assert.notCalled(request.fetch); + sinon.assert.notCalled(resumeRequest); }); it('continues to a matching log after a stream fails midway', async() => { @@ -465,7 +944,7 @@ describe('Jenkins resume', () => { }; assert.equal(await jobRunner.resume(), false); assert.equal(opened, 2); - sinon.assert.notCalled(request.fetch); + sinon.assert.notCalled(resumeRequest); }); it('checks later pages of changed files', async() => { @@ -475,37 +954,44 @@ describe('Jenkins resume', () => { { filename: 'test/parallel/test-example.js' } ]); assert.equal(await jobRunner.resume(), false); - sinon.assert.notCalled(request.fetch); + sinon.assert.notCalled(resumeRequest); }); it('refuses to resume if fetching changed files fails', async() => { request.json.withArgs(filesURL).rejects(new Error('Unavailable')); assert.equal(await jobRunner.resume(), false); - sinon.assert.notCalled(request.fetch); + sinon.assert.notCalled(resumeRequest); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + `Failed to check failures for PR CI job ${jobid}: Unavailable`, cli.SPINNER_STATUS.FAILED + ]); }); it('refuses to resume if GitHub returns an error response for changed files', async() => { request.json.withArgs(filesURL).resolves({ message: 'Not Found' }); assert.equal(await jobRunner.resume(), false); - sinon.assert.notCalled(request.fetch); + sinon.assert.notCalled(resumeRequest); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + `Failed to check failures for PR CI job ${jobid}: Unable to retrieve pull request files`, + cli.SPINNER_STATUS.FAILED + ]); }); it('allows resuming if failure logs cannot be downloaded', async() => { request.text.rejects(new Error('Unavailable')); assert.equal(await jobRunner.resume(), true); - sinon.assert.calledOnce(request.fetch); + sinon.assert.calledOnce(resumeRequest); }); it('allows resuming if failures cannot be parsed', async() => { request.text.resolves('Unrecognized failure output'); assert.equal(await jobRunner.resume(), true); - sinon.assert.calledOnce(request.fetch); + sinon.assert.calledOnce(resumeRequest); }); it('allows resuming if detailed build data cannot be downloaded', async() => { request.json.withArgs(fullAPIURL).rejects(new Error('Unavailable')); assert.equal(await jobRunner.resume(), true); - sinon.assert.calledOnce(request.fetch); + sinon.assert.calledOnce(resumeRequest); }); }); @@ -517,10 +1003,13 @@ describe('ncu-ci resume CLI', () => { const apiURL = `${jobURL}api/json?tree=${encodeURIComponent(resumeTree)}`; function run(t, args, hasCI = true, changedFile = 'README.md', - buildData = resumeBuildData, headSHA = approvedSHA, resumeResponse = { status: 200 }) { + buildData = resumeBuildData, headSHA = approvedSHA, resumeResponse = { status: 200 }, + contextMenu = { items: [{ url: 'resume' }] }, ancestor = null) { const dir = mkdtempSync(join(tmpdir(), 'ncu-ci-resume-')); t.after(() => rmSync(dir, { recursive: true, force: true })); writeFileSync(join(dir, 'ncurc'), JSON.stringify({ username: 'test', token: 'test' })); + const ancestorBuild = ancestor && new PRBuild(null, null, ancestor.id, undefined, resumeTree); + const ancestorFullAPIURL = ancestor && new PRBuild(null, null, ancestor.id).apiUrl; const script = ` import assert from 'node:assert/strict'; import Request from ${JSON.stringify(requestURL)}; @@ -535,6 +1024,14 @@ describe('ncu-ci resume CLI', () => { return []; }; Request.prototype.json = async (url) => { + if (${Boolean(ancestor)}) { + if (url === ${JSON.stringify(ancestorBuild?.apiUrl)}) { + return ${JSON.stringify(ancestor?.data)}; + } + if (url === ${JSON.stringify(ancestorFullAPIURL)}) { + return ${JSON.stringify(failureBuildData)}; + } + } if (url.endsWith('/crumbIssuer/api/json')) return { crumb: 'test-crumb' }; if (url === '/repos/nodejs/node/pulls/123456') { return { head: { sha: ${JSON.stringify(headSHA)} } }; @@ -550,7 +1047,16 @@ describe('ncu-ci resume CLI', () => { yield Buffer.from(${JSON.stringify(failureLog)}); }; Request.prototype.fetch = async (url, options) => { - assert.equal(url, ${JSON.stringify(`${jobURL}resume/`)}); + if (url === ${JSON.stringify(`${jobURL}contextMenu`)}) { + assert.deepEqual(options, { method: 'GET', redirect: 'error' }); + return { status: 200, json: async () => (${JSON.stringify(contextMenu)}) }; + } + if (${Boolean(ancestor)} && + url === ${JSON.stringify(`${ancestorBuild?.jobUrl}contextMenu`)}) { + assert.deepEqual(options, { method: 'GET', redirect: 'error' }); + return { status: 200, json: async () => ({ items: [{ url: 'resume/' }] }) }; + } + assert.equal(url, ${JSON.stringify(`${ancestorBuild?.jobUrl ?? jobURL}resume/`)}); assert.equal(options.method, 'POST'); assert.equal(options.headers['Jenkins-Crumb'], 'test-crumb'); return ${JSON.stringify(resumeResponse)}; @@ -621,7 +1127,68 @@ describe('ncu-ci resume CLI', () => { true, 'README.md', { ...resumeBuildData, result: 'ABORTED' }, approvedSHA, { status: 404, statusText: 'Not Found' }); assert.equal(status, 1, output); - assert.match(output, /Failed to resume PR CI: 404 Not Found/); + assert.match(output, /Cannot resume PR CI job 654321: Jenkins does not offer a "Resume build" action/); + assert.match(output, /ncu-ci run https:\/\/github.com\/nodejs\/node\/pull\/123456/); + assert.doesNotMatch(output, /PR CI job successfully resumed/); + }); + + it('exits 1 with a new-run command when Jenkins offers no resume action', (t) => { + const { status, output } = run(t, ['resume', 'https://github.com/nodejs/node/pull/123456'], + true, 'README.md', resumeBuildData, approvedSHA, { status: 200 }, { items: [] }); + assert.equal(status, 1, output); + assert.match(output, /Cannot resume PR CI job 654321: Jenkins does not offer a "Resume build" action/); + assert.match(output, /ncu-ci run https:\/\/github.com\/nodejs\/node\/pull\/123456/); + assert.ok(output.includes(jobURL)); + assert.doesNotMatch(output, /Checking failures|Resuming PR CI|PR CI job successfully resumed/); + }); + + it('reports which ancestor is resumed when the latest run has no action', (t) => { + const latest = { + ...resumeBuildData, + actions: [...resumeBuildData.actions, { + causes: [{ + _class: 'com.tikal.jenkins.plugins.multijob.ResumeCause', + upstreamProject: 'node-test-pull-request', + upstreamBuild: 654320, + upstreamUrl: 'job/node-test-pull-request/' + }] + }] + }; + const ancestor = { + id: 654320, + data: { + ...resumeBuildData, + actions: [...resumeBuildData.actions, { + parameters: [ + { name: 'TARGET_GITHUB_ORG', value: 'nodejs' }, + { name: 'TARGET_REPO_NAME', value: 'node' }, + { name: 'PR_ID', value: '123456' } + ] + }] + } + }; + const { status, output } = run(t, ['resume', 'https://github.com/nodejs/node/pull/123456'], + true, 'README.md', latest, approvedSHA, { status: 200 }, { items: [] }, ancestor); + assert.equal(status, 0, output); + assert.match(output, /Using resumable ancestor PR CI job 654320 for latest job 654321/); + assert.match(output, /PR CI job successfully resumed/); + }); + + it('exits 1 when Jenkins returns an invalid build context menu', (t) => { + const { status, output } = run(t, ['resume', 'https://github.com/nodejs/node/pull/123456'], + true, 'README.md', resumeBuildData, approvedSHA, { status: 200 }, {}); + assert.equal(status, 1, output); + assert.match(output, /Failed to check resume availability for PR CI job 654321/); + assert.match(output, /Jenkins returned an invalid build context menu/); + assert.doesNotMatch(output, /PR CI job successfully resumed/); + }); + + it('exits 1 with credential guidance when Jenkins denies the resume request', (t) => { + const { status, output } = run(t, ['resume', 'https://github.com/nodejs/node/pull/123456'], + true, 'README.md', resumeBuildData, approvedSHA, { status: 403, statusText: 'Forbidden' }); + assert.equal(status, 1, output); + assert.match(output, /Jenkins denied the request \(HTTP 403 Forbidden\)/); + assert.match(output, /Check your Jenkins credentials and build permissions/); assert.doesNotMatch(output, /PR CI job successfully resumed/); }); From 2551823a88d459e2b12c9005c9e6fa0616fd538d Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Fri, 25 Sep 2026 11:38:48 +0200 Subject: [PATCH 2/6] fix: guide CI recovery without bypassing duplicates Check resume availability before suggesting a recovery action. Keep same-commit duplicates blocked when neither the latest run nor an eligible ancestor can resume, and guide users to inspect the run and recover manually. Suggest resume-ci only when a resume action is available. Signed-off-by: Filip Skokan Assisted-by: Codex --- docs/ncu-ci.md | 7 +- lib/ci/ci_utils.js | 13 +++ lib/ci/resume_ci.js | 3 +- lib/ci/run_ci.js | 48 ++++++-- test/unit/ci_resume.test.js | 30 ++++- test/unit/ci_start.test.js | 212 +++++++++++++++++++++++++++++++++++- 6 files changed, 295 insertions(+), 18 deletions(-) diff --git a/docs/ncu-ci.md b/docs/ncu-ci.md index 46e1667c..f96f7643 100644 --- a/docs/ncu-ci.md +++ b/docs/ncu-ci.md @@ -150,6 +150,11 @@ Options: - `--owner `: GitHub repository owner, used when `` is not a GitHub URL - `--repo `: GitHub repository name, used when `` is not a GitHub URL +With `--check-for-duplicates`, an existing CI run for the same approved commit +prevents starting another run, including when a failed or aborted build has no +eligible resume action. Errors checking resume availability also prevent starting +another run. + Examples: Run CI for a PR number using repository information from config or flags: @@ -189,7 +194,7 @@ ancestor that offers the action, and reports which run it will resume. Each ancestor must have finished with `FAILURE` or `ABORTED` and match the PR, repository, and approved commit. It does not search unrelated older CI runs or skip past a mismatched ancestor. If no eligible ancestor is available, the command -reports the build URL and a command to start a new CI run. Errors checking +reports the build URL and exits unsuccessfully. Errors checking availability stop the command without attempting to resume. The CI-approved commit (`COMMIT_SHA_CHECK`) must match the PR's current HEAD. diff --git a/lib/ci/ci_utils.js b/lib/ci/ci_utils.js index a442ff2e..15d9694b 100644 --- a/lib/ci/ci_utils.js +++ b/lib/ci/ci_utils.js @@ -1,4 +1,5 @@ import qs from 'node:querystring'; +import { getPrURL } from '../links.js'; import { CI_DOMAIN, @@ -13,6 +14,18 @@ export const statusType = { UNSTABLE: 'UNSTABLE' }; +export function getCIActionAdvice(pr, action) { + const command = `ncu-ci ${action} ${getPrURL(pr)}`; + if (action === 'run') { + return 'Check the existing CI run in Jenkins and rebase the PR if needed. ' + + `To start a new CI run manually: ${command}`; + } + if (pr.owner === 'nodejs' && pr.repo === 'node') { + return `Resume CI by adding the "resume-ci" label to the PR, or run: ${command}`; + } + return `Resume CI with: ${command}`; +} + export function getPath(url) { return url.replace(`https://${CI_DOMAIN}/`, '').replace('api/json', ''); } diff --git a/lib/ci/resume_ci.js b/lib/ci/resume_ci.js index 3faedabc..101ceaec 100644 --- a/lib/ci/resume_ci.js +++ b/lib/ci/resume_ci.js @@ -5,6 +5,7 @@ import { getPrURL } from '../links.js'; import { debuglog } from '../verbosity.js'; import { FailureFileScanner } from './failure_file_scanner.js'; import { findResumableBuild, getApprovedSHA, RESUME_TREE } from './resumable_build.js'; +import { getCIActionAdvice } from './ci_utils.js'; export class ResumePRJob { constructor(cli, request, owner, repo, prid) { @@ -19,7 +20,7 @@ export class ResumePRJob { const { cli } = this; cli.stopSpinner( `Cannot resume PR CI job ${jobid}: Jenkins does not offer a "Resume build" action. ` + - `Start a new CI run with: ncu-ci run ${getPrURL(this)}`, + getCIActionAdvice(this, 'run'), cli.SPINNER_STATUS.FAILED); cli.error(build.jobUrl); } diff --git a/lib/ci/run_ci.js b/lib/ci/run_ci.js index 7963849f..489c9f8d 100644 --- a/lib/ci/run_ci.js +++ b/lib/ci/run_ci.js @@ -10,6 +10,12 @@ import PRData from '../pr_data.js'; import { debuglog } from '../verbosity.js'; import PRChecker from '../pr_checker.js'; import { PRBuild } from './build-types/pr_build.js'; +import { getCIActionAdvice } from './ci_utils.js'; +import { + findResumableBuild, + getApprovedSHA, + RESUME_TREE +} from './resumable_build.js'; export const CI_CRUMB_URL = `https://${CI_DOMAIN}/crumbIssuer/api/json`; const CI_PR_NAME = CI_TYPES.get(CI_TYPES_KEYS.PR).jobName; @@ -94,12 +100,12 @@ export class RunPRJob { if (checkForDuplicates) { await this.prData.getComments(); const { jobid, link } = new JobParser(this.prData.comments).parse().get('PR') ?? {}; - let actions; + let buildData; try { - ({ actions } = jobid - ? (await new PRBuild(cli, request, jobid, undefined, 'actions[parameters[name,value]]') - .getBuildData()) - : {}); + if (jobid) { + buildData = await new PRBuild(cli, request, jobid, undefined, RESUME_TREE) + .getBuildData(); + } } catch (err) { // Jenkins may no longer have the build (e.g. the record was lost) or // may answer with an HTML error page instead of JSON. Do not let that @@ -108,11 +114,35 @@ export class RunPRJob { cli.SPINNER_STATUS.WARN); cli.warn('Skipping the duplicate CI check'); } - const { parameters } = actions?.find(a => 'parameters' in a) ?? {}; - if (parameters?.find(c => c.name === 'COMMIT_SHA_CHECK')?.value === certifySafe) { + const matchingSHA = buildData?.actions?.some(action => + action.parameters?.some(parameter => + parameter.name === 'COMMIT_SHA_CHECK' && parameter.value === certifySafe)); + if (matchingSHA) { cli.info('Existing CI run found: ' + link); - cli.error('Refusing to start a potentially duplicate CI job. Use the ' + - '"Resume build" button in the Jenkins UI, or start a new CI manually.'); + let advice = 'Check the existing CI run in Jenkins before retrying.'; + if (getApprovedSHA(buildData) === certifySafe && buildData.building === false && + ['FAILURE', 'ABORTED'].includes(buildData.result)) { + try { + const { build } = await findResumableBuild(cli, request, { + jobid, + owner: this.owner, + repo: this.repo, + prid: this.prid + }, buildData); + advice = getCIActionAdvice(this, build ? 'resume' : 'run'); + } catch (err) { + cli.error(`Could not check whether existing CI run ${link} can be resumed: ` + + err.message); + cli.error('Refusing to start a potentially duplicate CI job. ' + + 'Retry after checking Jenkins access.'); + return false; + } + } else if (buildData.building === true) { + advice = 'The existing CI run is still in progress.'; + } else if (buildData.result === 'SUCCESS') { + advice = 'CI has already succeeded for this commit.'; + } + cli.error(`Refusing to start a potentially duplicate CI job. ${advice}`); return false; } } diff --git a/test/unit/ci_resume.test.js b/test/unit/ci_resume.test.js index 375beff0..1a6935a5 100644 --- a/test/unit/ci_resume.test.js +++ b/test/unit/ci_resume.test.js @@ -89,7 +89,8 @@ describe('Jenkins resume', () => { const jobURL = `https://ci.nodejs.org/job/node-test-pull-request/${jobid}/`; const menuURL = `${jobURL}contextMenu`; const unavailableMessage = `Cannot resume PR CI job ${jobid}: Jenkins does not offer a ` + - '"Resume build" action. Start a new CI run with: ' + + '"Resume build" action. Check the existing CI run in Jenkins and rebase the PR if needed. ' + + 'To start a new CI run manually: ' + `ncu-ci run https://github.com/${owner}/${repo}/pull/${prid}`; const apiURL = `${jobURL}api/json?tree=${encodeURIComponent(resumeTree)}`; const fullAPIURL = new PRBuild(null, null, jobid).apiUrl; @@ -163,10 +164,27 @@ describe('Jenkins resume', () => { assert.deepEqual(cli._calls.stopSpinner.at(-1), [ unavailableMessage, cli.SPINNER_STATUS.FAILED ]); + assert.doesNotMatch(cli._calls.stopSpinner.at(-1)[0], /request-ci|resume-ci/); assert.deepEqual(cli._calls.error, [[jobURL]]); }); } + it('suggests manual recovery for an unavailable nodejs/node run', async() => { + jobRunner = new ResumePRJob(cli, request, 'nodejs', 'node', prid); + request.json.withArgs(apiURL).resolves(getResumeBuildData('nodejs', 'node', prid)); + menuRequest.resolves({ status: 200, json: async() => ({ items: [] }) }); + assert.equal(await jobRunner.resume(), false); + sinon.assert.notCalled(resumeRequest); + assert.deepEqual(cli._calls.stopSpinner.at(-1), [ + `Cannot resume PR CI job ${jobid}: Jenkins does not offer a "Resume build" action. ` + + 'Check the existing CI run in Jenkins and rebase the PR if needed. ' + + 'To start a new CI run manually: ' + + `ncu-ci run https://github.com/nodejs/node/pull/${prid}`, + cli.SPINNER_STATUS.FAILED + ]); + assert.doesNotMatch(cli._calls.stopSpinner.at(-1)[0], /request-ci|resume-ci/); + }); + for (const [name, value] of [ ['TARGET_GITHUB_ORG', 'another-owner'], ['TARGET_GITHUB_ORG', undefined], ['TARGET_REPO_NAME', 'another-repo'], ['TARGET_REPO_NAME', undefined], @@ -1128,16 +1146,20 @@ describe('ncu-ci resume CLI', () => { { status: 404, statusText: 'Not Found' }); assert.equal(status, 1, output); assert.match(output, /Cannot resume PR CI job 654321: Jenkins does not offer a "Resume build" action/); - assert.match(output, /ncu-ci run https:\/\/github.com\/nodejs\/node\/pull\/123456/); + assert.ok(output.includes('Check the existing CI run in Jenkins and rebase the PR if needed. ' + + 'To start a new CI run manually: ncu-ci run https://github.com/nodejs/node/pull/123456')); + assert.doesNotMatch(output, /request-ci|resume-ci/); assert.doesNotMatch(output, /PR CI job successfully resumed/); }); - it('exits 1 with a new-run command when Jenkins offers no resume action', (t) => { + it('exits 1 with manual recovery guidance when Jenkins offers no resume action', (t) => { const { status, output } = run(t, ['resume', 'https://github.com/nodejs/node/pull/123456'], true, 'README.md', resumeBuildData, approvedSHA, { status: 200 }, { items: [] }); assert.equal(status, 1, output); assert.match(output, /Cannot resume PR CI job 654321: Jenkins does not offer a "Resume build" action/); - assert.match(output, /ncu-ci run https:\/\/github.com\/nodejs\/node\/pull\/123456/); + assert.ok(output.includes('Check the existing CI run in Jenkins and rebase the PR if needed. ' + + 'To start a new CI run manually: ncu-ci run https://github.com/nodejs/node/pull/123456')); + assert.doesNotMatch(output, /request-ci|resume-ci/); assert.ok(output.includes(jobURL)); assert.doesNotMatch(output, /Checking failures|Resuming PR CI|PR CI job successfully resumed/); }); diff --git a/test/unit/ci_start.test.js b/test/unit/ci_start.test.js index 7266ec78..2f6ece92 100644 --- a/test/unit/ci_start.test.js +++ b/test/unit/ci_start.test.js @@ -204,11 +204,18 @@ describe('Jenkins', () => { }); describe('--check-for-duplicates', { concurrency: false }, () => { + const jobid = 123456; + const jobURL = `https://ci.nodejs.org/job/node-test-pull-request/${jobid}/`; + const menuURL = `${jobURL}contextMenu`; + const duplicateRefusal = 'Refusing to start a potentially duplicate CI job. '; + const resumeHint = `Resume CI with: ncu-ci resume https://github.com/${owner}/${repo}/pull/${prid}`; + const manualHint = 'Check the existing CI run in Jenkins and rebase the PR if needed. ' + + `To start a new CI run manually: ncu-ci run https://github.com/${owner}/${repo}/pull/${prid}`; beforeEach(() => { sinon.replace(PRData.prototype, 'getComments', sinon.fake.resolves()); sinon.replace(PRData.prototype, 'getPR', sinon.fake.resolves()); sinon.replace(JobParser.prototype, 'parse', - sinon.fake.returns(new Map().set('PR', { jobid: 123456 }))); + sinon.fake.returns(new Map().set('PR', { jobid, link: jobURL }))); }); afterEach(() => { sinon.restore(); @@ -229,12 +236,12 @@ describe('Jenkins', () => { { _class: 'hudson.model.StringParameterValue', name: 'TARGET_GITHUB_ORG', - value: 'nodejs' + value: owner }, { _class: 'hudson.model.StringParameterValue', name: 'TARGET_REPO_NAME', - value: 'node' + value: repo }, { _class: 'hudson.model.StringParameterValue', @@ -290,6 +297,15 @@ describe('Jenkins', () => { } ] }); + const createRequest = () => { + const request = { + fetch: sinon.stub().rejects(new Error('Unexpected fetch request')), + json: sinon.stub().rejects(new Error('Unexpected JSON request')) + }; + request.json.withArgs(CI_CRUMB_URL).resolves({ crumb }); + request.fetch.withArgs(CI_PR_URL).resolves({ status: 201 }); + return request; + }; it('should return false if inferred commit already has CI', async() => { const cli = new TestCLI(); @@ -308,6 +324,196 @@ describe('Jenkins', () => { assert.strictEqual(await jobRunner.start(), false); assert.strictEqual(request.fetch.callCount, 0); }); + const inProgress = 'The existing CI run is still in progress.'; + const succeeded = 'CI has already succeeded for this commit.'; + const checkJenkins = 'Check the existing CI run in Jenkins before retrying.'; + for (const [state, reason] of [ + [{ building: true, result: null }, inProgress], + [{ building: true, result: 'FAILURE' }, inProgress], + [{ building: false, result: null }, checkJenkins], + [{ building: false, result: 'SUCCESS' }, succeeded], + [{ building: false, result: 'UNSTABLE' }, checkJenkins], + [{ building: false, result: 'NOT_BUILT' }, checkJenkins], + [{ building: false, result: 'UNKNOWN' }, checkJenkins], + [{ building: false }, checkJenkins], + [{ result: 'FAILURE' }, checkJenkins] + ]) { + it(`should reject duplicate CI without a resume lookup for ${ + JSON.stringify(state)}`, async() => { + const cli = new TestCLI(); + sinon.replace(PRBuild.prototype, 'getBuildData', sinon.fake.resolves({ + ...mockJenkinsResponse(getParameters('deadbeef')), + ...state + })); + const request = createRequest(); + const jobRunner = new RunPRJob(cli, request, owner, repo, prid, 'deadbeef', true); + assert.strictEqual(await jobRunner.start(), false); + sinon.assert.notCalled(request.fetch); + assert.deepStrictEqual(cli._calls.error, [[duplicateRefusal + reason]]); + }); + } + for (const result of ['FAILURE', 'ABORTED']) { + for (const resumable of [true, false]) { + it(`should reject duplicate CI for a ${ + result} job ${resumable ? 'with' : 'without'} a resume action`, async() => { + const cli = new TestCLI(); + sinon.replace(PRBuild.prototype, 'getBuildData', sinon.fake.resolves({ + ...mockJenkinsResponse(getParameters('deadbeef')), + result, + building: false + })); + const request = createRequest(); + const menuRequest = request.fetch.withArgs(menuURL).resolves({ + status: 200, + json: async() => ({ items: resumable ? [{ url: 'resume' }] : [] }) + }); + const jobRunner = new RunPRJob(cli, request, owner, repo, prid, 'deadbeef', true); + assert.strictEqual(await jobRunner.start(), false); + sinon.assert.calledOnceWithExactly(menuRequest, menuURL, { + method: 'GET', redirect: 'error' + }); + sinon.assert.notCalled(request.fetch.withArgs(CI_PR_URL)); + assert.deepStrictEqual(cli._calls.error, + [[duplicateRefusal + (resumable ? resumeHint : manualHint)]]); + }); + } + } + it('should suggest the resume-ci label for a resumable nodejs/node build', async() => { + const cli = new TestCLI(); + const parameters = getParameters('deadbeef').map(parameter => + parameter.name === 'TARGET_REPO_NAME' ? { ...parameter, value: 'node' } : parameter); + sinon.replace(PRBuild.prototype, 'getBuildData', sinon.fake.resolves({ + ...mockJenkinsResponse(parameters), + result: 'FAILURE', + building: false + })); + const request = createRequest(); + request.fetch.withArgs(menuURL).resolves({ + status: 200, + json: async() => ({ items: [{ url: 'resume' }] }) + }); + const jobRunner = new RunPRJob(cli, request, owner, 'node', prid, 'deadbeef', true); + assert.strictEqual(await jobRunner.start(), false); + sinon.assert.notCalled(request.fetch.withArgs(CI_PR_URL)); + assert.deepStrictEqual(cli._calls.error, [[duplicateRefusal + + 'Resume CI by adding the "resume-ci" label to the PR, or run: ' + + `ncu-ci resume https://github.com/nodejs/node/pull/${prid}`]]); + }); + for (const resumable of [true, false]) { + it(`should reject duplicate CI when the resume ancestor ${ + resumable ? 'has' : 'does not have'} a resume action`, async() => { + const cli = new TestCLI(); + const ancestorJobid = jobid - 1; + const ancestorURL = `https://ci.nodejs.org/job/node-test-pull-request/${ancestorJobid}/`; + const data = { + ...mockJenkinsResponse(getParameters('deadbeef')), + result: 'FAILURE', + building: false + }; + const latestData = { + ...data, + actions: [...data.actions, { + causes: [{ + _class: 'com.tikal.jenkins.plugins.multijob.ResumeCause', + upstreamProject: 'node-test-pull-request', + upstreamBuild: ancestorJobid, + upstreamUrl: 'job/node-test-pull-request/' + }] + }] + }; + const getBuildData = sinon.stub(PRBuild.prototype, 'getBuildData'); + getBuildData.onFirstCall().resolves(latestData); + getBuildData.onSecondCall().resolves(data); + const request = createRequest(); + request.fetch.withArgs(menuURL).resolves({ + status: 200, + json: async() => ({ items: [] }) + }); + const ancestorMenu = request.fetch.withArgs(`${ancestorURL}contextMenu`).resolves({ + status: 200, + json: async() => ({ items: resumable ? [{ url: 'resume' }] : [] }) + }); + const jobRunner = new RunPRJob(cli, request, owner, repo, prid, 'deadbeef', true); + assert.strictEqual(await jobRunner.start(), false); + sinon.assert.calledTwice(getBuildData); + assert.strictEqual(getBuildData.secondCall.thisValue.jobUrl, ancestorURL); + sinon.assert.calledOnce(ancestorMenu); + sinon.assert.notCalled(request.fetch.withArgs(CI_PR_URL)); + assert.deepStrictEqual(cli._calls.error, + [[duplicateRefusal + (resumable ? resumeHint : manualHint)]]); + }); + } + it('should reject duplicate CI when an ancestor cannot be queried', async() => { + const cli = new TestCLI(); + const data = { + ...mockJenkinsResponse(getParameters('deadbeef')), + result: 'FAILURE', + building: false + }; + data.actions.push({ + causes: [{ + _class: 'com.tikal.jenkins.plugins.multijob.ResumeCause', + upstreamProject: 'node-test-pull-request', + upstreamBuild: jobid - 1, + upstreamUrl: 'job/node-test-pull-request/' + }] + }); + const getBuildData = sinon.stub(PRBuild.prototype, 'getBuildData'); + getBuildData.onFirstCall().resolves(data); + getBuildData.onSecondCall().rejects(new Error('Ancestor metadata unavailable')); + const request = createRequest(); + request.fetch.withArgs(menuURL).resolves({ + status: 200, + json: async() => ({ items: [] }) + }); + const jobRunner = new RunPRJob(cli, request, owner, repo, prid, 'deadbeef', true); + assert.strictEqual(await jobRunner.start(), false); + sinon.assert.notCalled(request.fetch.withArgs(CI_PR_URL)); + assert.match(cli._calls.error[0][0], /Ancestor metadata unavailable/); + }); + it('should reject a potential duplicate CI with conflicting approved commits', async() => { + const cli = new TestCLI(); + const data = { + ...mockJenkinsResponse(getParameters('different-commit')), + result: 'FAILURE', + building: false + }; + data.actions.push({ parameters: getParameters('deadbeef') }); + sinon.replace(PRBuild.prototype, 'getBuildData', sinon.fake.resolves(data)); + const request = createRequest(); + const jobRunner = new RunPRJob(cli, request, owner, repo, prid, 'deadbeef', true); + assert.strictEqual(await jobRunner.start(), false); + sinon.assert.notCalled(request.fetch); + assert.deepStrictEqual(cli._calls.error, [[duplicateRefusal + checkJenkins]]); + }); + it('should reject duplicate CI when resume availability cannot be checked', async() => { + const cli = new TestCLI(); + sinon.replace(PRBuild.prototype, 'getBuildData', sinon.fake.resolves({ + ...mockJenkinsResponse(getParameters('deadbeef')), + result: 'FAILURE', + building: false + })); + const request = createRequest(); + request.fetch.withArgs(menuURL).resolves({ status: 403, statusText: 'Forbidden' }); + const jobRunner = new RunPRJob(cli, request, owner, repo, prid, 'deadbeef', true); + assert.strictEqual(await jobRunner.start(), false); + sinon.assert.notCalled(request.fetch.withArgs(CI_PR_URL)); + assert.match(cli._calls.error[0][0], /Could not check whether existing CI run/); + assert.ok(cli._calls.error[0][0].includes(jobURL)); + assert.match(cli._calls.error[0][0], /403 Forbidden/); + assert.match(cli._calls.error[1][0], /Retry after checking Jenkins access/); + }); + it('should not look up existing builds without the duplicate check flag', async() => { + const cli = new TestCLI(); + const getBuildData = sinon.fake.rejects(new Error('Unexpected metadata lookup')); + sinon.replace(PRBuild.prototype, 'getBuildData', getBuildData); + const request = createRequest(); + const jobRunner = new RunPRJob(cli, request, owner, repo, prid, 'deadbeef'); + assert.strictEqual(await jobRunner.start(), true); + sinon.assert.notCalled(PRData.prototype.getComments); + sinon.assert.notCalled(getBuildData); + sinon.assert.calledOnceWithMatch(request.fetch, CI_PR_URL, { method: 'POST' }); + }); it('should return true when last CI is on a different commit', async() => { const cli = new TestCLI(); sinon.replace(PRBuild.prototype, 'getBuildData', From 03f788173cdf55b96cf7fc061543e20771b1d429 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Fri, 25 Sep 2026 13:43:00 +0200 Subject: [PATCH 3/6] feat: add ncu-ci available Check Jenkins shutdown state and PR job readiness before processing CI requests. Report unavailable or unknown states with a nonzero exit code. Signed-off-by: Filip Skokan Assisted-by: Codex --- bin/ncu-ci.js | 26 +++++ docs/ncu-ci.md | 25 +++++ lib/ci/availability.js | 24 ++++ lib/ci/jenkins.js | 23 ++++ test/unit/ci_availability.test.js | 156 ++++++++++++++++++++++++++ test/unit/ci_preflight_cli.test.js | 171 +++++++++++++++++++++++++++++ 6 files changed, 425 insertions(+) create mode 100644 lib/ci/availability.js create mode 100644 lib/ci/jenkins.js create mode 100644 test/unit/ci_availability.test.js create mode 100644 test/unit/ci_preflight_cli.test.js diff --git a/bin/ncu-ci.js b/bin/ncu-ci.js index 808f28f7..fedab001 100755 --- a/bin/ncu-ci.js +++ b/bin/ncu-ci.js @@ -24,6 +24,7 @@ import { RunPRJob } from '../lib/ci/run_ci.js'; import { ResumePRJob } from '../lib/ci/resume_ci.js'; +import { checkAvailability } from '../lib/ci/availability.js'; import { writeJson, writeFile } from '../lib/file.js'; import { getMergedConfig } from '../lib/config.js'; import { runPromise } from '../lib/run.js'; @@ -54,6 +55,11 @@ const commandKeys = [ const args = yargs(hideBin(process.argv)) .completion('completion') + .command({ + command: 'available', + desc: 'Check whether Jenkins is available for PR CI requests', + handler + }) .command({ command: 'rate ', desc: 'Calculate the green rate of a CI job in the last 100 runs', @@ -598,7 +604,27 @@ class DailyCommand extends CICommand { } } +async function checkJenkins() { + try { + let jenkins; + try { + const credentials = await auth({ github: false, jenkins: true }); + jenkins = credentials.jenkins; + } catch { + throw new Error('Configure username and jenkins_token with ncu-config'); + } + const request = new Request({ jenkins }); + await checkAvailability(request); + } catch (err) { + console.error(`Unable to check Jenkins availability: ${err.message}`); + process.exitCode = 1; + } +} + async function main(command, argv) { + if (command === 'available') { + return checkJenkins(); + } const cli = new CLI(); const credentials = await auth({ github: true, diff --git a/docs/ncu-ci.md b/docs/ncu-ci.md index f96f7643..09249217 100644 --- a/docs/ncu-ci.md +++ b/docs/ncu-ci.md @@ -13,6 +13,7 @@ Supported jobs: ncu-ci Commands: + ncu-ci available Check whether Jenkins is available for PR CI requests ncu-ci rate Calculate the green rate of a CI job in the last 100 runs ncu-ci walk Walk the CI and display the failures @@ -35,6 +36,30 @@ Options: --help Show help [boolean] ``` +### `ncu-ci available` + +`ncu-ci available` checks whether Jenkins is available for PR CI requests. It +exits with status 0 and no output when the controller is not preparing for +shutdown and `node-test-pull-request` is enabled and buildable. Otherwise it exits +with status 1 and explains the reason on stderr. HTTP failures, timeouts, and +unrecognized responses are treated as unavailable. + +The command only reads Jenkins state. It does not start or resume a build, check +individual PRs, or require idle executors. Availability can change after the +check. + +The command uses the configured `username` and `jenkins_token`; no GitHub token, +repository configuration, or PR argument is required. It has a 20-second deadline +for its Jenkins requests, including response bodies. + +For example, skip processing requests unless Jenkins is available: + +```sh +ncu-ci available || exit 0 + +# Process requests here. +``` + ### `ncu-ci rate ` `ncu-ci rate ` calculate the success rate for CI jobs in the last 100 runs per [CI Health History](https://github.com/nodejs/reliability#ci-health-history), where `` can be either `pr` for `node-test-pull-request` or `commit` for `node-test-commit`. See `ncu-ci rate --help` for more. diff --git a/lib/ci/availability.js b/lib/ci/availability.js new file mode 100644 index 00000000..a1639bf4 --- /dev/null +++ b/lib/ci/availability.js @@ -0,0 +1,24 @@ +import { readJenkinsJSON } from './jenkins.js'; + +export async function checkAvailability(request) { + const signal = AbortSignal.timeout(20_000); + const controller = await readJenkinsJSON(request, '/api/json?tree=quietingDown', signal); + if (controller?.quietingDown === true) { + throw new Error('Jenkins is preparing for shutdown'); + } + if (controller?.quietingDown !== false) { + throw new Error('Jenkins quiet-down state is not confirmed'); + } + + const job = await readJenkinsJSON(request, + '/job/node-test-pull-request/api/json?tree=disabled,buildable', signal); + if (job?.disabled === true) { + throw new Error('Jenkins PR job is disabled'); + } + if (job?.buildable === false) { + throw new Error('Jenkins PR job is not buildable'); + } + if (job?.disabled !== false || job?.buildable !== true) { + throw new Error('Jenkins PR job availability is not confirmed'); + } +} diff --git a/lib/ci/jenkins.js b/lib/ci/jenkins.js new file mode 100644 index 00000000..c0caa408 --- /dev/null +++ b/lib/ci/jenkins.js @@ -0,0 +1,23 @@ +import { CI_DOMAIN } from './ci_type_parser.js'; + +export async function readJenkinsJSON(request, path, signal) { + const response = await request.fetch(`https://${CI_DOMAIN}${path}`, { + method: 'GET', + headers: { Accept: 'application/json' }, + redirect: 'error', + signal + }); + if (response.status !== 200) { + await response.body?.cancel(); + throw new Error(`Jenkins returned HTTP ${response.status} ${response.statusText ?? ''}`.trim()); + } + try { + return await response.json(); + } catch (cause) { + if (signal.aborted) throw signal.reason; + if (cause instanceof SyntaxError) { + throw new Error('Jenkins returned invalid JSON', { cause }); + } + throw cause; + } +} diff --git a/test/unit/ci_availability.test.js b/test/unit/ci_availability.test.js new file mode 100644 index 00000000..2e58c966 --- /dev/null +++ b/test/unit/ci_availability.test.js @@ -0,0 +1,156 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; + +import { checkAvailability } from '../../lib/ci/availability.js'; + +const controllerURL = 'https://ci.nodejs.org/api/json?tree=quietingDown'; +const jobURL = 'https://ci.nodejs.org/job/node-test-pull-request/api/json?tree=disabled,buildable'; +const availableController = { quietingDown: false }; +const availableJob = { disabled: false, buildable: true }; + +function requestFor(t, controller = availableController, job = availableJob) { + return { + fetch: t.mock.fn(async(url) => { + assert.ok(url === controllerURL || url === jobURL); + return { status: 200, json: async() => url === controllerURL ? controller : job }; + }) + }; +} + +describe('Jenkins availability', () => { + it('confirms controller and PR job availability with one shared deadline', async(t) => { + const controller = new AbortController(); + const timeout = t.mock.method(AbortSignal, 'timeout', () => controller.signal); + const request = requestFor(t); + + assert.equal(await checkAvailability(request), undefined); + assert.deepEqual(timeout.mock.calls.map(call => call.arguments), [[20_000]]); + assert.deepEqual(request.fetch.mock.calls.map(call => call.arguments), [ + [controllerURL, { + method: 'GET', redirect: 'error', + headers: { Accept: 'application/json' }, signal: controller.signal + }], + [jobURL, { + method: 'GET', redirect: 'error', + headers: { Accept: 'application/json' }, signal: controller.signal + }] + ]); + }); + + it('stops before reading the job when Jenkins is quieting down', async(t) => { + const request = requestFor(t, { quietingDown: true }); + await assert.rejects(checkAvailability(request), { + message: 'Jenkins is preparing for shutdown' + }); + assert.equal(request.fetch.mock.callCount(), 1); + }); + + for (const controller of [null, {}, { quietingDown: 'false' }, { quietingDown: 0 }]) { + it(`rejects unknown controller state: ${JSON.stringify(controller)}`, async(t) => { + const request = requestFor(t, controller); + await assert.rejects(checkAvailability(request), { + message: 'Jenkins quiet-down state is not confirmed' + }); + assert.equal(request.fetch.mock.callCount(), 1); + }); + } + + for (const job of [ + { disabled: true, buildable: false }, + { disabled: true, buildable: true } + ]) { + it(`rejects disabled PR job: ${JSON.stringify(job)}`, async(t) => { + await assert.rejects(checkAvailability(requestFor(t, availableController, job)), { + message: 'Jenkins PR job is disabled' + }); + }); + } + + it('rejects a PR job that cannot be built', async(t) => { + const request = requestFor(t, availableController, { disabled: false, buildable: false }); + await assert.rejects(checkAvailability(request), { + message: 'Jenkins PR job is not buildable' + }); + }); + + for (const job of [ + null, {}, { disabled: false }, { buildable: true }, + { disabled: 'false', buildable: true }, { disabled: false, buildable: 'true' } + ]) { + it(`rejects unknown PR job state: ${JSON.stringify(job)}`, async(t) => { + await assert.rejects(checkAvailability(requestFor(t, availableController, job)), { + message: 'Jenkins PR job availability is not confirmed' + }); + }); + } + + for (const failedURL of [controllerURL, jobURL]) { + it(`propagates network failures from ${failedURL}`, async(t) => { + const failure = new Error('Connection reset'); + const request = requestFor(t); + request.fetch.mock.mockImplementation(async(url) => { + if (url === failedURL) throw failure; + return { status: 200, json: async() => availableController }; + }); + await assert.rejects(checkAvailability(request), error => error === failure); + assert.equal(request.fetch.mock.callCount(), failedURL === controllerURL ? 1 : 2); + }); + + it(`rejects malformed JSON from ${failedURL}`, async(t) => { + const request = requestFor(t); + request.fetch.mock.mockImplementation(async(url) => ({ + status: 200, + json: async() => { + if (url === failedURL) throw new SyntaxError('Invalid JSON'); + return availableController; + } + })); + await assert.rejects(checkAvailability(request), /JSON/); + assert.equal(request.fetch.mock.callCount(), failedURL === controllerURL ? 1 : 2); + }); + + for (const status of [302, 401, 403, 404, 503]) { + it(`rejects HTTP ${status} from ${failedURL}`, async(t) => { + const request = requestFor(t); + const cancel = t.mock.fn(async() => {}); + const json = t.mock.fn(async() => availableJob); + request.fetch.mock.mockImplementation(async(url) => url === failedURL + ? { status, statusText: 'Unavailable', body: { cancel }, json } + : { status: 200, json: async() => availableController }); + await assert.rejects(checkAvailability(request), new RegExp(String(status))); + assert.equal(request.fetch.mock.callCount(), failedURL === controllerURL ? 1 : 2); + assert.equal(json.mock.callCount(), 0); + assert.equal(cancel.mock.callCount(), 1); + }); + } + } + + it('preserves connection failures while reading a response body', async() => { + const failure = new Error('Connection reset while reading response'); + const request = { + fetch: async() => ({ status: 200, json: async() => { throw failure; } }) + }; + await assert.rejects(checkAvailability(request), error => error === failure); + }); + + it('keeps the deadline active while reading the job response body', async(t) => { + const controller = new AbortController(); + t.mock.method(AbortSignal, 'timeout', () => controller.signal); + const timeout = new DOMException('Availability check timed out', 'TimeoutError'); + const request = requestFor(t); + request.fetch.mock.mockImplementation(async(url, { signal }) => ({ + status: 200, + json: async() => { + if (url === controllerURL) return availableController; + const body = new Promise((resolve, reject) => { + signal.addEventListener('abort', () => reject(signal.reason), { once: true }); + }); + controller.abort(timeout); + return body; + } + })); + + await assert.rejects(checkAvailability(request), error => error === timeout); + assert.equal(request.fetch.mock.callCount(), 2); + }); +}); diff --git a/test/unit/ci_preflight_cli.test.js b/test/unit/ci_preflight_cli.test.js new file mode 100644 index 00000000..624a7b1f --- /dev/null +++ b/test/unit/ci_preflight_cli.test.js @@ -0,0 +1,171 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const binaryURL = new URL('../../bin/ncu-ci.js', import.meta.url); +const requestURL = new URL('../../lib/request.js', import.meta.url); +const undiciURL = import.meta.resolve('undici'); +const root = (data, extra = {}) => ({ + path: '/api/json', tree: 'quietingDown', data, ...extra +}); +const job = (data, extra = {}) => ({ + path: '/job/node-test-pull-request/api/json', tree: 'disabled,buildable', data, ...extra +}); +function run(t, command, responses, credentials = { + username: 'test', jenkins_token: 'test-jenkins-token' +}) { + const dir = mkdtempSync(join(tmpdir(), 'ncu-ci-preflight-')); + t.after(() => rmSync(dir, { recursive: true, force: true })); + // These commands must not require a GitHub token or repository configuration. + writeFileSync(join(dir, 'ncurc'), JSON.stringify(credentials)); + const tracePath = join(dir, 'requests.json'); + const script = ` + import assert from 'node:assert/strict'; + import { writeFileSync } from 'node:fs'; + import http from 'node:http'; + import https from 'node:https'; + import { MockAgent, setGlobalDispatcher } from ${JSON.stringify(undiciURL)}; + import Request from ${JSON.stringify(requestURL.href)}; + + const agent = new MockAgent(); + agent.disableNetConnect(); + setGlobalDispatcher(agent); + http.request = https.request = http.get = https.get = () => { + assert.fail('Unexpected network access or GitHub authentication'); + }; + const trace = { requests: [], timeouts: [], jsonReads: 0, cancellations: 0 }; + process.on('exit', () => { + writeFileSync(${JSON.stringify(tracePath)}, JSON.stringify(trace)); + }); + const timeout = AbortSignal.timeout; + AbortSignal.timeout = (delay) => { + trace.timeouts.push(delay); + return timeout.call(AbortSignal, delay); + }; + let signal; + const responses = ${JSON.stringify(responses)}; + Request.prototype.fetch = async function (url, options) { + const response = responses[trace.requests.length]; + assert.ok(response, 'Unexpected request: ' + url); + const parsed = new URL(url); + assert.equal(parsed.origin, 'https://ci.nodejs.org'); + assert.equal(parsed.pathname, response.path); + assert.equal(parsed.searchParams.get('tree'), response.tree); + assert.equal([...parsed.searchParams].length, 1); + assert.equal(options.method, 'GET'); + assert.equal(options.headers.Accept, 'application/json'); + assert.equal(options.redirect, 'error'); + assert.ok(options.signal instanceof AbortSignal); + assert.equal(options.signal.aborted, false); + if (signal) assert.equal(options.signal, signal); + signal = options.signal; + assert.equal(this.getJenkinsHeaders().Authorization, + 'Basic ' + Buffer.from('test:test-jenkins-token').toString('base64')); + trace.requests.push(url); + if (response.error) throw new Error(response.error); + const status = response.status ?? 200; + return { + status, + statusText: response.statusText ?? 'OK', + ok: status >= 200 && status < 300, + body: { async cancel() { trace.cancellations++; } }, + async json() { + trace.jsonReads++; + if (response.jsonError) throw new SyntaxError(response.jsonError); + return response.data; + } + }; + }; + process.argv = [process.execPath, ${JSON.stringify(fileURLToPath(binaryURL))}, + ${JSON.stringify(command)}]; + await import(${JSON.stringify(binaryURL.href)}); + `; + const result = spawnSync(process.execPath, ['--input-type=module', '--eval', script], { + cwd: dir, + env: { ...process.env, XDG_CONFIG_HOME: dir, NCU_VERBOSITY: 'NONE' }, + encoding: 'utf8', + timeout: 10000 + }); + assert.ifError(result.error); + const trace = JSON.parse(readFileSync(tracePath, 'utf8')); + assert.equal(trace.requests.length, responses.length, result.stdout + result.stderr); + assert.deepEqual(trace.timeouts, responses.length ? [20000] : [], result.stdout + result.stderr); + assert.doesNotMatch(result.stdout + result.stderr, + /first time running|create an access token|Unexpected network|AssertionError/); + return { ...result, trace }; +} + +function assertFailure(result, reason) { + assert.equal(result.status, 1, result.stdout + result.stderr); + assert.equal(result.stdout, ''); + assert.notEqual(result.stderr.trim(), ''); + assert.equal(result.stderr.trim().split('\n').length, 1, result.stderr); + assert.doesNotMatch(result.stderr, /\n\s+at |SyntaxError|TypeError|\[DEBUG\]/); + if (reason) assert.match(result.stderr, reason); +} + +describe('ncu-ci available', () => { + it('quietly confirms readiness with Jenkins-only credentials and one shared timeout', (t) => { + const result = run(t, 'available', [ + root({ quietingDown: false }), job({ disabled: false, buildable: true }) + ]); + assert.equal(result.status, 0, result.stderr); + assert.equal(result.stdout, ''); + assert.equal(result.stderr, ''); + assert.equal(result.trace.jsonReads, 2); + }); + + for (const data of [ + { quietingDown: true }, {}, null, { quietingDown: null }, { quietingDown: 'false' } + ]) { + it(`skips the job query when readiness is unconfirmed: ${JSON.stringify(data)}`, (t) => { + assertFailure(run(t, 'available', [root(data)])); + }); + } + + for (const data of [ + { disabled: true, buildable: false }, + { disabled: false, buildable: false }, + { disabled: false }, + { buildable: true }, + { disabled: 'false', buildable: true }, + { disabled: false, buildable: 'true' }, + null + ]) { + it(`refuses an unavailable or unknown PR job state: ${JSON.stringify(data)}`, (t) => { + assertFailure(run(t, 'available', [root({ quietingDown: false }), job(data)])); + }); + } + + it('reports an HTTP failure without parsing the error response', (t) => { + const result = run(t, 'available', [ + root(null, { status: 503, statusText: 'Service Unavailable' }) + ]); + assertFailure(result, /503/); + assert.equal(result.trace.jsonReads, 0); + assert.equal(result.trace.cancellations, 1); + }); + + it('reports malformed Jenkins JSON without a stack trace', (t) => { + assertFailure(run(t, 'available', [root(null, { jsonError: 'Invalid Jenkins JSON' })]), + /Jenkins returned invalid JSON/); + }); + + it('reports a job lookup failure after Jenkins reports ready', (t) => { + assertFailure(run(t, 'available', [ + root({ quietingDown: false }), job(null, { error: 'Connection reset' }) + ]), /Connection reset/); + }); + + it('rejects malformed Jenkins credentials without printing the token or prompting', (t) => { + const result = run(t, 'available', [], { + username: 'test', jenkins_token: 'secret-invalid-token!' + }); + assertFailure(result, /Configure username and jenkins_token with ncu-config/); + assert.doesNotMatch(result.stdout + result.stderr, /secret-invalid-token/); + }); +}); From 711d58e7818d89c7a404d3bde434ffb5c9f0c638 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Fri, 25 Sep 2026 13:43:27 +0200 Subject: [PATCH 4/6] feat: add ncu-ci workload Count running and queued PR CI jobs for workload threshold checks. Include PR builds waiting for downstream tests and count requests only once when they move from the queue to an executor between reads. Signed-off-by: Filip Skokan Assisted-by: Codex --- bin/ncu-ci.js | 23 +++- docs/ncu-ci.md | 25 +++- lib/ci/workload.js | 72 +++++++++++ test/unit/ci_preflight_cli.test.js | 145 ++++++++++++++++++++++ test/unit/ci_workload.test.js | 193 +++++++++++++++++++++++++++++ 5 files changed, 449 insertions(+), 9 deletions(-) create mode 100644 lib/ci/workload.js create mode 100644 test/unit/ci_workload.test.js diff --git a/bin/ncu-ci.js b/bin/ncu-ci.js index fedab001..05ff3ca0 100755 --- a/bin/ncu-ci.js +++ b/bin/ncu-ci.js @@ -25,6 +25,7 @@ import { } from '../lib/ci/run_ci.js'; import { ResumePRJob } from '../lib/ci/resume_ci.js'; import { checkAvailability } from '../lib/ci/availability.js'; +import { getPRWorkload } from '../lib/ci/workload.js'; import { writeJson, writeFile } from '../lib/file.js'; import { getMergedConfig } from '../lib/config.js'; import { runPromise } from '../lib/run.js'; @@ -60,6 +61,11 @@ const args = yargs(hideBin(process.argv)) desc: 'Check whether Jenkins is available for PR CI requests', handler }) + .command({ + command: 'workload', + desc: 'Print the number of running and queued node-test-pull-request jobs', + handler + }) .command({ command: 'rate ', desc: 'Calculate the green rate of a CI job in the last 100 runs', @@ -604,7 +610,7 @@ class DailyCommand extends CICommand { } } -async function checkJenkins() { +async function checkJenkins(command) { try { let jenkins; try { @@ -614,16 +620,23 @@ async function checkJenkins() { throw new Error('Configure username and jenkins_token with ncu-config'); } const request = new Request({ jenkins }); - await checkAvailability(request); + if (command === 'available') { + await checkAvailability(request); + } else { + console.log(await getPRWorkload(request)); + } } catch (err) { - console.error(`Unable to check Jenkins availability: ${err.message}`); + const action = command === 'available' + ? 'check Jenkins availability' + : 'read the PR CI workload'; + console.error(`Unable to ${action}: ${err.message}`); process.exitCode = 1; } } async function main(command, argv) { - if (command === 'available') { - return checkJenkins(); + if (command === 'available' || command === 'workload') { + return checkJenkins(command); } const cli = new CLI(); const credentials = await auth({ diff --git a/docs/ncu-ci.md b/docs/ncu-ci.md index 09249217..ff144402 100644 --- a/docs/ncu-ci.md +++ b/docs/ncu-ci.md @@ -14,6 +14,7 @@ ncu-ci Commands: ncu-ci available Check whether Jenkins is available for PR CI requests + ncu-ci workload Print the number of running and queued node-test-pull-request jobs ncu-ci rate Calculate the green rate of a CI job in the last 100 runs ncu-ci walk Walk the CI and display the failures @@ -48,14 +49,30 @@ The command only reads Jenkins state. It does not start or resume a build, check individual PRs, or require idle executors. Availability can change after the check. -The command uses the configured `username` and `jenkins_token`; no GitHub token, -repository configuration, or PR argument is required. It has a 20-second deadline -for its Jenkins requests, including response bodies. +### `ncu-ci workload` -For example, skip processing requests unless Jenkins is available: +`ncu-ci workload` prints the number of running and queued `node-test-pull-request` +jobs, followed by a newline. This includes PR builds waiting for downstream tests. +Each PR build counts once; downstream test jobs do not add to the count. If no PR +builds are running or queued, it prints `0`. A failed query exits with status 1, +reports the reason on stderr, and does not print a count. + +The command reads Jenkins executors and the waiting queue, so older active builds +are counted without relying on a limited build history. A request that starts +between the two reads is counted once. The count is a snapshot and can change +before a new request starts. + +Both commands use the configured `username` and `jenkins_token`; no GitHub token, +repository configuration, or PR argument is required. Each command has a +20-second deadline for its Jenkins requests, including response bodies. + +For example, skip processing requests unless Jenkins is available and fewer than +five PR jobs are running or queued: ```sh ncu-ci available || exit 0 +workload=$(ncu-ci workload) || exit 0 +[ "$workload" -lt 5 ] || exit 0 # Process requests here. ``` diff --git a/lib/ci/workload.js b/lib/ci/workload.js new file mode 100644 index 00000000..da7e4f40 --- /dev/null +++ b/lib/ci/workload.js @@ -0,0 +1,72 @@ +import { CI_DOMAIN, CI_TYPES, CI_TYPES_KEYS } from './ci_type_parser.js'; +import { readJenkinsJSON } from './jenkins.js'; + +const PR_JOB_URL = `https://${CI_DOMAIN}/job/${CI_TYPES.get(CI_TYPES_KEYS.PR).jobName}`; + +export async function getPRWorkload(request) { + const signal = AbortSignal.timeout(20_000); + const data = await readJenkinsJSON(request, '/queue/api/json?tree=items[id,task[url]]', signal); + if (!Array.isArray(data?.items)) { + throw new Error('Jenkins returned an invalid queue'); + } + + const queued = new Set(); + for (const item of data.items) { + if (!item || typeof item !== 'object' || Array.isArray(item) || + !item.task || typeof item.task !== 'object' || Array.isArray(item.task)) { + throw new Error('Jenkins returned an invalid queue item'); + } + const { url } = item.task; + // Some Jenkins task types do not export a URL. + if (url === undefined || url === null) continue; + if (typeof url !== 'string') { + throw new Error('Jenkins returned an invalid queue task URL'); + } + if (url === PR_JOB_URL || url === `${PR_JOB_URL}/`) { + if (!Number.isSafeInteger(item.id) || item.id < 0) { + throw new Error('Jenkins returned an invalid queue item ID'); + } + queued.add(item.id); + } + } + + // PR multijobs stay on an executor while waiting for downstream tests. The + // waiting queue alone misses those builds, and build history can be truncated. + const computers = await readJenkinsJSON(request, + '/computer/api/json?tree=computer[executors[currentExecutable[url,queueId]],' + + 'oneOffExecutors[currentExecutable[url,queueId]]]', signal); + if (!Array.isArray(computers?.computer)) { + throw new Error('Jenkins returned invalid executor data'); + } + const running = new Set(); + for (const computer of computers.computer) { + if (!Array.isArray(computer?.executors) || !Array.isArray(computer?.oneOffExecutors)) { + throw new Error('Jenkins returned invalid executor lists'); + } + for (const executor of [...computer.executors, ...computer.oneOffExecutors]) { + if (!executor || typeof executor !== 'object' || Array.isArray(executor)) { + throw new Error('Jenkins returned an invalid executor'); + } + const build = executor.currentExecutable; + if (build === undefined || build === null) continue; + if (typeof build !== 'object' || Array.isArray(build)) { + throw new Error('Jenkins returned an invalid executable'); + } + const { url, queueId } = build; + if (url === undefined || url === null) continue; + if (typeof url !== 'string') { + throw new Error('Jenkins returned an invalid executable URL'); + } + if (!url.startsWith(`${PR_JOB_URL}/`)) continue; + const match = /^([1-9]\d*)\/?$/.exec(url.slice(PR_JOB_URL.length + 1)); + if (!match) continue; + if (!Number.isSafeInteger(queueId)) { + throw new Error('Jenkins returned an invalid executable queue ID'); + } + running.add(match[1]); + // A queued request can start between the two reads. Count it only once. + queued.delete(queueId); + } + } + return queued.size + running.size; +} diff --git a/test/unit/ci_preflight_cli.test.js b/test/unit/ci_preflight_cli.test.js index 624a7b1f..c4c88b92 100644 --- a/test/unit/ci_preflight_cli.test.js +++ b/test/unit/ci_preflight_cli.test.js @@ -9,12 +9,28 @@ import { fileURLToPath } from 'node:url'; const binaryURL = new URL('../../bin/ncu-ci.js', import.meta.url); const requestURL = new URL('../../lib/request.js', import.meta.url); const undiciURL = import.meta.resolve('undici'); +const jobURL = 'https://ci.nodejs.org/job/node-test-pull-request/'; const root = (data, extra = {}) => ({ path: '/api/json', tree: 'quietingDown', data, ...extra }); const job = (data, extra = {}) => ({ path: '/job/node-test-pull-request/api/json', tree: 'disabled,buildable', data, ...extra }); +const queue = (data, extra = {}) => ({ + path: '/queue/api/json', tree: 'items[id,task[url]]', data, ...extra +}); +const computers = (data, extra = {}) => ({ + path: '/computer/api/json', + tree: 'computer[executors[currentExecutable[url,queueId]],' + + 'oneOffExecutors[currentExecutable[url,queueId]]]', + data, + ...extra +}); +const idleComputers = () => computers({ computer: [] }); +const running = (number, queueId = number) => ({ + currentExecutable: { url: `${jobURL}${number}/`, queueId } +}); + function run(t, command, responses, credentials = { username: 'test', jenkins_token: 'test-jenkins-token' }) { @@ -169,3 +185,132 @@ describe('ncu-ci available', () => { assert.doesNotMatch(result.stdout + result.stderr, /secret-invalid-token/); }); }); + +describe('ncu-ci workload', () => { + it('prints only the number of unfinished and queued PR jobs', (t) => { + const result = run(t, 'workload', [queue({ + items: [ + { id: 1, task: { url: jobURL } }, + { task: { url: 'https://ci.nodejs.org/job/node-test-commit/' } }, + { task: { url: `${jobURL}123/` } }, + { task: { url: 'https://example.org/job/node-test-pull-request/' } }, + { task: { url: 'https://ci.nodejs.org/job/node-test-pull-request-other/' } }, + { id: 2, task: { url: jobURL.slice(0, -1) } }, + { task: {} }, + { task: { url: null } }, + { id: 3, task: { url: jobURL } } + ] + }), idleComputers()]); + assert.equal(result.status, 0, result.stderr); + assert.equal(result.stdout, '3\n'); + assert.equal(result.stderr, ''); + }); + + it('includes running PR builds when the waiting queue is empty', (t) => { + const result = run(t, 'workload', [queue({ items: [] }), computers({ + computer: [{ + executors: Array.from({ length: 3 }, (_, i) => running(i + 1)), + oneOffExecutors: Array.from({ length: 17 }, (_, i) => running(i + 4)) + }] + })]); + assert.equal(result.status, 0, result.stderr); + assert.equal(result.stdout, '20\n'); + assert.equal(result.stderr, ''); + assert.equal(result.trace.jsonReads, 2); + }); + + it('combines queued and working PR builds without double counting transitions', (t) => { + const result = run(t, 'workload', [queue({ + items: [ + { id: 10, task: { url: jobURL } }, + { id: 11, task: { url: jobURL } }, + { id: 12, task: { url: jobURL } } + ] + }), computers({ + computer: [{ + executors: [running(101, 10), running(102, 20), { currentExecutable: null }, {}], + oneOffExecutors: [ + running(102, 20), + { currentExecutable: { url: `${jobURL}102`, queueId: 20 } }, + { currentExecutable: { url: 'https://ci.nodejs.org/job/node-test-commit/1/' } }, + { currentExecutable: { url: 'https://example.org/job/node-test-pull-request/1/' } }, + { currentExecutable: {} } + ] + }] + })]); + assert.equal(result.status, 0, result.stderr); + assert.equal(result.stdout, '4\n'); + assert.equal(result.stderr, ''); + }); + + for (const items of [[], [{ task: { url: 'https://ci.nodejs.org/job/node-test-commit/' } }]]) { + it(`prints zero for a valid queue with no PR jobs: ${JSON.stringify(items)}`, (t) => { + const result = run(t, 'workload', [queue({ items }), idleComputers()]); + assert.equal(result.status, 0, result.stderr); + assert.equal(result.stdout, '0\n'); + assert.equal(result.stderr, ''); + }); + } + + for (const data of [null, {}, { items: null }, { items: {} }]) { + it(`fails without a misleading count for invalid queue data: ${JSON.stringify(data)}`, (t) => { + assertFailure(run(t, 'workload', [queue(data)])); + }); + } + + for (const item of [null, {}, { task: null }, { task: false }, { task: { url: 123 } }]) { + it(`fails for a malformed queue item: ${JSON.stringify(item)}`, (t) => { + assertFailure(run(t, 'workload', [queue({ items: [item] })])); + }); + } + + for (const id of [undefined, null, -1, 1.5, '1', Number.MAX_SAFE_INTEGER + 1]) { + it(`rejects a queued PR item with invalid ID: ${JSON.stringify(id)}`, (t) => { + assertFailure(run(t, 'workload', [queue({ items: [{ id, task: { url: jobURL } }] })])); + }); + } + + for (const data of [ + null, + {}, + { computer: null }, + { computer: {} }, + { computer: [{ executors: {}, oneOffExecutors: [] }] }, + { computer: [{ executors: [], oneOffExecutors: null }] }, + { computer: [{ executors: [{ currentExecutable: false }], oneOffExecutors: [] }] }, + { computer: [{ executors: [], oneOffExecutors: [{ currentExecutable: { url: 1 } }] }] } + ]) { + it(`fails without a partial count for invalid executor data: ${JSON.stringify(data)}`, (t) => { + assertFailure(run(t, 'workload', [ + queue({ items: [{ id: 1, task: { url: jobURL } }] }), computers(data) + ])); + }); + } + + it('reports executor API failure without printing the already counted queue', (t) => { + const result = run(t, 'workload', [ + queue({ items: [{ id: 1, task: { url: jobURL } }] }), + computers(null, { status: 503, statusText: 'Service Unavailable' }) + ]); + assertFailure(result, /503/); + assert.equal(result.trace.jsonReads, 1); + assert.equal(result.trace.cancellations, 1); + }); + + it('reports HTTP failures instead of printing zero', (t) => { + const result = run(t, 'workload', [queue(null, { status: 403, statusText: 'Forbidden' })]); + assertFailure(result, /403/); + assert.equal(result.trace.jsonReads, 0); + assert.equal(result.trace.cancellations, 1); + }); + + it('reports malformed JSON instead of printing zero', (t) => { + assertFailure(run(t, 'workload', [queue(null, { jsonError: 'Invalid queue JSON' })]), + /Jenkins returned invalid JSON/); + }); + + it('reports a network failure instead of printing zero', (t) => { + assertFailure(run(t, 'workload', [queue(null, { error: 'Connection reset' })]), + /Connection reset/); + }); +}); diff --git a/test/unit/ci_workload.test.js b/test/unit/ci_workload.test.js new file mode 100644 index 00000000..609bcc2b --- /dev/null +++ b/test/unit/ci_workload.test.js @@ -0,0 +1,193 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; + +import { getPRWorkload } from '../../lib/ci/workload.js'; + +const queueURL = 'https://ci.nodejs.org/queue/api/json?tree=items[id,task[url]]'; +const computerURL = 'https://ci.nodejs.org/computer/api/json?' + + 'tree=computer[executors[currentExecutable[url,queueId]],' + + 'oneOffExecutors[currentExecutable[url,queueId]]]'; +const prJobURL = 'https://ci.nodejs.org/job/node-test-pull-request/'; +const jsonResponse = data => new Response(JSON.stringify(data), { + headers: { 'content-type': 'application/json' } +}); +const requestFor = (data, computers = { computer: [] }) => ({ + fetch: async url => jsonResponse(url === computerURL ? computers : data) +}); +const executable = (number, queueId = number) => ({ + currentExecutable: { url: `${prJobURL}${number}/`, queueId } +}); + +describe('PR CI workload', () => { + it('counts only the canonical PR job, including repeated queued requests', async() => { + const items = [ + { id: 1, task: { url: prJobURL } }, + { id: 2, task: { url: prJobURL } }, + { id: 3, task: { url: prJobURL.slice(0, -1) } }, + { task: { url: 'https://ci.nodejs.org/job/node-test-commit/' } }, + { task: { url: 'https://ci.nodejs.org/job/node-test-pull-request-other/' } }, + { task: { url: 'https://ci.nodejs.org/job/folder/job/node-test-pull-request/' } }, + { task: { url: `${prJobURL}123/` } }, + { task: { url: `${prJobURL}?other=job` } }, + { task: { url: 'https://example.com/job/node-test-pull-request/' } } + ]; + assert.equal(await getPRWorkload(requestFor({ items })), 3); + }); + + it('returns zero when idle using two GETs with a shared deadline', async(t) => { + const signal = new AbortController().signal; + const timeout = t.mock.method(AbortSignal, 'timeout', () => signal); + const fetch = t.mock.fn(async(url, options) => { + assert.ok([queueURL, computerURL].includes(url)); + assert.equal(options.method, 'GET'); + assert.equal(options.redirect, 'error'); + assert.equal(options.signal, signal); + return jsonResponse(url === queueURL ? { items: [] } : { computer: [] }); + }); + assert.equal(await getPRWorkload({ fetch }), 0); + assert.deepEqual(fetch.mock.calls.map(call => call.arguments[0]), [queueURL, computerURL]); + assert.equal(timeout.mock.callCount(), 1); + assert.deepEqual(timeout.mock.calls[0].arguments, [20_000]); + }); + + it('ignores unrelated task types that do not export a URL', async() => { + const items = [ + { task: {} }, + { task: { _class: 'example.CustomTask', url: null } }, + { id: 1, task: { url: prJobURL } } + ]; + assert.equal(await getPRWorkload(requestFor({ items })), 1); + }); + + it('counts active PR builds when the waiting queue has no PR jobs', async() => { + const computers = { + computer: [{ + executors: Array.from({ length: 20 }, (_, index) => executable(77871 + index)), + oneOffExecutors: [] + }] + }; + assert.equal(await getPRWorkload(requestFor({ items: [] }, computers)), 20); + }); + + it('combines queued requests with regular and one-off executors without duplicates', async() => { + const items = [ + { id: 1, task: { url: prJobURL } }, + { id: 2, task: { url: prJobURL } } + ]; + const computers = { + computer: [{ + executors: [ + executable(123, 1), + executable(124, 3), + { currentExecutable: null }, + { currentExecutable: { url: 'https://ci.nodejs.org/job/node-test-commit/123/' } } + ], + oneOffExecutors: [ + executable(124, 3), + // An older active run must count even when newer builds have finished. + executable(1, -1), + { currentExecutable: { url: `${prJobURL}123`, queueId: 1 } }, + { currentExecutable: {} } + ] + }] + }; + assert.equal(await getPRWorkload(requestFor({ items }, computers)), 4); + }); + + it('ignores other job and child URLs on executors', async() => { + const urls = [ + 'https://example.com/job/node-test-pull-request/123/', + 'https://ci.nodejs.org/job/folder/job/node-test-pull-request/123/', + 'https://ci.nodejs.org/job/node-test-pull-request-other/123/', + `${prJobURL}123/child/`, `${prJobURL}123?other=job`, prJobURL + ]; + const computers = { + computer: [{ + executors: urls.map(url => ({ currentExecutable: { url, queueId: 1 } })), + oneOffExecutors: [] + }] + }; + assert.equal(await getPRWorkload(requestFor({ items: [] }, computers)), 0); + }); + + for (const id of [undefined, null, -1, '123', 0.5]) { + it(`rejects malformed PR queue IDs: ${id}`, async() => { + await assert.rejects(getPRWorkload(requestFor({ + items: [{ id, task: { url: prJobURL } }] + })), /invalid queue item ID/); + }); + } + + for (const computers of [ + null, {}, { computer: null }, { computer: {} }, + { computer: [null] }, { computer: [{}] }, + { computer: [{ executors: [], oneOffExecutors: null }] } + ]) { + it(`rejects malformed executor data: ${JSON.stringify(computers)}`, async() => { + await assert.rejects(getPRWorkload(requestFor({ items: [] }, computers)), + /invalid executor/); + }); + } + + for (const executor of [ + null, [], { currentExecutable: false }, { currentExecutable: [] }, + { currentExecutable: { url: 123 } }, + { currentExecutable: { url: `${prJobURL}123/` } } + ]) { + it(`rejects a malformed executable: ${JSON.stringify(executor)}`, async() => { + await assert.rejects(getPRWorkload(requestFor({ items: [] }, { + computer: [{ executors: [executor], oneOffExecutors: [] }] + })), /invalid exec/); + }); + } + + it('does not return a partial queue count if the executor query fails', async() => { + const error = new Error('Connection reset'); + const request = { + async fetch(url) { + if (url === computerURL) throw error; + return jsonResponse({ items: [{ id: 1, task: { url: prJobURL } }] }); + } + }; + await assert.rejects(getPRWorkload(request), error); + }); + + for (const data of [null, {}, [], { items: null }, { items: {} }]) { + it(`rejects malformed queue data: ${JSON.stringify(data)}`, async() => { + await assert.rejects(getPRWorkload(requestFor(data)), /invalid queue/); + }); + } + + for (const item of [null, false, [], {}, { task: null }, { task: 'job' }, { task: [] }]) { + it(`rejects a malformed queue item: ${JSON.stringify(item)}`, async() => { + await assert.rejects(getPRWorkload(requestFor({ items: [item] })), /invalid queue item/); + }); + } + + it('rejects a malformed task URL instead of reporting an empty queue', async() => { + await assert.rejects(getPRWorkload(requestFor({ items: [{ task: { url: 123 } }] })), + /invalid queue task URL/); + }); + + it('propagates HTTP errors without returning a queue count', async() => { + const request = { + fetch: async() => new Response('Forbidden', { status: 403, statusText: 'Forbidden' }) + }; + await assert.rejects(getPRWorkload(request), /403/); + }); + + it('propagates network failures', async() => { + const error = new Error('Connection reset'); + const request = { async fetch() { throw error; } }; + await assert.rejects(getPRWorkload(request), error); + }); + + it('propagates invalid JSON errors', async() => { + const request = { fetch: async() => new Response('Not JSON') }; + await assert.rejects(getPRWorkload(request), error => { + assert.equal(error.message, 'Jenkins returned invalid JSON'); + assert.ok(error.cause instanceof SyntaxError); + return true; + }); + }); +}); From 9dba994014c9002d88b15f386f4dab1ad53b0cd9 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Fri, 25 Sep 2026 16:06:23 +0200 Subject: [PATCH 5/6] feat: show failures that prevent CI resumes Print the matching diagnostic and console log URL when a changed file prevents resuming CI. Bound large excerpts and preserve early download cancellation. Assisted-by: Codex Signed-off-by: Filip Skokan --- docs/ncu-ci.md | 2 + lib/ci/failure_file_scanner.js | 81 +++++++--- lib/ci/resume_ci.js | 14 +- test/fixtures/ci-reliability-failures.json | 174 +++++++++++++++++++++ test/unit/ci_failure_file_scanner.test.js | 97 +++++++++++- test/unit/ci_resume.test.js | 33 ++++ 6 files changed, 367 insertions(+), 34 deletions(-) create mode 100644 test/fixtures/ci-reliability-failures.json diff --git a/docs/ncu-ci.md b/docs/ncu-ci.md index ff144402..805be8e7 100644 --- a/docs/ncu-ci.md +++ b/docs/ncu-ci.md @@ -247,6 +247,8 @@ Before resuming, the command streams failed-job console output and compares failure diagnostics with the PR's changed files. When recovering through resume ancestry, it checks the latest run and every ancestor visited. It refuses to resume if a failed test or a file referenced in a failure diagnostic is changed by the PR. +The refusal includes the first matching filename, failure excerpt, and console +log URL. Large excerpts are truncated with an explicit marker. Logs are scanned one at a time with bounded memory. HTTP compression is decoded as the response arrives. A match cancels the download and skips remaining logs. Unknown or unavailable failure details do not prevent resuming; the check uses the diff --git a/lib/ci/failure_file_scanner.js b/lib/ci/failure_file_scanner.js index cac3feeb..aa54e55f 100644 --- a/lib/ci/failure_file_scanner.js +++ b/lib/ci/failure_file_scanner.js @@ -14,6 +14,25 @@ const infrastructure = createMatcher(FAILURE_PATTERNS.infrastructure); const gitStart = createMatcher(FAILURE_MARKERS.git.map(({ start }) => start)); const gitEnd = createMatcher(FAILURE_MARKERS.git.map(({ end }) => end)); +// Keep both ends of large diagnostics without retaining whole lines or TAP blocks. +class FailureExcerpt { + head = ''; + tail = ''; + length = 0; + + append(text) { + const remaining = Math.max(0, 4096 - this.head.length); + this.head += text.slice(0, remaining); + this.tail = (this.tail + text.slice(remaining)).slice(-4096); + this.length += text.length; + } + + toString() { + const omitted = this.length > 8192 ? '\n[... failure output truncated ...]\n' : ''; + return this.head + omitted + this.tail; + } +} + function fileAliases(filename) { const paths = new Set([filename]); if (filename.startsWith('test/')) { @@ -35,15 +54,16 @@ async function * logWindows(source, overlap) { function * consume(raw) { raw = raw.replace(/\r/g, ''); - let text = raw.replace(/\\+/g, '/'); - if (afterBackslash && raw.startsWith('\\')) text = text.slice(1); - if (raw) afterBackslash = raw.endsWith('\\'); - for (let offset = 0; offset < text.length;) { - const newline = text.indexOf('\n', offset); - const end = Math.min(offset + 8192, newline < 0 ? text.length : newline + 1); - const window = tail + text.slice(offset, end); + for (let offset = 0; offset < raw.length;) { + const newline = raw.indexOf('\n', offset); + const end = Math.min(offset + 8192, newline < 0 ? raw.length : newline + 1); + const content = raw.slice(offset, end); + let normalized = content.replace(/\\+/g, '/'); + if (afterBackslash && content.startsWith('\\')) normalized = normalized.slice(1); + afterBackslash = content.endsWith('\\'); + const window = tail + normalized; const lineEnd = end === newline + 1; - yield { text: window, lineStart, lineEnd }; + yield { text: window, content, lineStart, lineEnd }; if (lineEnd) { tail = ''; lineStart = true; @@ -57,14 +77,15 @@ async function * logWindows(source, overlap) { for await (const chunk of source) yield * consume(decoder.write(chunk)); yield * consume(decoder.end()); - if (tail) yield { text: `${tail}\n`, lineStart, lineEnd: true }; + if (tail) yield { text: `${tail}\n`, content: '\n', lineStart, lineEnd: true }; } -// Each line is reduced to a filename and failure markers. Neither a giant line -// nor a giant TAP block needs to survive in memory. +// Retain failure markers and a bounded excerpt for each line. async function * failureLines(windows, matchFile) { let line = {}; - for await (const { text, lineStart, lineEnd } of windows) { + let excerpt = new FailureExcerpt(); + for await (const { text, content, lineStart, lineEnd } of windows) { + excerpt.append(content); line.file ??= matchFile(text, lineStart); line.diagnostic ||= diagnostic(text); line.infrastructure ||= infrastructure(text); @@ -75,8 +96,10 @@ async function * failureLines(windows, matchFile) { line.gitStart ||= gitStart(text); line.gitEnd ||= gitEnd(text); if (lineEnd) { + line.text = excerpt.toString(); yield line; line = {}; + excerpt = new FailureExcerpt(); } } } @@ -84,44 +107,60 @@ async function * failureLines(windows, matchFile) { async function findFailure(lines) { let history = []; let followingLines = 0; + let diagnosticExcerpt; let tap = null; let git = null; for await (const line of lines) { const precedingLines = history; - history = [...history, line.file].slice(-5); + history = [...history, line].slice(-5); if (line.tapStart) { - tap = {}; + tap = { excerpt: new FailureExcerpt() }; followingLines = 0; } if (tap) { + tap.excerpt.append(line.text); tap.file ??= line.file; tap.todo ||= line.todo; // A later TODO can mark this as an expected failure; wait for the ending. if (line.tapEnd) { - if (!tap.todo && tap.file) return tap.file; + if (!tap.todo && tap.file) { + return { filename: tap.file, reason: tap.excerpt.toString().trimEnd() }; + } tap = null; } continue; } - if (line.gitStart) git = {}; + if (line.gitStart) git = { excerpt: new FailureExcerpt() }; if (git) { + git.excerpt.append(line.text); git.file ??= line.file; if (line.gitEnd) { - if (git.file) return git.file; + if (git.file) return { filename: git.file, reason: git.excerpt.toString().trimEnd() }; git = null; } } if (line.infrastructure || line.cpp) { const before = line.cpp ? 5 : 1; - const file = line.file || precedingLines.slice(-before).find(Boolean); - if (file) return file; + const context = [...precedingLines.slice(-before), line]; + const filename = line.file || context.find(previous => previous.file)?.file; + if (filename) { + const excerpt = new FailureExcerpt(); + for (const previous of context) excerpt.append(previous.text); + return { filename, reason: excerpt.toString().trimEnd() }; + } + } + if (line.diagnostic) { + followingLines = 6; + diagnosticExcerpt = new FailureExcerpt(); } - if (line.diagnostic) followingLines = 6; if (followingLines > 0) { followingLines--; - if (line.file) return line.file; + diagnosticExcerpt.append(line.text); + if (line.file) { + return { filename: line.file, reason: diagnosticExcerpt.toString().trimEnd() }; + } } } } diff --git a/lib/ci/resume_ci.js b/lib/ci/resume_ci.js index 101ceaec..9073c2cd 100644 --- a/lib/ci/resume_ci.js +++ b/lib/ci/resume_ci.js @@ -33,7 +33,7 @@ export class ResumePRJob { filenames.add(filename); } } - const overlaps = new Set(); + let overlap; const buildRequest = Object.create(request); buildRequest.json = async(...args) => { const data = await request.json(...args); @@ -46,10 +46,10 @@ export class ResumePRJob { let pending = Promise.resolve(); buildRequest.text = (url) => { const scan = pending.then(async() => { - if (!filenames.size || overlaps.size) return ''; + if (!filenames.size || overlap) return ''; const scanner = new FailureFileScanner(filenames); const match = await scanner.scan(request.stream(url)); - if (match) overlaps.add(match); + if (match) overlap = { ...match, url }; return ''; }); pending = scan.catch(debuglog); @@ -62,10 +62,10 @@ export class ResumePRJob { debuglog(err); } await pending; - if (overlaps.size) { - for (const filename of [...overlaps].sort()) { - cli.error(filename); - } + if (overlap) { + cli.error(overlap.filename); + cli.info(overlap.url); + cli.log(overlap.reason); return false; } return true; diff --git a/test/fixtures/ci-reliability-failures.json b/test/fixtures/ci-reliability-failures.json new file mode 100644 index 00000000..a71db1b0 --- /dev/null +++ b/test/fixtures/ci-reliability-failures.json @@ -0,0 +1,174 @@ +[ + { + "name": "assertion with escaped output and a Windows file URL", + "kind": "failure", + "report": "https://github.com/nodejs/reliability/blob/main/reports/2026-09-23.md", + "url": "https://ci.nodejs.org/job/node-test-binary-windows-js-suites/RUN_SUBSET=0,nodes=win11-arm64-COMPILED_BY-vs2022_clang-arm64/43426/console", + "filenames": [ + "test/parallel/test-debugger-extract-function-name.mjs" + ], + "log": "not ok 294 parallel/test-debugger-extract-function-name\n ---\n duration_ms: 1399.99400\n severity: fail\n exitcode: 1\n stack: |-\n node:internal/modules/run_main:111\n triggerUncaughtException(\n ^\n \n AssertionError [ERR_ASSERTION]: The input did not match the regular expression /\\[GeneratorFunction\\]/. Input:\n \n '< \\n< \\n< #\\n< # Fatal error in , line 0\\n\\ndebug> '\n \n at file:///d:/workspace/node-test-binary-windows-js-suites/node/test/parallel/test-debugger-extract-function-name.mjs:34:10\n at process.processTicksAndRejections (node:internal/process/task_queues:104:5) {\n generatedMessage: true,\n code: 'ERR_ASSERTION',\n actual: '< \\n< \\n< #\\n< # Fatal error in , line 0\\n\\ndebug> ',\n expected: /\\[GeneratorFunction\\]/,\n operator: 'match',\n diff: 'simple'\n }\n \n Node.js v27.0.0-pre\n ..." + }, + { + "name": "assertion with prefixed stderr (sample 1)", + "kind": "failure", + "report": "https://github.com/nodejs/reliability/blob/main/reports/2026-09-25.md", + "url": "https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu2404_sharedlibs_icu_x64/59400/consoleText", + "filenames": [ + "test/parallel/test-inspector-async-hook-setup-at-signal.js" + ], + "log": "not ok 2417 parallel/test-inspector-async-hook-setup-at-signal\n ---\n duration_ms: 733.63600\n severity: fail\n exitcode: 1\n stack: |-\n [err] Waiting until a signal enables the inspector...\n [err] \n [err] started\n [err] \n [test] Connecting to a child Node process\n [test] Testing /json/list\n [err] Debugger listening on ws://127.0.0.1:43821/6b9a4948-8ecc-4ce7-9fd7-ee9c45aa4c55\n [err] For help, see: https://nodejs.org/learn/getting-started/debugging\n [err] \n [err] Signal received, waiting for debugger setup\n [err] (node:2142537) internal/test/binding: These APIs are for internal testing only. Do not use them.\n [err] (Use `node --trace-warnings ...` to show where the warning was created)\n [err] \n [err] Debugger attached.\n [err] \n [err] Debugger ready, setting up timeout with a break\n [err] \n [test] Waiting for initial setup\n [test] Setting up timeout for async stack trace\n [test] Verify basic properties of asyncStackTrace\n AssertionError [ERR_ASSERTION]: callFrames,reason,hitBreakpoints contains \"asyncStackTrace\" property\n at checkAsyncStackTrace (/home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-inspector-async-hook-setup-at-signal.js:59:3)\n at process.processTicksAndRejections (node:internal/process/task_queues:104:5)\n at async runTests (/home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-inspector-async-hook-setup-at-signal.js:80:3) {\n generatedMessage: false,\n code: 'ERR_ASSERTION',\n actual: undefined,\n expected: true,\n operator: '==',\n diff: 'simple'\n }\n 1\n ..." + }, + { + "name": "assertion with prefixed stderr (sample 2)", + "kind": "failure", + "report": "https://github.com/nodejs/reliability/blob/main/reports/2026-09-25.md", + "url": "https://ci.nodejs.org/job/node-test-commit-osx/nodes=macos15-x64/73649/consoleText", + "filenames": [ + "test/parallel/test-inspector-async-hook-setup-at-signal.js" + ], + "log": "not ok 2418 parallel/test-inspector-async-hook-setup-at-signal\n ---\n duration_ms: 2144.64200\n severity: fail\n exitcode: 1\n stack: |-\n [err] Waiting until a signal enables the inspector...\n [err] started\n [err] \n [test] Connecting to a child Node process\n [test] Testing /json/list\n [err] Debugger listening on ws://127.0.0.1:53035/aaf719fc-9123-43a1-ab43-109964bece98\n [err] For help, see: https://nodejs.org/learn/getting-started/debugging\n [err] \n [err] Signal received, waiting for debugger setup\n [err] (node:54211) internal/test/binding: These APIs are for internal testing only. Do not use them.\n [err] (Use `node --trace-warnings ...` to show where the warning was created)\n [err] \n [err] Debugger attached.\n [err] \n [err] Debugger ready, setting up timeout with a break\n [err] \n [test] Waiting for initial setup\n [test] Setting up timeout for async stack trace\n [test] Verify basic properties of asyncStackTrace\n AssertionError [ERR_ASSERTION]: callFrames,reason,hitBreakpoints contains \"asyncStackTrace\" property\n at checkAsyncStackTrace (/Users/admin/build/workspace/node-test-commit-osx/nodes/macos15-x64/test/parallel/test-inspector-async-hook-setup-at-signal.js:59:3)\n at process.processTicksAndRejections (node:internal/process/task_queues:104:5)\n at async runTests (/Users/admin/build/workspace/node-test-commit-osx/nodes/macos15-x64/test/parallel/test-inspector-async-hook-setup-at-signal.js:80:3) {\n generatedMessage: false,\n code: 'ERR_ASSERTION',\n actual: undefined,\n expected: true,\n operator: '==',\n diff: 'simple'\n }\n 1\n ..." + }, + { + "name": "assertion with a relative test location and a file URL (sample 1)", + "kind": "failure", + "report": "https://github.com/nodejs/reliability/blob/main/reports/2026-09-25.md", + "url": "https://ci.nodejs.org/job/node-test-commit-osx/nodes=macos15-x64/73654/consoleText", + "filenames": [ + "test/parallel/test-runner-run.mjs" + ], + "log": "not ok 3553 parallel/test-runner-run\n ---\n duration_ms: 24692.55700\n severity: fail\n exitcode: 1\n stack: |-\n Test failure: 'should support timeout'\n Location: test/parallel/test-runner-run.mjs:91:3\n AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:\n + actual - expected\n \n + 'uncaughtException'\n - 'testTimeoutFailure'\n \n at TestsStream. (file:///Users/admin/build/workspace/node-test-commit-osx/nodes/macos15-x64/test/parallel/test-runner-run.mjs:96:14)\n at TestsStream. (/Users/admin/build/workspace/node-test-commit-osx/nodes/macos15-x64/test/common/index.js:512:15)\n at TestsStream.emit (node:events:514:20)\n at [kEmitMessage] (node:internal/test_runner/tests_stream:198:10)\n at TestsStream.fail (node:internal/test_runner/tests_stream:39:23)\n at FileTest.report (node:internal/test_runner/test:1705:21)\n at FileTest.report (node:internal/test_runner/runner:398:13)\n at FileTest.finalize (node:internal/test_runner/test:1629:10)\n at Test.processReadySubtestRange (node:internal/test_runner/test:1022:15)\n at FileTest.postRun (node:internal/test_runner/test:1541:19) {\n generatedMessage: true,\n code: 'ERR_ASSERTION',\n actual: 'uncaughtException',\n expected: 'testTimeoutFailure',\n operator: 'strictEqual',\n diff: 'simple'\n }\n \n (node:15655) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 uncaughtException listeners added to [process]. MaxListeners is 10. Use emitter.setMaxListeners() to increase limit\n (Use `node --trace-warnings ...` to show where the warning was created)\n (node:15655) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 unhandledRejection listeners added to [process]. MaxListeners is 10. Use emitter.setMaxListeners() to increase limit\n (node:15655) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 beforeExit listeners added to [process]. MaxListeners is 10. Use emitter.setMaxListeners() to increase limit\n ..." + }, + { + "name": "assertion with a relative test location and a file URL (sample 2)", + "kind": "failure", + "report": "https://github.com/nodejs/reliability/blob/main/reports/2026-09-25.md", + "url": "https://ci.nodejs.org/job/node-test-commit-osx/nodes=macos15-x64/73650/consoleText", + "filenames": [ + "test/parallel/test-runner-run.mjs" + ], + "log": "not ok 3552 parallel/test-runner-run\n ---\n duration_ms: 19896.59500\n severity: fail\n exitcode: 1\n stack: |-\n Test failure: 'should support timeout'\n Location: test/parallel/test-runner-run.mjs:91:3\n AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:\n + actual - expected\n \n + 'uncaughtException'\n - 'testTimeoutFailure'\n \n at TestsStream. (file:///Users/admin/build/workspace/node-test-commit-osx/nodes/macos15-x64/test/parallel/test-runner-run.mjs:96:14)\n at TestsStream. (/Users/admin/build/workspace/node-test-commit-osx/nodes/macos15-x64/test/common/index.js:512:15)\n at TestsStream.emit (node:events:514:20)\n at [kEmitMessage] (node:internal/test_runner/tests_stream:198:10)\n at TestsStream.fail (node:internal/test_runner/tests_stream:39:23)\n at FileTest.report (node:internal/test_runner/test:1705:21)\n at FileTest.report (node:internal/test_runner/runner:398:13)\n at FileTest.finalize (node:internal/test_runner/test:1629:10)\n at Test.processReadySubtestRange (node:internal/test_runner/test:1022:15)\n at FileTest.postRun (node:internal/test_runner/test:1541:19) {\n generatedMessage: true,\n code: 'ERR_ASSERTION',\n actual: 'uncaughtException',\n expected: 'testTimeoutFailure',\n operator: 'strictEqual',\n diff: 'simple'\n }\n \n (node:54169) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 uncaughtException listeners added to [process]. MaxListeners is 10. Use emitter.setMaxListeners() to increase limit\n (Use `node --trace-warnings ...` to show where the warning was created)\n (node:54169) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 unhandledRejection listeners added to [process]. MaxListeners is 10. Use emitter.setMaxListeners() to increase limit\n (node:54169) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 beforeExit listeners added to [process]. MaxListeners is 10. Use emitter.setMaxListeners() to increase limit\n ..." + }, + { + "name": "native assertion with a stack trace", + "kind": "failure", + "report": "https://github.com/nodejs/reliability/blob/main/reports/2026-09-25.md", + "url": "https://ci.nodejs.org/job/node-test-commit-smartos/nodes=smartos23-x64/68645/consoleText", + "filenames": [ + "test/parallel/test-worker-init-failure.js" + ], + "log": "not ok 6641 parallel/test-worker-init-failure\n ---\n duration_ms: 1248.38300\n severity: fail\n exitcode: 1\n stack: |-\n child stdout: \n \n child stderr: \n # [35614]: node::InitializeOncePerProcessInternal(const std::vector >&, ProcessInitializationFlags::Flags):: at ../src/node.cc:1339\n # Assertion failed: (uv_random(nullptr, nullptr, buffer, length, 0, nullptr)) == (0)\n \n ----- Native stack trace -----\n \n 1: 1e56ace node::(anonymous namespace)::DefaultAbortHandler(char const*, char const*) [/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos23-x64/out/Release/node]\n 2: 1e57544 node::Assert(node::AssertionInfo const&) [/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos23-x64/out/Release/node]\n 3: 1de10b2 node::InitializeOncePerProcessInternal(std::vector, std::allocator >, std::allocator, std::allocator > > > const&, node::ProcessInitializationFlags::Flags)::{lambda(unsigned char*, unsigned long)#1}::_FUN(unsigned char*, unsigned long) [/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos23-x64/out/Release/node]\n 4: 3f9a8e0 v8::base::RandomNumberGenerator::RandomNumberGenerator() [/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos23-x64/out/Release/node]\n 5: 3f9d149 v8::base::VirtualAddressSpace::AllocateSubspace(unsigned long, unsigned long, unsigned long, v8::PagePermissions, std::optional, std::optional) [/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos23-x64/out/Release/node]\n 6: 2426c0a v8::internal::SegmentedTable::Initialize() [/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos23-x64/out/Release/node]\n 7: 242241a v8::internal::Isolate::Init(v8::internal::SnapshotData*, v8::internal::SnapshotData*, v8::internal::SnapshotData*, bool) [/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos23-x64/out/Release/node]\n 8: 29fd35d v8::internal::Snapshot::Initialize(v8::internal::Isolate*) [/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos23-x64/out/Release/node]\n 9: 22709cb v8::Isolate::Initialize(v8::Isolate*, v8::Isolate::CreateParams const&) [/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos23-x64/out/Release/node]\n 10: 1cab9b0 node::NewIsolate(v8::Isolate::CreateParams*, uv_loop_s*, node::MultiIsolatePlatform*, node::SnapshotData const*, node::IsolateSettings const&) [/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos23-x64/out/Release/node]\n 11: 202269a node::worker::WorkerThreadData::WorkerThreadData(node::worker::Worker*) [/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos23-x64/out/Release/node]\n 12: 2020143 node::worker::Worker::Run() [/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos23-x64/out/Release/node]\n 13: 2020c36 node::worker::Worker::StartThread(v8::FunctionCallbackInfo const&)::{lambda(void*)#1}::_FUN(void*) [/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos23-x64/out/Release/node]\n 14: fffffc7fef0405fc _thrp_setup [/lib/amd64/libc.so.1]\n 15: fffffc7fef040910 _lwp_start [/lib/amd64/libc.so.1]\n /bin/sh: 35614: Abort\n \n \n node:internal/assert/utils:146\n throw error;\n ^\n \n AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:\n \n null !== 0\n \n at ChildProcess. (/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos23-x64/test/parallel/test-worker-init-failure.js:67:12)\n at ChildProcess. (/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos23-x64/test/common/index.js:512:15)\n at ChildProcess.emit (node:events:514:20)\n at ChildProcess._handle.onexit (node:internal/child_process:323:12) {\n generatedMessage: true,\n code: 'ERR_ASSERTION',\n actual: null,\n expected: 0,\n operator: 'strictEqual',\n diff: 'simple'\n }\n \n Node.js v27.0.0-pre\n ..." + }, + { + "name": "TODO-marked TAP failure", + "kind": "todo", + "report": "https://github.com/nodejs/reliability/blob/main/reports/2026-09-25.md", + "url": "https://ci.nodejs.org/job/node-test-commit-osx/nodes=macos15-x64/73649/consoleText", + "filenames": [ + "test/sequential/test-watch-mode.mjs" + ], + "log": "not ok 7347 sequential/test-watch-mode # TODO : Fix flaky test\n ---\n duration_ms: 47606.28900\n severity: flaky\n exitcode: 1\n stack: |-\n Test failure: 'should reload env variables when --env-file changes'\n Location: test/sequential/test-watch-mode.mjs:228:3\n Error: Timed out waiting for restart\n at Timeout. (file:///Users/admin/build/workspace/node-test-commit-osx/nodes/macos15-x64/test/sequential/test-watch-mode.mjs:97:25)\n at listOnTimeout (node:internal/timers:685:17)\n at process.processTimers (node:internal/timers:618:7)\n \n Test failure: 'should strip multiple --watch-path entries from NODE_OPTIONS'\n Location: test/sequential/test-watch-mode.mjs:1080:3\n Error: Timed out waiting for restart\n at Timeout. (file:///Users/admin/build/workspace/node-test-commit-osx/nodes/macos15-x64/test/sequential/test-watch-mode.mjs:97:25)\n at listOnTimeout (node:internal/timers:685:17)\n at process.processTimers (node:internal/timers:618:7)\n \n ..." + }, + { + "name": "truncated TAP assertion with prefixed stderr", + "kind": "incomplete", + "report": "https://github.com/nodejs/reliability/blob/main/reports/2026-09-25.md", + "url": "https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu2404_sharedlibs_icu_x64/59400/console", + "filenames": [ + "test/parallel/test-inspector-async-hook-setup-at-signal.js" + ], + "log": "not ok 2417 parallel/test-inspector-async-hook-setup-at-signal\n ---\n duration_ms: 733.63600\n severity: fail\n exitcode: 1\n stack: |-\n [err] Waiting until a signal enables the inspector...\n [err] \n [err] started\n [err] \n [test] Connecting to a child Node process\n [test] Testing /json/list\n [err] Debugger listening on ws://127.0.0.1:43821/6b9a4948-8ecc-4ce7-9fd7-ee9c45aa4c55\n [err] For help, see: https://nodejs.org/learn/getting-started/debugging\n [err] \n [err] Signal received, waiting for debugger setup\n [err] (node:2142537) internal/test/binding: These APIs are for internal testing only. Do not use them.\n [err] (Use `node --trace-warnings ...` to show where the warning was created)\n [err] \n [err] Debugger attached.\n [err] \n [err] Debugger ready, setting up timeout with a break\n [err] \n [test] Waiting for initial setup\n [test] Setting up timeout for async stack trace\n [test] Verify basic properties of asyncStackTrace\n AssertionError [ERR_ASSERT..." + }, + { + "name": "truncated TAP assertion with a file URL", + "kind": "incomplete", + "report": "https://github.com/nodejs/reliability/blob/main/reports/2026-09-25.md", + "url": "https://ci.nodejs.org/job/node-test-commit-osx/nodes=macos15-x64/73654/console", + "filenames": [ + "test/parallel/test-runner-run.mjs" + ], + "log": "not ok 3553 parallel/test-runner-run\n ---\n duration_ms: 24692.55700\n severity: fail\n exitcode: 1\n stack: |-\n Test failure: 'should support timeout'\n Location: test/parallel/test-runner-run.mjs:91:3\n AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:\n + actual - expected\n \n + 'uncaughtException'\n - 'testTimeoutFailure'\n \n at TestsStream. (file:///Users/admin/build/workspace/node-test-commit-osx/nodes/macos15-x64/test/parallel/test-runner-run.mjs:96:14)\n at TestsStream. (/Users/admin/build/workspace/node-test-commit-osx/nodes/macos15-x64/test/common/index.js:512:15)\n at TestsStream.emit (node:events:514:20)\n at [kEmitMessage] (node:internal/test_runner/tests_stream:198:10)\n at TestsStream.fail (node:internal/test_runner/tests_stream:39:23)\n at FileTest.report (node:internal/test_runner/test:1705:21)\n at FileTest.report (node:internal/test_runner/runner:398:13)\n at FileTest.finalize (node..." + }, + { + "name": "truncated TAP native assertion", + "kind": "incomplete", + "report": "https://github.com/nodejs/reliability/blob/main/reports/2026-09-25.md", + "url": "https://ci.nodejs.org/job/node-test-commit-smartos/nodes=smartos23-x64/68645/console", + "filenames": [ + "test/parallel/test-worker-init-failure.js" + ], + "log": "not ok 6641 parallel/test-worker-init-failure\n ---\n duration_ms: 1248.38300\n severity: fail\n exitcode: 1\n stack: |-\n child stdout: \n \n child stderr: \n # [35614]: node::InitializeOncePerProcessInternal(const std::vector >&, ProcessInitializationFlags::Flags):: at ../src/node.cc:1339\n # Assertion failed: (uv_random(nullptr, nullptr, buffer, length, 0, nullptr)) == (0)\n \n ----- Native stack trace -----\n \n 1: 1e56ace node::(anonymous namespace)::DefaultAbortHandler(char const*, char const*) [/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos23-x64/out/Release/node]\n 2: 1e57544 node::Assert(node::AssertionInfo const&) [/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos23-x64/out/Release/node]\n 3: 1de10b2 node::InitializeOncePerProcessInternal(std::vector, std::allocator >, std::allocator `not ok 1 parallel/test-example\n ---\n${text}\n ...\n`; -async function scan(text, files = [filename], size = 8192) { +async function scanFailure(text, files = [filename], size = 8192) { const buffer = Buffer.from(text); async function * source() { for (let offset = 0; offset < buffer.length; offset += size) { @@ -23,7 +24,79 @@ async function scan(text, files = [filename], size = 8192) { return new FailureFileScanner(files).scan(source()); } +async function scan(...args) { + return (await scanFailure(...args))?.filename; +} + +// Complete console diagnostics and truncated report excerpts from September 23–25, 2026. +const reliabilityFailures = JSON.parse(readFileSync( + new URL('../fixtures/ci-reliability-failures.json', import.meta.url), 'utf8')); + +describe('Reliability report diagnostics', () => { + for (const { name, kind, filenames, log } of reliabilityFailures) { + it(name, async() => { + const expected = kind === 'failure' ? { filename: filenames[0], reason: log } : undefined; + for (const size of [1, 31, 8192]) { + assert.deepEqual(await scanFailure(`${log}\n`, filenames, size), expected); + assert.equal(await scanFailure(`${log}\n`, ['test/unrelated.js'], size), undefined); + if (kind === 'todo') { + const unexpected = log.replace(/ # TODO :[^\n]*/, ''); + assert.deepEqual(await scanFailure(`${unexpected}\n`, filenames, size), + { filename: filenames[0], reason: unexpected }); + } + } + }); + } +}); + describe('Streaming failure file scanner', () => { + it('returns the matching TAP failure without duplicating chunk overlaps', async() => { + const failure = tap(' severity: fail\n AssertionError: expected true, received false'); + for (const size of [1, 31, 8192]) { + assert.deepEqual(await scanFailure(`unrelated output\n${failure}trailing output\n`, + [filename], size), { filename, reason: failure.trimEnd() }); + } + }); + + it('returns the diagnostic preceding a matching file', async() => { + const reason = 'error: compilation failed\n src/node.cc:42'; + assert.deepEqual(await scanFailure(`unrelated output\n${reason}\n`, ['src/node.cc'], 1), + { filename: 'src/node.cc', reason }); + }); + + it('returns the matching C++ failure with its preceding context', async() => { + const reason = 'test/cctest/test-example.cc:42\nExpected: 1\nActual: 2\n[ FAILED ] Example'; + assert.deepEqual(await scanFailure(`${reason}\n`, ['test/cctest/test-example.cc'], 1), + { filename: 'test/cctest/test-example.cc', reason }); + }); + + it('returns the matching git failure block', async() => { + const reason = 'Changes not staged for commit:\n modified: src/node.cc\n' + + 'no changes added to commit'; + assert.deepEqual(await scanFailure(`${reason}\n`, ['src/node.cc'], 1), + { filename: 'src/node.cc', reason }); + }); + + it('bounds giant lines while retaining both ends of the diagnostic', async() => { + const reason = `src/node.cc ${'x'.repeat(200000)} error: failure`; + const failure = await scanFailure(`${reason}\n`, ['src/node.cc'], 31); + assert.equal(failure.filename, 'src/node.cc'); + assert.ok(failure.reason.length < 8300); + assert.match(failure.reason, /^src\/node\.cc /); + assert.match(failure.reason, /failure output truncated/); + assert.match(failure.reason, / error: failure$/); + }); + + it('bounds giant TAP blocks while retaining the failure and final diagnostic', async() => { + const text = tap(`${' context\n'.repeat(10000)} AssertionError: failure`); + const failure = await scanFailure(text); + assert.equal(failure.filename, filename); + assert.ok(failure.reason.length < 8300); + assert.match(failure.reason, /^not ok 1 parallel\/test-example/); + assert.match(failure.reason, /failure output truncated/); + assert.match(failure.reason, /AssertionError: failure\n {2}\.\.\.$/); + }); + for (const message of [ 'src/node.cc: Read-only file system', 'error C2143: src/node.cc', @@ -53,6 +126,14 @@ describe('Streaming failure file scanner', () => { const log = tap(' c:\\\\workspace\\\\test\\\\fixtures\\\\é-example.js:42:1') .replaceAll('\n', '\r\n'); assert.equal(await scan(log, [path], 1), path); + assert.deepEqual(await scanFailure(log, [path], 1), + { filename: path, reason: log.replaceAll('\r', '').trimEnd() }); + }); + + it('preserves literal backslashes in failure output', async() => { + const failure = tap(" actual: '\\\\n'\n expected: '\\\\t'"); + assert.deepEqual(await scanFailure(failure, [filename], 1), + { filename, reason: failure.trimEnd() }); }); it('requires a real path boundary across chunks', async() => { @@ -101,7 +182,8 @@ describe('Streaming failure file scanner', () => { assert.equal(await scanner.scan([Buffer.from('not ok 1 parallel/test-example\n')]), undefined); assert.equal(await scanner.scan([Buffer.from('unrelated output\n ...\n')]), undefined); - assert.equal(await scanner.scan([Buffer.from(tap(' actual failure'))]), filename); + assert.deepEqual(await scanner.scan([Buffer.from(tap(' actual failure'))]), + { filename, reason: tap(' actual failure').trimEnd() }); }); it('matches a compiler diagnostic without a final newline', async() => { @@ -137,8 +219,9 @@ describe('Streaming failure file scanner', () => { for (let i = 0; i < 2048; i++) yield chunk; yield Buffer.from('\\n ...\\n'); } - assert.equal(await new FailureFileScanner([${JSON.stringify(filename)}]).scan(source()), - ${JSON.stringify(filename)}); + const failure = await new FailureFileScanner([${JSON.stringify(filename)}]).scan(source()); + assert.equal(failure.filename, ${JSON.stringify(filename)}); + assert.ok(failure.reason.length < 8300); `], { encoding: 'utf8', timeout: 30000 }); assert.ifError(result.error); assert.equal(result.status, 0, result.stdout + result.stderr); @@ -174,7 +257,8 @@ describe('Streaming HTTP logs', () => { await setImmediate(); } }); - assert.equal(await new FailureFileScanner([filename]).scan(request.stream(url)), filename); + assert.deepEqual(await new FailureFileScanner([filename]).scan(request.stream(url)), + { filename, reason: tap(' failure').trimEnd() }); await responseClosed; assert.ok(sent < 1024 * 1024 * 1024, `Downloaded ${sent} trailing bytes`); }); @@ -185,7 +269,8 @@ describe('Streaming HTTP logs', () => { res.writeHead(200, { 'Content-Encoding': 'gzip' }); res.end(gzipSync(tap(' failure'))); }); - assert.equal(await new FailureFileScanner([filename]).scan(request.stream(url)), filename); + assert.deepEqual(await new FailureFileScanner([filename]).scan(request.stream(url)), + { filename, reason: tap(' failure').trimEnd() }); }); it('cancels an error response instead of scanning its body', async(t) => { diff --git a/test/unit/ci_resume.test.js b/test/unit/ci_resume.test.js index 1a6935a5..5ba70aef 100644 --- a/test/unit/ci_resume.test.js +++ b/test/unit/ci_resume.test.js @@ -57,8 +57,35 @@ const failureBuildData = { // Diagnostic excerpts captured with ncu-ci walk pr on 2026-09-09. const walkFailures = JSON.parse(readFileSync( new URL('../fixtures/ci-resume-walk.json', import.meta.url), 'utf8')); +const reliabilityFailures = JSON.parse(readFileSync( + new URL('../fixtures/ci-reliability-failures.json', import.meta.url), 'utf8')); describe('Resume file checks against real CI diagnostics', () => { + for (const { name, kind, filenames, log, url } of reliabilityFailures) { + if (kind !== 'failure') continue; + it(`prints the diagnostic and console URL for ${name}`, async() => { + const data = structuredClone(failureBuildData); + const buildURL = url.replace(/console(?:Text)?$/, ''); + data.subBuilds[0].build.subBuilds[0].url = buildURL; + const request = { + async json() { return data; }, + async * stream(url) { + assert.equal(url, `${buildURL}consoleText`); + yield Buffer.from(`${log}\n`); + }, + async * getPullRequestFiles() { + for (const filename of filenames) yield { filename }; + } + }; + const cli = new TestCLI(); + const runner = new ResumePRJob(cli, request, 'nodejs', 'node', 1); + assert.equal(await runner.checkFailures(1), false); + assert.deepEqual(cli._calls.error, [[filenames[0]]]); + assert.deepEqual(cli._calls.info, [[`${buildURL}consoleText`]]); + assert.deepEqual(cli._calls.log, [[log]]); + }); + } + for (const { filename, failure } of walkFailures) { it(`detects a PR change to ${filename}`, async() => { const request = { @@ -848,6 +875,10 @@ describe('Jenkins resume', () => { assert.equal(await jobRunner.resume(), false); sinon.assert.notCalled(resumeRequest); assert.deepEqual(cli._calls.error, [[filename]]); + assert.deepEqual(cli._calls.info, [ + ['https://ci.nodejs.org/job/node-test-commit-linux-freestyle/1/consoleText'] + ]); + assert.deepEqual(cli._calls.log, [[failureLog.trimEnd()]]); }); } @@ -1130,6 +1161,8 @@ describe('ncu-ci resume CLI', () => { assert.equal(status, 1, output); assert.match(output, /Refusing to resume CI: failures reference files changed by this PR/); assert.match(output, /test\/parallel\/test-example.js/); + assert.ok(output.includes('https://ci.nodejs.org/job/node-test-commit-linux-freestyle/1/consoleText')); + assert.ok(output.includes(failureLog.trimEnd())); assert.doesNotMatch(output, /PR CI job successfully resumed/); }); From 2c640e77399ccbc1ba1c45b86a80ba3d7f0e0cb7 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Fri, 25 Sep 2026 16:26:27 +0200 Subject: [PATCH 6/6] fix: ignore make exit summaries in resume checks A failed make target names its Makefile even when an unrelated test failed. Exclude these summaries from file attribution while retaining direct Makefile diagnostics and failures in changed tests. Assisted-by: Codex Signed-off-by: Filip Skokan --- docs/ncu-ci.md | 2 + lib/ci/failure_file_scanner.js | 12 ++++ test/fixtures/ci-resume-make-failure.json | 7 +++ test/unit/ci_failure_file_scanner.test.js | 70 +++++++++++++++++++++++ test/unit/ci_resume.test.js | 25 ++++++++ 5 files changed, 116 insertions(+) create mode 100644 test/fixtures/ci-resume-make-failure.json diff --git a/docs/ncu-ci.md b/docs/ncu-ci.md index 805be8e7..b4794eff 100644 --- a/docs/ncu-ci.md +++ b/docs/ncu-ci.md @@ -247,6 +247,8 @@ Before resuming, the command streams failed-job console output and compares failure diagnostics with the PR's changed files. When recovering through resume ancestry, it checks the latest run and every ancestor visited. It refuses to resume if a failed test or a file referenced in a failure diagnostic is changed by the PR. +Generic `make` recipe-failure summaries do not count as file references; direct +Makefile diagnostics, such as syntax errors, still do. The refusal includes the first matching filename, failure excerpt, and console log URL. Large excerpts are truncated with an explicit marker. Logs are scanned one at a time with bounded memory. HTTP compression is decoded as the diff --git a/lib/ci/failure_file_scanner.js b/lib/ci/failure_file_scanner.js index aa54e55f..763b4798 100644 --- a/lib/ci/failure_file_scanner.js +++ b/lib/ci/failure_file_scanner.js @@ -13,6 +13,11 @@ const diagnostic = createMatcher(FAILURE_PATTERNS.diagnostic); const infrastructure = createMatcher(FAILURE_PATTERNS.infrastructure); const gitStart = createMatcher(FAILURE_MARKERS.git.map(({ start }) => start)); const gitEnd = createMatcher(FAILURE_MARKERS.git.map(({ end }) => end)); +const makeRecipeFailure = createMatcher([ + /^\s*(?:\[(?:out|err)\]\s*)?g?make(?:\[\d+\])?: \*\*\* \[[^\r\n]+\] .+$/, + /^\s*(?:\[(?:out|err)\]\s*)?[^\r\n]+:\d+: recipe for target '.+' failed$/ +]); +const makefileDiagnostic = /^\s*(?:\[(?:out|err)\]\s*)?[^\r\n]+:\d+: \*\*\* /; // Keep both ends of large diagnostics without retaining whole lines or TAP blocks. class FailureExcerpt { @@ -97,6 +102,13 @@ async function * failureLines(windows, matchFile) { line.gitEnd ||= gitEnd(text); if (lineEnd) { line.text = excerpt.toString(); + if (makeRecipeFailure(line.text.trimEnd())) { + // The recipe location reports a child's exit, not the source of its + // failure. Keep the output without attributing it through TAP or history. + line = { text: line.text, todo: line.todo }; + } else { + line.diagnostic ||= makefileDiagnostic.test(line.text); + } yield line; line = {}; excerpt = new FailureExcerpt(); diff --git a/test/fixtures/ci-resume-make-failure.json b/test/fixtures/ci-resume-make-failure.json new file mode 100644 index 00000000..28286813 --- /dev/null +++ b/test/fixtures/ci-resume-make-failure.json @@ -0,0 +1,7 @@ +{ + "pr": "https://github.com/nodejs/node/pull/66228", + "url": "https://ci.nodejs.org/job/node-test-commit-linux/nodes=fedora-latest-x64/73461/consoleText", + "filename": "test/ffi/test-ffi-calls.js", + "failure": "not ok 81 ffi/test-ffi-calls\n ---\n duration_ms: 123029.13600\n severity: fail\n exitcode: -15\n stack: |-\n timeout\n (node:1962772) ExperimentalWarning: FFI is an experimental feature and might change at any time\n (Use `node --trace-warnings ...` to show where the warning was created)\n ...", + "summary": "make[1]: *** [Makefile:660: test-ci] Error 1" +} diff --git a/test/unit/ci_failure_file_scanner.test.js b/test/unit/ci_failure_file_scanner.test.js index 9426816d..a9d32926 100644 --- a/test/unit/ci_failure_file_scanner.test.js +++ b/test/unit/ci_failure_file_scanner.test.js @@ -31,6 +31,8 @@ async function scan(...args) { // Complete console diagnostics and truncated report excerpts from September 23–25, 2026. const reliabilityFailures = JSON.parse(readFileSync( new URL('../fixtures/ci-reliability-failures.json', import.meta.url), 'utf8')); +const makeFailure = JSON.parse(readFileSync( + new URL('../fixtures/ci-resume-make-failure.json', import.meta.url), 'utf8')); describe('Reliability report diagnostics', () => { for (const { name, kind, filenames, log } of reliabilityFailures) { @@ -49,6 +51,74 @@ describe('Reliability report diagnostics', () => { } }); +describe('Make recipe failure summaries', () => { + const summary = makeFailure.summary; + + it('attributes a test timeout to the test rather than the make recipe', async() => { + const log = `${makeFailure.failure}\n${summary}\n`; + for (const size of [1, 31, 8192]) { + assert.equal(await scan(log, ['Makefile'], size), undefined); + assert.deepEqual(await scanFailure(log, ['Makefile', makeFailure.filename], size), + { filename: makeFailure.filename, reason: makeFailure.failure }); + } + }); + + for (const [name, footer] of [ + ['recursive make', summary], + ['top-level make', 'make: *** [Makefile:660: test-ci] Error 2'], + ['gmake', 'gmake[2]: *** [Makefile:660: test-ci] Error 1'], + ['segmentation fault', 'make[1]: *** [Makefile:660: test-ci] Segmentation fault (core dumped)'], + ['abort', 'make[1]: *** [Makefile:660: test-ci] Aborted (core dumped)'], + ['failed recipe', "Makefile:660: recipe for target 'test-ci' failed"], + ['prefixed stdout', ' [out] make[1]: *** [Makefile:660: test-ci] Error 1'], + ['prefixed stderr', " [err] Makefile:660: recipe for target 'test-ci' failed"] + ]) { + it(`ignores a propagated exit summary: ${name}`, async() => { + for (const size of [1, 31, 8192]) { + assert.equal(await scan(`${footer}\n`, ['Makefile'], size), undefined); + } + }); + } + + for (const [name, log] of [ + ['preceding error', `error: unrelated failure\n${summary}\n`], + ['following filesystem error', `${summary}\nRead-only file system\n`], + ['following C++ failure', `${summary}\n[ FAILED ] Example\n`], + ['following filename', `${summary}\nMakefile\n`], + ['TAP block', tap(` ${summary}`)], + ['git failure block', + `Changes not staged for commit:\n${summary}\nno changes added to commit\n`] + ]) { + it(`does not use a propagated exit summary as failure context: ${name}`, async() => { + for (const size of [1, 31, 8192]) { + assert.equal(await scan(log, ['Makefile'], size), undefined); + } + }); + } + + it('continues to later failures and preserves the summary as output context', async() => { + const failure = tap(` ${summary}\n AssertionError: failure`); + const log = `${summary}\n${failure}`; + assert.deepEqual(await scanFailure(log, ['Makefile', filename], 1), + { filename, reason: failure.trimEnd() }); + }); + + for (const [name, reason] of [ + ['missing separator', 'Makefile:123: *** missing separator. Stop.'], + ['unterminated variable', 'Makefile:123: *** unterminated variable reference. Stop.'], + ['recipe before target', 'Makefile:123: *** recipe commences before first target. Stop.'], + ['invalid recipe', 'Makefile:123: error: invalid recipe'], + ['unreadable makefile', 'fatal: Unable to read Makefile: No such file or directory'] + ]) { + it(`retains a genuine Makefile diagnostic: ${name}`, async() => { + for (const size of [1, 31, 8192]) { + assert.deepEqual(await scanFailure(`${reason}\n`, ['Makefile'], size), + { filename: 'Makefile', reason }); + } + }); + } +}); + describe('Streaming failure file scanner', () => { it('returns the matching TAP failure without duplicating chunk overlaps', async() => { const failure = tap(' severity: fail\n AssertionError: expected true, received false'); diff --git a/test/unit/ci_resume.test.js b/test/unit/ci_resume.test.js index 5ba70aef..62b2f236 100644 --- a/test/unit/ci_resume.test.js +++ b/test/unit/ci_resume.test.js @@ -59,6 +59,8 @@ const walkFailures = JSON.parse(readFileSync( new URL('../fixtures/ci-resume-walk.json', import.meta.url), 'utf8')); const reliabilityFailures = JSON.parse(readFileSync( new URL('../fixtures/ci-reliability-failures.json', import.meta.url), 'utf8')); +const makeFailure = JSON.parse(readFileSync( + new URL('../fixtures/ci-resume-make-failure.json', import.meta.url), 'utf8')); describe('Resume file checks against real CI diagnostics', () => { for (const { name, kind, filenames, log, url } of reliabilityFailures) { @@ -178,6 +180,29 @@ describe('Jenkins resume', () => { assert.deepEqual(cli._calls.stopSpinner.at(-1), ['PR CI job successfully resumed']); }); + it('allows resuming when only a make exit summary references a changed file', async() => { + request.json.withArgs(filesURL).resolves([ + { filename: 'Makefile' }, + { filename: '.github/workflows/build-tarball.yml' }, + { filename: '.github/workflows/test-shared.yml' } + ]); + request.text.resolves(`${makeFailure.failure}\n${makeFailure.summary}\n`); + assert.equal(await jobRunner.resume(), true); + sinon.assert.calledOnce(resumeRequest); + assert.deepEqual(cli._calls.error, []); + }); + + it('refuses resuming when the failing test changed despite a make exit summary', async() => { + request.json.withArgs(filesURL).resolves([ + { filename: 'Makefile' }, { filename: makeFailure.filename } + ]); + request.text.resolves(`${makeFailure.failure}\n${makeFailure.summary}\n`); + assert.equal(await jobRunner.resume(), false); + sinon.assert.notCalled(resumeRequest); + assert.deepEqual(cli._calls.error, [[makeFailure.filename]]); + assert.deepEqual(cli._calls.log, [[makeFailure.failure]]); + }); + for (const result of ['FAILURE', 'ABORTED']) { it(`does not scan failures or POST when a ${result} job has no resume action`, async() => { request.json.withArgs(apiURL).resolves({ ...resumeBuildData, result });