Skip to content

build: npm_integration_test && angular_integration_test #33927

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

Closed
gregmagolan wants to merge 10 commits into from
Closed

build: npm_integration_test && angular_integration_test #33927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
68721dc
build: add npm_integration_test && angular_integration_test
gregmagolan Feb 4, 2020
18928b7
build: no-remote-exec for integration tests
gregmagolan Feb 4, 2020
8d49b93
ci: install java runtime in test job
gregmagolan Feb 11, 2020
a7af4f5
ci: bump CircleCi test job to xlarge2 CI job
gregmagolan Feb 8, 2020
71036e8
build: remove CI_CHROMEDIRVER_VERSION_ARG from integration/bazel-sche…
gregmagolan Feb 14, 2020
9745c79
build: remove dep on tsickle from platform-server
gregmagolan Feb 14, 2020
e9c027c
build: use puppeteer in integration/bazel-schematics
gregmagolan Feb 15, 2020
1a30a6c
build: add new integration tests & bazel-in-bazel tests to … glob
gregmagolan Feb 15, 2020
ff75f02
build: optimize integration tests on CI
gregmagolan Feb 15, 2020
9ec7ec8
build: add instructions for adding new integration tests to integrati…
gregmagolan Feb 18, 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
56 changes: 55 additions & 1 deletion .bazelignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,61 @@ dist
aio/content
aio/node_modules
aio/tools/examples/shared/node_modules
integration/bazel
integration/bazel/bazel-bazel
integration/bazel/bazel-bin
integration/bazel/bazel-out
integration/bazel/bazel-testlogs
integration/bazel/node_modules
integration/bazel/.yarn_local_cache
integration/bazel-schematics/node_modules
integration/bazel-schematics/.yarn_local_cache
integration/bazel-schematics/demo
Comment thread
gkalpak marked this conversation as resolved.
Outdated
integration/cli-hello-world/node_modules
integration/cli-hello-world-ivy-compat/node_modules
integration/cli-hello-world-ivy-i18n/node_modules
integration/cli-hello-world-ivy-minimal/node_modules
integration/cli-hello-world-lazy/node_modules
integration/cli-hello-world-lazy-rollup/node_modules
integration/dynamic-compiler/node_modules
integration/hello_world__closure/node_modules
integration/hello_world__systemjs_umd/node_modules
integration/i18n/node_modules
integration/injectable-def/node_modules
integration/ivy-i18n/node_modules
integration/language_service_plugin/node_modules
integration/ng_elements/node_modules
integration/ng_elements_schematics/node_modules
integration/ng_update/node_modules
integration/ng_update_migrations/node_modules
integration/ngcc/node_modules
integration/platform-server/node_modules
integration/service-worker-schema/node_modules
integration/side-effects/node_modules
integration/terser/node_modules
integration/typings_test_ts36/node_modules
integration/typings_test_ts37/node_modules
integration/cli-hello-world/.yarn_local_cache
Copy link
Copy Markdown
Contributor

@IgorMinar IgorMinar Jan 10, 2020

Choose a reason for hiding this comment

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

do we still need the local.yarn_local_cache for each integration test app? in the past we needed it because of how we installed the locally built packages into the integration folder and yarn bugs with handling file:// dependencies.

also doesn't .bazelignore support globs that could make this setup less verbose and less fragile? I suspect that as we add more tests, we will forget to ignore these folders in newly added tests. What will happen if we don't ignore these new folders? Will the mistake be obvious and break the build?

Copy link
Copy Markdown
Contributor

@IgorMinar IgorMinar Jan 23, 2020

Choose a reason for hiding this comment

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

one thing I tried recently was to use "preinstall": "rimraf -f node_modules" in package.json to work around the yarn issue and it appeared to work well (rimraf was provided by the root node_modules which are guaranteed to be available by the time we run these tests) - something to consider as an alternative to the current setup.

Copy link
Copy Markdown
Contributor

@IgorMinar IgorMinar Feb 13, 2020

Choose a reason for hiding this comment

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

@gregmagolan does this need an followup?

Copy link
Copy Markdown
Contributor Author

@gregmagolan gregmagolan Feb 13, 2020

Choose a reason for hiding this comment

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

There was an old issue about this for bazel and a new one just got opened up by Or from Wix. bazelbuild/bazel#7093. I commented there. Hopefully will get fixed soon as this file is getting out of hand.

Copy link
Copy Markdown
Contributor Author

@gregmagolan gregmagolan Feb 13, 2020

Choose a reason for hiding this comment

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

Oh. That issue is from Jan 11, 2019. Not 2020. 🙄

Copy link
Copy Markdown
Member

