Skip to content

Prepare bazel integration tests #17304

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.

Sign up for GitHub

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

Merged
merged 23 commits into from
Apr 7, 2020
Merged

Prepare bazel integration tests #17304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
457b046
build: fix bazel tests to run on windows
filipesilva Jan 29, 2020
9790ad3
build: use runfiles on windows
filipesilva Mar 5, 2020
3235d07
fix(@angular-devkit/architect): temporary workaround for TS bug with …
filipesilva Mar 5, 2020
fe680df
build: build and test @ngtools/webpack with Bazel
filipesilva Mar 5, 2020
bdda757
build: add module_name to @angular-devkit/build-optimizer Bazel rule
filipesilva Mar 9, 2020
0fc9189
refactor: use BUILD.bazel instead of BUILD
filipesilva Mar 11, 2020
fd9441a
build: use ts_json_schema's ts output directly
filipesilva Mar 11, 2020
ef82fc9
build: also produce tar.gz for bazel npm_package rules
filipesilva Mar 11, 2020
2513487
build: move BUILD.bazel files for secondary entry points into own folder
filipesilva Mar 11, 2020
425de7d
build: include json files in @angular-devkit/build-optimizer Bazel build
filipesilva Mar 17, 2020
c807ebd
build: update com_google_protobuf shallow_since arg
filipesilva Mar 19, 2020
96c3874
build: workaround bazel module resolution bug
filipesilva Mar 19, 2020
6c0ff8b
test(@angular-devkit/architect): run tests in Bazel
filipesilva Mar 23, 2020
3f2392b
fix(@schematics/schematics): add module_name and data files
filipesilva Mar 23, 2020
e1cb441
fix(@angular-devkit/schematics): add module_name and data files
filipesilva Mar 23, 2020
d09c6d5
test(@angular-devkit/schematics-cli): test with Bazel
filipesilva Mar 23, 2020
51fc2ea
fix(@schematics/angular): include assets in ts library
filipesilva Mar 23, 2020
9fcd794
test(@angular/pwa): test with Bazel
filipesilva Mar 23, 2020
b283747
test(@angular/cli): test with Bazel
filipesilva Mar 23, 2020
5dfa64e
build: update yarn lock
filipesilva Mar 25, 2020
ac384fd
style: reformat bazel files
filipesilva Mar 25, 2020
29137eb
build: add missing raw-loader dependency
filipesilva Mar 25, 2020
5dba778
docs: add windows note to bazel process doc
filipesilva Apr 7, 2020
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
6 changes: 5 additions & 1 deletion .bazelrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,4 +141,8 @@ common --experimental_allow_incremental_repository_updates

# Load any settings which are specific to the current user. Needs to be *last* statement
# in this config, as the user configuration should be able to overwrite flags from this file.
try-import .bazelrc.user
try-import .bazelrc.user

# Enable runfiles even on Windows.
# Architect resolves output files from data files, and this isn't possible without runfile support.
test --enable_runfiles
2 changes: 1 addition & 1 deletion WORKSPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ git_repository(
name = "com_google_protobuf",
commit = "6263268b8c1b78a8a9b65acd6f5dd5c04dd9b0e1",
remote = "https://github.com/protocolbuffers/protobuf",
shallow_since = "1559159889 -0400",
shallow_since = "1576607245 -0800",
)

load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")
Expand Down
19 changes: 19 additions & 0 deletions docs/process/bazel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,25 @@ well. This is to ensure that when users download a published version from NPM,
they will be able to install all dependencies correctly without Bazel. It is the
responsibility of the developer to keep both `package.json` in sync.

## Windows support

In general, any sort of node file lookup on Bazel should be subject to `require.resolve`.
This is how rules_nodejs resolves paths using the Bazel runfiles mechanism, where a given
Bazel target only has access to outputs from its dependencies.

In practice, this does not make a lot of difference on Linux.
A symlink forest is laid down where the target is going to actually run, and mostly the
files are resolved correctly whether you use `require.resolve` or not because the files are there.

On Windows though, that's a stricter. Bazel does not lay down a symlink forest on
windows by default. If you don't use `require.resolve`, it's still possible to correctly
resolve some files, like outputs from other rules. But other files, like node modules
dependencies and data files, need to be looked up in the runfiles.

Since the requirement is quite lax on Linux but quite strict on windows, what ends up
happening is that lack of `require.resolve` calls go unnoticed until someone tries to run
things on Windows, at which point it breaks.

