diff --git a/.ai/contexts/build-release.md b/.ai/contexts/build-release.md index 15ea19d32..fc9800edb 100644 --- a/.ai/contexts/build-release.md +++ b/.ai/contexts/build-release.md @@ -54,6 +54,60 @@ coverage stays on PR Node 22/Linux only. Lint and e2e still run for every PR and main push; changelog stays PR-only. Test jobs have 25 minutes; lint, changelog and e2e have 10 each. +Linux runs the full Node suite with coverage. Every non-Linux runner runs +`node scripts/run-tests.js --platform-only` without c8; its matrix and job +name stay unchanged, including `test (22, windows-2022)`. The coverage step +previously had no explicit environment; both test steps retain that inherited +environment. The PR patch gate reads `coverage/lcov.info` only on Node 22/Linux. + +`scripts/platform-tests.js` computes rule C from current files, with no stored +allowlist. It strips physical carriage returns before scanning each test for +strong platform markers: platform checks, Windows paths, conpty/node-pty, +Windows shells, links and filesystem errors, Windows environment variables, +escaped CRLF and case-insensitive paths. Real process calls (`spawn`, +`spawnSync`, `execFile`, `execFileSync`, `.kill`) and filesystem identity or +rename markers (`fs.rename`, sync/promises variants, `realpath`, `birthtime`, +`bigint: true`) also select a test. A tmpdir fixture using realpath therefore +stays selected, including on Windows with an 8.3 TEMP path. +The exported `STRONG_MARKERS` retains the study's POSIX/platform variants. +Relative production requires are followed transitively with a per-test visited +set: reaching a production file containing `process.platform` selects the test. +Test helpers are traversed to reach production dependencies, but their own +platform markers do not opt a caller in. Node_modules are excluded. Literal requires +resolve files, `.js`, `.cjs` and `index.js`; unresolved literals in test fixtures +are ignored. The read/dependency caches are scoped to one selection call. + +Limits: this is a source heuristic, not a JavaScript parser or proof that all +Windows-sensitive behavior is covered. It does not follow dynamic imports, +non-relative packages, worker filenames or source-extraction reads. It can +overselect dead code and mocked calls. Windows bug regressions with no visible +marker must retain the explicit opt-in: `task-review-regressions.test.js` +retains the #650 reader/worker regression family, and +`derive-project-path.test.js` exercises the separator expression in +`resolveWorktreePath` using real tmpdir paths, without a `process.platform` +branch. No production-module exclusion list is applied. The selection was 368 +of 556 files when this was written; selection above roughly 75% requires +reporting contributors before considering justified exclusions. + +To force Windows execution, add a relevant strong marker or the explicit test +source comment `// platform: windows`; extend the exported marker set when a +new category is needed. Physical CRLF endings alone do not opt in. The runner +filters both ordinary and isolated files, prints `platform-only: N of M test +files`, and refuses fewer than 150 selected files or any listed-file read +failure. Empty stages never fall back to Node's implicit test discovery. +Normal mode also refuses a missing `ISOLATED_FILES` inventory entry, naming it +before launching any stage; platform mode may intentionally filter that entry. +`test/platform-test-selection.test.js` pins selection, floor and stage behavior; +`test/build-workflow.test.js` pins coverage and the Windows command. +Its routing guard evaluates every matrix OS plus macOS and an unknown future +OS, requiring exactly one of coverage (`runner.os == 'Linux'`) and platform-only +(`runner.os != 'Linux'`). Matrix JSON, key order and job names stay unchanged. + +The fast CLI canary's source harness intercepts the runner's exit expression +before compiling it. The runner retains that expression outside `runSuite()`; +the platform-selection tests read the consumer's interception anchor and check +that it remains present, without executing the locally excluded canary. + The required-check list is documented in [releasing.md](../../docs/releasing.md). Build is not a required check. JB owns the external settings update that removes `test (20, windows-2022)` from both ruleset 17240725 (`main-protection`) and the diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9cdfad74d..d315ccc3f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -135,8 +135,13 @@ jobs: echo "SWITCHBOARD_REQUIRE_REAL_BWRAP=1" >> "$GITHUB_ENV" - name: Run tests with coverage + if: runner.os == 'Linux' run: npm run coverage + - name: Run platform-specific tests + if: runner.os != 'Linux' + run: node scripts/run-tests.js --platform-only + # Gate only NEW/changed lines (patch coverage), not the whole codebase: # main.js (Electron IPC/native) and the DOM glue cannot be unit-tested, so # a global threshold is meaningless. Runs once (Node 22 on ubuntu-latest) diff --git a/scripts/platform-tests.js b/scripts/platform-tests.js new file mode 100644 index 000000000..7da3624a4 --- /dev/null +++ b/scripts/platform-tests.js @@ -0,0 +1,59 @@ +'use strict'; + +const fs = require('node:fs'); +const path = require('node:path'); + +const STRONG_MARKERS = { + platform: /process\.platform|os\.platform\(\)|\bplatform\s*:\s*['"](win32|linux|darwin)|['"]win32['"]|path\.win32|path\.posix|isWindows|IS_WINDOWS/, + winpath: /[A-Za-z]:\\|\b[A-Za-z]:\/|RUNNER~1|~1[\/]|\\\\\?\\|\bUNC\b/, + conpty: /conpty|useConptyDll|node-pty/i, + shell: /cmd\.exe|powershell|pwsh|\.bat['"]|\.cmd['"]|COMSPEC|ComSpec/, + links: /junction|symlink|reparse|EPERM|EBUSY|EACCES/, + env: /USERPROFILE|LOCALAPPDATA|APPDATA|\bTEMP\b|process\.env\.TMP\b|HOMEDRIVE/, + crlf: /\\r\\n|CRLF/, + case: /toLowerCase\(\)[^;\n]{0,40}(path|dir|cwd)|case-?insensitive/i, + optIn: /\/\/\s*platform:\s*windows\b/i, + processes: /\b(?:spawn(?:Sync)?|execFile(?:Sync)?)\s*\(|\.kill\s*\(/, + filesystem: /\bfs\.(?:promises\.)?rename(?:Sync)?\s*\(|\brealpath(?:Sync)?\b|\bbirthtime(?:Ms|Ns)?\b|\bbigint\s*:\s*true/, +}; +const MIN_PLATFORM_TESTS = 150; + +function selectPlatformTests(files, { root = path.resolve(__dirname, '..'), minimum = MIN_PLATFORM_TESTS } = {}) { + const sources = new Map(); + const read = file => { + if (!sources.has(file)) sources.set(file, fs.readFileSync(file, 'utf8').replace(/\r/g, '')); + return sources.get(file); + }; + const testDir = path.join(root, 'test') + path.sep; + const dependencies = new Map(); + const relativeDependencies = file => { + if (!dependencies.has(file)) { + const resolved = []; + for (const match of read(file).matchAll(/require\(\s*['"](\.{1,2}\/[^'"]+)['"]\s*\)/g)) { + const base = path.resolve(path.dirname(file), match[1]); + const dependency = [base, base + '.js', base + '.cjs', path.join(base, 'index.js')] + .find(candidate => fs.existsSync(candidate) && fs.statSync(candidate).isFile()); + if (!dependency || dependency.includes(path.sep + 'node_modules' + path.sep)) continue; + resolved.push(dependency); + } + dependencies.set(file, resolved); + } + return dependencies.get(file); + }; + const reachesPlatform = (file, visited) => { + if (visited.has(file)) return false; + visited.add(file); + return relativeDependencies(file).some(dependency => + (!dependency.startsWith(testDir) && /process\.platform/.test(read(dependency))) || reachesPlatform(dependency, visited)); + }; + const selected = files.filter(file => { + const absolute = path.resolve(root, file); + return Object.values(STRONG_MARKERS).some(marker => marker.test(read(absolute))) || reachesPlatform(absolute, new Set()); + }); + if (selected.length < minimum) { + throw new Error(`platform-only: ${selected.length} of ${files.length} test files; below the minimum of ${minimum}`); + } + return selected; +} + +module.exports = { STRONG_MARKERS, MIN_PLATFORM_TESTS, selectPlatformTests }; diff --git a/scripts/run-tests.js b/scripts/run-tests.js index 50d029215..29dd92fc0 100644 --- a/scripts/run-tests.js +++ b/scripts/run-tests.js @@ -16,6 +16,7 @@ const { spawnSync } = require('child_process'); const fs = require('fs'); const os = require('os'); const path = require('path'); +const { selectPlatformTests } = require('./platform-tests'); const TEST_DIR = path.join(__dirname, '..', 'test'); const ISOLATED_FILES = ['trigger-watcher.test.js', 'trigger-delivery.test.js', 'trigger-continuation-delivery.test.js']; @@ -36,27 +37,49 @@ function stageOneConcurrency() { return Math.max(1, Math.min(DEFAULT_CONCURRENCY, available)); } -const mainFiles = fs.readdirSync(TEST_DIR) - .filter((name) => name.endsWith('.js') && !ISOLATED_FILES.includes(name)) - .map((name) => path.join('test', name)); - function run(args) { const result = spawnSync(process.execPath, args, { stdio: 'inherit' }); if (result.error) throw result.error; return result.status === null ? 1 : result.status; } -if (require.main === module) { - const mainStatus = run(['--test', `--test-concurrency=${stageOneConcurrency()}`, ...mainFiles]); +function runSuite() { + let files = fs.readdirSync(TEST_DIR) + .filter(name => name.endsWith('.js')) + .map(name => path.join('test', name)); + const platformOnly = process.argv.includes('--platform-only'); + const missing = ISOLATED_FILES.filter(name => !files.includes(path.join('test', name))); + if (!platformOnly && missing.length) { + console.error(`run-tests: missing isolated test files: ${missing.join(', ')}`); + return { mainStatus: 1, isolatedStatus: 0 }; + } + if (platformOnly) { + try { + const selected = selectPlatformTests(files); + console.log(`platform-only: ${selected.length} of ${files.length} test files`); + files = selected; + } catch (error) { + console.error(`run-tests: ${error.message}`); + return { mainStatus: 1, isolatedStatus: 0 }; + } + } + const mainFiles = files.filter(file => !ISOLATED_FILES.includes(path.basename(file))); + const isolatedFiles = ISOLATED_FILES.map(name => path.join('test', name)).filter(file => files.includes(file)); + const mainStatus = mainFiles.length ? run(['--test', `--test-concurrency=${stageOneConcurrency()}`, ...mainFiles]) : 0; // 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([ + const isolatedStatus = isolatedFiles.length ? run([ '--test', '--test-concurrency=1', - ...ISOLATED_FILES.map((name) => path.join('test', name)), - ]); + ...isolatedFiles, + ]) : 0; + return { mainStatus, isolatedStatus }; +} + +if (require.main === module) { + const { mainStatus, isolatedStatus } = runSuite(); process.exit(mainStatus !== 0 ? mainStatus : isolatedStatus); } diff --git a/test/build-workflow.test.js b/test/build-workflow.test.js index 835297d9b..69fe76ab3 100644 --- a/test/build-workflow.test.js +++ b/test/build-workflow.test.js @@ -167,6 +167,32 @@ test('Test keeps its triggers, real-bwrap, patch coverage, lint and e2e conditio assert.match(bwrap.run, /SWITCHBOARD_REQUIRE_REAL_BWRAP=1/); assert.equal(steps.find(step => step.name?.startsWith('Patch coverage gate')).if, "matrix.node-version == 22 && matrix.os == 'ubuntu-latest' && github.event_name == 'pull_request'"); - assert.equal(steps.find(step => step.run === 'npm run coverage').if, undefined); + assert.equal(steps.find(step => step.run === 'npm run coverage').if, "runner.os == 'Linux'"); assert.equal(testWorkflow.jobs.e2e.steps.find(step => step.uses?.startsWith('actions/upload-artifact@')).if, 'failure()'); }); + +test('Windows runs platform-only without coverage and Linux retains coverage', () => { + const steps = testWorkflow.jobs.test.steps; + const windows = steps.find(step => step.run === 'node scripts/run-tests.js --platform-only'); + assert.ok(windows, 'Windows must have its own platform-only test step'); + assert.equal(windows.run, 'node scripts/run-tests.js --platform-only'); + const linux = steps.find(step => step.name === 'Run tests with coverage'); + assert.equal(linux.if, "runner.os == 'Linux'"); + assert.equal(linux.run, 'npm run coverage'); + assert.equal(testWorkflow.jobs.test.name, undefined); +}); + +test('every matrix OS and a future OS run exactly one test step', () => { + const steps = testWorkflow.jobs.test.steps.filter(step => + step.run === 'npm run coverage' || step.run === 'node scripts/run-tests.js --platform-only'); + assert.equal(steps.length, 2); + const runnerOs = os => os.startsWith('ubuntu-') ? 'Linux' : os.startsWith('windows-') ? 'Windows' : 'macOS'; + for (const event of ['pull_request', 'push', 'workflow_dispatch']) { + const operatingSystems = evaluate(testWorkflow.jobs.test.strategy.matrix.include, event).map(cell => runnerOs(cell.os)); + for (const os of [...operatingSystems, 'macOS', 'FutureOS']) { + const matched = steps.filter(step => vm.runInNewContext(step.if, { runner: { os } }, { timeout: 1000 })); + assert.equal(matched.length, 1, `${event}: ${os} must run exactly one test step`); + assert.equal(matched[0].run, os === 'Linux' ? 'npm run coverage' : 'node scripts/run-tests.js --platform-only'); + } + } +}); diff --git a/test/derive-project-path.test.js b/test/derive-project-path.test.js index 0079d7adf..80edc4e84 100644 --- a/test/derive-project-path.test.js +++ b/test/derive-project-path.test.js @@ -1,3 +1,4 @@ +// platform: windows const test = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); diff --git a/test/platform-test-selection.test.js b/test/platform-test-selection.test.js new file mode 100644 index 000000000..63f62e79f --- /dev/null +++ b/test/platform-test-selection.test.js @@ -0,0 +1,192 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const vm = require('node:vm'); +const { createRequire } = require('node:module'); + +const root = path.resolve(__dirname, '..'); +const selectorPath = path.join(root, 'scripts/platform-tests.js'); + +function selector() { + assert.ok(fs.existsSync(selectorPath), 'platform test selector must exist'); + return require(selectorPath); +} + +function fixture(t, sources) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'platform-selection-')); + t.after(() => { + const tempRoot = fs.realpathSync(os.tmpdir()); + assert.ok(fs.realpathSync(dir).startsWith(tempRoot + path.sep), 'fixture cleanup stays inside the temp directory'); + fs.rmSync(dir, { recursive: true, force: true }); + }); + for (const [name, source] of Object.entries(sources)) { + fs.mkdirSync(path.dirname(path.join(dir, name)), { recursive: true }); + fs.writeFileSync(path.join(dir, name), source); + } + return { root: dir, minimum: 0 }; +} + +test('October Windows regressions remain selected without executing their sources', () => { + const files = ['worktree-delete-real-git', 'touched-editor-ipc', 'canary-cli-capabilities-fast'] + .map(name => `test/${name}.test.js`); + assert.deepEqual(selector().selectPlatformTests(files, { minimum: 0 }), files); +}); + +for (const name of ['run-to-exit', 'trigger-continuation-delivery', 'task-transcript-reader', + 'task-review-regressions', 'transcript-cwd-trust', 'transcript-cwd-trust-cache', 'derive-project-path']) { + test(`Windows bug regression ${name} remains selected`, () => { + const file = `test/${name}.test.js`; + assert.deepEqual(selector().selectPlatformTests([file], { minimum: 0 }), [file]); + }); +} + +for (const name of ['continuation-choice-menu', 'continuation-memory', 'get-projects-cold-start-reconcile', + 'projection-envelope-742', 'refusals-before-dialogs', 'reload-notices', 'restore-in-flight-persist', + 'restore-pending-notice-666', 'send-to-background-renderer']) { + test(`production platform dependency through a test helper selects ${name}`, () => { + const file = `test/${name}.test.js`; + assert.deepEqual(selector().selectPlatformTests([file], { minimum: 0 }), [file]); + }); +} + +test('test helpers forward production dependencies without opting callers into helper-only platform code', t => { + const options = fixture(t, { + 'test/through.js': "require('./bridge.cjs')", + 'test/bridge.cjs': "require('../src/platform')", + 'src/platform.js': 'process.platform', + 'test/only.js': "require('./only.cjs')", + 'test/only.cjs': 'process.platform', + }); + assert.deepEqual(selector().selectPlatformTests(['test/through.js', 'test/only.js'], options), ['test/through.js']); +}); + +test('generic sources and physical CRLF line endings stay out', t => { + const options = fixture(t, { 'test/plain.js': "'use strict';\nconst value = 1;\n", + 'test/crlf.js': "'use strict';\r\nconst value = 'https://example.com';\r\n" }); + assert.deepEqual(selector().selectPlatformTests(['test/plain.js', 'test/crlf.js'], options), []); +}); + +test('carriage returns are stripped before strong-marker matching', t => { + const options = fixture(t, { 'test/normalized.js': 'process.\rplatform' }); + assert.deepEqual(selector().selectPlatformTests(['test/normalized.js'], options), ['test/normalized.js']); +}); + +test('each strong marker and the explicit comment opt-in select a file', t => { + const samples = ["process.platform", "'win32'", 'path.win32', 'C:\\repo', 'C:/repo', 'RUNNER~1', + 'conpty', 'node-pty', 'cmd.exe', 'powershell', 'junction', 'symlink', 'EPERM', 'EBUSY', + 'EACCES', 'USERPROFILE', 'APPDATA', 'TEMP', String.raw`'\r\n'`, 'case-insensitive', + '// platform: windows', 'spawn(', 'spawnSync(', 'execFile(', 'execFileSync(', + 'child.kill(', 'fs.rename(', 'fs.renameSync(', 'fs.promises.rename(', + 'fs.realpathSync(', 'stat.birthtime', '{ bigint: true }', 'os.tmpdir(); realpath()']; + const sources = Object.fromEntries(samples.map((source, i) => [`test/marker-${i}.js`, source])); + const options = fixture(t, sources); + const files = Object.keys(sources); + assert.deepEqual(selector().selectPlatformTests(files, options), files); + assert.ok(Object.keys(selector().STRONG_MARKERS).length > 0); +}); + +test('production platform dependencies are followed transitively through cycles and index modules', t => { + const options = fixture(t, { + 'test/direct.js': "require('../src/direct')", + 'test/indirect.js': "require('../src/indirect')", + 'test/helper.js': "require('./helper.cjs')", + 'test/helper.cjs': 'process.platform', + 'test/shell.js': "require('../src/shell')", + 'src/direct.js': 'process.platform', + 'src/indirect.js': "require('./direct')", + 'src/shell.js': "'powershell'", + 'test/cycle.js': "require('../src/cycle')", + 'src/cycle/index.js': "require('../cycle'); require('../indirect')", + }); + assert.deepEqual(selector().selectPlatformTests([ + 'test/direct.js', 'test/indirect.js', 'test/helper.js', 'test/shell.js', 'test/cycle.js', + ], options), ['test/direct.js', 'test/indirect.js', 'test/cycle.js']); +}); + +test('missing listed files fail closed', t => { + const options = fixture(t, { 'test/dep.js': "require('../src/missing')" }); + assert.throws(() => selector().selectPlatformTests(['test/missing.js'], options), /ENOENT/); +}); + +test('the selection floor rejects 149 files and accepts 150', t => { + const sources = Object.fromEntries(Array.from({ length: 150 }, (_, i) => [`test/${i}.js`, 'process.platform'])); + const options = fixture(t, sources); + delete options.minimum; + assert.throws(() => selector().selectPlatformTests(Object.keys(sources).slice(1), options), /149.*150/); + assert.equal(selector().selectPlatformTests(Object.keys(sources), options).length, 150); +}); + +function runRunner(platformOnly, inventory, selected, error) { + const filename = path.join(root, 'scripts/run-tests.js'); + const realRequire = createRequire(filename); + const calls = [], logs = []; + let status; + const entry = { exports: {} }; + const requireForRunner = name => { + if (name === 'fs') return { readdirSync: () => inventory }; + if (name === 'child_process') return { spawnSync: (_exe, args) => { calls.push(args); return { status: 0 }; } }; + if (name === './platform-tests') return { selectPlatformTests: files => { + if (error) throw new Error(error); + return files.filter(file => selected.includes(path.basename(file))); + } }; + return realRequire(name); + }; + requireForRunner.main = entry; + vm.runInNewContext(fs.readFileSync(filename, 'utf8'), { + require: requireForRunner, module: entry, __dirname: path.dirname(filename), + process: { execPath: process.execPath, argv: ['node', filename, ...(platformOnly ? ['--platform-only'] : [])], + env: {}, exit: code => { status = code; } }, + console: { log: line => logs.push(line), error: line => logs.push(line) }, + }, { filename }); + return { calls, logs, status }; +} + +test('platform-only filters both stages and prints the inventory count', () => { + const result = runRunner(true, ['plain.test.js', 'windows.test.js', 'trigger-watcher.test.js', 'trigger-delivery.test.js'], + ['windows.test.js', 'trigger-delivery.test.js']); + assert.deepEqual(result.calls.map(args => Array.from(args).filter(arg => arg.startsWith('test'))), + [[path.join('test', 'windows.test.js')], [path.join('test', 'trigger-delivery.test.js')]]); + assert.deepEqual(result.logs, ['platform-only: 2 of 4 test files']); + assert.equal(result.status, 0); +}); + +test('empty selected stages never launch Node with implicit test discovery', () => { + const result = runRunner(true, ['plain.test.js'], []); + assert.deepEqual(result.calls, []); +}); + +test('selection read errors fail before any test process starts', () => { + const result = runRunner(true, ['plain.test.js'], [], 'ENOENT: listed file'); + assert.equal(result.status, 1); + assert.deepEqual(result.calls, []); + assert.match(result.logs.join('\n'), /ENOENT: listed file/); +}); + +test('ordinary runner retains both unfiltered stages', () => { + const result = runRunner(false, ['plain.test.js', ...require('../scripts/run-tests').ISOLATED_FILES], []); + assert.equal(result.calls.length, 2); + assert.ok(result.calls[0].includes(path.join('test', 'plain.test.js'))); + assert.ok(result.calls[1].includes(path.join('test', 'trigger-watcher.test.js'))); +}); + +for (const missing of require('../scripts/run-tests').ISOLATED_FILES) { + test(`ordinary runner fails closed when isolated file ${missing} is missing`, () => { + const inventory = ['plain.test.js', ...require('../scripts/run-tests').ISOLATED_FILES.filter(file => file !== missing)]; + const result = runRunner(false, inventory, []); + assert.equal(result.status, 1, 'missing isolated file must fail the run'); + assert.deepEqual(result.calls, [], 'missing inventory entry must fail before launching tests'); + assert.ok(result.logs.some(line => line.includes(missing)), 'failure must name the missing isolated file'); + }); +} + +test('the excluded canary source harness can still intercept the runner exit', () => { + const consumer = fs.readFileSync(path.join(root, 'test/canary-cli-capabilities-fast.test.js'), 'utf8'); + const anchor = consumer.match(/\.replace\('(process\.exit\([^']+;)'/); + assert.ok(anchor, 'the recorded canary consumer must expose its exit interception anchor'); + const runner = fs.readFileSync(path.join(root, 'scripts/run-tests.js'), 'utf8'); + assert.ok(runner.includes(anchor[1]), 'runner must preserve the canary exit interception anchor'); +}); diff --git a/test/task-review-regressions.test.js b/test/task-review-regressions.test.js index 46014a739..828dca840 100644 --- a/test/task-review-regressions.test.js +++ b/test/task-review-regressions.test.js @@ -1,4 +1,5 @@ 'use strict'; +// platform: windows const test = require('node:test'); const assert = require('node:assert/strict');