From 9e3e970dbf683b5064e1952da647b9987f80a11e Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Thu, 24 Mar 2022 17:58:38 +0100 Subject: [PATCH] test: refactor compiler-cli compliance test to work on windows (#45431) Recent changes in `rules_nodejs` caused the test case copy file actions to be transitioned into the `exec` configuration, resulting in much larger file paths. These paths break on Windows with the shell argument limit, and with the path limit, causing errors like: ``` ERROR: C:/users/circleci/ng/packages/compiler-cli/test/compliance/test_cases/BUILD.bazel:9:12: Copying file packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/value_composition/structural_directives_if_directive_def.js failed: (Exit 1): cmd.exe failed: error executing command cd /d C:/users/circleci/_bazel_circleci/u4uoan2j/execroot/angular SET PATH=C:\Program Files\Git\usr\bin;C:\Program Files\Git\bin;C:\Windows;C:\Windows\System32;C:\Windows\System32\WindowsPowerShell\v1.0 SET RUNFILES_MANIFEST_ONLY=1 cmd.exe /C bazel-out\x64_windows-opt-exec-2B5CBBC6\bin\packages\compiler-cli\test\compliance\test_cases\test_cases--1973427149-cmd.bat The system cannot find the path specified ``` https://app.circleci.com/pipelines/github/angular/angular/44038/workflows/4b530cb2-f232-4e1d-b35a-e6e085151d08/jobs/1140017 PR Close #45431 --- .../test/compliance/partial/cli.ts | 5 +++- .../partial/generate_golden_partial.ts | 9 +++--- .../partial/partial_compliance_goldens.bzl | 6 ++-- .../test/compliance/test_cases/BUILD.bazel | 6 +--- .../test/compliance/test_helpers/BUILD.bazel | 1 + .../test_helpers/get_compliance_tests.ts | 30 +++++++------------ 6 files changed, 22 insertions(+), 35 deletions(-) diff --git a/packages/compiler-cli/test/compliance/partial/cli.ts b/packages/compiler-cli/test/compliance/partial/cli.ts index afcd0eb002a..60fc63ffd85 100644 --- a/packages/compiler-cli/test/compliance/partial/cli.ts +++ b/packages/compiler-cli/test/compliance/partial/cli.ts @@ -5,6 +5,9 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ + +import {fs} from '../test_helpers/get_compliance_tests'; + import {generateGoldenPartial} from './generate_golden_partial'; -generateGoldenPartial(process.argv[2]); +generateGoldenPartial(fs.resolve(process.argv[2])); diff --git a/packages/compiler-cli/test/compliance/partial/generate_golden_partial.ts b/packages/compiler-cli/test/compliance/partial/generate_golden_partial.ts index ed4c3ef51f8..be5e44f9bd1 100644 --- a/packages/compiler-cli/test/compliance/partial/generate_golden_partial.ts +++ b/packages/compiler-cli/test/compliance/partial/generate_golden_partial.ts @@ -5,7 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import {FileSystem} from '../../../src/ngtsc/file_system'; +import {AbsoluteFsPath, FileSystem} from '../../../src/ngtsc/file_system'; import {compileTest, getBuildOutputDirectory, initMockTestFileSystem} from '../test_helpers/compile_test'; import {ComplianceTest, getComplianceTests} from '../test_helpers/get_compliance_tests'; import {PartiallyCompiledFile, renderGoldenPartial} from '../test_helpers/golden_partials'; @@ -13,12 +13,11 @@ import {PartiallyCompiledFile, renderGoldenPartial} from '../test_helpers/golden /** * Generate the golden partial output for the tests described in the `testConfigPath` config file. * - * @param testConfigPath Path (relative to the `test_cases` directory) of the `TEST_CASES.json` file - * that describes the tests. + * @param testConfigPath Absolute disk path of the `TEST_CASES.json` file that describes the tests. */ -export function generateGoldenPartial(testConfigPath: string): void { +export function generateGoldenPartial(absTestConfigPath: AbsoluteFsPath): void { const files: PartiallyCompiledFile[] = []; - const tests = getComplianceTests(testConfigPath); + const tests = getComplianceTests(absTestConfigPath); for (const test of tests) { const fs = initMockTestFileSystem(test.realTestPath); for (const file of compilePartials(fs, test)) { diff --git a/packages/compiler-cli/test/compliance/partial/partial_compliance_goldens.bzl b/packages/compiler-cli/test/compliance/partial/partial_compliance_goldens.bzl index 83bc2d85cc8..0967ebe587f 100644 --- a/packages/compiler-cli/test/compliance/partial/partial_compliance_goldens.bzl +++ b/packages/compiler-cli/test/compliance/partial/partial_compliance_goldens.bzl @@ -16,14 +16,12 @@ def partial_compliance_golden(filePath): nodejs_binary( name = generate_partial_name, testonly = True, - data = data, + data = data + [filePath], visibility = [":__pkg__"], entry_point = "//packages/compiler-cli/test/compliance/partial:cli.ts", templated_args = [ - filePath, - # TODO(josephperrott): update dependency usages to no longer need bazel patch module resolver - # See: https://github.com/bazelbuild/rules_nodejs/wiki#--bazel_patch_module_resolver-now-defaults-to-false-2324 "--bazel_patch_module_resolver", + "$(execpath %s)" % filePath, ], ) diff --git a/packages/compiler-cli/test/compliance/test_cases/BUILD.bazel b/packages/compiler-cli/test/compliance/test_cases/BUILD.bazel index 51d995fb081..84b47a5aa7e 100644 --- a/packages/compiler-cli/test/compliance/test_cases/BUILD.bazel +++ b/packages/compiler-cli/test/compliance/test_cases/BUILD.bazel @@ -1,12 +1,8 @@ -load("@build_bazel_rules_nodejs//:index.bzl", "copy_to_bin") load("//packages/compiler-cli/test/compliance/partial:partial_compliance_goldens.bzl", "partial_compliance_golden") package(default_visibility = ["//packages/compiler-cli/test/compliance:__subpackages__"]) -# Normally we would use `file_group` here but this doesn't work on Windows because there -# appears to be a bug with making the `runfiles` folder available. -# See https://github.com/bazelbuild/rules_nodejs/issues/1689 -copy_to_bin( +filegroup( name = "test_cases", srcs = glob(["*/**"]), ) diff --git a/packages/compiler-cli/test/compliance/test_helpers/BUILD.bazel b/packages/compiler-cli/test/compliance/test_helpers/BUILD.bazel index f7db924eca4..feff9f3f1af 100644 --- a/packages/compiler-cli/test/compliance/test_helpers/BUILD.bazel +++ b/packages/compiler-cli/test/compliance/test_helpers/BUILD.bazel @@ -18,6 +18,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/logging", "//packages/compiler-cli/src/ngtsc/sourcemaps", "//packages/compiler-cli/src/ngtsc/testing", + "@npm//@bazel/runfiles", "@npm//typescript", ], ) diff --git a/packages/compiler-cli/test/compliance/test_helpers/get_compliance_tests.ts b/packages/compiler-cli/test/compliance/test_helpers/get_compliance_tests.ts index f1ecd56e4e5..97800620e44 100644 --- a/packages/compiler-cli/test/compliance/test_helpers/get_compliance_tests.ts +++ b/packages/compiler-cli/test/compliance/test_helpers/get_compliance_tests.ts @@ -5,10 +5,15 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import {runfiles} from '@bazel/runfiles'; + import {AbsoluteFsPath, NodeJSFileSystem, PathSegment, ReadonlyFileSystem} from '../../../src/ngtsc/file_system'; -const fs = new NodeJSFileSystem(); -const basePath = fs.resolve(__dirname, '../test_cases'); +export const fs = new NodeJSFileSystem(); + +/** Path to the test case sources. */ +const basePath = fs.resolve( + runfiles.resolveWorkspaceRelative('packages/compiler-cli/test/compliance/test_cases')); /** * Search the `test_cases` directory, in the real file-system, for all the compliance tests. @@ -25,11 +30,10 @@ export function* getAllComplianceTests(): Generator { /** * Extract all the compliance tests from the TEST_CASES.json file at the `testConfigPath`. * - * @param testConfigPath The path, relative to the `test_cases` basePath, of the `TEST_CASES.json` - * config file. + * @param testConfigPath Absolute disk path of the `TEST_CASES.json` file that describes the tests. + */ -export function* getComplianceTests(testConfigPath: string): Generator { - const absTestConfigPath = fs.resolve(basePath, testConfigPath); +export function* getComplianceTests(absTestConfigPath: AbsoluteFsPath): Generator { const realTestPath = fs.dirname(absTestConfigPath); const testConfigJSON = loadTestCasesFile(fs, absTestConfigPath, basePath).cases; const testConfig = Array.isArray(testConfigJSON) ? testConfigJSON : [testConfigJSON]; @@ -93,20 +97,6 @@ function getStringOrFail(container: any, property: string, testPath: AbsoluteFsP return value; } -function getBooleanOrDefault( - container: any, property: string, testPath: AbsoluteFsPath, defaultValue: boolean): boolean { - const value = container[property]; - if (typeof value === 'undefined') { - return defaultValue; - } - if (typeof value !== 'boolean') { - throw new Error( - `Test has invalid "${property}" property in TEST_CASES.json - expected boolean: ` + - testPath); - } - return value; -} - function getStringArrayOrDefault( container: any, property: string, testPath: AbsoluteFsPath, defaultValue: string[]): string[] { const value = container[property];