Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[19.1.x]: build: migrate @angular-devkit/build-angular tests to rules_js #29515

Merged
merged 1 commit into from
Jan 29, 2025
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -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
14 changes: 14 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion modules/testing/builder/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"devDependencies": {
"@angular-devkit/core": "workspace:*",
"@angular-devkit/architect": "workspace:*",
"@angular/ssr": "workspace:*"
"@angular/ssr": "workspace:*",
"@angular-devkit/build-angular": "workspace:*"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"compilerOptions": {
"outDir": "../dist-server",
"baseUrl": "./",
"types": ["@angular/localize"]
"types": ["@angular/localize", "node"]
},
"files": [
"main.server.ts"
Expand Down
8 changes: 8 additions & 0 deletions modules/testing/builder/src/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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),
Expand Down
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,10 @@
}
},
"pnpm": {
"onlyBuiltDependencies": [],
"onlyBuiltDependencies": [
"puppeteer",
"webdriver-manager"
],
"overrides": {
"@angular/build": "workspace:*"
}
Expand Down
10 changes: 3 additions & 7 deletions packages/angular/build/src/utils/service-worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -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);
Expand Down
47 changes: 31 additions & 16 deletions packages/angular_devkit/build_angular/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
],
)

Expand All @@ -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",
Expand All @@ -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(
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand All @@ -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
]
13 changes: 7 additions & 6 deletions packages/angular_devkit/build_angular/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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/) }),
);
});
});
Expand Down Expand Up @@ -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/) }),
);
});

Expand All @@ -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/) }),
);
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -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__"],
)
Loading
Loading