Skip to content

Commit 29d4705

Browse files
committed
refactor(bazel): update integration test rule to use rules_js
Technically this wouldn't be necessary for `rules_js` as the rule is pretty agnostic, but it means we don't end up hitting the `@npm` workspace, or more `rules_nodejs` dependencies— so it's part of our migration. Also this commit simplifies some logic around the temporary directory creation to spare an extra dependency. We also remove some unnecessary Windows logic. Finally, we fix runfile resolution to work with `rules_js`, without relying on any runfile helpes, as we can expect a runfiles directory with proper real files.
1 parent 77b2017 commit 29d4705

15 files changed

+74
-100
lines changed

bazel/BUILD.bazel

-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ filegroup(
2727
"//bazel/esbuild:files",
2828
"//bazel/git-toolchain:files",
2929
"//bazel/http-server:files",
30-
"//bazel/integration:files",
3130
"//bazel/karma:files",
3231
"//bazel/map-size-tracking:files",
3332
"//bazel/private:files",

bazel/integration/BUILD.bazel

-9
Original file line numberDiff line numberDiff line change
@@ -1,9 +0,0 @@
1-
package(default_visibility = ["//visibility:public"])
2-
3-
# Make source files available for distribution via pkg_npm
4-
filegroup(
5-
name = "files",
6-
srcs = glob(["*"]) + [
7-
"//bazel/integration/test_runner:files",
8-
],
9-
)

bazel/integration/index.bzl

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_test")
1+
load("@aspect_rules_js//js:defs.bzl", "js_test")
22

33
def _serialize_file(file):
44
"""Serializes a file into a struct that matches the `BazelFileInfo` type in the
@@ -259,11 +259,11 @@ def integration_test(
259259
toolchains = toolchains,
260260
)
261261

262-
nodejs_test(
262+
js_test(
263263
name = name,
264-
data = ["//bazel/integration/test_runner", ":" + config_target],
265-
templated_args = ["--nobazel_run_linker", "$(rootpath :%s)" % config_target],
266-
entry_point = "//bazel/integration/test_runner:main.ts",
264+
data = ["@devinfra//bazel/integration/test_runner", ":%s" % config_target],
265+
fixed_args = ["$(rootpath :%s)" % config_target],
266+
entry_point = "@devinfra//bazel/integration/test_runner:main.mjs",
267267
tags = tags,
268268
**kwargs
269269
)
+6-18
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,13 @@
1-
load("//bazel:defaults.bzl", "ts_library")
1+
load("@aspect_rules_ts//ts:defs.bzl", "ts_project")
22

33
package(default_visibility = ["//visibility:public"])
44

5-
ts_library(
5+
ts_project(
66
name = "test_runner",
7-
srcs = glob(["*.ts"]),
8-
module_name = "@angular/build-tooling/bazel/integration/test_runner",
9-
# A tsconfig needs to be specified as otherwise `ts_library` will look for the config
10-
# in `//:package.json` and this breaks when the BUILD file is copied to `@npm//`.
11-
tsconfig = "//:tsconfig.json",
7+
srcs = glob(["*.mts"]),
8+
tsconfig = "//bazel:tsconfig",
129
deps = [
13-
"@npm//@bazel/runfiles",
14-
"@npm//@types/node",
15-
"@npm//@types/tmp",
16-
"@npm//tmp",
17-
"@npm//true-case-path",
10+
"//bazel:node_modules/@types/node",
11+
"//bazel:node_modules/true-case-path",
1812
],
1913
)
20-
21-
# Make source files available for distribution via pkg_npm
22-
filegroup(
23-
name = "files",
24-
srcs = glob(["*"]),
25-
)

bazel/integration/test_runner/bazel.ts renamed to bazel/integration/test_runner/bazel.mts

+8-8
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,9 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {debug} from './debug';
10-
import {runfiles} from '@bazel/runfiles';
11-
12-
// Exposing the runfiles to keep the Bazel-specific code local to this file.
13-
export {runfiles};
9+
import path from 'node:path';
10+
import assert from 'node:assert';
11+
import {debug} from './debug.mjs';
1412

1513
/**
1614
* Interface describing a file captured in the Bazel action.
@@ -38,8 +36,9 @@ export interface BazelExpandedValue {
3836
}
3937

4038
/** Resolves the specified Bazel file to an absolute disk path. */
41-
export function resolveBazelFile(file: BazelFileInfo): string {
42-
return runfiles.resolveWorkspaceRelative(file.shortPath);
39+
export function resolveBazelFile(file: Pick<BazelFileInfo, 'shortPath'>): string {
40+
// CWD is runfiles root inside workspace. All short paths naturally work.
41+
return path.join(process.cwd(), file.shortPath);
4342
}
4443

4544
/**
@@ -53,7 +52,8 @@ export function resolveBazelFile(file: BazelFileInfo): string {
5352
*/
5453
export async function resolveBinaryWithRunfilesGracefully(binary: string): Promise<string> {
5554
try {
56-
const resolved = runfiles.resolveWorkspaceRelative(binary);
55+
// CWD is runfiles root inside workspace. All short paths naturally work.
56+
const resolved = path.join(process.cwd(), binary);
5757
debug(`Resolved ${binary} to ${resolved} using runfile resolution.`);
5858
return resolved;
5959
} catch {

bazel/integration/test_runner/debug.ts renamed to bazel/integration/test_runner/debug.mts

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import * as util from 'util';
9+
import util from 'node:util';
1010

1111
/** Function for printing debug messages when working with the test runner. */
1212
export const debug = util.debuglog('ng-integration-test');

bazel/integration/test_runner/file_system_utils.ts renamed to bazel/integration/test_runner/file_system_utils.mts

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import * as fs from 'fs';
9+
import fs from 'node:fs';
1010
import {trueCasePath} from 'true-case-path';
1111

1212
/** Gets whether the file is executable or not. */

bazel/integration/test_runner/main.ts renamed to bazel/integration/test_runner/main.mts

+7-5
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,11 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {TestRunner} from './runner';
10-
import {BazelExpandedValue, BazelFileInfo, runfiles} from './bazel';
11-
import * as fs from 'fs';
12-
import {debug} from './debug';
9+
import {TestRunner} from './runner.mjs';
10+
import {BazelExpandedValue, BazelFileInfo} from './bazel.mjs';
11+
import fs from 'node:fs';
12+
import path from 'node:path';
13+
import {debug} from './debug.mjs';
1314

1415
/**
1516
* Test config that is passed as JSON from the Bazel action.
@@ -30,7 +31,8 @@ async function main(): Promise<void> {
3031
debug(`Running test with arguments: ${process.argv.join(' ')}`);
3132
debug(`Current working directory: ${process.cwd()}`);
3233

33-
const configPath = runfiles.resolveWorkspaceRelative(process.argv[2]);
34+
// Config is passed as short path, relative to current working dir.
35+
const configPath = path.resolve(process.argv[2]);
3436
const configContent = await fs.promises.readFile(configPath, 'utf8');
3537
const config = JSON.parse(configContent) as TestConfig;
3638

bazel/integration/test_runner/package_json.ts renamed to bazel/integration/test_runner/package_json.mts

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

99
import * as fs from 'fs';
10-
import {debug} from './debug';
10+
import {debug} from './debug.mjs';
1111

1212
/** Record capturing dependencies in a `package.json`. */
1313
export type DependencyRecord = Record<string, string>;

bazel/integration/test_runner/process_utils.ts renamed to bazel/integration/test_runner/process_utils.mts

+5-6
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import * as childProcess from 'child_process';
10-
import * as path from 'path';
11-
import {debug} from './debug';
12-
import {getCaseExactRealpath} from './file_system_utils';
9+
import childProcess from 'node:child_process';
10+
import path from 'node:path';
11+
import {debug} from './debug.mjs';
12+
import {getCaseExactRealpath} from './file_system_utils.mjs';
1313

1414
/**
1515
* Regular expression matching environment variable substitutions
@@ -81,7 +81,6 @@ export function prependToPathVariable(pathToAdd: string, existingPathVar: string
8181
*/
8282
export function getBinaryPassThroughScript(binaryFilePath: string) {
8383
return {
84-
cmd: `@echo off\nCALL "${path.win32.normalize(binaryFilePath)}" %*`,
85-
bash: `${path.posix.normalize(binaryFilePath)} $@`,
84+
bash: `${path.posix.normalize(binaryFilePath)} "$@"`,
8685
};
8786
}

bazel/integration/test_runner/runner.ts renamed to bazel/integration/test_runner/runner.mts

+29-43
Original file line numberDiff line numberDiff line change
@@ -6,32 +6,30 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import * as fs from 'fs';
10-
import * as path from 'path';
11-
import * as tmp from 'tmp';
12-
import * as os from 'os';
9+
import fs from 'node:fs/promises';
10+
import path from 'node:path';
11+
import os from 'node:os';
1312

1413
import {
1514
BazelExpandedValue,
1615
BazelFileInfo,
1716
resolveBazelFile,
1817
resolveBinaryWithRunfilesGracefully,
19-
} from './bazel';
18+
} from './bazel.mjs';
2019
import {
2120
PackageMappings,
2221
readPackageJsonContents,
2322
updateMappingsForPackageJson,
24-
} from './package_json';
25-
import {addWritePermissionFlag, writeExecutableFile} from './file_system_utils';
23+
} from './package_json.mjs';
24+
import {addWritePermissionFlag, writeExecutableFile} from './file_system_utils.mjs';
2625
import {
2726
expandEnvironmentVariableSubstitutions,
2827
getBinaryPassThroughScript,
2928
prependToPathVariable,
3029
runCommandInChildProcess,
31-
} from './process_utils';
32-
33-
import {ENVIRONMENT_TMP_PLACEHOLDER} from './constants';
34-
import {debug} from './debug';
30+
} from './process_utils.mjs';
31+
import {ENVIRONMENT_TMP_PLACEHOLDER} from './constants.mjs';
32+
import {debug} from './debug.mjs';
3533

3634
/** Error class that is used when an integration command fails. */
3735
class IntegrationTestCommandError extends Error {}
@@ -55,7 +53,7 @@ export class TestRunner {
5553
private readonly testFiles: BazelFileInfo[],
5654
private readonly testPackage: string,
5755
private readonly testPackageRelativeWorkingDir: string,
58-
private readonly toolMappings: Record<string, BazelFileInfo>,
56+
private readonly toolMappings: Record<string, Pick<BazelFileInfo, 'shortPath'>>,
5957
private readonly npmPackageMappings: Record<string, BazelFileInfo>,
6058
private readonly commands: [[binary: BazelExpandedValue, ...args: string[]]],
6159
environment: EnvironmentConfig,
@@ -70,13 +68,13 @@ export class TestRunner {
7068

7169
// Create the test sandbox directory. The working directory does not need to
7270
// be explicitly created here as the test file copying should create the folder.
73-
await fs.promises.mkdir(testSandboxDir);
71+
await fs.mkdir(testSandboxDir);
7472

7573
const toolMappings = await this._setupToolMappingsForTest(testTmpDir);
7674
const testEnv = await this._buildTestProcessEnvironment(testTmpDir, toolMappings.binDir);
7775

7876
debug(`Temporary directory for integration test: ${path.normalize(testTmpDir)}`);
79-
debug(`Test files are copied into: ${path.normalize(testSandboxDir)}`);
77+
debug(`Test files are copied into: ${path.normalize(testWorkingDir)}`);
8078
console.info(`Running test in directory: ${path.normalize(testWorkingDir)}`);
8179

8280
await this._copyTestFilesToDirectory(testSandboxDir);
@@ -90,7 +88,7 @@ export class TestRunner {
9088
// We keep the temporary directory on disk if the users wants to debug the test.
9189
if (!this.isTestDebugMode) {
9290
debug('Deleting the integration test temporary directory..');
93-
await fs.promises.rm(testTmpDir, {force: true, recursive: true, maxRetries: 3});
91+
await fs.rm(testTmpDir, {force: true, recursive: true, maxRetries: 3});
9492
}
9593
}
9694
}
@@ -111,17 +109,7 @@ export class TestRunner {
111109
// system directories are always writable. See:
112110
// Linux: https://github.com/bazelbuild/bazel/blob/d35f923b098e4dc9c90b1ab66b413c216bdee638/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java#L280.
113111
// Darwin: https://github.com/bazelbuild/bazel/blob/d35f923b098e4dc9c90b1ab66b413c216bdee638/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java#L170.
114-
return new Promise((resolve, reject) => {
115-
tmp.dir(
116-
{
117-
template: 'ng-integration-test-XXXXXX',
118-
// Always keep the directory for debugging. We will handle the deletion
119-
// manually and need full control over the directory persistence.
120-
keep: true,
121-
},
122-
(err, tmpPath) => (err ? reject(err) : resolve(path.resolve(tmpPath))),
123-
);
124-
});
112+
return fs.mkdtemp(path.join(os.tmpdir(), 'ng-integration-test'));
125113
}
126114

127115
/**
@@ -149,8 +137,8 @@ export class TestRunner {
149137
const outAbsolutePath = path.join(tmpDir, outRelativePath);
150138
const resolvedFilePath = resolveBazelFile(file);
151139

152-
await fs.promises.mkdir(path.dirname(outAbsolutePath), {recursive: true});
153-
await fs.promises.copyFile(resolvedFilePath, outAbsolutePath);
140+
await fs.mkdir(path.dirname(outAbsolutePath), {recursive: true});
141+
await fs.copyFile(resolvedFilePath, outAbsolutePath);
154142

155143
// Bazel removes the write permission from all generated files. Since we copied
156144
// the test files over to a directory, we want to re-add the write permission in
@@ -164,17 +152,16 @@ export class TestRunner {
164152
* directory can be added to the `$PATH` later when commands are executed.
165153
*/
166154
private async _setupToolMappingsForTest(testDir: string) {
167-
const toolBinDir = path.join(testDir, '.integration-bazel-tool-bin');
155+
const toolBinDir = path.join(testDir, 'integration-bazel-tool-bin');
168156

169157
// Create the bin directory for the tool mappings.
170-
await fs.promises.mkdir(toolBinDir, {recursive: true});
158+
await fs.mkdir(toolBinDir, {recursive: true});
171159

172160
for (const [toolName, toolFile] of Object.entries(this.toolMappings)) {
173161
const toolAbsolutePath = resolveBazelFile(toolFile);
174162
const passThroughScripts = getBinaryPassThroughScript(toolAbsolutePath);
175163
const toolDelegateBasePath = path.join(toolBinDir, toolName);
176164

177-
await writeExecutableFile(`${toolDelegateBasePath}.cmd`, passThroughScripts.cmd);
178165
await writeExecutableFile(`${toolDelegateBasePath}.sh`, passThroughScripts.bash);
179166
await writeExecutableFile(toolDelegateBasePath, passThroughScripts.bash);
180167
}
@@ -207,7 +194,7 @@ export class TestRunner {
207194

208195
// Write the new `package.json` file to the test directory, overwriting
209196
// the `package.json` file initially copied as a test input/file.
210-
await fs.promises.writeFile(pkgJsonPath, JSON.stringify(newPkgJson, null, 2));
197+
await fs.writeFile(pkgJsonPath, JSON.stringify(newPkgJson, null, 2));
211198
}
212199

213200
/**
@@ -230,15 +217,21 @@ export class TestRunner {
230217
if (value.containsExpansion) {
231218
envValue = await resolveBinaryWithRunfilesGracefully(envValue);
232219
} else if (envValue === ENVIRONMENT_TMP_PLACEHOLDER) {
233-
envValue = path.join(testTmpDir, `.tmp-env-${i++}`);
234-
await fs.promises.mkdir(envValue);
220+
envValue = path.join(testTmpDir, `tmp-env-${i++}`);
221+
await fs.mkdir(envValue);
235222
}
236223

237224
testEnv[variableName] = envValue;
238225
}
239226

240227
const commandPath = prependToPathVariable(toolsBinDir, testEnv.PATH ?? '');
241-
return {...testEnv, PATH: commandPath};
228+
return {
229+
...testEnv,
230+
PATH: commandPath,
231+
// `rules_js` binaries are never build actions, so can be set to `.`.
232+
// See: https://github.com/aspect-build/rules_js/blob/674f689ff56b962c3cb0509a4b97e99af049a6eb/js/private/js_binary.sh.tpl#L200-L207
233+
BAZEL_BINDIR: '.',
234+
};
242235
}
243236

244237
/**
@@ -301,16 +294,9 @@ export class TestRunner {
301294
private _assignDefaultEnvironmentVariables(baseEnv: EnvironmentConfig): EnvironmentConfig {
302295
const defaults: EnvironmentConfig = {
303296
'HOME': {value: ENVIRONMENT_TMP_PLACEHOLDER, containsExpansion: false},
297+
'PROFILE': {value: ENVIRONMENT_TMP_PLACEHOLDER, containsExpansion: false},
304298
};
305299

306-
// Support windows-specific system variables. We don't want to always assign these as
307-
// it would result in unnecessary directories being created all the time.
308-
if (os.platform() === 'win32') {
309-
defaults.USERPROFILE = {value: ENVIRONMENT_TMP_PLACEHOLDER, containsExpansion: false};
310-
defaults.APPDATA = {value: ENVIRONMENT_TMP_PLACEHOLDER, containsExpansion: false};
311-
defaults.LOCALAPPDATA = {value: ENVIRONMENT_TMP_PLACEHOLDER, containsExpansion: false};
312-
}
313-
314300
return {...defaults, ...baseEnv};
315301
}
316302
}

bazel/integration/tests/custom_env_variables/test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ if (process.env.MANUAL_ROOT_PATH !== '../npm/node_modules/semver/package.json')
2020
process.exit(1);
2121
}
2222

23-
if (!process.env.HOME.includes('.tmp-env-')) {
23+
if (!process.env.HOME.includes('tmp-env-')) {
2424
console.error('Expected `HOME` to point to a temporary environment directory.');
2525
process.exit(1);
2626
}

bazel/package.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
"@microsoft/api-extractor": "7.52.3",
55
"@types/node": "22.14.0",
66
"piscina": "^4.9.2",
7-
"typescript": "5.8.2"
7+
"typescript": "5.8.2",
8+
"true-case-path": "2.2.1"
89
},
910
"pnpm": {
1011
"onlyBuiltDependencies": []

0 commit comments

Comments
 (0)