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

Rules nodejs 1.0 redo #34736

Closed
wants to merge 19 commits into from
Closed

Conversation

@gregmagolan
Copy link
Contributor

gregmagolan commented Jan 11, 2020

Redo of #34589 which broke jasmine_node_test and was reverted.

Added a commit tests for this regression which will fail without the fix that is coming in rules_nodejs.

@googlebot googlebot added the cla: yes label Jan 11, 2020
@gregmagolan gregmagolan force-pushed the gregmagolan:rules_nodejs_1.0_redo branch 3 times, most recently from de6cf2e to 1698621 Jan 11, 2020
@gregmagolan gregmagolan force-pushed the gregmagolan:rules_nodejs_1.0_redo branch from b268283 to 6e5516e Jan 12, 2020
@ngbot ngbot bot added this to the needsTriage milestone Jan 12, 2020
@gregmagolan

This comment has been minimized.

Copy link
Contributor Author

gregmagolan commented Jan 12, 2020

Manually verified that a jasmine_node_test can fail:

PASSING TEST

greg@Air-Force-One angular (rules_nodejs_1.0_redo) $ yarn bazel test --config=ivy packages/core/test/acceptance
yarn run v1.21.1
$ bazel test --config=ivy packages/core/test/acceptance
INFO: Writing tracer profile to '/private/var/tmp/_bazel_greg/5e8f8a9cd1c6fbc1afd11e37ee1fe2e5/command.profile.gz'
INFO: Options provided by the client:
  Inherited 'common' options: --isatty=1 --terminal_columns=243
INFO: Reading rc options for 'test' from /Users/greg/google/angular/.bazelrc:
  Inherited 'common' options: --experimental_allow_incremental_repository_updates
INFO: Reading rc options for 'test' from /Users/greg/.bazelrc:
  Inherited 'common' options: --announce_rc
INFO: Reading rc options for 'test' from /Users/greg/google/angular/.bazelrc:
  Inherited 'build' options: --symlink_prefix=dist/ --nolegacy_external_runfiles --incompatible_strict_action_env --define=angular_ivy_enabled=False
INFO: Reading rc options for 'test' from /Users/greg/google/angular/.bazelrc:
  'test' options: --nolegacy_external_runfiles --incompatible_strict_action_env --test_output=errors
INFO: Reading rc options for 'test' from /Users/greg/.bazelrc:
  'test' options: --test_output=errors
INFO: Found applicable config definition build:ivy in file /Users/greg/google/angular/.bazelrc: --define=angular_ivy_enabled=True
INFO: Build option --action_env has changed, discarding analysis cache.
INFO: Analyzed target //packages/core/test/acceptance:acceptance (304 packages loaded, 11869 targets configured).
INFO: Found 1 test target...
Target //packages/core/test/acceptance:acceptance up-to-date:
  dist/bin/packages/core/test/acceptance/acceptance.sh
  dist/bin/packages/core/test/acceptance/acceptance_loader.js
  dist/bin/packages/core/test/acceptance/acceptance_require_patch.js
INFO: Elapsed time: 63.070s, Critical Path: 28.34s
INFO: 19 processes: 3 darwin-sandbox, 16 worker.
INFO: Build completed successfully, 20 total actions
//packages/core/test/acceptance:acceptance                               PASSED in 7.4s

Executed 1 out of 1 test: 1 test passes.
INFO: Build completed successfully, 20 total actions
✨  Done in 63.29s.

FORCED TO FAIL:

greg@Air-Force-One angular (rules_nodejs_1.0_redo) $ yarn bazel test --config=ivy packages/core/test/acceptance
yarn run v1.21.1
$ bazel test --config=ivy packages/core/test/acceptance
INFO: Writing tracer profile to '/private/var/tmp/_bazel_greg/5e8f8a9cd1c6fbc1afd11e37ee1fe2e5/command.profile.gz'
INFO: Options provided by the client:
  Inherited 'common' options: --isatty=1 --terminal_columns=243
INFO: Reading rc options for 'test' from /Users/greg/google/angular/.bazelrc:
  Inherited 'common' options: --experimental_allow_incremental_repository_updates
INFO: Reading rc options for 'test' from /Users/greg/.bazelrc:
  Inherited 'common' options: --announce_rc
INFO: Reading rc options for 'test' from /Users/greg/google/angular/.bazelrc:
  Inherited 'build' options: --symlink_prefix=dist/ --nolegacy_external_runfiles --incompatible_strict_action_env --define=angular_ivy_enabled=False
INFO: Reading rc options for 'test' from /Users/greg/google/angular/.bazelrc:
  'test' options: --nolegacy_external_runfiles --incompatible_strict_action_env --test_output=errors
INFO: Reading rc options for 'test' from /Users/greg/.bazelrc:
  'test' options: --test_output=errors
