Skip to content

Commit 832bfff

Browse files
devversionalan-agius4
devversion
authored and
alan-agius4
committed
build: migrate @angular-devkit/build-angular tests to rules_js
Migrates the `@angular-devkit/build-angular` tests to `rules_js`. This was a rather larger undertaking as the tests were very reliant on e.g. the directory structure or specific node module layout; so some changes were needed. - the Sass files include a much larger file header now. That is because the npm Sass files have much larger paths, given being inside a symlinked pnpm store directory. E.g. ``` /*!**********************************************************************************************************************************************************************************************************************************************************************************************************************************************************************************!*\ !*** css ../../../../../node_modules/.aspect_rules_js/css-loader@7.1.2_webpack_5.97.1/node_modules/css-loader/dist/cjs.js??ruleSet[1].rules[4].rules[0].oneOf[0].use[1]!../../../../../node_modules/.aspect_rules_js/postcss-loader@8.1.1_1462687623/node_modules/postcss-loader/dist/cjs.js??ruleSet[1].rules[4].rules[0].oneOf[0].use[2]!./src/test-style-a.css?ngGlobalStyle ***! \**********************************************************************************************************************************************************************************************************************************************************************************************************************************************************************************/ .test-a {color: red} ``` - Similarly to above, hashed chunk files can change given different paths of e.g. Webpack, or external sources. - Tests for verifying the lazy module chunks may enable `preserveSymlinks` just to make the chunk names shorter and easier to verify, avoiding truncatd super long paths to the e.g. pnpm stores again. - the ngsw-worker.js file cannot be copied using `copyFile` as that results in permissions being copied as well. In Bazel, now that the npm files are properly captured, are readonly, so subsequent builds (e.g. the watch tests) will fail to copy/override the file again! Reading and writing the file consistently seems appropriate. - Tests relying on puppeteer and webdriver-manager worked in the past, by accident, because postinstall scripts (from e.g. puppeteer) were able to modify content of other packages (e.g. the puppeteer-core cache of browsers then). This does not work with `rules_js` anymore, so we need to keep the cache local to the puppeteer postinstall script. This requires a little trickery right now to ensure resolution of the browsers at runtime works.. - server tests did miss the `node` types to be explicitly listed (as they would be in a fresh project), and this caused failures. Likely because we no longer patch resolution. - avoid npm-module style imports from tests within the same package. This is not allowed with `rules_js` and also is inconsistent.
1 parent 0fe4a54 commit 832bfff

File tree

17 files changed

+300
-430
lines changed

17 files changed

+300
-430
lines changed

‎.aspect/rules/external_repository_action_cache/npm_translate_lock_MzA5NzUwNzMx

+5-5
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,21 @@
22
# Input hashes for repository rule npm_translate_lock(name = "npm2", pnpm_lock = "@//:pnpm-lock.yaml").
33
# This file should be checked into version control along with the pnpm-lock.yaml file.
44
.npmrc=-1406867100
5-
modules/testing/builder/package.json=-1196120648
6-
package.json=550134286
5+
modules/testing/builder/package.json=973445093
6+
package.json=-744581142
77
packages/angular/build/package.json=1783072863
88
packages/angular/cli/package.json=1301682969
99
packages/angular/pwa/package.json=1108903917
1010
packages/angular/ssr/package.json=1104313629
1111
packages/angular_devkit/architect/package.json=-1496633956
1212
packages/angular_devkit/architect_cli/package.json=1551210941
13-
packages/angular_devkit/build_angular/package.json=1732310149
13+
packages/angular_devkit/build_angular/package.json=-1206066740
1414
packages/angular_devkit/build_webpack/package.json=373950017
1515
packages/angular_devkit/core/package.json=339935828
1616
packages/angular_devkit/schematics/package.json=673943597
1717
packages/angular_devkit/schematics_cli/package.json=-169616762
1818
packages/ngtools/webpack/package.json=1463215526
1919
packages/schematics/angular/package.json=251715148
20-
pnpm-lock.yaml=1489986000
21-
pnpm-workspace.yaml=-1847919625
20+
pnpm-lock.yaml=1844603164
21+
pnpm-workspace.yaml=-1056556036
2222
yarn.lock=346921654

‎WORKSPACE

