Skip to content

build(docs-infra): use local version of Zone.js when testing against local packages#35780

Closed
gkalpak wants to merge 4 commits intoangular:masterfrom
gkalpak:build-aio-use-local-zones
Closed

build(docs-infra): use local version of Zone.js when testing against local packages#35780
gkalpak wants to merge 4 commits intoangular:masterfrom
gkalpak:build-aio-use-local-zones

Conversation

@gkalpak
Copy link
Copy Markdown
Member

@gkalpak gkalpak commented Feb 29, 2020

In some cases, we want to test the AIO app or docs examples against the locally built Angular packages (for example to ensure that the changes in a commit do not introduce a breaking change). In order to achieve this, we have the ng-packages-installer script that handles updating a project's package.json file to use the locally built Angular packages (and appropriate versions for their (dev-/peer-)dependencies).

Previously, ng-packages-installer would only consider the locally built Angular packages (from dist/packages-dist/). However, given that Zone.js is now part of the angular/angular repo, it makes sense to also use the locally built Zone.js package (from dist/zone.js-dist/). Otherwise, the tests might fail for commits that update both the Angular packages (and related docs examples) and the Zone.js package. An example of such a simultaneous change (that would have broken tests) is #33838.

This commit updates the script to install the locally built Zone.js package (in addition to the Angular ones). The commit ensures that the Zone.js package will always be available alongside the Angular packages (i.e. that the Zone.js package will be built by the same script that builds the Angular packages and that the dist/zone.js-dist/ directory will be cached on CI).

Note: This problem was discovered while enabling docs examples unit tests in #34374.

@gkalpak gkalpak added area: build & ci Related the build and CI infrastructure of the project comp: docs-infra action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Feb 29, 2020
@ngbot ngbot Bot modified the milestone: needsTriage Feb 29, 2020
@mary-poppins
Copy link
Copy Markdown

@gkalpak gkalpak force-pushed the build-aio-use-local-zones branch from 8367c72 to cc93ccc Compare February 29, 2020 20:39
@pullapprove pullapprove Bot requested a review from mhevery February 29, 2020 20:39
@mary-poppins
Copy link
Copy Markdown

@gkalpak gkalpak force-pushed the build-aio-use-local-zones branch from cc93ccc to 6fdc27f Compare February 29, 2020 21:49
@mary-poppins
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@IgorMinar IgorMinar left a comment

Choose a reason for hiding this comment

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

Lgtm with a nit.

Comment thread scripts/build/build-packages-dist.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Most of us have the original path to this file in our muscle memory. Can you please leave a placeholder script in the old location that will error instructing the developers to use the new path?

Also please leave a comment there that the placeholder can be removed two months from now (early May).

@IgorMinar IgorMinar added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Mar 3, 2020
gkalpak added 3 commits March 4, 2020 15:21
This commit moves the build-related scripts
(`build-ivy-npm-packages.js`, `build-packages-dist.js` and
`package-builder.js`) to a dedicated directory to keep the `scripts/`
directory cleaner.

It also moves the logic for building the `zone.js` package to a separate
script, `zone-js-builder.js`, to make it re-usable. A subsequent commit
will use it to build the `zone.js` package when building the Ivy Angular
packages as well.
…al package detection

Previously, `NgPackagesInstaller` would only look for Angular local
packages and do so by listing all (deeply nested) files in
`dist/packages-dist/` and looking for `package.json` files nested two
levels deep (i.e. `dist/packages-dist/*/package.json`). Thus, it would
unnecessarily check a large number of files.

This commit changes the package detection logic to instead look for
a `package.json` file directly inside each subdirectory of
`dist/packages-dist/`, which speeds up the operation.
It also refactors the code to make it easier to look for packages in
other directories (besides `dist/packages-dist/`). This will be useful
in a subsequent commit to look for and use the locally built `zone.js`
package (from `dist/zone.js-dist/`).
…local packages

In some cases, we want to test the AIO app or docs examples against the
locally built Angular packages (for example to ensure that the changes
in a commit do not introduce a breaking change). In order to achieve
this, we have the `ng-packages-installer` script that handles updating
a project's `package.json` file to use the locally built Angular
packages (and appropriate versions for their (dev-/peer-)dependencies).

Previously, `ng-packages-installer` would only consider the locally
built Angular packages (from `dist/packages-dist/`). However, given that
Zone.js is now part of the `angular/angular` repo, it makes sense to
also use the locally built Zone.js package (from `dist/zone.js-dist/`).
Otherwise, the tests might fail for commits that update both the Angular
packages (and related docs examples) and the Zone.js package. An example
of such a simultaneous change (that would have broken tests) is angular#33838.

This commit updates the script to install the locally built Zone.js
package (in addition to the Angular ones). The commit ensures that the
Zone.js package will always be available alongside the Angular packages
(i.e. that the Zone.js package will be built by the same script that
builds the Angular packages and that the `dist/zone.js-dist/` directory
will be cached on CI).

Note: This problem was discovered while enabling docs examples unit
tests in angular#34374.
@gkalpak gkalpak force-pushed the build-aio-use-local-zones branch from 6fdc27f to e4b198a Compare March 4, 2020 13:48
@gkalpak gkalpak force-pushed the build-aio-use-local-zones branch from e4b198a to 2d9c2f3 Compare March 4, 2020 13:53
@gkalpak gkalpak removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Mar 4, 2020
@mary-poppins
Copy link
Copy Markdown

