mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
fix(test-cache): stop three id shapes from making a test permanently uncacheable
A happy-dom test goes through the web transform, so a node builtin it imports
resolves to `__vite-browser-external:crypto` — an id no path can name. The key
fingerprinted it as a file, got `missing`, and the reporter read that as a module
deleted during the run. All 44 happy-dom test files in the repo were recorded on
no run and re-ran on every one. Vite virtual ids (a `\0` prefix, a plugin scheme)
fail the same way, so the rule is written for the shape, not the one spelling.
`src/hooks/useDateLocale.ts` imports `dayjs/locale/${tag}.js`, which the
computed-import rule treats as opaque — but whatever it computes resolves under
node_modules, which the lockfile already covers. Narrowed to a template whose
literal head is a bare specifier, excluding `@civitai/*` (a workspace symlink
into packages/), any head carrying a `:` (`node:${mod}` can be child_process)
and `./` or `~/`. That one file made another 44 test files uncacheable.
`toRel` also THREW on a POSIX `file://` id on Windows, where fileURLToPath raises
ERR_INVALID_FILE_URL_PATH. That is red on main today, in this file's own POSIX
invariant test, on every Windows run.
The e2e gains a happy-dom fixture so the control is end to end rather than a
predicate assertion: reverting the first fix fails it with
`expected { recorded: 1, notRecorded: { "imported module gone": 1 } } to deeply
equal { recorded: 2, notRecorded: {} }`.
Verified: full unit suite 1927 passed | 3 skipped (1930), exit 0, 0 false skips;
typecheck 0 errors; test:lint-rules 47 files, 660 tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -158,6 +158,11 @@ describe('tests that always run', () => {
|
||||
["const { Worker } = require('worker_threads');"],
|
||||
['const m = await import(`./pages/${name}`);'],
|
||||
['const m = await import(target);'],
|
||||
// A bare-specifier computed import is cacheable (below), but only where its head cannot name
|
||||
// first-party source or a builtin. Whoever widens that: these three are why it is narrow.
|
||||
['const m = await import(`~/server/${name}`);'],
|
||||
['const m = await import(`@civitai/ui/${name}`);'],
|
||||
['const m = await import(`node:${mod}`);'],
|
||||
// The form this repo uses, which the first version of the pattern let through.
|
||||
['return import(/* @vite-ignore */ file);'],
|
||||
["const files = globSync('src/**/*.ts');"],
|
||||
@@ -178,6 +183,9 @@ describe('tests that always run', () => {
|
||||
// test's setup. Matching the bare word made all 1880 unit tests uncacheable.
|
||||
["const opts = { client: 'cluster' };"],
|
||||
["// Module not found: Can't resolve 'cluster'"],
|
||||
// Whatever it computes lives under node_modules, which the lockfile covers. The real site is
|
||||
// src/hooks/useDateLocale.ts, and treating it as opaque cost 44 test files.
|
||||
['const m = await import(`dayjs/locale/${tag}.js`);'],
|
||||
])('leaves %s cacheable', (source) => {
|
||||
expect(alwaysRuns(source)).toBe(false);
|
||||
});
|
||||
|
||||
@@ -44,13 +44,15 @@ describe('the cache, run for real', () => {
|
||||
|
||||
const cold = runOnce(cacheDir);
|
||||
expect(cold.status).toBe(0);
|
||||
// Both fixtures, one of which is a happy-dom file: that one resolves a node builtin to a vite
|
||||
// virtual id, and a key that treats such an id as a path records nothing for it.
|
||||
expect({ recorded: cold.last.recorded, notRecorded: cold.last.notRecorded }).toEqual({
|
||||
recorded: 1,
|
||||
recorded: 2,
|
||||
notRecorded: {},
|
||||
});
|
||||
|
||||
const warm = runOnce(cacheDir);
|
||||
expect(warm.status).toBe(0);
|
||||
expect({ ran: warm.last.ran, skipped: warm.last.skipped }).toEqual({ ran: 0, skipped: 1 });
|
||||
expect({ ran: warm.last.ran, skipped: warm.last.skipped }).toEqual({ ran: 0, skipped: 2 });
|
||||
}, 300_000);
|
||||
});
|
||||
|
||||
@@ -0,0 +1,11 @@
|
||||
// @vitest-environment happy-dom
|
||||
import { randomUUID } from 'crypto';
|
||||
import { expect, it } from 'vitest';
|
||||
|
||||
// A node builtin imported from a test the WEB transform handles resolves to
|
||||
// `__vite-browser-external:crypto` — a module id no path can name. Fingerprinting it as a file
|
||||
// said "missing", which the reporter read as a module deleted mid-run, and all 44 happy-dom tests
|
||||
// in the repo went unrecorded. Whoever deletes this: the point is the id, not the UUID.
|
||||
it('passes', () => {
|
||||
expect(typeof randomUUID).toBe('function');
|
||||
});
|
||||
@@ -96,7 +96,16 @@ export function toRel(id, root) {
|
||||
// `toRel('/C:/notes/x.md', '/C:')` returned 'notes/x.md' before and null after. Only a
|
||||
// `file://` id can carry the platform artefact, so only a `file://` id needs the repair,
|
||||
// and confining it here leaves every non-URL input byte-for-byte as it was.
|
||||
p = fileURLToPath(p).replace(/^\/([A-Za-z]:)/, '$1');
|
||||
// 🔴 And it THROWS rather than returning the other platform's spelling: on Windows a POSIX
|
||||
// `file:///home/u/x.ts` is `ERR_INVALID_FILE_URL_PATH`, which took this file's own POSIX
|
||||
// invariant test red on every Windows run of `main`. The URL's pathname is the same string
|
||||
// `fileURLToPath` would have produced on the host that wrote it, so fall back to it.
|
||||
try {
|
||||
p = fileURLToPath(p);
|
||||
} catch {
|
||||
p = decodeURIComponent(new URL(p).pathname);
|
||||
}
|
||||
p = p.replace(/^\/([A-Za-z]:)/, '$1');
|
||||
}
|
||||
p = p.split('?')[0].replace(/\\/g, '/');
|
||||
const r = root.replace(/\\/g, '/').replace(/\/$/, '');
|
||||
@@ -111,13 +120,31 @@ export function isCoveredElsewhere(rel) {
|
||||
// like `src`, which the convention guards list — dropping the one read that sees a new file.
|
||||
if (isBuiltin(rel)) return true;
|
||||
if (rel.startsWith('node:')) return true;
|
||||
// A vite virtual module is not a file on disk, so fingerprinting it yields `missing` and the
|
||||
// reporter reads that as a module deleted mid-run. Measured 2026-09-19: every happy-dom test
|
||||
// depends on `__vite-browser-external:crypto`, and all 44 of them fell out of the cache this
|
||||
// way. A `\0` prefix, a plugin scheme and vite's browser shims are all ids no path can name.
|
||||
if (rel.startsWith('\0') || /^[A-Za-z_][A-Za-z\d_+.-]*:/.test(rel)) return true;
|
||||
if (rel.startsWith('node_modules/') || rel.includes('/node_modules/')) return true;
|
||||
if (rel.startsWith('.git/')) return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* A computed import whose literal head is a BARE package specifier — `dayjs/locale/${tag}.js` —
|
||||
* resolves under node_modules whatever it computes, and the lockfile already covers that. Three
|
||||
* heads are deliberately not bare: `@civitai/*` is a workspace symlink into `packages/`, a head
|
||||
* carrying a `:` can be `node:${mod}` (a builtin, and one of them spawns), and `./` or `~/` is
|
||||
* first-party source. Measured 2026-09-19: `src/hooks/useDateLocale.ts` is the repo's only such
|
||||
* site, and it alone made 44 test files uncacheable.
|
||||
*/
|
||||
const BARE_COMPUTED_IMPORT = new RegExp(
|
||||
String.raw`import\(\s*(?:/\*[\s\S]*?\*/\s*)*\x60(?!@civitai/)[A-Za-z@][^\x60$:]*\$\{[^\x60]*\x60\s*\)`,
|
||||
'g'
|
||||
);
|
||||
|
||||
export function alwaysRuns(source) {
|
||||
return ALWAYS_RUN_SOURCE.some((re) => re.test(source));
|
||||
return ALWAYS_RUN_SOURCE.some((re) => re.test(source.replace(BARE_COMPUTED_IMPORT, '')));
|
||||
}
|
||||
|
||||
const RESOLVABLE_EXTS = ['ts', 'tsx', 'mts', 'cts', 'js', 'jsx', 'mjs', 'cjs', 'json'];
|
||||
@@ -173,7 +200,9 @@ export function makeFingerprinter(root) {
|
||||
let fp;
|
||||
try {
|
||||
const st = statSync(abs);
|
||||
fp = st.isDirectory() ? `dir:${sha(listTree(abs).join('\n'))}` : `file:${sha(readFileSync(abs))}`;
|
||||
fp = st.isDirectory()
|
||||
? `dir:${sha(listTree(abs).join('\n'))}`
|
||||
: `file:${sha(readFileSync(abs))}`;
|
||||
} catch {
|
||||
fp = 'missing';
|
||||
}
|
||||
@@ -243,7 +272,9 @@ export function keyFor({ salt, project, testRel, entries, fingerprint }) {
|
||||
const shadows = [...new Set(deps.flatMap(shadowCandidates))]
|
||||
.filter((rel) => fingerprint(rel) !== 'missing')
|
||||
.sort();
|
||||
return sha([salt, project, testRel, fingerprint(testRel), ...parts, '--shadows--', ...shadows].join('\n'));
|
||||
return sha(
|
||||
[salt, project, testRel, fingerprint(testRel), ...parts, '--shadows--', ...shadows].join('\n')
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -295,7 +326,11 @@ export function cacheDir(root) {
|
||||
const common = execFileSync('git', ['rev-parse', '--git-common-dir'], { cwd: root })
|
||||
.toString()
|
||||
.trim();
|
||||
return join(isAbsolute(common) ? common : join(root, common), 'civitai-test-cache', `v${CACHE_FORMAT}`);
|
||||
return join(
|
||||
isAbsolute(common) ? common : join(root, common),
|
||||
'civitai-test-cache',
|
||||
`v${CACHE_FORMAT}`
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -309,7 +344,10 @@ export function recordsFor(dir, project, testRel) {
|
||||
for (const name of readdirSync(d)) {
|
||||
if (!name.endsWith('.json')) continue;
|
||||
try {
|
||||
out.push({ ...JSON.parse(readFileSync(join(d, name), 'utf8')), mtime: statSync(join(d, name)).mtimeMs });
|
||||
out.push({
|
||||
...JSON.parse(readFileSync(join(d, name), 'utf8')),
|
||||
mtime: statSync(join(d, name)).mtimeMs,
|
||||
});
|
||||
} catch {
|
||||
/* a half-written or foreign file is not a record */
|
||||
}
|
||||
@@ -329,7 +367,8 @@ export function writeRecord(dir, project, testRel, record) {
|
||||
const byAge = all
|
||||
.map((n) => ({ n, t: statSync(join(d, n)).mtimeMs }))
|
||||
.sort((a, b) => a.t - b.t);
|
||||
for (const { n } of byAge.slice(0, all.length - RECORDS_PER_TEST)) rmSync(join(d, n), { force: true });
|
||||
for (const { n } of byAge.slice(0, all.length - RECORDS_PER_TEST))
|
||||
rmSync(join(d, n), { force: true });
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user