+14
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,10 @@ load("@aspect_rules_js//npm:repositories.bzl", "npm_translate_lock")
183183

184184
npm_translate_lock(
185185
name = "npm2",
186+
custom_postinstalls = {
187+
# TODO: Standardize browser management for `rules_js`
188+
"webdriver-manager": "node ./bin/webdriver-manager update --standalone false --gecko false --versions.chrome 106.0.5249.21",
189+
},
186190
data = [
187191
"//:package.json",
188192
"//:pnpm-workspace.yaml",
@@ -201,6 +205,16 @@ npm_translate_lock(
201205
"//packages/ngtools/webpack:package.json",
202206
"//packages/schematics/angular:package.json",
203207
],
208+
lifecycle_hooks_envs = {
209+
# TODO: Standardize browser management for `rules_js`
210+
"puppeteer": ["PUPPETEER_DOWNLOAD_PATH=./downloads"],
211+
},
212+
lifecycle_hooks_execution_requirements = {
213+
# Needed for downloading chromedriver.
214+
# Also `update-config` of webdriver manager would store an absolute path;
215+
# which would then break execution.
216+
"webdriver-manager": ["local"],
217+
},
204218
npmrc = "//:.npmrc",
205219
patches = {
206220
# Note: Patches not needed as the existing patches are only

‎modules/testing/builder/package.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
"devDependencies": {
33
"@angular-devkit/core": "workspace:*",
44
"@angular-devkit/architect": "workspace:*",
5-
"@angular/ssr": "workspace:*"
5+
"@angular/ssr": "workspace:*",
6+
"@angular-devkit/build-angular": "workspace:*"
67
}
78
}

‎modules/testing/builder/projects/hello-world-app/src/tsconfig.server.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"compilerOptions": {
44
"outDir": "../dist-server",
55
"baseUrl": "./",
6-
"types": ["@angular/localize"]
6+
"types": ["@angular/localize", "node"]
77
},
88
"files": [
99
"main.server.ts"

‎modules/testing/builder/src/test-utils.ts

+8
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
virtualFs,
2222
workspaces,
2323
} from '@angular-devkit/core';
24+
import path from 'node:path';
2425
import { firstValueFrom } from 'rxjs';
2526

2627
// Default timeout for large specs is 2.5 minutes.
@@ -42,6 +43,13 @@ export async function createArchitect(workspaceRoot: Path) {
4243
registry.addPostTransform(schema.transforms.addUndefinedDefaults);
4344
const workspaceSysPath = getSystemPath(workspaceRoot);
4445

46+
// The download path is relative (set from Starlark), so before potentially
47+
// changing directories, or executing inside a temporary directory, ensure
48+
// the path is absolute.
49+
if (process.env['PUPPETEER_DOWNLOAD_PATH']) {
50+
process.env.PUPPETEER_DOWNLOAD_PATH = path.resolve(process.env['PUPPETEER_DOWNLOAD_PATH']);
51+
}
52+
4553
const { workspace } = await workspaces.readWorkspace(
4654
workspaceSysPath,
4755
workspaces.createWorkspaceHost(host),

‎package.json

+4-1
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,10 @@
221221
}
222222
},
223223
"pnpm": {
224-
"onlyBuiltDependencies": [],
224+
"onlyBuiltDependencies": [
225+
"puppeteer",
226+
"webdriver-manager"
227+
],
225228
"overrides": {
226229
"@angular/build": "workspace:*"
227230
}

‎packages/angular/build/src/utils/service-worker.ts

+3-7
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export async function augmentAppWithServiceWorker(
126126
outputPath: string,
127127
baseHref: string,
128128
ngswConfigPath?: string,
129-
inputputFileSystem = fsPromises,
129+
inputFileSystem = fsPromises,
130130
outputFileSystem = fsPromises,
131131
): Promise<void> {
132132
// Determine the configuration file path
@@ -137,7 +137,7 @@ export async function augmentAppWithServiceWorker(
137137
// Read the configuration file
138138
let config: Config | undefined;
139139
try {
140-
const configurationData = await inputputFileSystem.readFile(configPath, 'utf-8');
140+
const configurationData = await inputFileSystem.readFile(configPath, 'utf-8');
141141
config = JSON.parse(configurationData) as Config;
142142
} catch (error) {
143143
assertIsError(error);
@@ -161,11 +161,7 @@ export async function augmentAppWithServiceWorker(
161161
const copy = async (src: string, dest: string): Promise<void> => {
162162
const resolvedDest = path.join(outputPath, dest);
163163

164-
return inputputFileSystem === outputFileSystem
165-
? // Native FS (Builder).
166-
inputputFileSystem.copyFile(src, resolvedDest, fsConstants.COPYFILE_FICLONE)
167-
: // memfs (Webpack): Read the file from the input FS (disk) and write it to the output FS (memory).
168-
outputFileSystem.writeFile(resolvedDest, await inputputFileSystem.readFile(src));
164+
return outputFileSystem.writeFile(resolvedDest, await inputFileSystem.readFile(src));
169165
};
170166

171167
await outputFileSystem.writeFile(path.join(outputPath, 'ngsw.json'), result.manifest);

‎packages/angular_devkit/build_angular/BUILD.bazel

+31-16
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,16 @@
44
# found in the LICENSE file at https://angular.dev/license
55

66
load("@npm//@angular/build-tooling/bazel/api-golden:index.bzl", "api_golden_test_npm_package")
7-
load("@npm//@bazel/jasmine:index.bzl", "jasmine_node_test")
8-
load("//tools:defaults2.bzl", "npm_package", "ts_project")
7+
load("@npm2//:defs.bzl", "npm_link_all_packages")
8+
load("//tools:defaults2.bzl", "jasmine_test", "npm_package", "ts_project")
99
load("//tools:ts_json_schema.bzl", "ts_json_schema")
1010

1111
licenses(["notice"])
1212

1313
package(default_visibility = ["//visibility:public"])
1414

15+
npm_link_all_packages()
16+
1517
ts_json_schema(
1618
name = "app_shell_schema",
1719
src = "src/builders/app-shell/schema.json",
@@ -122,6 +124,12 @@ ts_project(
122124
data = RUNTIME_ASSETS,
123125
module_name = "@angular-devkit/build-angular",
124126
deps = [
127+
":node_modules/@angular-devkit/architect",
128+
":node_modules/@angular-devkit/build-webpack",
129+
":node_modules/@angular-devkit/core",
130+
":node_modules/@angular/build",
131+
":node_modules/@angular/ssr",
132+
":node_modules/@ngtools/webpack",
125133
"//:node_modules/@ampproject/remapping",
126134
"//:node_modules/@angular/common",
127135
"//:node_modules/@angular/compiler-cli",
@@ -193,14 +201,6 @@ ts_project(
193201
"//:node_modules/webpack-dev-server",
194202
"//:node_modules/webpack-merge",
195203
"//:node_modules/webpack-subresource-integrity",
196-
"//packages/angular/build:build_rjs",
197-
"//packages/angular/build/private:private_rjs",
198-
"//packages/angular/ssr:ssr_rjs",
199-
"//packages/angular_devkit/architect:architect_rjs",
200-
"//packages/angular_devkit/build_webpack:build_webpack_rjs",
201-
"//packages/angular_devkit/core:core_rjs",
202-
"//packages/angular_devkit/core/node:node_rjs",
203-
"//packages/ngtools/webpack:webpack_rjs",
204204
],
205205
)
206206

@@ -215,7 +215,9 @@ ts_project(
215215
"src/builders/**/*_spec.ts",
216216
],
217217
),
218-
data = glob(["test/**/*"]),
218+
data = [
219+
"//packages/angular_devkit/build_angular/test/hello-world-lib",
220+
],
219221
deps = [
220222
":build_angular_rjs",
221223
":build_angular_test_utils_rjs",
@@ -228,9 +230,9 @@ ts_project(
228230
],
229231
)
230232

231-
jasmine_node_test(
233+
jasmine_test(
232234
name = "build_angular_test",
233-
srcs = [":build_angular_test_lib"],
235+
data = [":build_angular_test_lib_rjs"],
234236
)
235237

236238
genrule(
@@ -284,7 +286,9 @@ ts_project(
284286
"src/**/*_spec.ts",
285287
],
286288
),
287-
data = glob(["test/**/*"]),
289+
data = [
290+
"//packages/angular_devkit/build_angular/test/hello-world-lib",
291+
],
288292
deps = [
289293
":build_angular_rjs",
290294
"//:node_modules/@types/jasmine",
@@ -408,9 +412,21 @@ LARGE_SPECS = {
408412
]
409413

410414
[
411-
jasmine_node_test(
415+
jasmine_test(
412416
name = "build_angular_" + spec + "_test",
413417
size = LARGE_SPECS[spec].get("size", "medium"),
418+
data = [
419+
":build_angular_" + spec + "_test_lib_rjs",
420+
# Helpers for `testing/builder` rely on the npm artifact, so we'll make
421+
# sure it's available during this test. Notably that the package needs to
422+
# available as a parent of `modules/testing/builder` for resolution to work!
423+
"//modules/testing/builder:node_modules/@angular-devkit/build-angular",
424+
],
425+
env = {
426+
# TODO: Replace Puppeteer downloaded browsers with Bazel-managed browsers,
427+
# or standardize to avoid complex configuration like this!
428+
"PUPPETEER_DOWNLOAD_PATH": "../../../node_modules/puppeteer/downloads",
429+
},
414430
flaky = LARGE_SPECS[spec].get("flaky", False),
415431
shard_count = LARGE_SPECS[spec].get("shards", 2),
416432
# These tests are resource intensive and should not be over-parallized as they will
@@ -419,7 +435,6 @@ LARGE_SPECS = {
419435
tags = [
420436
"cpu:2",
421437
] + LARGE_SPECS[spec].get("tags", []),
422-
deps = [":build_angular_" + spec + "_test_lib"],
423438
)
424439
for spec in LARGE_SPECS
425440
]

‎packages/angular_devkit/build_angular/package.json

+7-6
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@
77
"builders": "builders.json",
88
"dependencies": {
99
"@ampproject/remapping": "2.3.0",
10-
"@angular-devkit/architect": "0.0.0-EXPERIMENTAL-PLACEHOLDER",
11-
"@angular-devkit/build-webpack": "0.0.0-EXPERIMENTAL-PLACEHOLDER",
12-
"@angular-devkit/core": "0.0.0-PLACEHOLDER",
13-
"@angular/build": "0.0.0-PLACEHOLDER",
10+
"@angular-devkit/architect": "workspace:0.0.0-EXPERIMENTAL-PLACEHOLDER",
11+
"@angular-devkit/build-webpack": "workspace:0.0.0-EXPERIMENTAL-PLACEHOLDER",
12+
"@angular-devkit/core": "workspace:0.0.0-PLACEHOLDER",
13+
"@angular/build": "workspace:0.0.0-PLACEHOLDER",
1414
"@babel/core": "7.26.7",
1515
"@babel/generator": "7.26.5",
1616
"@babel/helper-annotate-as-pure": "7.25.9",
@@ -21,7 +21,7 @@
2121
"@babel/preset-env": "7.26.7",
2222
"@babel/runtime": "7.26.7",
2323
"@discoveryjs/json-ext": "0.6.3",
24-
"@ngtools/webpack": "0.0.0-PLACEHOLDER",
24+
"@ngtools/webpack": "workspace:0.0.0-PLACEHOLDER",
2525
"@vitejs/plugin-basic-ssl": "1.2.0",
2626
"ansi-colors": "4.1.3",
2727
"autoprefixer": "10.4.20",
@@ -66,7 +66,8 @@
6666
"esbuild": "0.24.2"
6767
},
6868
"devDependencies": {
69-
"undici": "7.3.0"
69+
"undici": "7.3.0",
70+
"@angular/ssr": "workspace:*"
7071
},
7172
"peerDependencies": {
7273
"@angular/compiler-cli": "0.0.0-ANGULAR-FW-PEER-DEP",

‎packages/angular_devkit/build_angular/src/builders/browser/specs/cross-origin_spec.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,11 @@
77
*/
88

99
import { Architect } from '@angular-devkit/architect';
10-
import { BrowserBuilderOutput, CrossOrigin } from '@angular-devkit/build-angular';
1110
import { join, normalize, virtualFs } from '@angular-devkit/core';
1211
import { lastValueFrom } from 'rxjs';
1312
import { createArchitect, host } from '../../../testing/test-utils';
13+
import { BrowserBuilderOutput } from '../index';
14+
import { CrossOrigin } from '../schema';
1415

1516
describe('Browser Builder crossOrigin', () => {
1617
const targetSpec = { project: 'app', target: 'build' };

‎packages/angular_devkit/build_angular/src/builders/browser/specs/lazy-module_spec.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,12 @@ describe('Browser Builder lazy modules', () => {
149149
'src/main.ts': `import('./one'); import('./two');`,
150150
});
151151

152-
const { files } = await browserBuild(architect, host, target);
152+
const { files } = await browserBuild(architect, host, target, {
153+
// Preserve symlinks to reliably verify the chunk names. When symlinks
154+
// would be dereferenced, the `@angular/common` file can originate from a
155+
// less predictable path in e.g. node_modules/.pnpm/<...>`.
156+
preserveSymlinks: true,
157+
});
153158
expect(files['src_one_ts.js']).toBeDefined();
154159
expect(files['src_two_ts.js']).toBeDefined();
155160
expect(files['default-node_modules_angular_common_fesm2022_http_mjs.js']).toBeDefined();

‎packages/angular_devkit/build_angular/src/builders/browser/specs/source-map_spec.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
*/
88

99
import { Architect } from '@angular-devkit/architect';
10-
import { OutputHashing } from '@angular-devkit/build-angular';
1110
import { browserBuild, createArchitect, host } from '../../../testing/test-utils';
11+
import { OutputHashing } from '../schema';
1212

1313
describe('Browser Builder source map', () => {
1414
const target = { project: 'app', target: 'build' };

‎packages/angular_devkit/build_angular/src/builders/browser/tests/options/named-chunks_spec.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup';
1111

1212
const MAIN_OUTPUT = 'dist/main.js';
1313
const NAMED_LAZY_OUTPUT = 'dist/src_lazy-module_ts.js';
14-
const UNNAMED_LAZY_OUTPUT = 'dist/358.js';
14+
const UNNAMED_LAZY_OUTPUT = 'dist/321.js';
1515

1616
describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
1717
describe('Option: "namedChunks"', () => {

‎packages/angular_devkit/build_angular/src/builders/browser/tests/options/styles_spec.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
143143
expect(result?.success).toBe(true);
144144

145145
expect(logs).toContain(
146-
jasmine.objectContaining({ message: jasmine.stringMatching(/styles\.css.+\d+ bytes/) }),
146+
jasmine.objectContaining({ message: jasmine.stringMatching(/styles\.css.+\d+ kB/) }),
147147
);
148148
});
149149
});
@@ -410,7 +410,7 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
410410
expect(result?.success).toBe(true);
411411

412412
expect(logs).toContain(
413-
jasmine.objectContaining({ message: jasmine.stringMatching(/styles\.css.+\d+ bytes/) }),
413+
jasmine.objectContaining({ message: jasmine.stringMatching(/styles\.css.+\d+ kB/) }),
414414
);
415415
});
416416

@@ -427,7 +427,7 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
427427
expect(result?.success).toBe(true);
428428

429429
expect(logs).toContain(
430-
jasmine.objectContaining({ message: jasmine.stringMatching(/extra\.css.+\d+ bytes/) }),
430+
jasmine.objectContaining({ message: jasmine.stringMatching(/extra\.css.+\d+ kB/) }),
431431
);
432432
});
433433
});

‎packages/angular_devkit/build_angular/test/hello-world-lib/BUILD.bazel

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
load("@aspect_bazel_lib//lib:copy_to_bin.bzl", "copy_to_bin")
2+
load("@aspect_rules_js//npm:defs.bzl", "npm_link_package")
3+
4+
# Note: Link the package into node modules for testing. Notably, tests
5+
# of a package generally don't use the associated npm package, to e.g. allow for relative
6+
# imports, but here this is an exception as the package needs to be resolvable at runtime
7+
# to replicate a CLI environment.
8+
npm_link_package(
9+
name = "node_modules/@angular-devkit/build-angular",
10+
src = "//packages/angular_devkit/build_angular:pkg",
11+
package = "@angular-devkit/build-angular",
12+
root_package = package_name(),
13+
)
14+
15+
copy_to_bin(
16+
name = "hello-world-lib",
17+
srcs = glob(["**/*"]) + [
18+
":node_modules/@angular-devkit/build-angular",
19+
],
20+
visibility = ["//packages/angular_devkit/build_angular:__pkg__"],
21+
)

0 commit comments

Comments
 (0)