From 6f076af187691af3d11a51610b5801a5bee20949 Mon Sep 17 00:00:00 2001 From: 4gray Date: Mon, 28 Sep 2026 22:41:17 +0200 Subject: [PATCH] ci(test): prefetch Electron only when the selected specs exec it Review feedback: the unconditional unit-job step made PRs that skip the Tier A suite depend on an Electron download, and the coverage:unit:ci prefix did the same for filtered local runs of projects that never spawn Electron. The Tier A runner now prefetches only when a selected project has a spec calling createRequire(...)('electron'), and the CI step runs after the scope decision and only when the suite is in scope. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 22 +++++++++------ docs/architecture/validation-map.md | 11 ++++---- package.json | 2 +- tools/coverage/coverage-run-pool.mjs | 34 +++++++++++++++-------- tools/coverage/coverage-run-pool.test.mjs | 24 ++++++++++++++++ tools/coverage/run-tier-a-coverage.mjs | 20 ++++++++++++- 6 files changed, 86 insertions(+), 27 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a7973b710..05947a5a1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -371,15 +371,6 @@ jobs: - name: Install dependencies run: pnpm install --frozen-lockfile - # pnpm skips Electron's postinstall (it is not in - # onlyBuiltDependencies), so the binary is fetched on the first - # require('electron'). Specs that run SQLite under - # ELECTRON_RUN_AS_NODE resolve it that way while Tier A projects - # run concurrently, and one exec'ing it mid-extraction fails with - # ETXTBSY. Download and verify it once, before any test starts. - - name: Download Electron binary - run: node tools/testing/ensure-electron-binary.mjs - - name: Validate agent guidance run: pnpm run agents:validate @@ -436,6 +427,19 @@ jobs: --paginate --jq '.[] | .filename, (.previous_filename // empty)' | node tools/coverage/unit-coverage-scope.mjs --base FETCH_HEAD --github-output + # pnpm skips Electron's postinstall (it is not in + # onlyBuiltDependencies), so the binary is fetched on the first + # require('electron'). Specs that run SQLite under + # ELECTRON_RUN_AS_NODE resolve it that way while Tier A projects + # run concurrently, and one exec'ing it mid-extraction fails with + # ETXTBSY. Download and verify it once, before any test starts; + # the Tier A runner does the same, so this step only separates a + # download failure from test failures. Skipped suites need no + # binary. + - name: Download Electron binary + if: steps.scope.outputs.run == 'true' + run: node tools/testing/ensure-electron-binary.mjs + # Jest's transform cache (TypeScript/Angular transpilation plus # coverage instrumentation, keyed by file content) is persisted # between runs. Only master pushes and maintainer dispatches save diff --git a/docs/architecture/validation-map.md b/docs/architecture/validation-map.md index 5a9a5f9e6..1bc623490 100644 --- a/docs/architecture/validation-map.md +++ b/docs/architecture/validation-map.md @@ -113,11 +113,12 @@ pnpm does not run Electron's postinstall (`electron` is deliberately absent from `onlyBuiltDependencies`, see [workspace shell](workspace-shell.md)), so the first `require` downloads and extracts the binary. With projects and Jest workers in parallel, one process can exec it while another is still extracting -it (`spawnSync … ETXTBSY` on Linux). `coverage:unit:ci` and a CI step right -after `pnpm install` therefore run `tools/testing/ensure-electron-binary.mjs` -first, which downloads the binary once and fails unless it runs and reports -the pinned version. In a fresh worktree, run it before starting several of -these specs at once by other means. +it (`spawnSync … ETXTBSY` on Linux). When a selected project has a spec that +calls `createRequire(...)('electron')`, `coverage:unit:ci` first runs +`tools/testing/ensure-electron-binary.mjs`, which downloads the binary once and +fails unless it runs and reports the pinned version; CI also runs it as its own +step when the Tier A suite is in scope. In a fresh worktree, run it before +starting several of these specs at once by other means. In CI, a pull request skips the Tier A suite (and the merged-coverage upload) when every changed file is outside Tier A test inputs: diff --git a/package.json b/package.json index e25fad65c..c5c2fb931 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "styles:inputs:check": "node tools/nx/check-stylesheet-inputs.mjs", "styles:inputs:validate": "pnpm run styles:inputs:test && pnpm run styles:inputs:check", "coverage:tools:test": "node --test tools/coverage/coverage-integrity.test.mjs tools/coverage/e2e-shard-reports.test.mjs tools/coverage/coverage-run-pool.test.mjs tools/coverage/unit-coverage-scope.test.mjs", - "coverage:unit:ci": "node tools/testing/ensure-electron-binary.mjs && node tools/coverage/run-tier-a-coverage.mjs", + "coverage:unit:ci": "node tools/coverage/run-tier-a-coverage.mjs", "coverage:merge": "node tools/coverage/merge-coverage.mjs", "coverage:health": "node tools/coverage/coverage-health.mjs", "coverage:policy:check": "node tools/coverage/check-coverage-policy.mjs", diff --git a/tools/coverage/coverage-run-pool.mjs b/tools/coverage/coverage-run-pool.mjs index d6e56b804..5379f0d8c 100644 --- a/tools/coverage/coverage-run-pool.mjs +++ b/tools/coverage/coverage-run-pool.mjs @@ -8,29 +8,41 @@ * bounded worker count, so the runner's total CPU budget stays close to the * machine's core count instead of multiplying with it. */ -import { readdirSync } from 'node:fs'; +import { readdirSync, readFileSync } from 'node:fs'; import path from 'node:path'; const SPEC_FILE = /\.(spec|test)\.ts$/; -/** Counts spec files under a directory; used to start the big projects first. */ -export function countSpecFiles(directory) { - let count = 0; +function listSpecFiles(directory) { let entries; try { entries = readdirSync(directory, { withFileTypes: true }); } catch { - return 0; + return []; } - for (const entry of entries) { + return entries.flatMap((entry) => { const fullPath = path.join(directory, entry.name); if (entry.isDirectory()) { - count += countSpecFiles(fullPath); - } else if (entry.isFile() && SPEC_FILE.test(entry.name)) { - count += 1; + return listSpecFiles(fullPath); } - } - return count; + return entry.isFile() && SPEC_FILE.test(entry.name) ? [fullPath] : []; + }); +} + +/** Counts spec files under a directory; used to start the big projects first. */ +export function countSpecFiles(directory) { + return listSpecFiles(directory).length; +} + +// `createRequire(__filename)('electron')` resolves the binary path, which +// downloads it on first use (see tools/testing/ensure-electron-binary.mjs). +const ELECTRON_BINARY_REQUIRE = /createRequire\([^)]*\)\(\s*['"]electron['"]\s*\)/; + +/** Whether any spec under a directory resolves the Electron binary to exec it. */ +export function specsResolveElectronBinary(directory) { + return listSpecFiles(directory).some((file) => + ELECTRON_BINARY_REQUIRE.test(readFileSync(file, 'utf8')) + ); } /** diff --git a/tools/coverage/coverage-run-pool.test.mjs b/tools/coverage/coverage-run-pool.test.mjs index 608c0d273..4a7b7e6bd 100644 --- a/tools/coverage/coverage-run-pool.test.mjs +++ b/tools/coverage/coverage-run-pool.test.mjs @@ -13,6 +13,7 @@ import { resolveConcurrency, resolveWorkersPerProject, runWithConcurrency, + specsResolveElectronBinary, } from './coverage-run-pool.mjs'; let workDir; @@ -35,6 +36,29 @@ test('counts spec and test files recursively and ignores sources', async () => { assert.equal(countSpecFiles(path.join(workDir, 'missing')), 0); }); +test('detects specs that resolve the Electron binary, including split calls', async () => { + const mocked = path.join(workDir, 'mocked'); + await mkdir(mocked, { recursive: true }); + await writeFile( + path.join(mocked, 'a.spec.ts'), + "jest.mock('electron', () => ({}));\nimport { app } from 'electron';\n" + ); + await writeFile( + path.join(mocked, 'b.ts'), + "createRequire(__filename)('electron');\n" + ); + assert.equal(specsResolveElectronBinary(mocked), false); + + const spawning = path.join(workDir, 'spawning', 'nested'); + await mkdir(spawning, { recursive: true }); + await writeFile( + path.join(spawning, 'c.spec.ts'), + "const electronPath = createRequire(__filename)(\n 'electron'\n) as string;\n" + ); + assert.equal(specsResolveElectronBinary(path.dirname(spawning)), true); + assert.equal(specsResolveElectronBinary(path.join(workDir, 'missing')), false); +}); + test('orders longest first and keeps policy order for ties', () => { const projects = [ { name: 'small' }, diff --git a/tools/coverage/run-tier-a-coverage.mjs b/tools/coverage/run-tier-a-coverage.mjs index 716a4c57b..a9a4fbb53 100644 --- a/tools/coverage/run-tier-a-coverage.mjs +++ b/tools/coverage/run-tier-a-coverage.mjs @@ -1,6 +1,6 @@ #!/usr/bin/env node -import { spawn } from 'node:child_process'; +import { spawn, spawnSync } from 'node:child_process'; import { existsSync, readFileSync, rmSync } from 'node:fs'; import os from 'node:os'; import path from 'node:path'; @@ -19,6 +19,7 @@ import { resolveConcurrency, resolveWorkersPerProject, runWithConcurrency, + specsResolveElectronBinary, } from './coverage-run-pool.mjs'; const workspaceRoot = process.cwd(); @@ -253,6 +254,23 @@ const specCounts = new Map( countSpecFiles(path.join(workspaceRoot, project.sourceRoot)), ]) ); +// Concurrent specs would otherwise race to download and extract the binary +// (ETXTBSY), so fetch it once before any project starts. +if ( + tierAProjects.some((project) => + specsResolveElectronBinary(path.join(workspaceRoot, project.sourceRoot)) + ) +) { + const prefetch = spawnSync( + process.execPath, + [path.join(workspaceRoot, 'tools/testing/ensure-electron-binary.mjs')], + { stdio: 'inherit' } + ); + if (prefetch.status !== 0) { + process.exit(prefetch.status ?? 1); + } +} + const ordered = orderLongestFirst(tierAProjects, (project) => specCounts.get(project.name) );