diff --git a/.ai/agent-practices.md b/.ai/agent-practices.md index 79a737ad..012853c5 100644 --- a/.ai/agent-practices.md +++ b/.ai/agent-practices.md @@ -78,14 +78,37 @@ execution environment. Rules of thumb: `SWITCHBOARD_TEST_TIME_SCALE= node --test test/trigger-watcher.test.js` (env, default 1) before assuming the code broke. -## 5. Memory / notes hygiene +## 5. Test concurrency + +`npm test` runs stage 1 at **4 parallel workers**, not at node's default of +`os.availableParallelism()`. On a 14-core workstation that default means a +dozen-plus test processes at once, which saturates the machine the developer is +using and starves `test/trigger-watcher.test.js`, whose wall-clock budgets are +the suite's most contention-sensitive assertions. + +Override it when you want the machine's full width: + +```bash +SWITCHBOARD_TEST_CONCURRENCY=12 npm test +``` + +A value that is not a positive integer is ignored with a message on stderr, and +the cap applies. The cap is also bounded by what the machine actually has, so a +2-core CI runner still gets 2. + +When several agents run the suite at once, cap the whole process tree instead — +`taskset -c 0-3 npm test` on Linux, which node's `availableParallelism()` +honours, so it reduces the number of workers rather than crowding them onto +fewer cores. + +## 6. Memory / notes hygiene Any saved note, memory, or prior observation is a **point-in-time snapshot**, not live state. Before citing something more than about a week old as a current fact — a file's line count, a commit range, a "this is safe" claim — re-verify it against the actual repo (read the file, run the check). If it's stale, correct or delete it in the same pass rather than repeating it. -## 6. Scope discipline +## 7. Scope discipline Fix exactly what was asked. When a task names N specific findings, apply N fixes — no unrelated cleanup, no rewriting adjacent prose "while you're in there". If you notice something diff --git a/README.md b/README.md index e0bdbf3c..7cb4c9aa 100644 --- a/README.md +++ b/README.md @@ -120,7 +120,7 @@ Grab the latest release for your platform: ```bash task install # npm install task dev # launch Electron (--no-sandbox, required on Linux) -task test # node --test +task test # node --test (4 workers; SWITCHBOARD_TEST_CONCURRENCY=N to change) task lint # eslint . task check # test + lint — pre-commit / pre-push gate task ci # same as check but sequential, verbose diff --git a/scripts/run-tests.js b/scripts/run-tests.js index e0a528ac..8d7a6c74 100644 --- a/scripts/run-tests.js +++ b/scripts/run-tests.js @@ -2,8 +2,8 @@ // Runs the node:test suite in two stages, cross-platform (no shell-specific // syntax, so this works the same under cmd.exe and under a POSIX shell). // -// Stage 1: every test file except trigger-watcher.test.js, node's own default -// concurrency. +// Stage 1: every test file except trigger-watcher.test.js, at +// DEFAULT_CONCURRENCY workers (override with SWITCHBOARD_TEST_CONCURRENCY). // Stage 2: trigger-watcher.test.js alone, serially, with a generous timeout. // It uses real timers + real fs.watch against wall-clock budgets (no fake-timer // injection yet -- see .ai/contexts/trigger-watcher.md, "timing tests and host @@ -14,11 +14,28 @@ const { spawnSync } = require('child_process'); const fs = require('fs'); +const os = require('os'); const path = require('path'); const TEST_DIR = path.join(__dirname, '..', 'test'); const ISOLATED_FILE = 'trigger-watcher.test.js'; +// Node defaults stage 1 to os.availableParallelism() workers, which on a large +// workstation is a dozen-plus test processes at once. See +// .ai/agent-practices.md, "Test concurrency". +const DEFAULT_CONCURRENCY = 4; + +function stageOneConcurrency() { + const raw = process.env.SWITCHBOARD_TEST_CONCURRENCY; + if (raw !== undefined && raw !== '') { + const n = Number(raw); + if (Number.isInteger(n) && n > 0) return n; + console.error(`run-tests: ignoring SWITCHBOARD_TEST_CONCURRENCY=${raw} (want a positive integer)`); + } + const available = typeof os.availableParallelism === 'function' ? os.availableParallelism() : os.cpus().length; + return Math.max(1, Math.min(DEFAULT_CONCURRENCY, available)); +} + const mainFiles = fs.readdirSync(TEST_DIR) .filter((name) => name.endsWith('.js') && name !== ISOLATED_FILE) .map((name) => path.join('test', name)); @@ -29,14 +46,18 @@ function run(args) { return result.status === null ? 1 : result.status; } -const mainStatus = run(['--test', ...mainFiles]); +if (require.main === module) { + const mainStatus = run(['--test', `--test-concurrency=${stageOneConcurrency()}`, ...mainFiles]); -// No --test-timeout: with an explicit file operand node applies it to the -// file-level entry too, and this file legitimately runs for minutes on CI. -const isolatedStatus = run([ - '--test', - '--test-concurrency=1', - path.join('test', ISOLATED_FILE), -]); + // No --test-timeout: with an explicit file operand node applies it to the + // file-level entry too, and this file legitimately runs for minutes on CI. + const isolatedStatus = run([ + '--test', + '--test-concurrency=1', + path.join('test', ISOLATED_FILE), + ]); + + process.exit(mainStatus !== 0 ? mainStatus : isolatedStatus); +} -process.exit(mainStatus !== 0 ? mainStatus : isolatedStatus); +module.exports = { stageOneConcurrency, DEFAULT_CONCURRENCY }; diff --git a/test/run-tests-concurrency.test.js b/test/run-tests-concurrency.test.js new file mode 100644 index 00000000..fb290f97 --- /dev/null +++ b/test/run-tests-concurrency.test.js @@ -0,0 +1,54 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert'); +const os = require('node:os'); + +const { stageOneConcurrency, DEFAULT_CONCURRENCY } = require('../scripts/run-tests.js'); + +function withEnv(value, fn) { + const had = Object.prototype.hasOwnProperty.call(process.env, 'SWITCHBOARD_TEST_CONCURRENCY'); + const previous = process.env.SWITCHBOARD_TEST_CONCURRENCY; + if (value === undefined) delete process.env.SWITCHBOARD_TEST_CONCURRENCY; + else process.env.SWITCHBOARD_TEST_CONCURRENCY = value; + try { + return fn(); + } finally { + if (had) process.env.SWITCHBOARD_TEST_CONCURRENCY = previous; + else delete process.env.SWITCHBOARD_TEST_CONCURRENCY; + } +} + +function available() { + return typeof os.availableParallelism === 'function' ? os.availableParallelism() : os.cpus().length; +} + +test('unset: caps at DEFAULT_CONCURRENCY rather than taking every core (mutation target: dropping the cap)', () => { + const got = withEnv(undefined, stageOneConcurrency); + assert.equal(got, Math.min(DEFAULT_CONCURRENCY, available())); + assert.ok(got <= DEFAULT_CONCURRENCY, `expected at most ${DEFAULT_CONCURRENCY}, got ${got}`); +}); + +test('unset: never exceeds what the machine has, on a machine smaller than the cap', () => { + assert.ok(withEnv(undefined, stageOneConcurrency) <= available()); +}); + +test('a positive integer in the environment wins over the cap, in both directions', () => { + assert.equal(withEnv('1', stageOneConcurrency), 1); + assert.equal(withEnv('64', stageOneConcurrency), 64); +}); + +test('an unusable value falls back to the cap instead of failing the run', () => { + for (const bad of ['0', '-3', '2.5', 'banana', ' ']) { + assert.equal(withEnv(bad, stageOneConcurrency), Math.min(DEFAULT_CONCURRENCY, available()), `for ${JSON.stringify(bad)}`); + } +}); + +test('an empty value is treated as unset, not as an error', () => { + assert.equal(withEnv('', stageOneConcurrency), Math.min(DEFAULT_CONCURRENCY, available())); +}); + +test('requiring the module does not run the suite', () => { + assert.equal(typeof stageOneConcurrency, 'function'); + assert.equal(typeof DEFAULT_CONCURRENCY, 'number'); +});