INFO: Found applicable config definition build:ivy in file /Users/greg/google/angular/.bazelrc: --define=angular_ivy_enabled=True
INFO: Analyzed target //packages/core/test/acceptance:acceptance (1 packages loaded, 72 targets configured).
INFO: Found 1 test target...
[140 / 141] [Prepa] Testing //packages/core/test/acceptance:acceptance

FAIL: //packages/core/test/acceptance:acceptance (see /private/var/tmp/_bazel_greg/5e8f8a9cd1c6fbc1afd11e37ee1fe2e5/execroot/angular/bazel-out/darwin-fastbuild/testlogs/packages/core/test/acceptance/acceptance/test.log)
INFO: From Testing //packages/core/test/acceptance:acceptance:
==================== Test output for //packages/core/test/acceptance:acceptance:
Randomized with seed 81737
Started
..............................................................................................................WARNING: sanitizing unsafe URL value javascript:alert("I am a h4xx0rz1!!"); (see http://g.co/ng/security#xss)
.WARNING: sanitizing unsafe URL value javascript:alert("haha, I am taking over your computer!!!"); (see http://g.co/ng/security#xss)
.................................................................WARNING: sanitizing unsafe URL value javascript:alert("haha, I am taking over your computer!!!"); (see http://g.co/ng/security#xss)
..........................................Angular is running in the development mode. Call enableProdMode() to enable the production mode.
......................................................................................F.....WARNING: sanitizing unsafe URL value javascript:true (see http://g.co/ng/security#xss)
...................................................................................................................*............................................................................................................................................................WARNING: sanitizing unsafe URL value javascript:alert(3) (see http://g.co/ng/security#xss)
.WARNING: sanitizing unsafe URL value javascript:alert(1.2) (see http://g.co/ng/security#xss)
.WARNING: sanitizing unsafe URL value javascript:alert(2.2) (see http://g.co/ng/security#xss)
.WARNING: sanitizing unsafe URL value javascript:alert(2.1) (see http://g.co/ng/security#xss)
.WARNING: sanitizing unsafe URL value javascript:alert(1.1) (see http://g.co/ng/security#xss)
.WARNING: sanitizing unsafe URL value javascript:alert(1) (see http://g.co/ng/security#xss)
.WARNING: sanitizing unsafe URL value javascript:alert(2) (see http://g.co/ng/security#xss)
.....................................................................................................................................................................................................................................................................................................................................................................................................

Failures:
1) attribute creation should create an element
  Message:
    Expected 'Hello' to equal 'FORCED FAIL'.
  Stack:
    Error: Expected 'Hello' to equal 'FORCED FAIL'.
        at <Jasmine>
        at UserContext.<anonymous> (packages/core/test/acceptance/attributes_spec.ts:26:23)
        at ZoneDelegate.invoke (packages/zone.js/lib/zone.ts:1135:20)
        at ProxyZoneSpec.onInvoke (packages/zone.js/lib/zone-spec/proxy.ts:129:33)
        at ZoneDelegate.invoke (packages/zone.js/lib/zone.ts:1132:26)
        at Zone.run (packages/zone.js/lib/zone.ts:814:35)
        at runInTestZone (packages/zone.js/lib/jasmine/jasmine.ts:173:28)
        at UserContext.<anonymous> (packages/zone.js/lib/jasmine/jasmine.ts:189:22)
        at <Jasmine>
        at ZoneDelegate.invokeTask (packages/zone.js/lib/zone.ts:1173:25)
        at Zone.runTask (packages/zone.js/lib/zone.ts:864:37)
        at drainMicroTaskQueue (packages/zone.js/lib/zone.ts:1365:23)
        at ZoneTask.invokeTask (packages/zone.js/lib/zone.ts:1271:11)
        at Immediate.ZoneTask.invoke (packages/zone.js/lib/zone.ts:1256:38)
        at Immediate.timer [as _onImmediate] (packages/zone.js/lib/common/timers.ts:34:21)
        at runCallback (timers.js:705:18)
        at tryOnImmediate (timers.js:676:5)
        at processImmediate (timers.js:658:5)
Pending:

1) styling should not set inputs called class if they are not being used in the template
  Temporarily disabled with xit

977 specs, 1 failure, 1 pending spec
Finished in 6.648 seconds
Randomized with seed 81737 (jasmine --random=true --seed=81737)
================================================================================
Target //packages/core/test/acceptance:acceptance up-to-date:
  dist/bin/packages/core/test/acceptance/acceptance.sh
  dist/bin/packages/core/test/acceptance/acceptance_loader.js
  dist/bin/packages/core/test/acceptance/acceptance_require_patch.js
INFO: Elapsed time: 27.964s, Critical Path: 21.10s
INFO: 3 processes: 2 darwin-sandbox, 1 worker.
INFO: Build completed, 1 test FAILED, 3 total actions
//packages/core/test/acceptance:acceptance                               FAILED in 10.2s
  /private/var/tmp/_bazel_greg/5e8f8a9cd1c6fbc1afd11e37ee1fe2e5/execroot/angular/bazel-out/darwin-fastbuild/testlogs/packages/core/test/acceptance/acceptance/test.log

INFO: Build completed, 1 test FAILED, 3 total actions
error Command failed with exit code 3.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
@gregmagolan gregmagolan marked this pull request as ready for review Jan 12, 2020
@gregmagolan gregmagolan requested review from IgorMinar and angular/dev-infra-framework as code owners Jan 12, 2020
matsko added a commit that referenced this pull request Jan 15, 2020
This brings in a few minor fixes including a better way to patch require for bootstrap scripts

Also remove install_source_map_support attribute from nodejs_binary targets This attribute will be removed from nodejs_binary in the future

PR Close #34736
matsko added a commit that referenced this pull request Jan 15, 2020
matsko added a commit that referenced this pull request Jan 15, 2020
…34736)

This removes the churn in the existing bootstrap scripts.

PR Close #34736
matsko added a commit that referenced this pull request Jan 15, 2020
This is recommended in the Bazel docs as $(location) is ambiguous and can mean either $(execpath) or $(rootpath) depending on the context.

PR Close #34736
matsko added a commit that referenced this pull request Jan 15, 2020
matsko added a commit that referenced this pull request Jan 15, 2020
matsko added a commit that referenced this pull request Jan 15, 2020
…4736)

Adds tests to verify that jasmine_node_test targets fail as expected.
This is to catch any future regressions to jasmine_node_test where tests pass silently without executing.
See bazelbuild/rules_nodejs#1540 for an example of a potential regression.

PR Close #34736
matsko added a commit that referenced this pull request Jan 15, 2020
This release resolves the bootstrap require patching issue with jasmine_node_test. Require patches are now included before any bootstrap scripts.

PR Close #34736
@matsko matsko closed this in 93c2df2 Jan 15, 2020
matsko added a commit that referenced this pull request Jan 15, 2020
matsko added a commit that referenced this pull request Jan 15, 2020
The major one that affects the angular repo is the removal of the bootstrap attribute in nodejs_binary, nodejs_test and jasmine_node_test in favor of using templated_args --node_options=--require=/path/to/script. The side-effect of this is that the bootstrap script does not get the require.resolve patches with explicitly loading the targets _loader.js file.

PR Close #34736
matsko added a commit that referenced this pull request Jan 15, 2020
…ng (#34736)

bazel query now using: kind("ng_package|pkg_npm", //packages/...)

PR Close #34736
matsko added a commit that referenced this pull request Jan 15, 2020
…nodejs 1.0.0 (#34736)

For the purposes of the integration test the zone.js script & bundle script tags can just go into the source index.html itself. The purpose of the integration test is is to test @angular/bazel & ng_module & ng_package so there is no need to exercise html_insert_assets in integration/bazel.

PR Close #34736
matsko added a commit that referenced this pull request Jan 15, 2020
This brings in a required fix to allow for 0.0.0 dev version of @angular/core for integration testing. Without this the following error is now observed:

```
This version of CLI is only compatible with Angular versions ^9.0.0-beta || >=9.0.0 <10.0.0,
but Angular version 0.0.0 was found instead.
```

NB: rc.7 breaks the cli-hello-world-ivy-i18n integration test

PR Close #34736
matsko added a commit that referenced this pull request Jan 15, 2020
This brings in a few minor fixes including a better way to patch require for bootstrap scripts

Also remove install_source_map_support attribute from nodejs_binary targets This attribute will be removed from nodejs_binary in the future

PR Close #34736
matsko added a commit that referenced this pull request Jan 15, 2020
matsko added a commit that referenced this pull request Jan 15, 2020
…ional_root_paths (#34736)

PR Close #34736
matsko added a commit that referenced this pull request Jan 15, 2020
matsko added a commit that referenced this pull request Jan 15, 2020
…34736)

This removes the churn in the existing bootstrap scripts.

PR Close #34736
matsko added a commit that referenced this pull request Jan 15, 2020
This is recommended in the Bazel docs as $(location) is ambiguous and can mean either $(execpath) or $(rootpath) depending on the context.

PR Close #34736
matsko added a commit that referenced this pull request Jan 15, 2020
… it ends in `_es5` (#34736)

PR Close #34736
matsko added a commit that referenced this pull request Jan 15, 2020
matsko added a commit that referenced this pull request Jan 15, 2020
matsko added a commit that referenced this pull request Jan 15, 2020
matsko added a commit that referenced this pull request Jan 15, 2020
…4736)

Adds tests to verify that jasmine_node_test targets fail as expected.
This is to catch any future regressions to jasmine_node_test where tests pass silently without executing.
See bazelbuild/rules_nodejs#1540 for an example of a potential regression.

PR Close #34736
matsko added a commit that referenced this pull request Jan 15, 2020
This release resolves the bootstrap require patching issue with jasmine_node_test. Require patches are now included before any bootstrap scripts.

PR Close #34736
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.