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/[email protected]_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/[email protected]_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.
angular · Jan 29, 2025 · 7532aca · 7532aca
1 parent bb41345
commit 7532aca
Show file tree

Hide file tree

Showing 17 changed files with 299 additions and 428 deletions.
diff --git a/.aspect/rules/external_repository_action_cache/npm_translate_lock_MzA5NzUwNzMx b/.aspect/rules/external_repository_action_cache/npm_translate_lock_MzA5NzUwNzMx
@@ -2,21 +2,21 @@
 # Input hashes for repository rule npm_translate_lock(name = "npm2", pnpm_lock = "@//:pnpm-lock.yaml").
 # This file should be checked into version control along with the pnpm-lock.yaml file.
 .npmrc=-1406867100
-modules/testing/builder/package.json=-1196120648
-package.json=-865553716
+modules/testing/builder/package.json=973445093
+package.json=-1236730584
 packages/angular/build/package.json=954937711
 packages/angular/cli/package.json=349838588
 packages/angular/pwa/package.json=-1352285148
 packages/angular/ssr/package.json=120782115
 packages/angular_devkit/architect/package.json=-1496633956
 packages/angular_devkit/architect_cli/package.json=1551210941
-packages/angular_devkit/build_angular/package.json=-1437596637
+packages/angular_devkit/build_angular/package.json=1670359618
 packages/angular_devkit/build_webpack/package.json=373950017
 packages/angular_devkit/core/package.json=339935828
 packages/angular_devkit/schematics/package.json=673943597
 packages/angular_devkit/schematics_cli/package.json=-356386813
 packages/ngtools/webpack/package.json=-942726894
 packages/schematics/angular/package.json=251715148
-pnpm-lock.yaml=1087719969
-pnpm-workspace.yaml=-1847919625
+pnpm-lock.yaml=-657686225
+pnpm-workspace.yaml=-1056556036
 yarn.lock=969972397
