From 76dd8c099e88d8b73cedc1c4cb256406e441f7d9 Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Tue, 29 Sep 2026 08:57:07 +0200 Subject: [PATCH] ci(test): download the Electron binary once before unit specs run (#1739) --- .github/workflows/ci.yml | 16 ++++-- docs/architecture/validation-map.md | 13 +++++ tools/coverage/coverage-run-pool.mjs | 34 +++++++++---- tools/coverage/coverage-run-pool.test.mjs | 24 +++++++++ tools/coverage/run-tier-a-coverage.mjs | 20 +++++++- tools/testing/ensure-electron-binary.mjs | 62 +++++++++++++++++++++++ 6 files changed, 152 insertions(+), 17 deletions(-) create mode 100644 tools/testing/ensure-electron-binary.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8d5906f52..a57d6e28e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -324,9 +324,12 @@ jobs: # and never launch a Playwright browser, so no `playwright # install`. The runner image ships Electron's shared libraries and # xvfb; fail fast with a clear message if an image update drops one. + # pnpm skips Electron's postinstall, so download the binary first; + # otherwise ldd sees no file and the check passes vacuously. - name: Check Electron runtime dependencies run: | command -v xvfb-run || { echo "::error::xvfb-run is missing on the runner"; exit 1; } + node tools/testing/ensure-electron-binary.mjs || { echo "::error::Electron binary download failed"; exit 1; } missing="$(ldd node_modules/electron/dist/electron | grep 'not found' || true)" if [ -n "$missing" ]; then echo "::error::Electron is missing shared libraries:" @@ -466,13 +469,16 @@ jobs: # pnpm only runs build scripts for allow-listed packages # (pnpm-workspace.yaml), so electron's postinstall never fetches # its binary. `require('electron')` then downloads it lazily, and - # parallel Jest workers that spawn Electron race on the same - # download: one executes the half-written binary (ETXTBSY). - # Fetch it once before the Tier A suite, the only step here whose - # specs spawn Electron; install.js skips an existing binary. + # parallel Jest workers and Tier A projects that spawn Electron + # race on the same download: one executes the half-written binary + # (ETXTBSY). Fetch it once before the Tier A suite, the only step + # here whose specs spawn Electron. Unlike a bare install.js, the + # helper also runs the binary, so a partial extraction fails here. + # The Tier A runner calls it too; this step only separates a + # download failure from test failures. - name: Install the Electron binary if: steps.scope.outputs.run == 'true' - run: node node_modules/electron/install.js + run: node tools/testing/ensure-electron-binary.mjs # Jest's transform cache (TypeScript/Angular transpilation plus # coverage instrumentation, keyed by file content) is persisted diff --git a/docs/architecture/validation-map.md b/docs/architecture/validation-map.md index ff50576c9..1bc623490 100644 --- a/docs/architecture/validation-map.md +++ b/docs/architecture/validation-map.md @@ -107,6 +107,19 @@ with `diagnostics: false`); `isolatedModules`-incompatible syntax such as a type re-export without `export type` still fails at load time, and spec type errors are caught by `typecheck:spec` (see Unit And Type Checks). +Some `electron-backend` and `database` specs run SQLite code in the Electron +binary under `ELECTRON_RUN_AS_NODE`, resolving it with `require('electron')`. +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). 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: `tools/coverage/unit-coverage-scope.mjs` holds the allowlist (Markdown, `docs/`, 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) ); diff --git a/tools/testing/ensure-electron-binary.mjs b/tools/testing/ensure-electron-binary.mjs new file mode 100644 index 000000000..1f0946d34 --- /dev/null +++ b/tools/testing/ensure-electron-binary.mjs @@ -0,0 +1,62 @@ +#!/usr/bin/env node + +// Downloads the Electron binary once and proves it runs, before anything +// spawns it concurrently. +// +// pnpm does not run Electron's postinstall (it is not in +// `onlyBuiltDependencies`), so `pnpm install` leaves no binary behind and +// `require('electron')` downloads and extracts it on first use. Unit specs +// that execute SQLite code under `ELECTRON_RUN_AS_NODE` resolve it that way, +// and Tier A projects and Jest workers run in parallel: one process can exec +// the binary while another is still extracting it (ETXTBSY on Linux, clobbered +// framework symlinks on macOS). Running this first leaves nothing to download. +// +// Usage: node tools/testing/ensure-electron-binary.mjs + +import { execFileSync } from 'node:child_process'; +import { createRequire } from 'node:module'; +import process from 'node:process'; + +const require = createRequire(import.meta.url); +const { version } = require('electron/package.json'); + +let electronPath; +try { + electronPath = require('electron'); +} catch (error) { + console.error( + `Electron ${version} binary download failed: ${error.message}` + ); + process.exit(1); +} + +let reported; +try { + reported = execFileSync( + electronPath, + ['-e', 'process.stdout.write(process.versions.electron)'], + { + encoding: 'utf8', + // The child's stderr (e.g. a dyld or loader error) goes straight + // to the log instead of being repeated in the message below. + stdio: ['ignore', 'pipe', 'inherit'], + env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' }, + timeout: 60_000, + } + ).trim(); +} catch (error) { + console.error( + `Electron binary at ${electronPath} does not run (${error.code ?? `exit ${error.status}`}). ` + + 'If an interrupted download left a partial copy, delete node_modules/electron/dist and path.txt, then rerun.' + ); + process.exit(1); +} + +if (reported !== version) { + console.error( + `Electron binary at ${electronPath} reports ${reported || 'no version'}, expected ${version}.` + ); + process.exit(1); +} + +console.log(`Electron ${version} binary ready at ${electronPath}`);