mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-10 01:09:43 +02:00
fix(test-vendor): repair a poisoned bundle, track all bundle inputs, pin esbuild
Follow-ups to #241 (thanks @Lint111), from an independent review of that PR. The script is a real fix for a real gap; these are the four defects the review found, each reproduced before and after. 1. A wrong-but-fresh output was never repaired. The zerolag bundle is finished by a SECOND step (the alias append), so anything landing between esbuild and the append is permanent: the file looks complete, carries a current mtime, and the mtime-only cache reports "up to date" forever while the suite dies on `LocalEchoOverlay is not defined`. Reproduced by replaying #241's own two commits: running the first and then pulling the second kept the broken bundle. Fixed twice over, because the two halves address different cases. Builds now go to a temp file and `renameSync` into place, so this script can never publish a half-written output (that also covers an interrupted esbuild or copy, and two concurrent runs). And `isFresh` verifies the bundle actually contains its alias tail, which is what repairs a file an EARLIER version already poisoned; a rename alone cannot fix what is already on disk. 2. Freshness compared against the entry file only, but esbuild bundles its four siblings too, so editing overlay-renderer.ts left the suite testing a stale overlay while reporting "up to date". Editing those siblings is exactly the single-source workflow CLAUDE.md mandates. It now stats every `.ts` in the package source dir. A full rebuild is ~2s, so the cache was not buying much. 3. `execFileSync('npx', ...)` passed no cwd, unlike scripts/build.mjs, so a run from another directory missed the repo's pinned esbuild and would fetch an unpinned one from the registry. Both calls now pass `cwd: ROOT`. 4. Every invocation in test/mobile/README.md was a bare `npx vitest`, which skips the `pretest:mobile` hook npm only fires for `npm run test:mobile`, so the documented commands all bypassed the fix. Rewritten, with a note on why. Also: an esbuild failure printed a raw stack; it now names the asset and its input, matching the missing-input message. And the header comment no longer implies the vendor dir is always empty: scripts/postinstall.js already writes these same seven outputs, so what this script adds is freshness and independence from install time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -4,22 +4,42 @@
|
|||||||
*
|
*
|
||||||
* The mobile suite (test/mobile/**) drives a real browser against a WebServer
|
* The mobile suite (test/mobile/**) drives a real browser against a WebServer
|
||||||
* started from TypeScript source, so fastify-static serves
|
* started from TypeScript source, so fastify-static serves
|
||||||
* `join(__dirname, 'public')` = `src/web/public` — NOT `dist/web/public`, where
|
* `join(__dirname, 'public')` = `src/web/public`, NOT `dist/web/public`, where
|
||||||
* `npm run build` puts the vendor bundles. Every `/vendor/xterm*` request 404s,
|
* `npm run build` puts the vendor bundles. Without them every `/vendor/xterm*`
|
||||||
* so `Terminal` is never defined, `initTerminal()` never runs, and every test
|
* request 404s, so `Terminal` is never defined, `initTerminal()` never runs, and
|
||||||
* touching `app.terminal` dies with `Cannot read properties of null`.
|
* every test touching `app.terminal` dies with `Cannot read properties of null`.
|
||||||
*
|
*
|
||||||
* That stayed invisible because config/vitest.ci.config.ts excludes
|
* That stayed invisible because config/vitest.ci.config.ts excludes
|
||||||
* `test/mobile/**`, so CI never ran the suite.
|
* `test/mobile/**`, so CI never ran the suite.
|
||||||
*
|
*
|
||||||
* Mirrors the vendor steps in scripts/build.mjs, targeting the source tree.
|
* ⚠️ scripts/postinstall.js:238-303 already writes these same 7 outputs (same
|
||||||
* Same inputs and output names, so the page markup needs no test-only branch.
|
* names, same alias tail), so a plain `npm install` leaves the suite working. What
|
||||||
|
* this script adds is FRESHNESS and independence from install time: a checkout
|
||||||
|
* installed with `--ignore-scripts`, or one borrowing another tree's
|
||||||
|
* `node_modules`, never ran postinstall, and an edit to the zerolag package after
|
||||||
|
* install leaves the bundle stale. It runs as `pretest:mobile`.
|
||||||
|
*
|
||||||
|
* Mirrors the vendor steps in scripts/build.mjs, targeting the source tree. Same
|
||||||
|
* inputs and output names, so the page markup needs no test-only branch. That
|
||||||
|
* makes THREE hand-synced copies of this asset table (here, build.mjs:45-51,
|
||||||
|
* postinstall.js:255-303); keep them in step or a missing entry becomes a 404 that
|
||||||
|
* silently disables the terminal.
|
||||||
* `src/web/public/vendor/` is gitignored, so these stay build artifacts.
|
* `src/web/public/vendor/` is gitignored, so these stay build artifacts.
|
||||||
*
|
*
|
||||||
* Idempotent: skips outputs already newer than their source.
|
* Idempotent: skips outputs newer than every input they derive from.
|
||||||
*/
|
*/
|
||||||
import { execFileSync } from 'node:child_process';
|
import { execFileSync } from 'node:child_process';
|
||||||
import { appendFileSync, copyFileSync, existsSync, mkdirSync, statSync } from 'node:fs';
|
import {
|
||||||
|
appendFileSync,
|
||||||
|
copyFileSync,
|
||||||
|
existsSync,
|
||||||
|
mkdirSync,
|
||||||
|
readFileSync,
|
||||||
|
readdirSync,
|
||||||
|
renameSync,
|
||||||
|
rmSync,
|
||||||
|
statSync,
|
||||||
|
} from 'node:fs';
|
||||||
import { dirname, join, resolve } from 'node:path';
|
import { dirname, join, resolve } from 'node:path';
|
||||||
import { fileURLToPath } from 'node:url';
|
import { fileURLToPath } from 'node:url';
|
||||||
|
|
||||||
@@ -54,14 +74,45 @@ const ASSETS = [
|
|||||||
out: 'xterm-zerolag-input.js',
|
out: 'xterm-zerolag-input.js',
|
||||||
mode: 'bundle',
|
mode: 'bundle',
|
||||||
globalName: 'XtermZerolagInput',
|
globalName: 'XtermZerolagInput',
|
||||||
|
// The alias tail appended below. Its absence means the output is a partial
|
||||||
|
// write from an older version of this script, whatever its mtime says.
|
||||||
|
mustContain: 'window.LocalEchoOverlay',
|
||||||
},
|
},
|
||||||
];
|
];
|
||||||
|
|
||||||
function isFresh(src, dest) {
|
/**
|
||||||
|
* Every input an asset is derived from. For the bundle that is the whole package
|
||||||
|
* source dir, not just the entry: esbuild pulls in the entry's siblings, so
|
||||||
|
* comparing against the entry alone reports "up to date" after an edit to
|
||||||
|
* overlay-renderer.ts and the suite then tests a stale overlay. Editing those
|
||||||
|
* siblings is exactly the single-source workflow CLAUDE.md mandates.
|
||||||
|
*/
|
||||||
|
function sourcesOf(asset) {
|
||||||
|
if (asset.mode !== 'bundle') return [asset.src];
|
||||||
|
const dir = dirname(asset.src);
|
||||||
|
try {
|
||||||
|
return readdirSync(dir)
|
||||||
|
.filter((f) => f.endsWith('.ts'))
|
||||||
|
.map((f) => join(dir, f));
|
||||||
|
} catch {
|
||||||
|
return [asset.src];
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function isFresh(asset, dest) {
|
||||||
if (!existsSync(dest)) return false;
|
if (!existsSync(dest)) return false;
|
||||||
try {
|
try {
|
||||||
return statSync(dest).mtimeMs >= statSync(src).mtimeMs;
|
// Content check before the mtime check, because mtime cannot see a WRONG file.
|
||||||
|
// The atomic rename below stops this script from ever publishing a half-written
|
||||||
|
// bundle, but it cannot repair one already on disk: anyone who ran an earlier
|
||||||
|
// version that appended the aliases in place has a complete-looking file with a
|
||||||
|
// current mtime and no alias tail, and a pure mtime cache calls that "up to
|
||||||
|
// date" forever while the suite dies on `LocalEchoOverlay is not defined`.
|
||||||
|
if (asset.mustContain && !readFileSync(dest, 'utf-8').includes(asset.mustContain)) return false;
|
||||||
|
const destMs = statSync(dest).mtimeMs;
|
||||||
|
return sourcesOf(asset).every((src) => destMs >= statSync(src).mtimeMs);
|
||||||
} catch {
|
} catch {
|
||||||
|
// an unreadable or vanished input: rebuild rather than trust the cache
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -76,28 +127,43 @@ for (const asset of ASSETS) {
|
|||||||
console.error(`[test-vendor] missing input: ${asset.src}\n run \`npm install\` first`);
|
console.error(`[test-vendor] missing input: ${asset.src}\n run \`npm install\` first`);
|
||||||
process.exit(1);
|
process.exit(1);
|
||||||
}
|
}
|
||||||
if (isFresh(asset.src, dest)) {
|
if (isFresh(asset, dest)) {
|
||||||
skipped += 1;
|
skipped += 1;
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
if (asset.mode === 'copy') {
|
// Build into a temp path and rename into place at the very end. The zerolag
|
||||||
copyFileSync(asset.src, dest);
|
// bundle is finished by a SECOND step (the alias append below), so writing
|
||||||
} else if (asset.mode === 'minify') {
|
// `dest` directly leaves a window where a complete-looking file with a current
|
||||||
execFileSync('npx', ['esbuild', asset.src, '--minify', `--outfile=${dest}`], { stdio: 'inherit' });
|
// mtime is missing its tail: `isFresh` then reports "up to date" forever and the
|
||||||
} else {
|
// suite dies on `LocalEchoOverlay is not defined`, which is the exact failure
|
||||||
execFileSync(
|
// this script exists to prevent. An interrupted esbuild or copy poisons the
|
||||||
'npx',
|
// cache the same way. rename(2) is atomic within a directory, so a reader sees
|
||||||
[
|
// either the old file or the finished new one, never a half-written one.
|
||||||
|
const tmp = `${dest}.tmp`;
|
||||||
|
rmSync(tmp, { force: true });
|
||||||
|
// cwd: ROOT so `npx` resolves the repo's pinned esbuild. Without it a run from
|
||||||
|
// another directory misses the local install and fetches an unpinned one.
|
||||||
|
const run = (args) => execFileSync('npx', args, { stdio: 'inherit', cwd: ROOT });
|
||||||
|
try {
|
||||||
|
if (asset.mode === 'copy') {
|
||||||
|
copyFileSync(asset.src, tmp);
|
||||||
|
} else if (asset.mode === 'minify') {
|
||||||
|
run(['esbuild', asset.src, '--minify', `--outfile=${tmp}`]);
|
||||||
|
} else {
|
||||||
|
run([
|
||||||
'esbuild',
|
'esbuild',
|
||||||
asset.src,
|
asset.src,
|
||||||
'--bundle',
|
'--bundle',
|
||||||
'--minify',
|
'--minify',
|
||||||
'--format=iife',
|
'--format=iife',
|
||||||
`--global-name=${asset.globalName}`,
|
`--global-name=${asset.globalName}`,
|
||||||
`--outfile=${dest}`,
|
`--outfile=${tmp}`,
|
||||||
],
|
]);
|
||||||
{ stdio: 'inherit' }
|
}
|
||||||
);
|
} catch (err) {
|
||||||
|
rmSync(tmp, { force: true });
|
||||||
|
console.error(`[test-vendor] failed to build ${asset.out} from ${asset.src}\n ${err.message}`);
|
||||||
|
process.exit(1);
|
||||||
}
|
}
|
||||||
built += 1;
|
built += 1;
|
||||||
|
|
||||||
@@ -108,7 +174,7 @@ for (const asset of ASSETS) {
|
|||||||
// every later step (including the mobile touch handlers) silently never runs.
|
// every later step (including the mobile touch handlers) silently never runs.
|
||||||
if (asset.out === 'xterm-zerolag-input.js') {
|
if (asset.out === 'xterm-zerolag-input.js') {
|
||||||
appendFileSync(
|
appendFileSync(
|
||||||
dest,
|
tmp,
|
||||||
'\n// Global aliases for browser usage\n' +
|
'\n// Global aliases for browser usage\n' +
|
||||||
'if(typeof window!=="undefined"){' +
|
'if(typeof window!=="undefined"){' +
|
||||||
'window.ZerolagInputAddon=XtermZerolagInput.ZerolagInputAddon;' +
|
'window.ZerolagInputAddon=XtermZerolagInput.ZerolagInputAddon;' +
|
||||||
@@ -121,6 +187,9 @@ for (const asset of ASSETS) {
|
|||||||
'}\n'
|
'}\n'
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Only now is the output complete, so publish it.
|
||||||
|
renameSync(tmp, dest);
|
||||||
}
|
}
|
||||||
|
|
||||||
console.log(`[test-vendor] ${built} built, ${skipped} up to date -> src/web/public/vendor/`);
|
console.log(`[test-vendor] ${built} built, ${skipped} up to date -> src/web/public/vendor/`);
|
||||||
|
|||||||
+14
-6
@@ -16,22 +16,30 @@ Validates Codeman's mobile UI across 136 devices, covering:
|
|||||||
|
|
||||||
## Quick Start
|
## Quick Start
|
||||||
|
|
||||||
|
⚠️ Go through `npm run test:mobile`, not `npx vitest` directly. The suite serves the
|
||||||
|
page from `src/web/public`, but `npm run build` puts the xterm vendor bundles in
|
||||||
|
`dist/web/public`, so without them every `/vendor/xterm*` request 404s, `Terminal` is
|
||||||
|
never defined and every test touching `app.terminal` fails on a null. The
|
||||||
|
`pretest:mobile` hook (`scripts/prepare-test-vendor.mjs`) is what puts them in place,
|
||||||
|
and npm only fires it for `npm run test:mobile`. Run the prepare script by hand first
|
||||||
|
if you really need a bare `npx vitest`.
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
# Run all mobile tests
|
# Run all mobile tests
|
||||||
npx vitest run --config test/mobile/vitest.config.ts
|
npm run test:mobile
|
||||||
|
|
||||||
# Run a single test file
|
# Run a single test file
|
||||||
npx vitest run --config test/mobile/vitest.config.ts test/mobile/keyboard.test.ts
|
npm run test:mobile -- test/mobile/keyboard.test.ts
|
||||||
|
|
||||||
# Quick mode — 6 representative devices, skip full matrix
|
# Quick mode: 6 representative devices, skip full matrix
|
||||||
CI_QUICK=1 npx vitest run --config test/mobile/vitest.config.ts
|
CI_QUICK=1 npm run test:mobile
|
||||||
|
|
||||||
# Full device matrix only (136 devices)
|
# Full device matrix only (136 devices)
|
||||||
npx vitest run --config test/mobile/vitest.config.ts test/mobile/device-matrix.test.ts
|
npm run test:mobile -- test/mobile/device-matrix.test.ts
|
||||||
|
|
||||||
# Update visual baselines (delete old baselines, re-run)
|
# Update visual baselines (delete old baselines, re-run)
|
||||||
rm -rf test/mobile/snapshots/*.png
|
rm -rf test/mobile/snapshots/*.png
|
||||||
npx vitest run --config test/mobile/vitest.config.ts test/mobile/visual-regression.test.ts
|
npm run test:mobile -- test/mobile/visual-regression.test.ts
|
||||||
```
|
```
|
||||||
|
|
||||||
## Test Files
|
## Test Files
|
||||||
|
|||||||
Reference in New Issue
Block a user