@gkalpak gkalpak Feb 16, 2020

Choose a reason for hiding this comment

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

We will definitely forget about updating this file as we add/remove integration projects.

  • What will be the consequences of use forgetting to ignore something here?
  • Could we have some instructions on what one needs to do to add a new integration project (including what needs to go in this file)?

integration/cli-hello-world-ivy-compat/.yarn_local_cache
integration/cli-hello-world-ivy-i18n/.yarn_local_cache
integration/cli-hello-world-ivy-minimal/.yarn_local_cache
integration/cli-hello-world-lazy/.yarn_local_cache
integration/cli-hello-world-lazy-rollup/.yarn_local_cache
integration/dynamic-compiler/.yarn_local_cache
integration/hello_world__closure/.yarn_local_cache
integration/hello_world__systemjs_umd/.yarn_local_cache
integration/i18n/.yarn_local_cache
integration/injectable-def/.yarn_local_cache
integration/ivy-i18n/.yarn_local_cache
integration/language_service_plugin/.yarn_local_cache
integration/ng_elements/.yarn_local_cache
integration/ng_elements_schematics/.yarn_local_cache
integration/ng_update/.yarn_local_cache
integration/ng_update_migrations/.yarn_local_cache
integration/ngcc/.yarn_local_cache
integration/platform-server/.yarn_local_cache
integration/service-worker-schema/.yarn_local_cache
integration/side-effects/.yarn_local_cache
integration/terser/.yarn_local_cache
integration/typings_test_ts36/.yarn_local_cache
integration/typings_test_ts37/.yarn_local_cache
packages/bazel/node_modules
11 changes: 10 additions & 1 deletion .bazelrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@ test --test_output=errors

# Bazel flags for CircleCI are in /.circleci/bazel.linux.rc and /.circleci/bazel.windows.rc

##################################
# Settings for integration tests #
##################################

# Trick bazel into treating BUILD files under integration/bazel as being regular files
# This lets us glob() up all the files inside this integration test to make them inputs to tests
# (Note, we cannot use common --deleted_packages because the bazel version command doesn't support it)
build --deleted_packages=integration/bazel,integration/bazel/src,integration/bazel/src/hello-world,integration/bazel/test,integration/bazel/test/e2e
query --deleted_packages=integration/bazel,integration/bazel/src,integration/bazel/src/hello-world,integration/bazel/test,integration/bazel/test/e2e

################################
# Temporary Settings for Ivy #
################################
Expand Down Expand Up @@ -100,7 +110,6 @@ build:remote --javabase=@rbe_ubuntu1604_angular//java:jdk
build:remote --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_hostjdk8
build:remote --java_toolchain=@bazel_tools//tools/jdk:toolchain_hostjdk8
build:remote --crosstool_top=@rbe_ubuntu1604_angular//cc:toolchain
build:remote --action_env=BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1
build:remote --extra_toolchains=@rbe_ubuntu1604_angular//config:cc-toolchain
build:remote --extra_execution_platforms=//tools:rbe_ubuntu1604-angular
build:remote --host_platform=//tools:rbe_ubuntu1604-angular
Expand Down
24 changes: 22 additions & 2 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,19 @@ commands:
# circleci/node:x.x.x-browsers image.
sudo apt-get -y install libgtk-3-0 libasound2 libnss3 libxss1

# Install java runtime which is required by some integration tests such as
# //integration:hello_world__closure_test, //integration:i18n_test and
# //integration:ng_elements_test to run the closure compiler
install_java:
Copy link
Copy Markdown
Contributor

@filipesilva filipesilva Feb 14, 2020

Choose a reason for hiding this comment

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

Why was this not needed before?

Copy link
Copy Markdown
Contributor Author

@gregmagolan gregmagolan Feb 14, 2020

Choose a reason for hiding this comment

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

The circle node browsers docker image included Java but now we're using the bare circleci node image since we have puppeteer :)

Copy link
Copy Markdown
Contributor

@filipesilva filipesilva Feb 14, 2020

Choose a reason for hiding this comment

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

Oh wow that was surprising, didn't expect the browser imagines to also randomly include that. Good thing it's explicit now. Again it highlights on how much we rely on hidden stuff from those images.

description: Install java
steps:
- run:
name: Install java
command: |
sudo apt-get update
# Install java runtime
sudo apt-get install default-jre

# Initializes the CI environment by setting up common environment variables.
init_environment:
description: Initializing environment (setting up variables)
Expand Down Expand Up @@ -281,10 +294,16 @@ jobs:
test:
executor:
name: default-executor
resource_class: xlarge
# Now that large integration tests are running locally in parallel (they can't run on RBE yet
# as they require network access for yarn install), this test is running out of memory
# consistently with the xlarge machine.
# TODO: switch back to xlarge once integration tests are running on remote-exec
resource_class: 2xlarge+
Copy link
Copy Markdown
Contributor

