From 950cb338d8705798db3bad1f227e82d162eaff48 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 9 Jul 2026 20:25:25 +0200 Subject: [PATCH 1/3] feat(git-node): add `git node benchmark` --- components/git/benchmark.js | 112 ++++++++++++ docs/git-node.md | 66 ++++++- lib/benchmark.js | 239 +++++++++++++++++++++++++ lib/request.js | 41 +++++ test/unit/benchmark.test.js | 235 ++++++++++++++++++++++++ test/unit/benchmark_identifier.test.js | 33 ++++ 6 files changed, 725 insertions(+), 1 deletion(-) create mode 100644 components/git/benchmark.js create mode 100644 lib/benchmark.js create mode 100644 test/unit/benchmark.test.js create mode 100644 test/unit/benchmark_identifier.test.js diff --git a/components/git/benchmark.js b/components/git/benchmark.js new file mode 100644 index 00000000..9c04e557 --- /dev/null +++ b/components/git/benchmark.js @@ -0,0 +1,112 @@ +import auth from '../../lib/auth.js'; +import { parsePRFromURL } from '../../lib/links.js'; +import CLI from '../../lib/cli.js'; +import Request from '../../lib/request.js'; +import { runPromise } from '../../lib/run.js'; +import BenchmarkSession from '../../lib/benchmark.js'; + +export const command = 'benchmark '; +export const describe = + 'Trigger the benchmark GitHub Actions workflow for a pull request'; + +const options = { + owner: { + alias: 'o', + describe: 'GitHub owner of the PR repository', + default: 'nodejs', + type: 'string' + }, + repo: { + alias: 'r', + describe: 'GitHub repository of the PR', + default: 'node', + type: 'string' + }, + 'workflow-owner': { + describe: 'GitHub owner of the repository hosting the benchmark workflow ' + + '(defaults to the PR owner)', + type: 'string' + }, + 'workflow-repo': { + describe: 'GitHub repository hosting the benchmark workflow ' + + '(defaults to the PR repository)', + type: 'string' + }, + workflow: { + describe: 'The workflow file to dispatch', + default: 'benchmark.yml', + type: 'string' + }, + ref: { + describe: 'The git ref of the workflow repository to run the workflow from', + default: 'main', + type: 'string' + }, + runs: { + describe: 'How many times to repeat each benchmark', + type: 'number' + } +}; + +let yargsInstance; + +export function builder(yargs) { + yargsInstance = yargs; + return yargs + .options(options) + .positional('identifier', { + type: 'string', + describe: 'ID or URL of the pull request. A commit URL ' + + '(…/pull//commits/) benchmarks that commit instead of the ' + + 'PR head.' + }) + .example('git node benchmark 12344', + 'Pick benchmark categories to run on ' + + 'https://github.com/nodejs/node/pull/12344') + .example('git node benchmark https://github.com/nodejs/node/pull/12344', + 'Pick benchmark categories to run on ' + + 'https://github.com/nodejs/node/pull/12344') + .example( + 'git node benchmark ' + + 'https://github.com/nodejs/node/pull/12344/commits/abc1234', + 'Benchmark commit abc1234 rather than the current PR head') + .wrap(90); +} + +const COMMIT_URL_RE = /\/pull\/\d+\/commits\/([0-9a-f]{7,40})/i; + +// Parse the positional identifier into { prid, owner?, repo?, commit? }. +// A PR commit URL (…/pull//commits/) also yields the commit SHA. +export function parseIdentifier(identifier) { + const prid = Number.parseInt(identifier); + if (!Number.isNaN(prid)) { + return { prid }; + } + const parsed = parsePRFromURL(identifier); + if (!parsed) { + return undefined; + } + const commit = COMMIT_URL_RE.exec(identifier); + if (commit) { + parsed.commit = commit[1]; + } + return parsed; +} + +export function handler(argv) { + const parsed = parseIdentifier(argv.identifier); + if (!parsed) { + return yargsInstance.showHelp(); + } + Object.assign(argv, parsed); + + return runPromise(main(argv)); +} + +async function main(argv) { + const cli = new CLI(process.stderr); + const credentials = await auth({ github: true }); + const request = new Request(credentials); + const session = new BenchmarkSession(cli, request, argv); + return session.start(); +} diff --git a/docs/git-node.md b/docs/git-node.md index dc93303a..be76df9d 100644 --- a/docs/git-node.md +++ b/docs/git-node.md @@ -28,8 +28,10 @@ A custom Git command for managing pull requests. You can run it as - [Usage](#usage) - [`git node status`](#git-node-status) - [Example](#example-2) - - [`git node wpt`](#git-node-wpt) + - [`git node benchmark`](#git-node-benchmark) - [Example](#example-3) + - [`git node wpt`](#git-node-wpt) + - [Example](#example-4) ## `git node land` @@ -568,6 +570,66 @@ Upstream: upstream Branch: main ``` +## `git node benchmark` + +Trigger the [`benchmark.yml`][] GitHub Actions workflow on a pull request. The +command fetches the PR, then prompts you to pick which benchmark categories to +run (the folders in [`benchmark/`][] at the PR head). Categories whose files are +touched by the PR are checked by default; if the PR touches no benchmark file, +the pre-selection is guessed from the PR's subsystem labels. You are then prompted for an optional +filter substring; if the PR touches a single benchmark file and only that file's +category is selected, the filter is pre-filled with that file's name. Finally you +are asked whether the workflow should post the results as a comment on the PR. It +then dispatches the workflow with the PR number and its head commit. + +``` +git-node benchmark + +Trigger the benchmark GitHub Actions workflow for a pull request + +Positionals: + identifier ID or URL of the pull request; a commit URL + (…/pull//commits/) benchmarks that commit [string] [required] + +Options: + --version Show version number [boolean] + --help Show help [boolean] + --owner, -o GitHub owner of the PR repository [string] [default: "nodejs"] + --repo, -r GitHub repository of the PR [string] [default: "node"] + --workflow-owner GitHub owner of the repository hosting the benchmark workflow + (defaults to the PR owner) [string] + --workflow-repo GitHub repository hosting the benchmark workflow (defaults to the PR + repository) [string] + --workflow The workflow file to dispatch [string] [default: "benchmark.yml"] + --ref The git ref of the workflow repository to run the workflow from + [string] [default: "main"] + --runs How many times to repeat each benchmark [number] +``` + +The workflow is dispatched on the PR's repository by default. When the workflow +lives in a different repository (for example a fork that hosts `benchmark.yml`), +pass `--workflow-owner`/`--workflow-repo`, and the PR repository is forwarded to +the workflow so it fetches the PR from the right place. + +### Example + +```bash +# Interactively pick benchmark categories to run on nodejs/node#12344 +# (categories touched by the PR are pre-selected) +$ git node benchmark 12344 +# is equivalent to +$ git node benchmark https://github.com/nodejs/node/pull/12344 + +# Repeat each benchmark 10 times (the filter is asked interactively) +$ git node benchmark 12344 --runs 10 + +# Dispatch the workflow hosted on a fork for a nodejs/node PR +$ git node benchmark 12344 --workflow-owner my-user --workflow-repo node + +# Benchmark a specific PR commit instead of the current PR head +$ git node benchmark https://github.com/nodejs/node/pull/12344/commits/abc1234 +``` + ## `git node wpt` Update or patch the Web Platform Tests in core. @@ -603,3 +665,5 @@ $ git node wpt url --commit=43feb7f612fe9160639e09a47933a29834904d69 [node.js abi version registry]: https://github.com/nodejs/node/blob/main/doc/abi_version_registry.json [Security Release Process]: https://github.com/nodejs/node/blob/main/doc/contributing/security-release-process.md +[`benchmark.yml`]: https://github.com/nodejs/node/blob/main/.github/workflows/benchmark.yml +[`benchmark/`]: https://github.com/nodejs/node/tree/main/benchmark diff --git a/lib/benchmark.js b/lib/benchmark.js new file mode 100644 index 00000000..613ffeb3 --- /dev/null +++ b/lib/benchmark.js @@ -0,0 +1,239 @@ +import { basename } from 'node:path'; + +import { getPrURL } from './links.js'; +import { IGNORE } from './run.js'; + +const BENCHMARK_DIR = 'benchmark'; +// Entries in the benchmark folder that are not benchmark categories. +const NON_CATEGORY_DIRS = new Set(['fixtures']); + +export default class BenchmarkSession { + constructor(cli, request, argv) { + this.cli = cli; + this.request = request; + this.argv = argv; + this.workflow = argv.workflow || 'benchmark.yml'; + } + + get prUrl() { + const { owner, repo, prid } = this.argv; + return getPrURL({ owner, repo, prid }); + } + + async getPullRequest() { + const { cli } = this; + cli.startSpinner(`Fetching data for ${this.prUrl}`); + const pr = await this.request.getPullRequest(this.prUrl); + if (!pr || !pr.head) { + cli.stopSpinner( + `Could not find pull request ${this.prUrl}`, + cli.SPINNER_STATUS.FAILED); + throw new Error(IGNORE); + } + cli.stopSpinner(`Fetched data for ${this.prUrl}`); + this.pr = pr; + return pr; + } + + async getCategories() { + const { cli, pr } = this; + const head = pr.head; + // Prefer listing the benchmark folder on the PR head, falling back to the + // base branch if the head repository is unavailable (e.g. deleted fork). + const source = head.repo + ? { fullName: head.repo.full_name, ref: head.sha } + : { fullName: pr.base.repo.full_name, ref: pr.base.ref }; + const [owner, repo] = source.fullName.split('/'); + + cli.startSpinner('Fetching available benchmark categories'); + let entries; + try { + entries = await this.request.listDirectory({ + owner, + repo, + path: BENCHMARK_DIR, + ref: source.ref + }); + } catch (e) { + cli.stopSpinner( + 'Could not fetch benchmark categories', + cli.SPINNER_STATUS.FAILED); + throw e; + } + + const categories = entries + .filter((entry) => entry.type === 'dir') + .map((entry) => entry.name) + .filter((name) => !NON_CATEGORY_DIRS.has(name)) + .sort(); + + if (categories.length === 0) { + cli.stopSpinner( + `No benchmark categories found in ${source.fullName}/${BENCHMARK_DIR}`, + cli.SPINNER_STATUS.FAILED); + throw new Error(IGNORE); + } + + cli.stopSpinner(`Found ${categories.length} benchmark categories`); + this.categories = categories; + return categories; + } + + async getTouchedBenchmarks() { + const { cli, argv } = this; + const touched = []; + try { + const files = this.request.getPullRequestFiles({ + owner: argv.owner, + repo: argv.repo, + prid: argv.prid + }); + for await (const file of files) { + const match = /^benchmark\/([^/]+)\/(.+\.js)$/.exec(file.filename); + if (match && this.categories.includes(match[1])) { + touched.push({ + category: match[1], + name: basename(match[2], '.js') + }); + } + } + } catch { + // Not being able to fetch the changed files is not fatal: fall back to + // an empty pre-selection. + cli.warn('Could not determine which benchmark files the PR touches.'); + } + this.touched = touched; + return touched; + } + + // Guess relevant categories from the PR subsystem labels, matching a label + // to a category name up to a trailing plural "s" (e.g. `buffer` -> `buffers`). + categoriesFromLabels() { + const normalize = (name) => name.toLowerCase().replace(/s$/, ''); + const labels = (this.pr.labels ?? []) + .map((label) => (typeof label === 'string' ? label : label.name)) + .filter(Boolean) + .map(normalize); + return new Set( + this.categories.filter((category) => + labels.includes(normalize(category)))); + } + + async promptCategories() { + const { cli, categories } = this; + // Fall back to the PR labels when the PR does not touch any benchmark file. + const preselected = this.touched.length + ? new Set(this.touched.map((f) => f.category)) + : this.categoriesFromLabels(); + + cli.info(`Based on the PR ${this.touched.length ? 'changed files' : 'labels'}, this/these benchmark(s) seem relevant:`) + cli.info([...preselected].join(', ')); + cli.info('If not relevant, do not forget to unselect it/them.') + + const selected = await cli.promptCheckbox( + 'Select the benchmark categories to run:', + categories.map((name) => ({ + name, + value: name, + checked: preselected.has(name) + }))); + + if (!selected || selected.length === 0) { + cli.warn('No benchmark category selected, aborting.'); + throw new Error(IGNORE); + } + return selected; + } + + async promptFilter(selected) { + const { cli, touched } = this; + + // When a single benchmark file was touched and only its category is + // selected, pre-fill the filter with that file so it runs on its own. + const defaultAnswer = + touched.length === 1 && selected.length === 1 && + selected[0] === touched[0].category + ? touched[0].name + : ''; + + const filter = await cli.prompt( + 'Substring to filter which benchmarks run (leave empty to run all):', + { questionType: cli.QUESTION_TYPE.INPUT, defaultAnswer }); + return filter?.trim() || undefined; + } + + async start() { + const { cli, argv } = this; + + await this.getPullRequest(); + await this.getCategories(); + cli.startSpinner('Checking for benchmark files listed in the PR changes...'); + await this.getTouchedBenchmarks(); + cli.stopSpinner(`Found ${this.touched.length} benchmark file(s) in the PR changes`) + const selected = await this.promptCategories(); + const filter = await this.promptFilter(selected); + + const workflowOwner = argv.workflowOwner || argv.owner; + const workflowRepo = argv.workflowRepo || argv.repo; + const prRepo = `${argv.owner}/${argv.repo}`; + // The workflow token can only comment when it runs in the PR repository; + // default to not commenting when the workflow lives elsewhere. + const sameRepo = `${workflowOwner}/${workflowRepo}` === prRepo; + const postComment = await cli.prompt( + 'Should the workflow post the results as a comment on the PR?', + { defaultAnswer: sameRepo }); + + const category = selected.join(' '); + // Benchmark the commit provided on the command line, defaulting to the + // current PR head. + const commit = argv.commit || this.pr.head.sha; + + const inputs = { + pr_id: `${argv.prid}`, + commit, + category + }; + // The benchmark workflow checks out `inputs.repo || github.repository`; only + // set it when the workflow lives in a different repository than the PR. + if (!sameRepo) { + inputs.repo = prRepo; + } + if (filter) { + inputs.filter = filter; + } + if (argv.runs != null) { + inputs.runs = `${argv.runs}`; + } + inputs['post-comment'] = `${postComment}`; + + cli.separator('Benchmark'); + cli.table('PR:', this.prUrl); + cli.table('Commit:', commit); + cli.table('Categories:', category); + if (inputs.filter) cli.table('Filter:', inputs.filter); + if (inputs.runs) cli.table('Runs:', inputs.runs); + cli.table('Comment on PR:', postComment ? 'yes' : 'no'); + cli.table('Workflow:', + `${workflowOwner}/${workflowRepo} (${this.workflow}@${argv.ref})`); + + const confirmed = await cli.prompt( + 'Trigger the benchmark workflow with these settings?', + { defaultAnswer: true }); + if (!confirmed) { + cli.warn('Aborted, no workflow was triggered.'); + throw new Error(IGNORE); + } + + cli.startSpinner('Dispatching benchmark workflow'); + await this.request.dispatchWorkflow(this.workflow, { + owner: workflowOwner, + repo: workflowRepo, + ref: argv.ref, + inputs + }); + cli.stopSpinner('Benchmark workflow dispatched'); + cli.info('Follow the run at ' + + `https://github.com/${workflowOwner}/${workflowRepo}/actions/` + + `workflows/${this.workflow}`); + } +} diff --git a/lib/request.js b/lib/request.js index c28e92a0..4e2cf5dc 100644 --- a/lib/request.js +++ b/lib/request.js @@ -100,6 +100,47 @@ export default class Request { return this.json(prUrl); } + async * getPullRequestFiles({ owner, repo, prid }) { + let page = 1; + for (;;) { + const url = + `/repos/${owner}/${repo}/pulls/${prid}/files?per_page=100&page=${page}`; + const batch = await this.json(url); + if (!Array.isArray(batch) || batch.length === 0) { + break; + } + yield * batch; + if (batch.length < 100) { + break; + } + page++; + } + } + + async listDirectory({ owner, repo, path, ref }) { + let url = `/repos/${owner}/${repo}/contents/${path}`; + if (ref) { + url += `?ref=${encodeURIComponent(ref)}`; + } + return this.json(url); + } + + async dispatchWorkflow(workflowId, { owner, repo, ref, inputs }) { + const url = + `/repos/${owner}/${repo}/actions/workflows/${workflowId}/dispatches`; + const res = await this.fetch(url, { + method: 'POST', + body: JSON.stringify({ ref, inputs }) + }); + if (!res.ok) { + const text = await res.text(); + throw new Error( + `Failed to dispatch workflow ${workflowId} on ${owner}/${repo}: ` + + `${res.status} ${res.statusText}${text ? `\n${text}` : ''}`); + } + return res; + } + async createPullRequest(title, body, { owner, repo, head, base }) { const url = `/repos/${owner}/${repo}/pulls`; const options = { diff --git a/test/unit/benchmark.test.js b/test/unit/benchmark.test.js new file mode 100644 index 00000000..fd56db48 --- /dev/null +++ b/test/unit/benchmark.test.js @@ -0,0 +1,235 @@ +import { describe, it, beforeEach } from 'node:test'; +import assert from 'node:assert'; + +import * as sinon from 'sinon'; + +import BenchmarkSession from '../../lib/benchmark.js'; + +const QUESTION_TYPE = { INPUT: 'input', CONFIRM: 'confirm' }; + +// prompt() is used both for yes/no confirmations and for the free-text filter +// input; honour the prompt's defaultAnswer for confirmations and return an +// empty string for inputs. +function defaultPrompt(message, opts = {}) { + if (opts.questionType === QUESTION_TYPE.INPUT) { + return Promise.resolve(''); + } + return Promise.resolve(opts.defaultAnswer ?? true); +} + +function makeCli() { + return { + SPINNER_STATUS: { SUCCESS: 'success', FAILED: 'failed' }, + QUESTION_TYPE, + startSpinner: sinon.stub(), + stopSpinner: sinon.stub(), + updateSpinner: sinon.stub(), + separator: sinon.stub(), + table: sinon.stub(), + info: sinon.stub(), + warn: sinon.stub(), + error: sinon.stub(), + log: sinon.stub(), + prompt: sinon.stub().callsFake(defaultPrompt), + promptCheckbox: sinon.stub() + }; +} + +const pr = { + number: 123, + head: { + sha: 'deadbeef', + repo: { full_name: 'nodejs/node' } + }, + base: { + ref: 'main', + repo: { full_name: 'nodejs/node' } + } +}; + +const contents = [ + { type: 'dir', name: 'buffers' }, + { type: 'dir', name: 'url' }, + { type: 'dir', name: 'fixtures' }, + { type: 'file', name: '_http-benchmarkers.js' } +]; + +function filesGenerator(files) { + return async function * () { + yield * files; + }; +} + +function makeRequest() { + return { + getPullRequest: sinon.stub().resolves(pr), + getPullRequestFiles: sinon.stub().callsFake(filesGenerator([])), + listDirectory: sinon.stub().resolves(contents), + dispatchWorkflow: sinon.stub().resolves({ ok: true }) + }; +} + +describe('BenchmarkSession', () => { + let cli; + let request; + const baseArgv = { owner: 'nodejs', repo: 'node', prid: 123, ref: 'main' }; + + beforeEach(() => { + cli = makeCli(); + request = makeRequest(); + }); + + it('lists categories excluding fixtures and non-dir entries', async() => { + cli.promptCheckbox.resolves(['buffers']); + const session = new BenchmarkSession(cli, request, { ...baseArgv }); + await session.start(); + + const [, choices] = cli.promptCheckbox.firstCall.args; + assert.deepStrictEqual(choices.map((c) => c.value), ['buffers', 'url']); + assert.ok(request.listDirectory.calledOnce); + assert.deepStrictEqual(request.listDirectory.firstCall.args[0], { + owner: 'nodejs', + repo: 'node', + path: 'benchmark', + ref: 'deadbeef' + }); + }); + + it('dispatches the workflow with the selected categories', async() => { + cli.promptCheckbox.resolves(['buffers', 'url']); + const session = new BenchmarkSession(cli, request, { ...baseArgv }); + await session.start(); + + assert.ok(request.dispatchWorkflow.calledOnce); + const [workflow, opts] = request.dispatchWorkflow.firstCall.args; + assert.strictEqual(workflow, 'benchmark.yml'); + assert.strictEqual(opts.owner, 'nodejs'); + assert.strictEqual(opts.repo, 'node'); + assert.strictEqual(opts.ref, 'main'); + assert.deepStrictEqual(opts.inputs, { + pr_id: '123', + commit: 'deadbeef', + category: 'buffers url', + 'post-comment': 'true' + }); + }); + + it('pre-checks categories touched by the PR', async() => { + request.getPullRequestFiles.callsFake(filesGenerator([ + { filename: 'benchmark/url/url-parse.js' }, + { filename: 'lib/url.js' }, + { filename: 'benchmark/fixtures/foo.js' } + ])); + cli.promptCheckbox.resolves(['url']); + const session = new BenchmarkSession(cli, request, { ...baseArgv }); + await session.start(); + + const [, choices] = cli.promptCheckbox.firstCall.args; + assert.deepStrictEqual(choices, [ + { name: 'buffers', value: 'buffers', checked: false }, + { name: 'url', value: 'url', checked: true } + ]); + }); + + it('falls back to PR labels when no benchmark file is touched', async() => { + request.getPullRequest.resolves({ + ...pr, + labels: [{ name: 'buffer' }, { name: 'dont-land-on-v20.x' }] + }); + cli.promptCheckbox.resolves(['buffers']); + const session = new BenchmarkSession(cli, request, { ...baseArgv }); + await session.start(); + + const [, choices] = cli.promptCheckbox.firstCall.args; + assert.deepStrictEqual(choices, [ + { name: 'buffers', value: 'buffers', checked: true }, + { name: 'url', value: 'url', checked: false } + ]); + }); + + it('pre-fills the filter when a single benchmark file is touched', async() => { + request.getPullRequestFiles.callsFake(filesGenerator([ + { filename: 'benchmark/url/url-parse.js' } + ])); + cli.promptCheckbox.resolves(['url']); + // Accept the pre-filled default answer for the filter prompt. + cli.prompt.callsFake((message, opts = {}) => + Promise.resolve(opts.questionType === QUESTION_TYPE.INPUT + ? opts.defaultAnswer + : true)); + const session = new BenchmarkSession(cli, request, { ...baseArgv }); + await session.start(); + + const [, opts] = request.dispatchWorkflow.firstCall.args; + assert.strictEqual(opts.inputs.filter, 'url-parse'); + }); + + it('uses the interactive filter prompt otherwise', async() => { + cli.promptCheckbox.resolves(['buffers']); + cli.prompt.callsFake((message, opts = {}) => + Promise.resolve(opts.questionType === QUESTION_TYPE.INPUT ? 'foo ' : true)); + const session = new BenchmarkSession(cli, request, { ...baseArgv }); + await session.start(); + + const [, opts] = request.dispatchWorkflow.firstCall.args; + assert.strictEqual(opts.inputs.filter, 'foo'); + }); + + it('sets repo input when the workflow lives in another repository', async() => { + cli.promptCheckbox.resolves(['buffers']); + const session = new BenchmarkSession(cli, request, { + ...baseArgv, + workflowOwner: 'aduh95-evals', + workflowRepo: 'node', + runs: 10 + }); + await session.start(); + + const [, opts] = request.dispatchWorkflow.firstCall.args; + assert.strictEqual(opts.owner, 'aduh95-evals'); + assert.deepStrictEqual(opts.inputs, { + pr_id: '123', + commit: 'deadbeef', + category: 'buffers', + repo: 'nodejs/node', + runs: '10', + // The workflow lives in a different repo, so commenting defaults to off. + 'post-comment': 'false' + }); + }); + + it('benchmarks the provided commit instead of the PR head', async() => { + cli.promptCheckbox.resolves(['buffers']); + const session = new BenchmarkSession(cli, request, { + ...baseArgv, + commit: 'cafebabe' + }); + await session.start(); + + const [, opts] = request.dispatchWorkflow.firstCall.args; + assert.strictEqual(opts.inputs.commit, 'cafebabe'); + }); + + it('sets post-comment to false when the user declines the prompt', async() => { + cli.promptCheckbox.resolves(['buffers']); + // Decline the "comment on the PR?" prompt but confirm the final dispatch. + cli.prompt.callsFake((message, opts = {}) => { + if (/comment/i.test(message)) return Promise.resolve(false); + return Promise.resolve(opts.questionType === QUESTION_TYPE.INPUT ? '' : true); + }); + const session = new BenchmarkSession(cli, request, { ...baseArgv }); + await session.start(); + + const [, opts] = request.dispatchWorkflow.firstCall.args; + assert.strictEqual(opts.inputs['post-comment'], 'false'); + }); + + it('does not dispatch when the confirmation prompt is declined', async() => { + cli.promptCheckbox.resolves(['buffers']); + cli.prompt.callsFake((message, opts = {}) => + Promise.resolve(opts.questionType === QUESTION_TYPE.INPUT ? '' : false)); + const session = new BenchmarkSession(cli, request, { ...baseArgv }); + await assert.rejects(session.start(), /__ignore__/); + assert.ok(request.dispatchWorkflow.notCalled); + }); +}); diff --git a/test/unit/benchmark_identifier.test.js b/test/unit/benchmark_identifier.test.js new file mode 100644 index 00000000..fe4f8f25 --- /dev/null +++ b/test/unit/benchmark_identifier.test.js @@ -0,0 +1,33 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert'; + +import { parseIdentifier } from '../../components/git/benchmark.js'; + +describe('git node benchmark parseIdentifier', () => { + it('parses a numeric PR id', () => { + assert.deepStrictEqual(parseIdentifier('12344'), { prid: 12344 }); + }); + + it('parses a PR URL', () => { + assert.deepStrictEqual( + parseIdentifier('https://github.com/nodejs/node/pull/12344'), + { owner: 'nodejs', repo: 'node', prid: 12344 }); + }); + + it('parses a PR commit URL and captures the commit SHA', () => { + assert.deepStrictEqual( + parseIdentifier( + 'https://github.com/aduh95-evals/node-core-utils/pull/2/commits/' + + 'eea2127204972210d1705f2a4d9c25c58b9cf436'), + { + owner: 'aduh95-evals', + repo: 'node-core-utils', + prid: 2, + commit: 'eea2127204972210d1705f2a4d9c25c58b9cf436' + }); + }); + + it('returns undefined for an unrecognized identifier', () => { + assert.strictEqual(parseIdentifier('not-a-pr'), undefined); + }); +}); From 034f7c2ca4733c64389789ddf45333ba9b439239 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 9 Jul 2026 20:37:13 +0200 Subject: [PATCH 2/3] fixup! feat(git-node): add `git node benchmark` --- lib/benchmark.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/benchmark.js b/lib/benchmark.js index 613ffeb3..8f6da2c2 100644 --- a/lib/benchmark.js +++ b/lib/benchmark.js @@ -126,9 +126,11 @@ export default class BenchmarkSession { ? new Set(this.touched.map((f) => f.category)) : this.categoriesFromLabels(); - cli.info(`Based on the PR ${this.touched.length ? 'changed files' : 'labels'}, this/these benchmark(s) seem relevant:`) + cli.info(`Based on the PR ${ + this.touched.length ? 'changed files' : 'labels' + }, this/these benchmark(s) seem(s) relevant:`); cli.info([...preselected].join(', ')); - cli.info('If not relevant, do not forget to unselect it/them.') + cli.info('If not relevant, do not forget to unselect it/them.'); const selected = await cli.promptCheckbox( 'Select the benchmark categories to run:', @@ -169,7 +171,7 @@ export default class BenchmarkSession { await this.getCategories(); cli.startSpinner('Checking for benchmark files listed in the PR changes...'); await this.getTouchedBenchmarks(); - cli.stopSpinner(`Found ${this.touched.length} benchmark file(s) in the PR changes`) + cli.stopSpinner(`Found ${this.touched.length} benchmark file(s) in the PR changes`); const selected = await this.promptCategories(); const filter = await this.promptFilter(selected); From d274ef4572de5539c78b1e738a9847c8986cf338 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 10 Jul 2026 07:23:05 +0200 Subject: [PATCH 3/3] fixup! feat(git-node): add `git node benchmark` --- lib/benchmark.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/benchmark.js b/lib/benchmark.js index 8f6da2c2..6c1d6eab 100644 --- a/lib/benchmark.js +++ b/lib/benchmark.js @@ -109,7 +109,7 @@ export default class BenchmarkSession { // Guess relevant categories from the PR subsystem labels, matching a label // to a category name up to a trailing plural "s" (e.g. `buffer` -> `buffers`). categoriesFromLabels() { - const normalize = (name) => name.toLowerCase().replace(/s$/, ''); + const normalize = (name) => name.toLowerCase().replace(/s$| /, ''); const labels = (this.pr.labels ?? []) .map((label) => (typeof label === 'string' ? label : label.name)) .filter(Boolean) @@ -126,11 +126,13 @@ export default class BenchmarkSession { ? new Set(this.touched.map((f) => f.category)) : this.categoriesFromLabels(); - cli.info(`Based on the PR ${ + if (preselected.size) { + cli.info(`Based on the PR ${ this.touched.length ? 'changed files' : 'labels' - }, this/these benchmark(s) seem(s) relevant:`); - cli.info([...preselected].join(', ')); - cli.info('If not relevant, do not forget to unselect it/them.'); + }, this/these benchmark(s) seem(s) relevant:`); + cli.info([...preselected].join(', ')); + cli.info('If not relevant, do not forget to unselect it/them.'); + } const selected = await cli.promptCheckbox( 'Select the benchmark categories to run:',