-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
fix(zone.js): async-test should only call done once #45025
fix(zone.js): async-test should only call done once #45025
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
84587b5
to
c787e82
Compare
`AsyncTestZoneSpec` triggers jasmine `done()` function multiple times and causes warning ``` An asynchronous function called its 'done' callback more than once. This is a bug in the spec, beforeAll, beforeEach, afterAll, or afterEach function in question. This will be treated as an error in a future version. See<https://jasmine.github.io/tutorials/upgrading_to_Jasmine_4.0#deprecations-due-to-calling-done-multiple-times> for more information ``` The reproduce case will be running some `Zone.run()` inside `waitForAsync()`. ``` it('multiple done', waitForAsync(() => { Zone.current.run(() => {}); Zone.current.run(() => {}); })); ``` The reason the `done()` is called in the `onInvoke()` hook is to handle the case that the testBody is totally sync, but we should only do this check for the entry function not for all `Zone.run()` scenario. Another issue is if we run nested zone inside `waitForAsync()`, the `onHasTask()` hook will be triggered multiple times, and cause `done()` be triggered multiple times, so we need to only trigger the `done()` when the zone is `AsyncTestZone`.
…times Several test cases trigger the `done()` multiple times and cause warning from jasmine. This commit fixes these test cases.
c787e82
to
4d6674c
Compare
Presubmit passed. Marking as merge-ready as discussed. |
This PR was merged into the repository by commit d5565cc. |
@JiaLiPassion @alxhub is it possible to include this fix in the v13 branch? We're stuck to an older jasmine version because of it, and getting to v14 will probably take a while until the entire ecosystem catches up. |
@coyoteecd, this fix need to be released with new version of |
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/animations](https://github.com/angular/angular) | dependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2fanimations/13.2.4/13.2.5) | | [@angular/common](https://github.com/angular/angular) | dependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2fcommon/13.2.4/13.2.5) | | [@angular/compiler](https://github.com/angular/angular) | dependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/13.2.4/13.2.5) | | [@angular/compiler-cli](https://github.com/angular/angular) | devDependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/13.2.4/13.2.5) | | [@angular/core](https://github.com/angular/angular) | dependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2fcore/13.2.4/13.2.5) | | [@angular/forms](https://github.com/angular/angular) | dependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2fforms/13.2.4/13.2.5) | | [@angular/platform-browser](https://github.com/angular/angular) | dependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/13.2.4/13.2.5) | | [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/13.2.4/13.2.5) | | [@angular/router](https://github.com/angular/angular) | dependencies | patch | [`13.2.4` -> `13.2.5`](https://renovatebot.com/diffs/npm/@angular%2frouter/13.2.4/13.2.5) | | [zone.js](https://github.com/angular/angular) ([changelog](https://github.com/angular/angular/blob/master/packages/zone.js/CHANGELOG.md)) | dependencies | patch | [`0.11.4` -> `0.11.5`](https://renovatebot.com/diffs/npm/zone.js/0.11.4/0.11.5) | --- ### Release Notes <details> <summary>angular/angular (@​angular/animations)</summary> ### [`v13.2.5`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#​1325-2022-03-02) [Compare Source](angular/angular@13.2.4...13.2.5) ##### animations | Commit | Type | Description | | -- | -- | -- | | [6c61d20476](angular/angular@6c61d20) | fix | allow animations with unsupported CSS properties ([#​45185](angular/angular#45185)) | ##### common | Commit | Type | Description | | -- | -- | -- | | [64da1daa78](angular/angular@64da1da) | fix | canceled JSONP requests won't throw console error with missing callback function ([#​36807](angular/angular#36807)) | | [56ca7d385b](angular/angular@56ca7d3) | perf | make `NgLocalization` token tree-shakable ([#​45118](angular/angular#45118)) ([#​45226](angular/angular#45226)) | ##### compiler-cli | Commit | Type | Description | | -- | -- | -- | | [6c906a5bb9](angular/angular@6c906a5) | fix | Support resolve animation name from the DTS ([#​45169](angular/angular#45169)) | ##### core | Commit | Type | Description | | -- | -- | -- | | [e8fd452bd2](angular/angular@e8fd452) | fix | remove individual commands for updating gold files ([#​45198](angular/angular#45198)) | | [82d772857c](angular/angular@82d7728) | perf | make `Compiler`, `ApplicationRef` and `ApplicationInitStatus` tree-shakable ([#​45102](angular/angular#45102)) ([#​45222](angular/angular#45222)) | | [71ff12c1cc](angular/angular@71ff12c) | perf | make `LOCALE_ID` and other tokens from `ApplicationModule` tree-shakable ([#​45102](angular/angular#45102)) ([#​45222](angular/angular#45222)) | ##### localize | Commit | Type | Description | | -- | -- | -- | | [d388522745](angular/angular@d388522) | fix | avoid imports into `compiler-cli` package ([#​45180](angular/angular#45180)) | #### Special Thanks Andrew Kushnir, Andrew Scott, Charles Lyding, Guillaume Bonnet, Jessica Janiuk, JoostK, Martin Sikora, Paul Gschwendtner, Theodore Brown, dario-piotrowicz and ivanwonder <!-- CHANGELOG SPLIT MARKER --> </details> <details> <summary>angular/angular (zone.js)</summary> ### [`v0.11.5`](https://github.com/angular/angular/blob/HEAD/packages/zone.js/CHANGELOG.md#​0115-httpsgithubcomangularangularcomparezonejs-0114zonejs-0115-2022-03-03) [Compare Source](angular/angular@zone.js-0.11.4...zone.js-0.11.5) ##### Bug Fixes - **zone.js:** async-test should only call done once ([#​45025](angular/angular#45025)) ([dea7234](angular/angular@dea7234)) - **zone.js:** defineProperties should also set symbol props ([#​45098](angular/angular#45098)) ([b437d12](angular/angular@b437d12)), closes [#​44095](angular/angular#44095) - **zone.js:** fix several test cases which trigger `done()` multiple times ([#​45025](angular/angular#45025)) ([d5565cc](angular/angular@d5565cc)) - **zone.js:** only one listener should also re-throw an error correctly ([#​41868](angular/angular#41868)) ([299f92c](angular/angular@299f92c)), closes [#​41867](angular/angular#41867) [/github.com/angular/angular/pull/41562#issuecomment-822696973](https://github.com//github.com/angular/angular/pull/41562/issues/issuecomment-822696973) - **zone.js:** patch global instead of Mocha object ([#​45047](angular/angular#45047)) ([8efbdb5](angular/angular@8efbdb5)), closes [#​42834](angular/angular#42834) - **zone.js:** should continue to executue listeners when throw error ([#​41562](angular/angular#41562)) ([008eaf3](angular/angular@008eaf3)), closes [#​41522](angular/angular#41522) - **zone.js:** update several flaky cases ([#​41526](angular/angular#41526)) ([25a83eb](angular/angular@25a83eb)), closes [#​41434](angular/angular#41434) ##### Features - **zone.js:** add Promise.any() implementation ([#​45064](angular/angular#45064)) ([4d494d2](angular/angular@4d494d2)), closes [#​44393](angular/angular#44393) - **zone.js:** update electron patch to support electron/remote 14 ([#​45073](angular/angular#45073)) ([d65706a](angular/angular@d65706a)), closes [#​43346](angular/angular#43346) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1197 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
`AsyncTestZoneSpec` triggers jasmine `done()` function multiple times and causes warning ``` An asynchronous function called its 'done' callback more than once. This is a bug in the spec, beforeAll, beforeEach, afterAll, or afterEach function in question. This will be treated as an error in a future version. See<https://jasmine.github.io/tutorials/upgrading_to_Jasmine_4.0#deprecations-due-to-calling-done-multiple-times> for more information ``` The reproduce case will be running some `Zone.run()` inside `waitForAsync()`. ``` it('multiple done', waitForAsync(() => { Zone.current.run(() => {}); Zone.current.run(() => {}); })); ``` The reason the `done()` is called in the `onInvoke()` hook is to handle the case that the testBody is totally sync, but we should only do this check for the entry function not for all `Zone.run()` scenario. Another issue is if we run nested zone inside `waitForAsync()`, the `onHasTask()` hook will be triggered multiple times, and cause `done()` be triggered multiple times, so we need to only trigger the `done()` when the zone is `AsyncTestZone`. PR Close angular#45025
…times (angular#45025) Several test cases trigger the `done()` multiple times and cause warning from jasmine. This commit fixes these test cases. PR Close angular#45025
AsyncTestZoneSpec
triggers jasminedone()
function multiple timesand causes warning
The reproduce case will be running some
Zone.run()
insidewaitForAsync()
.The reason the
done()
is called in theonInvoke()
hook is to handlethe case that the testBody is totally sync, but we should only do this
check for the entry function not for all
Zone.run()
scenario.Another issue is if we have nested zone inside
waitForAsync()
, theonHasTask()
hook will be triggered multiple times and cause thedone()
callback to be triggered multiple times, so we should only trigger it
when the
zone
isAsyncTestZone
.This fix will also fix a lot of flaky errors caused by
done()
is beinginvoked too earlier.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
related to this PR #44923