@IgorMinar IgorMinar Feb 21, 2020

Choose a reason for hiding this comment

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

I heard from Joey that 2xlarge+ didn't decrease the job execution time over 2xlarge, did it turn out that we can utilize the biggest vm afterall?

Copy link
Copy Markdown
Contributor Author

@gregmagolan gregmagolan Feb 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I made a difference once I moved the integration/bazel & integration/bazel-schematics tests to work in the general bazel test ... case without the requiring the browser docker image and with the cpu:3 tags. Those tests use a lot of resources as they are bazel-in-bazel and the additional 4 cores resulted in reduced time. Moving those over we have fewer jobs so overall we're using fewer credits now.

steps:
- custom_attach_workspace
- init_environment
- install_chrome_libs
- install_java
- run:
command: yarn bazel test //... --build_tag_filters=-ivy-only --test_tag_filters=-ivy-only
no_output_timeout: 20m
Expand All @@ -297,6 +316,7 @@ jobs:
steps:
- custom_attach_workspace
- init_environment
- install_chrome_libs
# We need to explicitly specify the --symlink_prefix option because otherwise we would
# not be able to easily find the output bin directory when uploading artifacts for size
# measurements.
Expand Down Expand Up @@ -561,7 +581,7 @@ jobs:
# of memory. Together with the system under test, this can exhaust the RAM
# on a 4G worker so we use a larger machine here too.
resource_class: xlarge
parallelism: 4
parallelism: 3
Comment thread
IgorMinar marked this conversation as resolved.
Outdated
steps:
- custom_attach_workspace
- init_environment
Expand Down
1 change: 1 addition & 0 deletions .pullapprove.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -953,6 +953,7 @@ groups:
'tools/ng_rollup_bundle/**',
'tools/ngcontainer/**',
'tools/npm/**',
'tools/npm_integration_test/**',
'tools/public_api_guard/BUILD.bazel',
'tools/public_api_guard/public_api_guard.bzl',
'tools/pullapprove/**',
Expand Down
3 changes: 3 additions & 0 deletions WORKSPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,11 @@ node_repositories(
package_json = ["//:package.json"],
)

load("//integration:angular_integration_test.bzl", "npm_package_archives")

yarn_install(
name = "npm",
manual_build_file_contents = npm_package_archives(),
package_json = "//:package.json",
yarn_lock = "//:yarn.lock",
)
Expand Down
100 changes: 100 additions & 0 deletions integration/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
load(":angular_integration_test.bzl", "angular_integration_test")

# Some integration ports must be managed manually to be unique and in other
# cases the tests are able to select a random free port.
#
# Where `ng e2e` is used we pass `ng e2e --port 0` which prompts the cli
# to select a random free port for the the e2e test. The protractor.conf is
# automaticaly updated to use this port.
#
# Karma automatically finds a free port so no effort is needed there.
#
# The manually configured ports are as follows:
#
# TEST PORT CONFIGURATION
# ==== ==== =============
# dynamic-compiler 4201 /e2e/browser.config.json: "port": 4201
# hello_world__closure 4202 /e2e/browser.config.json: "port": 4202
# hello_world__systemjs_umd 4203 /bs-config.e2e.json: "port": 4203
# i18n 4204 /e2e/browser.config.json: "port": 4204
# ng_elements 4205 /e2e/browser.config.json: "port": 4205
# platform-server 4206 /src/server.ts: app.listen(4206,...
Comment thread
IgorMinar marked this conversation as resolved.
Outdated