@gkalpak gkalpak removed the request for review from mhevery March 4, 2020 14:22
@atscott atscott closed this in 3f88de9 Mar 4, 2020
atscott pushed a commit that referenced this pull request Mar 4, 2020
…al package detection (#35780)

Previously, `NgPackagesInstaller` would only look for Angular local
packages and do so by listing all (deeply nested) files in
`dist/packages-dist/` and looking for `package.json` files nested two
levels deep (i.e. `dist/packages-dist/*/package.json`). Thus, it would
unnecessarily check a large number of files.

This commit changes the package detection logic to instead look for
a `package.json` file directly inside each subdirectory of
`dist/packages-dist/`, which speeds up the operation.
It also refactors the code to make it easier to look for packages in
other directories (besides `dist/packages-dist/`). This will be useful
in a subsequent commit to look for and use the locally built `zone.js`
package (from `dist/zone.js-dist/`).

PR Close #35780
atscott pushed a commit that referenced this pull request Mar 4, 2020
…local packages (#35780)

In some cases, we want to test the AIO app or docs examples against the
locally built Angular packages (for example to ensure that the changes
in a commit do not introduce a breaking change). In order to achieve
this, we have the `ng-packages-installer` script that handles updating
a project's `package.json` file to use the locally built Angular
packages (and appropriate versions for their (dev-/peer-)dependencies).

Previously, `ng-packages-installer` would only consider the locally
built Angular packages (from `dist/packages-dist/`). However, given that
Zone.js is now part of the `angular/angular` repo, it makes sense to
also use the locally built Zone.js package (from `dist/zone.js-dist/`).
Otherwise, the tests might fail for commits that update both the Angular
packages (and related docs examples) and the Zone.js package. An example
of such a simultaneous change (that would have broken tests) is #33838.

This commit updates the script to install the locally built Zone.js
package (in addition to the Angular ones). The commit ensures that the
Zone.js package will always be available alongside the Angular packages
(i.e. that the Zone.js package will be built by the same script that
builds the Angular packages and that the `dist/zone.js-dist/` directory
will be cached on CI).

Note: This problem was discovered while enabling docs examples unit
tests in #34374.

PR Close #35780
atscott pushed a commit that referenced this pull request Mar 4, 2020
This commit moves the build-related scripts
(`build-ivy-npm-packages.js`, `build-packages-dist.js` and
`package-builder.js`) to a dedicated directory to keep the `scripts/`
directory cleaner.

It also moves the logic for building the `zone.js` package to a separate
script, `zone-js-builder.js`, to make it re-usable. A subsequent commit
will use it to build the `zone.js` package when building the Ivy Angular
packages as well.

PR Close #35780
atscott pushed a commit that referenced this pull request Mar 4, 2020
…al package detection (#35780)

Previously, `NgPackagesInstaller` would only look for Angular local
packages and do so by listing all (deeply nested) files in
`dist/packages-dist/` and looking for `package.json` files nested two
levels deep (i.e. `dist/packages-dist/*/package.json`). Thus, it would
unnecessarily check a large number of files.

This commit changes the package detection logic to instead look for
a `package.json` file directly inside each subdirectory of
`dist/packages-dist/`, which speeds up the operation.
It also refactors the code to make it easier to look for packages in
other directories (besides `dist/packages-dist/`). This will be useful
in a subsequent commit to look for and use the locally built `zone.js`
package (from `dist/zone.js-dist/`).

PR Close #35780
atscott pushed a commit that referenced this pull request Mar 4, 2020
…local packages (#35780)

In some cases, we want to test the AIO app or docs examples against the
locally built Angular packages (for example to ensure that the changes
in a commit do not introduce a breaking change). In order to achieve
this, we have the `ng-packages-installer` script that handles updating
a project's `package.json` file to use the locally built Angular
packages (and appropriate versions for their (dev-/peer-)dependencies).

Previously, `ng-packages-installer` would only consider the locally
built Angular packages (from `dist/packages-dist/`). However, given that
Zone.js is now part of the `angular/angular` repo, it makes sense to
also use the locally built Zone.js package (from `dist/zone.js-dist/`).
Otherwise, the tests might fail for commits that update both the Angular
packages (and related docs examples) and the Zone.js package. An example
of such a simultaneous change (that would have broken tests) is #33838.

This commit updates the script to install the locally built Zone.js
package (in addition to the Angular ones). The commit ensures that the
Zone.js package will always be available alongside the Angular packages
(i.e. that the Zone.js package will be built by the same script that
builds the Angular packages and that the `dist/zone.js-dist/` directory
will be cached on CI).

Note: This problem was discovered while enabling docs examples unit
tests in #34374.

PR Close #35780
@gkalpak gkalpak deleted the build-aio-use-local-zones branch March 4, 2020 16:37
atscott added a commit to atscott/angular that referenced this pull request Mar 4, 2020
…against local packages (angular#35780)"

This reverts commit 7d832ae; breaks CI
with error `Concurrent upstream jobs persisted the same file(s) into the workspace:`
atscott added a commit that referenced this pull request Mar 4, 2020
…against local packages (#35780)" (#35857)

This reverts commit 7d832ae; breaks CI
with error `Concurrent upstream jobs persisted the same file(s) into the workspace:`

PR Close #35857
atscott added a commit that referenced this pull request Mar 4, 2020
…against local packages (#35780)" (#35857)

This reverts commit 7d832ae; breaks CI
with error `Concurrent upstream jobs persisted the same file(s) into the workspace:`

PR Close #35857
@angular-automatic-lock-bot
Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Apr 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: build & ci Related the build and CI infrastructure of the project cla: yes target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants