Skip to content

Commit a56f203

Browse files
atscottcrisbeto
authored andcommitted
test(compiler-cli): port preprocessor CLI, sidecar isolation, and golden typecheck tests
- Port `ngp.test.ts` to `test/ngp_spec.ts` to test the `main.ts` CLI entrypoint under Bazel. - Fix the inverted `fs.access(ngAnalyzeDir)` fallback in `main.ts`: when `alxhub/ng-hybrid-preprocessor` commit `c707e178` converted `if (!fs.existsSync(ngAnalyzeDir))` to async `fs.access(ngAnalyzeDir)`, it placed `ngAnalyzeDir = path.resolve(__dirname, '../../ng-analyze')` inside the `try` block instead of the `catch` block, overwriting a valid `./ng-analyze` directory with `../../ng-analyze` whenever `./ng-analyze` existed. - Port `scripts/check-sidecar-isolation.mjs` to `test/sidecar_isolation_spec.ts` to guard against static imports of `analyzer_loader`, `analyzer_napi`, `analyzer_wasm`, or `wasm_host_fs` from sidecar-mode entrypoints. - Port `scripts/typecheck-goldens.ts` to `test/typecheck_goldens_spec.ts` with a dedicated sharded `//packages/compiler-cli/preprocessor/test:typecheck_goldens_test` Bazel target to verify emitted TypeScript across all golden test cases.
1 parent 61e0558 commit a56f203

6 files changed

Lines changed: 651 additions & 5 deletions

File tree

‎packages/compiler-cli/preprocessor/main.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,9 @@ import {run} from './ngp.js';
134134

135135
try {
136136
await fs.access(ngAnalyzeDir);
137+
} catch {
137138
ngAnalyzeDir = path.resolve(__dirname, '../../ng-analyze');
138-
} catch {}
139+
}
139140

