Skip to content

Commit 85098a4

Browse files
committed
test(objectql): the reader enumeration scans once, in one top-down walk
Each of its three scanning tests re-parsed the package source and climbed from every node to the root to name its site: about 1.5 s per scan unloaded. Under the CPU contention a Test Core shard runs at, two of them measured past vitest's 5 s default timeout. The walk now hands the enclosing site down, the scan is computed once per run, and the tests that may pay for it declare an explicit budget. The READERS table is unchanged, and the pin's own exact-set assertions hold against it. Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7c2888a commit 85098a4

1 file changed

Lines changed: 52 additions & 30 deletions

File tree

‎packages/objectql/src/federated-injected-column-readers.test.ts‎

Lines changed: 52 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -360,29 +360,37 @@ function injectedColumnNames(): ReadonlySet<string> {
360360
return new Set(Object.keys(injectedSystemColumnDefs({ name: 'probe', external: { remoteName: 'probe' }, fields: {} })));
361361
}
362362

363-
/** The name of the function a node sits in, or `<module>` for top-level code. */
364-
function siteOf(node: ts.Node): string {
365-
for (let p: ts.Node | undefined = node.parent; p; p = p.parent) {
366-
if (
367-
(ts.isMethodDeclaration(p) || ts.isFunctionDeclaration(p) || ts.isGetAccessorDeclaration(p) || ts.isSetAccessorDeclaration(p)) &&
368-
p.name
369-
) {
370-
return p.name.getText();
371-
}
372-
if (ts.isConstructorDeclaration(p)) return 'constructor';
373-
const fnInit = (init: ts.Expression | undefined) => !!init && (ts.isArrowFunction(init) || ts.isFunctionExpression(init));
374-
if (ts.isPropertyDeclaration(p) && fnInit(p.initializer)) return p.name.getText();
375-
if (
376-
ts.isVariableDeclaration(p) &&
377-
fnInit(p.initializer) &&
378-
ts.isVariableDeclarationList(p.parent) &&
379-
ts.isVariableStatement(p.parent.parent) &&
380-
ts.isSourceFile(p.parent.parent.parent)
381-
) {
382-
return p.name.getText();
383-
}
363+
/**
364+
* The name a node gives the code inside it when it is a function-like
365+
* container, or `undefined` when it is not. A node's site is the name of its
366+
* NEAREST such ancestor, or `<module>` for top-level code. The walk below
367+
* hands that name down as it descends, so each node's site costs nothing.
368+
* The first version asked for every node by climbing to the root: O(nodes x
369+
* depth) over 2.8 MB of source, about 1.5 s a scan and three scans a run.
370+
* Under the CPU contention a Test Core shard runs at (four suites of three
371+
* workers on four cores), two of those scans measured past vitest's 5 s
372+
* default timeout.
373+
*/
374+
function containerName(p: ts.Node, sf: ts.SourceFile): string | undefined {
375+
if (
376+
(ts.isMethodDeclaration(p) || ts.isFunctionDeclaration(p) || ts.isGetAccessorDeclaration(p) || ts.isSetAccessorDeclaration(p)) &&
377+
p.name
378+
) {
379+
return p.name.getText(sf);
384380
}
385-
return '<module>';
381+
if (ts.isConstructorDeclaration(p)) return 'constructor';
382+
const fnInit = (init: ts.Expression | undefined) => !!init && (ts.isArrowFunction(init) || ts.isFunctionExpression(init));
383+
if (ts.isPropertyDeclaration(p) && fnInit(p.initializer)) return p.name.getText(sf);
384+
if (
385+
ts.isVariableDeclaration(p) &&
386+
fnInit(p.initializer) &&
387+
ts.isVariableDeclarationList(p.parent) &&
388+
ts.isVariableStatement(p.parent.parent) &&
389+
ts.isSourceFile(p.parent.parent.parent)
390+
) {
391+
return p.name.getText(sf);
392+
}
393+
return undefined;
386394
}
387395

388396
function calleeName(call: ts.CallExpression): string | undefined {
@@ -392,7 +400,19 @@ function calleeName(call: ts.CallExpression): string | undefined {
392400
return undefined;
393401
}
394402

403+
/** The one scan this file makes: every test reads the same answer. */
404+
let scanned: Scan | undefined;
405+
406+
/**
407+
* Parsing the package source is the one costly step in this file, so a test
408+
* that may be the first to call this declares {@link SCAN_BUDGET_MS} rather
409+
* than borrowing vitest's 5 s default, which is a budget for a unit, not for a
410+
* parse of the whole package on a shared runner.
411+
*/
412+
const SCAN_BUDGET_MS = 30_000;
413+
395414
function scanObjectqlSources(): Scan {
415+
if (scanned) return scanned;
396416
// Located from THIS test file's own path, as `engine-middleware-operation-vocabulary.test.ts`
397417
// does: the package's build config targets CommonJS, where `import.meta` is TS1470.
398418
const testPath = expect.getState().testPath;
@@ -416,8 +436,8 @@ function scanObjectqlSources(): Scan {
416436
for (const file of files) {
417437
const rel = relative(srcDir, file).split(sep).join('/');
418438
const sf = ts.createSourceFile(file, readFileSync(file, 'utf8'), ts.ScriptTarget.Latest, true);
419-
const visit = (n: ts.Node): void => {
420-
const site = `${rel}#${siteOf(n)}`;
439+
const visit = (n: ts.Node, siteName: string): void => {
440+
const site = `${rel}#${siteName}`;
421441
if (ts.isCallExpression(n)) {
422442
const name = calleeName(n);
423443
if (name) {
@@ -454,11 +474,13 @@ function scanObjectqlSources(): Scan {
454474
) {
455475
seams.add(`${site} :: organization_id`);
456476
}
457-
ts.forEachChild(n, visit);
477+
const inner = containerName(n, sf) ?? siteName;
478+
ts.forEachChild(n, (child) => visit(child, inner));
458479
};
459-
visit(sf);
480+
visit(sf, '<module>');
460481
}
461-
return { seams, calls, files: files.map((f) => relative(srcDir, f).split(sep).join('/')), columns };
482+
scanned = { seams, calls, files: files.map((f) => relative(srcDir, f).split(sep).join('/')), columns };
483+
return scanned;
462484
}
463485

464486
const siteKeyOf = (seamKey: string): string => seamKey.slice(0, seamKey.indexOf(' :: '));
@@ -474,7 +496,7 @@ describe('[#21918] every engine reader of an injected column has a disposition t
474496
expect([...scan.columns]).toEqual(
475497
expect.arrayContaining(['organization_id', 'owning_business_unit_id', 'owner_id', 'created_by', 'updated_by']),
476498
);
477-
});
499+
}, SCAN_BUDGET_MS);
478500

479501
it('has a row for every seam use in the source, and no row for a use that is gone', () => {
480502
const scan = scanObjectqlSources();
@@ -488,7 +510,7 @@ describe('[#21918] every engine reader of an injected column has a disposition t
488510
'its row to READERS in this file.',
489511
).toEqual([]);
490512
expect(stale, 'A READERS row names a seam use the source no longer has: delete or re-key the row.').toEqual([]);
491-
});
513+
}, SCAN_BUDGET_MS);
492514

493515
it('a disposition that says the site asks a federated predicate is true of the source', () => {
494516
const scan = scanObjectqlSources();
@@ -509,7 +531,7 @@ describe('[#21918] every engine reader of an injected column has a disposition t
509531
expect(asks.some((p) => own.has(p)), `${key}: the site asks none of ${asks.join(', ')}`).toBe(true);
510532
}
511533
}
512-
});
534+
}, SCAN_BUDGET_MS);
513535

514536
it('the sites that skip are exactly the cascade scan, its plan, and the lifecycle tenant partition', () => {
515537
const skipping = new Set(

0 commit comments

Comments
 (0)