## Issues

1. Yarn workspaces is not compatible with Bazel-managed deps
Expand Down
13 changes: 10 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,13 @@
"conventional-changelog": "^3.0.0",
"conventional-commits-parser": "^3.0.0",
"debug": "^4.1.1",
"enhanced-resolve": "4.1.1",
"fast-json-stable-stringify": "2.1.0",
"gh-got": "^9.0.0",
"git-raw-commits": "^2.0.0",
"glob": "^7.0.3",
"husky": "^4.0.10",
"inquirer": "7.1.0",
"jasmine": "^3.3.1",
"jasmine-spec-reporter": "~5.0.0",
"karma": "~4.4.1",
Expand All @@ -123,18 +125,21 @@
"node-fetch": "^2.2.0",
"npm-registry-client": "8.6.0",
"ora": "^4.0.2",
"pacote": "9.5.12",
"pidtree": "^0.4.0",
"pacote": "11.1.4",
"parse5-html-rewriting-stream": "5.1.1",
"pidtree": "^0.3.0",
"pidusage": "^2.0.17",
"prettier": "^2.0.0",
"puppeteer": "2.1.1",
"quicktype-core": "^6.0.15",
"raw-loader": "4.0.0",
"rxjs": "6.5.4",
"sauce-connect-proxy": "https://saucelabs.com/downloads/sc-4.5.4-linux.tar.gz",
"semver": "7.1.3",
"source-map": "^0.7.3",
"source-map-support": "^0.5.0",
"spdx-satisfies": "^5.0.0",
"symbol-observable": "1.2.0",
"tar": "^6.0.0",
"temp": "^0.9.0",
"through2": "^3.0.0",
Expand All @@ -145,7 +150,9 @@
"tslint-no-circular-imports": "^0.7.0",
"tslint-sonarts": "1.9.0",
"typescript": "3.8.3",
"verdaccio": "4.5.1"
"verdaccio": "4.5.1",
"webpack": "4.42.0",
"webpack-sources": "1.4.3"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having to put all the dependencies in two places is really awkward and a potential maintenance burden. Hopefully this can be rectified in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At its core, this is needed because rules_nodejs doesn't understand yarn workspaces. It's not super clear to me whether it should though, or how yarn pnp plays into that either.

We could reduce this burden with some health checks on our repository, like checking that these versions are the same everywhere they are mentioned, plus some exceptions. Maybe at that point what we need to to also loop in dev-infra to share that tooling between fw/comp/cli. We're the only ones that use yarn workspaces afaik but we're not the only ones that have versions sprinkled around that need to be matched.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, I think extra validation checks are going to be paramount. Especially to ensure that the tests are using the same dependency versions that the users are using when they install the package.

I think this is also problematic if we need to use a different version of a dependency for a particular package (recent build optimizer TS issue, for example) or is there a mechanism for that situation? I don't think there are any of these currently though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @josephperrott

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also update renovate configuration now, so that it doesn't create separate PRs for dependencies and dev deps, as otherwise we'll have 2 PRs for each dependency update.

},
"husky": {
"hooks": {
Expand Down
82 changes: 50 additions & 32 deletions packages/angular/cli/BUILD → packages/angular/cli/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
licenses(["notice"]) # MIT

load("@npm_bazel_typescript//:index.bzl", "ts_library")
load("@npm_bazel_jasmine//:index.bzl", "jasmine_node_test")
load("//tools:ts_json_schema.bzl", "ts_json_schema")

package(default_visibility = ["//visibility:public"])
Expand All @@ -18,21 +19,41 @@ ts_library(
"**/*_spec.ts",
"**/*_spec_large.ts",
],
),
) + [
"//packages/angular/cli:lib/config/schema.ts",
"//packages/angular/cli:commands/analytics.ts",
"//packages/angular/cli:commands/add.ts",
"//packages/angular/cli:commands/build.ts",
"//packages/angular/cli:commands/deploy.ts",
"//packages/angular/cli:commands/config.ts",
"//packages/angular/cli:commands/deprecated.ts",
"//packages/angular/cli:commands/doc.ts",
"//packages/angular/cli:commands/e2e.ts",
"//packages/angular/cli:commands/easter-egg.ts",
"//packages/angular/cli:commands/generate.ts",
"//packages/angular/cli:commands/help.ts",
"//packages/angular/cli:commands/lint.ts",
"//packages/angular/cli:commands/new.ts",
"//packages/angular/cli:commands/serve.ts",
"//packages/angular/cli:commands/test.ts",
"//packages/angular/cli:commands/update.ts",
"//packages/angular/cli:commands/version.ts",
"//packages/angular/cli:commands/run.ts",
"//packages/angular/cli:commands/xi18n.ts",
],
data = glob([
"**/*.json",
"**/*.md",
]),
module_name = "@angular/cli",
# strict_checks = False,
deps = [
":command_schemas",
"//packages/angular_devkit/architect",
"//packages/angular_devkit/architect:node",
"//packages/angular_devkit/architect/node",
"//packages/angular_devkit/core",
"//packages/angular_devkit/core:node",
"//packages/angular_devkit/core/node",
"//packages/angular_devkit/schematics",
"//packages/angular_devkit/schematics:tools",
"//packages/angular_devkit/schematics/tools",
# @node_module: typescript:es2017.object
"@npm//@types/debug",
"@npm//@types/node",
Expand All @@ -45,33 +66,6 @@ ts_library(
],
)

