fix: should not tick nested timer in fakeAsync test by JiaLiPassion · Pull Request #33838 · angular/angular

JiaLiPassion · 2019-11-15T00:37:13Z

Close #33799

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

Issue Number: #33799

function waitTwice(callback: () => any): void {
  setTimeout(() =>
    setTimeout(() => callback())
  );
}

describe('fakeAsync', () => {
  it('nested timers', fakeAsync(() => {
    const callback = jasmine.createSpy('callback');
    waitTwice(callback);
    expect(callback).not.toHaveBeenCalled();
    tick();
    expect(callback).toHaveBeenCalled();
    tick();
    expect(callback).toHaveBeenCalled();
  }));
});

By default,
The callback will be called after the first tick.

describe('fakeAsync', () => {
  it('nested timers', fakeAsync(() => {
    const callback = jasmine.createSpy('callback');
    waitTwice(callback);
    expect(callback).not.toHaveBeenCalled();
    tick(0, true);
    expect(callback).not.toHaveBeenCalled();
    tick();
    expect(callback).toHaveBeenCalled();
  }));
});

## What is the new behavior?
with `tick(0, true)`, the second parameter is `ignoreNestedTimeout`, after the `first tick`, the `nested timeout` will not be invoked.

## Does this PR introduce a breaking change?

- [ ] Yes
- [x] No

mhevery · 2019-11-18T20:07:09Z

presubmit

mhevery · 2019-11-18T20:41:36Z

So this change is breaky in g3 where 30+ targets fail just in blueprint.

mhevery

could we add some API docs to the change?

petebacondarwin

The last "fix: format" commit should be a fixup commit for the previous commit

JiaLiPassion · 2019-12-04T08:20:28Z

@mhevery , @petebacondarwin , thank you for the review, I have updated the PR, use new API instead of breaking changes. Please review again, thanks!

petebacondarwin

LGTM

mhevery · 2020-02-11T18:04:44Z

presubmit

…Synchronously. This option will control whether to invoke the new macro tasks when ticking. Close angular#33799

mary-poppins · 2020-02-12T01:32:23Z

You can preview 284f8b2 at https://pr33838-284f8b2.ngbuilds.io/.

mhevery · 2020-02-20T19:24:45Z

presubmit

…Synchronously. (#33838) This option will control whether to invoke the new macro tasks when ticking. Close #33799 PR Close #33838

…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.

…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

…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.

…local packages (#35858) 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 #35858

angular-automatic-lock-bot · 2020-03-22T16:06:35Z

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.}

JiaLiPassion requested a review from a team November 15, 2019 00:37

googlebot added the cla: yes label Nov 15, 2019

JiaLiPassion force-pushed the fakeasync-nested-timeout branch from 461d422 to ef9a2c1 Compare November 15, 2019 02:48

JiaLiPassion requested review from a team and mhevery November 15, 2019 02:48

JiaLiPassion added area: testing Issues related to Angular testing features, such as TestBed area: zones Issues related to zone.js breaking changes risk: high labels Nov 15, 2019

ngbot Bot modified the milestone: needsTriage Nov 15, 2019

mhevery reviewed Nov 18, 2019

View reviewed changes

Comment thread packages/zone.js/lib/zone-spec/fake-async-test.ts Outdated

petebacondarwin reviewed Dec 2, 2019

View reviewed changes

JiaLiPassion force-pushed the fakeasync-nested-timeout branch from 4ad7ef7 to 69c7eda Compare December 2, 2019 15:57

JiaLiPassion requested review from a team December 3, 2019 23:56

JiaLiPassion force-pushed the fakeasync-nested-timeout branch 2 times, most recently from 094d6dc to 7b39c5b Compare December 4, 2019 01:26

JiaLiPassion removed breaking changes risk: high labels Dec 4, 2019

JiaLiPassion force-pushed the fakeasync-nested-timeout branch from 7b39c5b to 111ddf4 Compare December 4, 2019 05:50

JiaLiPassion requested a review from a team December 4, 2019 05:50

JiaLiPassion force-pushed the fakeasync-nested-timeout branch 2 times, most recently from 5f05545 to 6ad4193 Compare December 4, 2019 07:29

petebacondarwin approved these changes Dec 4, 2019

View reviewed changes

kara added the action: review The PR is still awaiting reviews from at least one requested reviewer label Feb 11, 2020

feat: add an tickOptions parameter with property processNewMacroTasks…

284f8b2

…Synchronously. This option will control whether to invoke the new macro tasks when ticking. Close angular#33799

JiaLiPassion force-pushed the fakeasync-nested-timeout branch from 246b0bb to 284f8b2 Compare February 12, 2020 01:22

mhevery removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Feb 20, 2020

mhevery pushed a commit that referenced this pull request Feb 20, 2020

feat: add an tickOptions parameter with property processNewMacroTasks…

7d2ea93

…Synchronously. (#33838) This option will control whether to invoke the new macro tasks when ticking. Close #33799 PR Close #33838

mhevery closed this in 17b862c Feb 20, 2020

This was referenced Feb 26, 2020

Edit copy for tick function #35696

Closed

docs: edit copy for tick() in testing and api docs #35697

Closed

gkalpak mentioned this pull request Feb 29, 2020

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

Closed

angular-automatic-lock-bot Bot locked and limited conversation to collaborators Mar 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: should not tick nested timer in fakeAsync test#33838

fix: should not tick nested timer in fakeAsync test#33838
JiaLiPassion wants to merge 1 commit intoangular:masterfrom
JiaLiPassion:fakeasync-nested-timeout

JiaLiPassion commented Nov 15, 2019 •

edited

Loading

Uh oh!

mhevery commented Nov 18, 2019

Uh oh!

mhevery commented Nov 18, 2019

Uh oh!

mhevery left a comment

Uh oh!

Uh oh!

petebacondarwin left a comment

Uh oh!

JiaLiPassion commented Dec 4, 2019

Uh oh!

petebacondarwin left a comment

Uh oh!

mhevery commented Feb 11, 2020

Uh oh!

mary-poppins commented Feb 12, 2020

Uh oh!

mhevery commented Feb 20, 2020

Uh oh!

angular-automatic-lock-bot Bot commented Mar 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

JiaLiPassion commented Nov 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

PR Type

What is the current behavior?

Uh oh!

mhevery commented Nov 18, 2019

Uh oh!

mhevery commented Nov 18, 2019

Uh oh!

mhevery left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

petebacondarwin left a comment

Choose a reason for hiding this comment

Uh oh!

JiaLiPassion commented Dec 4, 2019

Uh oh!

petebacondarwin left a comment

Choose a reason for hiding this comment

Uh oh!

mhevery commented Feb 11, 2020

Uh oh!

mary-poppins commented Feb 12, 2020

Uh oh!

mhevery commented Feb 20, 2020

Uh oh!

angular-automatic-lock-bot Bot commented Mar 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

JiaLiPassion commented Nov 15, 2019 •

edited

Loading