diff --git a/WORKSPACE b/WORKSPACE
@@ -183,6 +183,10 @@ load("@aspect_rules_js//npm:repositories.bzl", "npm_translate_lock")
 
 npm_translate_lock(
     name = "npm2",
+    custom_postinstalls = {
+        # TODO: Standardize browser management for `rules_js`
+        "webdriver-manager": "node ./bin/webdriver-manager update --standalone false --gecko false --versions.chrome 106.0.5249.21",
+    },
     data = [
         "//:package.json",
         "//:pnpm-workspace.yaml",
@@ -201,6 +205,16 @@ npm_translate_lock(
         "//packages/ngtools/webpack:package.json",
         "//packages/schematics/angular:package.json",
     ],
+    lifecycle_hooks_envs = {
+        # TODO: Standardize browser management for `rules_js`
+        "puppeteer": ["PUPPETEER_DOWNLOAD_PATH=./downloads"],
+    },
+    lifecycle_hooks_execution_requirements = {
+        # Needed for downloading chromedriver.
+        # Also `update-config` of webdriver manager would store an absolute path;
+        # which would then break execution.
+        "webdriver-manager": ["local"],
+    },
     npmrc = "//:.npmrc",
     patches = {
         # Note: Patches not needed as the existing patches are only

diff --git a/modules/testing/builder/package.json b/modules/testing/builder/package.json
@@ -2,6 +2,7 @@
   "devDependencies": {
     "@angular-devkit/core": "workspace:*",
     "@angular-devkit/architect": "workspace:*",
-    "@angular/ssr": "workspace:*"
+    "@angular/ssr": "workspace:*",
+    "@angular-devkit/build-angular": "workspace:*"
   }
 }
diff --git a/modules/testing/builder/projects/hello-world-app/src/tsconfig.server.json b/modules/testing/builder/projects/hello-world-app/src/tsconfig.server.json
@@ -3,7 +3,7 @@
   "compilerOptions": {
     "outDir": "../dist-server",
     "baseUrl": "./",
-    "types": ["@angular/localize"]
+    "types": ["@angular/localize", "node"]
   },
   "files": [
     "main.server.ts"

diff --git a/modules/testing/builder/src/test-utils.ts b/modules/testing/builder/src/test-utils.ts
@@ -21,6 +21,7 @@ import {
   virtualFs,
   workspaces,
 } from '@angular-devkit/core';
+import path from 'node:path';
 import { firstValueFrom } from 'rxjs';
 
 // Default timeout for large specs is 2.5 minutes.
@@ -42,6 +43,13 @@ export async function createArchitect(workspaceRoot: Path) {
   registry.addPostTransform(schema.transforms.addUndefinedDefaults);
   const workspaceSysPath = getSystemPath(workspaceRoot);
 
+  // The download path is relative (set from Starlark), so before potentially
+  // changing directories, or executing inside a temporary directory, ensure
+  // the path is absolute.
+  if (process.env['PUPPETEER_DOWNLOAD_PATH']) {
+    process.env.PUPPETEER_DOWNLOAD_PATH = path.resolve(process.env['PUPPETEER_DOWNLOAD_PATH']);
+  }
+
   const { workspace } = await workspaces.readWorkspace(
     workspaceSysPath,
     workspaces.createWorkspaceHost(host),

diff --git a/package.json b/package.json
@@ -221,7 +221,10 @@
     }
   },
   "pnpm": {
-    "onlyBuiltDependencies": [],
+    "onlyBuiltDependencies": [
+      "puppeteer",
+      "webdriver-manager"
+    ],
     "overrides": {
       "@angular/build": "workspace:*"
     }

diff --git a/packages/angular/build/src/utils/service-worker.ts b/packages/angular/build/src/utils/service-worker.ts
@@ -126,7 +126,7 @@ export async function augmentAppWithServiceWorker(
   outputPath: string,
   baseHref: string,
   ngswConfigPath?: string,
-  inputputFileSystem = fsPromises,
+  inputFileSystem = fsPromises,
   outputFileSystem = fsPromises,
 ): Promise<void> {
   // Determine the configuration file path
@@ -137,7 +137,7 @@ export async function augmentAppWithServiceWorker(
   // Read the configuration file
   let config: Config | undefined;
   try {
-    const configurationData = await inputputFileSystem.readFile(configPath, 'utf-8');
+    const configurationData = await inputFileSystem.readFile(configPath, 'utf-8');
     config = JSON.parse(configurationData) as Config;
   } catch (error) {
     assertIsError(error);
@@ -161,11 +161,7 @@ export async function augmentAppWithServiceWorker(
   const copy = async (src: string, dest: string): Promise<void> => {
     const resolvedDest = path.join(outputPath, dest);
 
-    return inputputFileSystem === outputFileSystem
-      ? // Native FS (Builder).
-        inputputFileSystem.copyFile(src, resolvedDest, fsConstants.COPYFILE_FICLONE)
-      : // memfs (Webpack): Read the file from the input FS (disk) and write it to the output FS (memory).
-        outputFileSystem.writeFile(resolvedDest, await inputputFileSystem.readFile(src));
+    return outputFileSystem.writeFile(resolvedDest, await inputFileSystem.readFile(src));
   };
 
   await outputFileSystem.writeFile(path.join(outputPath, 'ngsw.json'), result.manifest);

diff --git a/packages/angular_devkit/build_angular/BUILD.bazel b/packages/angular_devkit/build_angular/BUILD.bazel
@@ -4,14 +4,16 @@
 # found in the LICENSE file at https://angular.dev/license
 
 load("@npm//@angular/build-tooling/bazel/api-golden:index.bzl", "api_golden_test_npm_package")
-load("@npm//@bazel/jasmine:index.bzl", "jasmine_node_test")
-load("//tools:defaults2.bzl", "npm_package", "ts_project")
+load("@npm2//:defs.bzl", "npm_link_all_packages")
+load("//tools:defaults2.bzl", "jasmine_test", "npm_package", "ts_project")
 load("//tools:ts_json_schema.bzl", "ts_json_schema")
 
 licenses(["notice"])
 
 package(default_visibility = ["//visibility:public"])
 
+npm_link_all_packages()
+
 ts_json_schema(
     name = "app_shell_schema",
     src = "src/builders/app-shell/schema.json",
@@ -122,6 +124,12 @@ ts_project(
     data = RUNTIME_ASSETS,
     module_name = "@angular-devkit/build-angular",
     deps = [
+        ":node_modules/@angular-devkit/architect",
+        ":node_modules/@angular-devkit/build-webpack",
+        ":node_modules/@angular-devkit/core",
+        ":node_modules/@angular/build",
+        ":node_modules/@angular/ssr",
+        ":node_modules/@ngtools/webpack",
         "//:node_modules/@ampproject/remapping",
         "//:node_modules/@angular/common",
         "//:node_modules/@angular/compiler-cli",
@@ -193,14 +201,6 @@ ts_project(
         "//:node_modules/webpack-dev-server",
         "//:node_modules/webpack-merge",
         "//:node_modules/webpack-subresource-integrity",
-        "//packages/angular/build:build_rjs",
-        "//packages/angular/build/private:private_rjs",
-        "//packages/angular/ssr:ssr_rjs",
-        "//packages/angular_devkit/architect:architect_rjs",
-        "//packages/angular_devkit/build_webpack:build_webpack_rjs",
-        "//packages/angular_devkit/core:core_rjs",
-        "//packages/angular_devkit/core/node:node_rjs",
-        "//packages/ngtools/webpack:webpack_rjs",
     ],
 )
 
@@ -215,7 +215,9 @@ ts_project(
             "src/builders/**/*_spec.ts",
         ],
     ),
-    data = glob(["test/**/*"]),
+    data = [
+        "//packages/angular_devkit/build_angular/test/hello-world-lib",
+    ],
     deps = [
         ":build_angular_rjs",
         ":build_angular_test_utils_rjs",
@@ -228,9 +230,9 @@ ts_project(
     ],
 )
 
-jasmine_node_test(
+jasmine_test(
     name = "build_angular_test",
-    srcs = [":build_angular_test_lib"],
+    data = [":build_angular_test_lib_rjs"],
 )
 
 genrule(
@@ -284,7 +286,9 @@ ts_project(
             "src/**/*_spec.ts",
         ],
     ),
-    data = glob(["test/**/*"]),
+    data = [
+        "//packages/angular_devkit/build_angular/test/hello-world-lib",
+    ],
     deps = [
         ":build_angular_rjs",
         "//:node_modules/@types/jasmine",
@@ -408,9 +412,21 @@ LARGE_SPECS = {
 ]
 
 [
-    jasmine_node_test(
+    jasmine_test(
         name = "build_angular_" + spec + "_test",
         size = LARGE_SPECS[spec].get("size", "medium"),
+        data = [
+            ":build_angular_" + spec + "_test_lib_rjs",
+            # Helpers for `testing/builder` rely on the npm artifact, so we'll make
+            # sure it's available during this test. Notably that the package needs to
+            # available as a parent of `modules/testing/builder` for resolution to work!
+            "//modules/testing/builder:node_modules/@angular-devkit/build-angular",
+        ],
+        env = {
+            # TODO: Replace Puppeteer downloaded browsers with Bazel-managed browsers,
+            # or standardize to avoid complex configuration like this!
+            "PUPPETEER_DOWNLOAD_PATH": "../../../node_modules/puppeteer/downloads",
+        },
         flaky = LARGE_SPECS[spec].get("flaky", False),
         shard_count = LARGE_SPECS[spec].get("shards", 2),
         # These tests are resource intensive and should not be over-parallized as they will
@@ -419,7 +435,6 @@ LARGE_SPECS = {
         tags = [
             "cpu:2",
         ] + LARGE_SPECS[spec].get("tags", []),
-        deps = [":build_angular_" + spec + "_test_lib"],
     )
     for spec in LARGE_SPECS
 ]
diff --git a/packages/angular_devkit/build_angular/package.json b/packages/angular_devkit/build_angular/package.json
@@ -7,10 +7,10 @@
   "builders": "builders.json",
   "dependencies": {
     "@ampproject/remapping": "2.3.0",
-    "@angular-devkit/architect": "0.0.0-EXPERIMENTAL-PLACEHOLDER",
-    "@angular-devkit/build-webpack": "0.0.0-EXPERIMENTAL-PLACEHOLDER",
-    "@angular-devkit/core": "0.0.0-PLACEHOLDER",
-    "@angular/build": "0.0.0-PLACEHOLDER",
+    "@angular-devkit/architect": "workspace:0.0.0-EXPERIMENTAL-PLACEHOLDER",
+    "@angular-devkit/build-webpack": "workspace:0.0.0-EXPERIMENTAL-PLACEHOLDER",
+    "@angular-devkit/core": "workspace:0.0.0-PLACEHOLDER",
+    "@angular/build": "workspace:0.0.0-PLACEHOLDER",
     "@babel/core": "7.26.0",
     "@babel/generator": "7.26.3",
     "@babel/helper-annotate-as-pure": "7.25.9",
@@ -21,7 +21,7 @@
     "@babel/preset-env": "7.26.0",
     "@babel/runtime": "7.26.0",
     "@discoveryjs/json-ext": "0.6.3",
-    "@ngtools/webpack": "0.0.0-PLACEHOLDER",
+    "@ngtools/webpack": "workspace:0.0.0-PLACEHOLDER",
     "@vitejs/plugin-basic-ssl": "1.2.0",
     "ansi-colors": "4.1.3",
     "autoprefixer": "10.4.20",
@@ -66,7 +66,8 @@
     "esbuild": "0.24.2"
   },
   "devDependencies": {
-    "undici": "7.2.0"
+    "undici": "7.2.0",
+    "@angular/ssr": "workspace:*"
   },
   "peerDependencies": {
     "@angular/compiler-cli": "^19.0.0",

diff --git a/packages/angular_devkit/build_angular/src/builders/browser/specs/cross-origin_spec.ts b/packages/angular_devkit/build_angular/src/builders/browser/specs/cross-origin_spec.ts
@@ -7,10 +7,11 @@
  */
 
 import { Architect } from '@angular-devkit/architect';
-import { BrowserBuilderOutput, CrossOrigin } from '@angular-devkit/build-angular';
 import { join, normalize, virtualFs } from '@angular-devkit/core';
 import { lastValueFrom } from 'rxjs';
 import { createArchitect, host } from '../../../testing/test-utils';
+import { BrowserBuilderOutput } from '../index';
+import { CrossOrigin } from '../schema';
 
 describe('Browser Builder crossOrigin', () => {
   const targetSpec = { project: 'app', target: 'build' };

diff --git a/packages/angular_devkit/build_angular/src/builders/browser/specs/lazy-module_spec.ts b/packages/angular_devkit/build_angular/src/builders/browser/specs/lazy-module_spec.ts
@@ -149,7 +149,12 @@ describe('Browser Builder lazy modules', () => {
       'src/main.ts': `import('./one'); import('./two');`,
     });
 
-    const { files } = await browserBuild(architect, host, target);
+    const { files } = await browserBuild(architect, host, target, {
+      // Preserve symlinks to reliably verify the chunk names. When symlinks
+      // would be dereferenced, the `@angular/common` file can originate from a
+      // less predictable path in e.g. node_modules/.pnpm/<...>`.
+      preserveSymlinks: true,
+    });
     expect(files['src_one_ts.js']).toBeDefined();
     expect(files['src_two_ts.js']).toBeDefined();
     expect(files['default-node_modules_angular_common_fesm2022_http_mjs.js']).toBeDefined();

diff --git a/packages/angular_devkit/build_angular/src/builders/browser/specs/source-map_spec.ts b/packages/angular_devkit/build_angular/src/builders/browser/specs/source-map_spec.ts
@@ -7,8 +7,8 @@
  */
 
 import { Architect } from '@angular-devkit/architect';
-import { OutputHashing } from '@angular-devkit/build-angular';
 import { browserBuild, createArchitect, host } from '../../../testing/test-utils';
+import { OutputHashing } from '../schema';
 
 describe('Browser Builder source map', () => {
   const target = { project: 'app', target: 'build' };

diff --git a/...ages/angular_devkit/build_angular/src/builders/browser/tests/options/named-chunks_spec.ts b/...ages/angular_devkit/build_angular/src/builders/browser/tests/options/named-chunks_spec.ts
@@ -11,7 +11,7 @@ import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup';
 
 const MAIN_OUTPUT = 'dist/main.js';
 const NAMED_LAZY_OUTPUT = 'dist/src_lazy-module_ts.js';
-const UNNAMED_LAZY_OUTPUT = 'dist/358.js';
+const UNNAMED_LAZY_OUTPUT = 'dist/321.js';
 
 describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
   describe('Option: "namedChunks"', () => {

diff --git a/packages/angular_devkit/build_angular/src/builders/browser/tests/options/styles_spec.ts b/packages/angular_devkit/build_angular/src/builders/browser/tests/options/styles_spec.ts
@@ -143,7 +143,7 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
         expect(result?.success).toBe(true);
 
         expect(logs).toContain(
-          jasmine.objectContaining({ message: jasmine.stringMatching(/styles\.css.+\d+ bytes/) }),
+          jasmine.objectContaining({ message: jasmine.stringMatching(/styles\.css.+\d+ kB/) }),
         );
       });
     });
@@ -410,7 +410,7 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
         expect(result?.success).toBe(true);
 
         expect(logs).toContain(
-          jasmine.objectContaining({ message: jasmine.stringMatching(/styles\.css.+\d+ bytes/) }),
+          jasmine.objectContaining({ message: jasmine.stringMatching(/styles\.css.+\d+ kB/) }),
         );
       });
 
@@ -427,7 +427,7 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
         expect(result?.success).toBe(true);
 
         expect(logs).toContain(
-          jasmine.objectContaining({ message: jasmine.stringMatching(/extra\.css.+\d+ bytes/) }),
+          jasmine.objectContaining({ message: jasmine.stringMatching(/extra\.css.+\d+ kB/) }),
         );
       });
     });

diff --git a/packages/angular_devkit/build_angular/test/hello-world-lib/BUILD.bazel b/packages/angular_devkit/build_angular/test/hello-world-lib/BUILD.bazel
@@ -0,0 +1,21 @@
+load("@aspect_bazel_lib//lib:copy_to_bin.bzl", "copy_to_bin")
+load("@aspect_rules_js//npm:defs.bzl", "npm_link_package")
+
+# Note: Link the package into node modules for testing. Notably, tests
+# of a package generally don't use the associated npm package, to e.g. allow for relative
+# imports, but here this is an exception as the package needs to be resolvable at runtime
+# to replicate a CLI environment.
+npm_link_package(
+    name = "node_modules/@angular-devkit/build-angular",
+    src = "//packages/angular_devkit/build_angular:pkg",
+    package = "@angular-devkit/build-angular",
+    root_package = package_name(),
+)
+
+copy_to_bin(
+    name = "hello-world-lib",
+    srcs = glob(["**/*"]) + [
+        ":node_modules/@angular-devkit/build-angular",
+    ],
+    visibility = ["//packages/angular_devkit/build_angular:__pkg__"],
+)