Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions scripts/__tests__/eager-closure-budgets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,24 @@ test('the committed-tree reader walks what was committed, not the working direct
expect(eagerClosureGraphOf(entry).size, 'working tree: one edge').toBe(2);
});

test('a JSON data import is not an edge, so the committed-tree walk does not read it', () => {
// The committed reader holds only walked TypeScript sources; following a `with { type: 'json' }`
// import into it threw at the merge-base once a façade closure first reached a JSON fixture.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: This comment describes the past failure rather than the invariant. Drop the history sentence and keep one line stating the rule, e.g. "A JSON import is data, not an edge; the committed reader does not hold it." The repo guide says not to record review history in comments.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/__tests__/eager-closure-budgets.test.ts, line 478:

<comment>This comment describes the past failure rather than the invariant. Drop the history sentence and keep one line stating the rule, e.g. "A JSON import is data, not an edge; the committed reader does not hold it." The repo guide says not to record review history in comments.</comment>

<file context>
@@ -473,6 +473,24 @@ test('the committed-tree reader walks what was committed, not the working direct
 
+test('a JSON data import is not an edge, so the committed-tree walk does not read it', () => {
+  // The committed reader holds only walked TypeScript sources; following a `with { type: 'json' }`
+  // import into it threw at the merge-base once a façade closure first reached a JSON fixture.
+  const repo = mkGitFixtureRepo('eager-closure-json-import-');
+  const entry = path.join(repo, 'packages/demo/src/entry.ts');
</file context>

const repo = mkGitFixtureRepo('eager-closure-json-import-');
const entry = path.join(repo, 'packages/demo/src/entry.ts');
fs.writeFileSync(path.join(repo, 'packages/demo/src/capture.raw.json'), '{"nodes":[]}\n');
fs.writeFileSync(
entry,
"import capture from './capture.raw.json' with { type: 'json' };\nexport const a = capture;\n",
);
execFileSync('git', ['add', '-A'], { cwd: repo });
execFileSync('git', ['commit', '-qm', 'json import'], { cwd: repo });

const committed = createCommittedSourceTree(repo, 'HEAD');
expect([...eagerClosureGraphOf(entry, committed).keys()]).toEqual([entry]);
expect([...eagerClosureGraphOf(entry).keys()]).toEqual([entry]);
});

test('a renamed entry is followed to its path at the base, not treated as first-introduced', () => {
const repo = mkGitFixtureRepo('eager-closure-renamed-entry-');
execFileSync('git', ['mv', 'packages/demo/src/entry.ts', 'packages/demo/src/moved.ts'], {
Expand Down
34 changes: 1 addition & 33 deletions scripts/__tests__/eager-closure-budgets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,39 +132,7 @@ export const NEW_ENTRY_CEILINGS: Readonly<Record<EntryCategory, number>> = Objec
*/
export const APPROVED_OVER_CEILING: Readonly<
Record<string, { issue: string; reason: string; owner: string }>
> = Object.freeze({
// The single owner of the client-collected replay source bundles tests carry (#1802). Its
// weight is the real collector it exists to share, not fixture code.
'packages/replay-port/src/replay-script-source.fixtures.ts': {
issue: '#3517',
reason:
'Test-only request fixture whose whole contract is running the real client collector ' +
'(script-source-bundle + source-discovery) so the root and daemon test trees cannot ' +
'drift from it; the weight is the ad-script format parser behind resolveReplayFormat. ' +
'Making that lazy would fork a second collector, which is the duplication #3517 removes. ' +
'No platform implementation; never bundled.',
owner: 'replay-port',
},
// The two Android fixture subpaths are by design the REAL walker on archived device trees:
// their whole contract is running the shipped projection over captures no hand-written node
// could stand in for, so their closure IS the mechanics the subpath exposes.
'packages/platform-android/src/snapshot-capture.fixtures.ts': {
issue: '#3517',
reason:
'Test-only Android capture builders over the package mechanics facade, shared by the ' +
'daemon and root test trees; a fixture that simulated the walk would prove nothing about ' +
'production inclusion decisions. Test-only subpath; never bundled.',
owner: 'platform-android',
},
'packages/platform-android/src/ui-hierarchy.fixtures.ts': {
issue: '#3517',
reason:
'Test-only archived device trees (IME, QS shade) plus the real non-raw/interactive walks ' +
'over them, shared by the daemon and root test trees; the closure is the mechanics facade ' +
'the fixtures must run. Test-only subpath; never bundled.',
owner: 'platform-android',
},
});
> = Object.freeze({});

/** The category is a function of the path, never a hand-written column. */
export function entryCategoryOf(entryFile: string): EntryCategory {
Expand Down
4 changes: 3 additions & 1 deletion src/__tests__/eager-import-closure.fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,9 @@ const workingTreeReader: SourceTreeReader = {
*/
function resolveRelative(from: string, specifier: string, tree: SourceTreeReader): string | null {
const candidate = path.resolve(path.dirname(from), specifier);
if (tree.isFile(candidate)) return candidate;
// A JSON import (`with { type: 'json' }`) is data with no edges, and the committed tree reader
// only reads walked TypeScript sources, so it is not an edge.
if (tree.isFile(candidate)) return candidate.endsWith('.ts') ? candidate : null;
for (const suffix of ['.ts', '/index.ts']) {
if (tree.exists(`${candidate}${suffix}`)) return `${candidate}${suffix}`;
}
Expand Down
Loading