ts_library(
name = "command_schemas",
srcs = [],
deps = [
":add_schema",
":analytics_schema",
":build_schema",
":cli_schema",
":config_schema",
":deploy_schema",
":deprecated_schema",
":doc_schema",
":e2e_schema",
":easter_egg_schema",
":generate_schema",
":help_schema",
":lint_schema",
":new_schema",
":run_schema",
":serve_schema",
":test_schema",
":update_schema",
":version_schema",
":xi18n_schema",
],
)

ts_json_schema(
name = "cli_schema",
src = "lib/config/schema.json",
Expand Down Expand Up @@ -228,3 +222,27 @@ ts_json_schema(
"commands/definitions.json",
],
)

ts_library(
name = "angular-cli_test_lib",
testonly = True,
srcs = glob(
include = [
"**/*_spec.ts",
"**/*_spec_large.ts",
],
),
# strict_checks = False,
tsconfig = "//:tsconfig-test.json",
deps = [
":angular-cli",
"//packages/angular_devkit/core",
"@npm//@types/jasmine",
"@npm//@types/node",
],
)

jasmine_node_test(
name = "angular-cli_test",
srcs = [":angular-cli_test_lib"],
)
38 changes: 37 additions & 1 deletion packages/angular/pwa/BUILD → packages/angular/pwa/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
licenses(["notice"]) # MIT

load("@npm_bazel_typescript//:index.bzl", "ts_library")
load("@npm_bazel_jasmine//:index.bzl", "jasmine_node_test")
load("//tools:ts_json_schema.bzl", "ts_json_schema")

package(default_visibility = ["//visibility:public"])
Expand All @@ -24,12 +25,23 @@ ts_library(
"**/*_spec.ts",
"**/*_spec_large.ts",
],
) + [
"//packages/angular/pwa:pwa/schema.ts",
],
data = glob(
include = [
"collection.json",
"package.json",
"pwa/schema.json",
"pwa/files/**/*",
],
),
deps = [
":pwa_schema",
"//packages/angular_devkit/core",
"//packages/angular_devkit/schematics",
"//packages/schematics/angular",
"@npm//@types/node",
"@npm//parse5-html-rewriting-stream",
"@npm//rxjs",
],
)
Expand All @@ -38,3 +50,27 @@ ts_json_schema(
name = "pwa_schema",
src = "pwa/schema.json",
)

ts_library(
name = "pwa_test_lib",
testonly = True,
srcs = glob(
include = [
"pwa/**/*_spec.ts",
"pwa/**/*_spec_large.ts",
],
),
# strict_checks = False,
tsconfig = "//:tsconfig-test.json",
deps = [
":pwa",
"//packages/angular_devkit/schematics/testing",
"@npm//@types/jasmine",
"@npm//@types/node",
],
)

jasmine_node_test(
name = "pwa_test",
srcs = [":pwa_test_lib"],
)
2 changes: 1 addition & 1 deletion packages/angular/pwa/pwa/index_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { Schema as PwaOptions } from './schema';
describe('PWA Schematic', () => {
const schematicRunner = new SchematicTestRunner(
'@angular/pwa',
path.join(__dirname, '../collection.json'),
require.resolve(path.join(__dirname, '../collection.json')),
);
const defaultOptions: PwaOptions = {
project: 'bar',
Expand Down
Loading