140141
innerAnalyzer = await NapiAnalyzer.create(tsconfigPath, {
141142
optimize,

‎packages/compiler-cli/preprocessor/test/BUILD.bazel‎

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,36 @@
11
load("//tools:defaults.bzl", "jasmine_test", "ts_project")
22

3+
ts_project(
4+
name = "test_support",
5+
testonly = True,
6+
srcs = [
7+
"golden_cases.ts",
8+
"golden_markdown.ts",
9+
"utils.ts",
10+
],
11+
deps = [
12+
"//:node_modules/@types/jasmine",
13+
"//:node_modules/@types/node",
14+
"//:node_modules/diff",
15+
"//:node_modules/prettier",
16+
"//:node_modules/typescript",
17+
"//packages/compiler",
18+
"//packages/compiler-cli/preprocessor",
19+
],
20+
)
21+
322
ts_project(
423
name = "test_lib",
524
testonly = True,
625
srcs = glob(
7-
[
8-
"**/*_spec.ts",
9-
"*.ts",
26+
["**/*_spec.ts"],
27+
exclude = [
28+
"golden/**",
29+
"typecheck_goldens_spec.ts",
1030
],
11-
exclude = ["golden/**"],
1231
),
1332
deps = [
33+
":test_support",
1434
"//:node_modules/@types/jasmine",
1535
"//:node_modules/@types/node",
1636
"//:node_modules/diff",
@@ -37,3 +57,31 @@ jasmine_test(
3757
] + glob(["golden/**"]),
3858
shard_count = 4,
3959
)
60+
61+
ts_project(
62+
name = "typecheck_goldens_lib",
63+
testonly = True,
64+
srcs = ["typecheck_goldens_spec.ts"],
65+
deps = [
66+
":test_support",
67+
"//:node_modules/@types/jasmine",
68+
"//:node_modules/@types/node",
69+
"//:node_modules/typescript",
70+
],
71+
)
72+
73+
jasmine_test(
74+
name = "typecheck_goldens_test",
75+
timeout = "long",
76+
data = [
77+
":typecheck_goldens_lib",
78+
"//:node_modules/rxjs",
79+
"//:node_modules/tslib",
80+
"//packages/common:npm_package",
81+
"//packages/compiler-cli/preprocessor/ng-analyze:ng_analyze_bin",
82+
"//packages/compiler-cli/preprocessor/ng-analyze:ng_analyze_wasm",
83+
"//packages/compiler-cli/preprocessor/ng-analyze:package_json",
84+
"//packages/core:npm_package",
85+
] + glob(["golden/**"]),
86+
shard_count = 4,
87+
)
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import {execFileSync} from 'child_process';
10+
import * as fs from 'node:fs/promises';
11+
import * as os from 'os';
12+
import * as path from 'path';
13+
import {resolveGoldenRoot} from './golden_cases.js';
14+
import {getOrCreateSyntheticNodeModules, parseMarkdownTestCase, pathExists} from './utils.js';
15+
16+
describe('ngp CLI (main.ts)', () => {
17+
const ngpPath = path.resolve(import.meta.dirname, '../main.js');
18+
const fixtureDir = path.join(resolveGoldenRoot(), 'directive_host_directives');
19+
let tempDir: string;
20+
let tsconfigPath: string;
21+
22+
const runNgp = (args: string[]) => {
23+
return execFileSync(process.execPath, [...process.execArgv, ngpPath, ...args], {
24+
encoding: 'utf-8',
25+
stdio: 'pipe',
26+
env: process.env,
27+
});
28+
};
29+
30+
beforeAll(async () => {
31+
const mdPath = path.join(fixtureDir, 'source.md');
32+
if (!(await pathExists(mdPath))) {
33+
throw new Error(`Source file not found: ${mdPath}`);
34+
}
35+
const files = await parseMarkdownTestCase(mdPath);
36+
37+
tempDir = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'ngp-cli-test-')));
38+
tsconfigPath = path.join(tempDir, 'tsconfig.json');
39+
40+
const nodeModules = getOrCreateSyntheticNodeModules();
41+
if (nodeModules) {
42+
await fs.symlink(nodeModules, path.join(tempDir, 'node_modules'), 'dir');
43+
}
44+
45+
await Promise.all(
46+
files.map(async (file) => {
47+
const relativePath = file.path.startsWith('/') ? file.path.slice(1) : file.path;
48+
const filePath = path.join(tempDir, relativePath);
49+
await fs.mkdir(path.dirname(filePath), {recursive: true});
50+
await fs.writeFile(filePath, file.content);
51+
}),
52+
);
53+
});
54+
55+
afterAll(async () => {
56+
if (tempDir) {
57+
await fs.rm(tempDir, {recursive: true, force: true});
58+
}
59+
});
60+
61+
it('should exit with an error when missing tsconfig argument', () => {
62+
expect(() => runNgp([])).toThrowError(
63+
/Usage: ngp <project-dir\|angular\.json\|tsconfig\.json>/,
64+
);
65+
});
66+
67+
it('should process files and output to console by default', () => {
68+
const output = runNgp([tsconfigPath, '--wasm']);
69+
70+
expect(output).toContain('Running ngp...');
71+
expect(output).toContain('Processing files...');
72+
expect(output).toContain('test.ts ---');
73+
});
74+
75+
it('should create output files when --out is specified', async () => {
76+
const outDir = await fs.mkdtemp(path.join(os.tmpdir(), 'ngp-cli-out-'));
77+
78+
try {
79+
const output = runNgp([tsconfigPath, '--wasm', '--out', outDir, '--root', tempDir]);
80+
81+
expect(output).toContain(`(output: ${outDir})`);
82+
expect(output).toContain(`(root: ${tempDir})`);
83+
84+
const generatedSourcePath = path.join(outDir, 'test.ts');
85+
expect(await pathExists(generatedSourcePath)).toBe(true);
86+
expect(await pathExists(`${generatedSourcePath}.map`)).toBe(true);
87+
} finally {
88+
await fs.rm(outDir, {recursive: true, force: true});
89+
}
90+
});
91+
});
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import * as fs from 'node:fs/promises';
10+
import * as path from 'path';
11+
import {pathExists} from './utils.js';
12+
13+
/**
14+
* Guards the napi-less deployment path.
15+
*
16+
* The compiler also runs where N-API is unavailable, driven by `--sidecar <path>`,
17+
* which talks to an out-of-process engine over stdio. That works only because engine
18+
* resolution is never reached by a *static* import: `main.js` pulls the loader in
19+
* behind a dynamic `import()` inside its non-sidecar branch, and `api.js` exposes it
20+
* only through `createAnalyzer()`.
21+
*
22+
* A single static `import` added to `api.ts`, `hybrid_compiler.ts` or
23+
* `analyzer_sidecar.ts` would drag the loader into module evaluation and break those
24+
* deployments at import time. This walks the emitted static import graph from each
25+
* entry below and fails if it can reach a forbidden module.
26+
*/
27+
describe('sidecar isolation', () => {
28+
const packageDir = path.resolve(import.meta.dirname, '..');
29+
30+
/** Entry points that must stay free of engine-resolution code. */
31+
const ENTRIES = ['src/analyzer_sidecar.js', 'src/hybrid_compiler.js'];
32+
33+
/** Modules that must not be statically reachable from those entries. */
34+
const FORBIDDEN = ['analyzer_loader', 'analyzer_napi', 'analyzer_wasm', 'wasm_host_fs'];
35+
36+
/**
37+
* Static `import ... from '...'` / `export ... from '...'` specifiers.
38+
* Deliberately does not match dynamic `import(...)`, which is the permitted escape hatch.
39+
*/
40+
const STATIC_IMPORT = /(?:^|\n)\s*(?:import|export)\b[^;\n]*?\sfrom\s*['"]([^'"]+)['"]/g;
41+
42+
async function resolveSpecifier(fromFile: string, specifier: string): Promise<string | null> {
43+
if (!specifier.startsWith('.')) {
44+
return null; // bare specifier: an external package, not our graph
45+
}
46+
const resolved = path.resolve(path.dirname(fromFile), specifier);
47+
for (const candidate of [resolved, `${resolved}.js`, path.join(resolved, 'index.js')]) {
48+
const stat = await fs.stat(candidate).catch(() => null);
49+
if (stat?.isFile()) {
50+
return candidate;
51+
}
52+
}
53+
return null;
54+
}
55+
56+
/** Walks the static import graph, returning the first offending path found. */
57+
async function findForbiddenPath(entry: string): Promise<string[] | null> {
58+
const seen = new Set<string>();
59+
const stack: Array<[string, string[]]> = [[entry, [path.relative(packageDir, entry)]]];
60+
61+
while (stack.length > 0) {
62+
const [file, chain] = stack.pop()!;
63+
if (seen.has(file)) {
64+
continue;
65+
}
66+
seen.add(file);
67+
68+
const source = await fs.readFile(file, 'utf-8');
69+
for (const match of source.matchAll(STATIC_IMPORT)) {
70+
const target = await resolveSpecifier(file, match[1]);
71+
if (target === null) {
72+
continue;
73+
}
74+
const nextChain = [...chain, path.relative(packageDir, target)];
75+
const base = path.basename(target, '.js');
76+
if (FORBIDDEN.includes(base)) {
77+
return nextChain;
78+
}
79+
stack.push([target, nextChain]);
80+
}
81+
}
82+
return null;
83+
}
84+
85+
for (const entry of ENTRIES) {
86+
it(`does not statically reach engine-resolution modules from ${entry}`, async () => {
87+
const file = path.join(packageDir, entry);
88+
expect(await pathExists(file))
89+
.withContext(`Missing build output: ${entry}`)
90+
.toBe(true);
91+
92+
const chain = await findForbiddenPath(file);
93+
expect(chain)
94+
.withContext(
95+
chain
96+
? `Engine-resolution code is statically reachable:\n ${chain.join('\n -> ')}`
97+
: '',
98+
)
99+
.toBeNull();
100+
});
101+
}
102+
});

0 commit comments

Comments
 (0)