# Map of integration tests to tags.
# A subset of these tests fail or are not meant to be run with ivy bundles. These are tagged
# "no-ivy-aot".
INTEGRATION_TESTS = {
"bazel": [
# Bazel-in-bazel tests are resource intensive and should not be over-parallized
# as they will compete for the resources of other parallel tests slowing
# everything down. Ask Bazel to allocate multiple CPUs for these tests with "cpu:n" tag.
"cpu:3",
"no-ivy-aot",
],
"bazel-schematics": [
# Bazel-in-bazel tests are resource intensive and should not be over-parallized
# as they will complete for the resources of other parallel tests slowing
# everything down. Ask Bazel to allocate multiple CPUs for these tests with "cpu:n" tag.
"cpu:3",
"no-ivy-aot",
],
"cli-hello-world": [],
"cli-hello-world-ivy-compat": [],
"cli-hello-world-ivy-i18n": ["no-ivy-aot"],
"cli-hello-world-ivy-minimal": [],
"cli-hello-world-lazy": [],
"cli-hello-world-lazy-rollup": [],
"dynamic-compiler": ["no-ivy-aot"],
"hello_world__closure": ["no-ivy-aot"],
"i18n": ["no-ivy-aot"],
"injectable-def": ["no-ivy-aot"],
"ivy-i18n": ["no-ivy-aot"],
"language_service_plugin": [],
"ng_elements": ["no-ivy-aot"],
# TODO: fix ng_elements_schematics with Bazel which was added recently and uses a new pattern
# "ng_elements_schematics": ["no-ivy-aot"],
"ng_update": [],
"ng_update_migrations": ["no-ivy-aot"],
"ngcc": ["no-ivy-aot"],
"platform-server": ["no-ivy-aot"],
"service-worker-schema": [],
"side-effects": ["no-ivy-aot"],
"terser": [],
}

[
angular_integration_test(
name = test_folder + "_test",
tags = INTEGRATION_TESTS[test_folder],
test_folder = test_folder,
)
for test_folder in INTEGRATION_TESTS
]

# Special case for `typings_test_ts36` test as we want to pin
# `typescript` at version 3.6.x for that test and not link to the
# root @npm//typescript package.
angular_integration_test(
name = "typings_test_ts36_test",
pinned_npm_packages = ["typescript"],
test_folder = "typings_test_ts36",
)

# Special case for `typings_test_ts37` test as we want to pin
# `typescript` at version 3.7.x for that test and not link to the
# root @npm//typescript package.
angular_integration_test(
name = "typings_test_ts37_test",
pinned_npm_packages = ["typescript"],
test_folder = "typings_test_ts37",
)

# Special case for `hello_world__systemjs_umd` test as we want to pin
# `systems` at version 0.20.2 and not link to the the root @npm//systemjs
# which is stuck at 0.18.10 and can't be updated to 0.20.2 without
# breaking the legacy saucelabs job.
angular_integration_test(
name = "hello_world__systemjs_umd_test",
pinned_npm_packages = ["systemjs"],
test_folder = "hello_world__systemjs_umd",
)
41 changes: 41 additions & 0 deletions integration/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,47 @@ $ ./integration/run_tests.sh
The test runner will first re-build any stale npm packages, then `cd` into each subdirectory to
execute the test.

## Running integration tests under Bazel

The PR https://github.com/angular/angular/pull/33927 added the ability to run integration tests with Bazel. These tests can be resource intensive so it is recommended to limit the number of concurrent test jobs with the `--local_test_jobs` bazel flag.

Locally, if Bazel uses all of your cores to run the maximum number of integration tests in parallel then this can lead to test timeouts and flakes and freeze up your machine while these tests are running. You can limit the number of concurrent local integration tests that run with:

```
yarn bazel test --local_test_jobs=<N> //integration/...
```

Set a reasonable `local_test_jobs` limit for your local machine to prevent full cpu utilization during local development test runs.

To avoid having to specify this command line flag, you may want to include it in your `.bazelrc.user` file:

```
test --local_test_jobs=<N>
```

The downside of this is that this will apply to all tests and not just the resource intensive integration tests.

### Bazel-in-bazel integration tests

Two of the integration tests that run Bazel-in-Bazel are particularly resource intensive and are tagged "manual" and "exclusive". To run these tests use,

```
yarn bazel test //integration:bazel_test
yarn bazel test //integration:bazel-schematics_test
```

## Adding a new integration test

When adding a new integration test, follow the steps below to add a bazel test target for the new test.

1. Add new test to `INTEGRATION_TESTS` object in `/integration/BUILD.bazel` (and tag as `"no-ivy-aot"` if not meant to be run against ivy bundles). *NB: if the test requires any special attribute then make a new angular_integration_test target instead.*
2. If test requires ports and does not support ethereal ports then make sure the port is unique and add it to the "manually configured ports" comment to document which port it is using
3. Add at least the following two entries `.bazelignore` (as they may contain BUILD files)
1. `integration/new_test/node_modules`
2. `integration/new_test/.yarn_local_cache`
4. Add any other untracked folders to `.bazelignore` that may contain `BUILD` files
5. If there are tracked BUILD files in the integration test folder (`integration/bazel` has these for example) add those folders to the `build --deleted_packages` and `query --deleted_packages` lines in `.bazelrc`

## Browser tests

For integration tests we use the puppeteer provisioned version of Chrome. For both Karma and Protractor tests we set a number of browser testing flags. To avoid duplication, they will be listed and explained here and the code will reference this file for more information.
Expand Down
Loading