Skip to content

fix(zone.js): Update the default behavior of fakeAsync to flush after the test#57240

Closed
atscott wants to merge 1 commit intoangular:mainfrom
atscott:flipdefaultflush
Closed

fix(zone.js): Update the default behavior of fakeAsync to flush after the test#57240
atscott wants to merge 1 commit intoangular:mainfrom
atscott:flipdefaultflush

Conversation

@atscott
Copy link
Copy Markdown
Contributor

@atscott atscott commented Aug 2, 2024

From the internal issue on the matter:

When using the standard Jasmine version of it promises returned by the body function are automatically awaited. The Catalyst version of it is fake-async, so awaiting the promise does not make sense; however it would be nice if Catalyst automatically flushed the promise to replicate the experience of using standard it. This would allow users to do the following:

it('should fail later', async () => {
  await new Promise(r => setTimeout(r));
  fail('failure');
});

In Catalyst today the above test will pass. If this proposal to automatically flush the resulting promise were implemented it would fail.

Flushing after the tests complete has been the default behavior inside Google since 2020. Very few tests remain that use the old behavior of only flushing microtasks. The example above would actually fail with fakeAsync due to the pending timer, but the argument still remains the same. We might as well just flush if we're going to fail the test anyways by throwing if there's no flush at the end.

BREAKING CHANGE: fakeAsync will now flush pending timers at the end of the given function by default. To opt-out of this, you can use {flush: false} in options parameter of fakeAsync

@atscott atscott added the area: zones Issues related to zone.js label Aug 2, 2024
@ngbot ngbot Bot added this to the Backlog milestone Aug 2, 2024
@pullapprove pullapprove Bot requested a review from JiaLiPassion August 2, 2024 16:32
@angular-robot angular-robot Bot added the detected: breaking change PR contains a commit with a breaking change label Aug 2, 2024
@shannonvc
Copy link
Copy Markdown
Contributor

Typo in the commit message header; fix(zon.js) -> fix(zone.js)

@atscott atscott changed the title fix(zon.js): Update the default behavior of fakeAsync to flush after the test fix(zone.js): Update the default behavior of fakeAsync to flush after the test Aug 3, 2024
… the test

From the internal issue on the matter:

> When using the standard Jasmine version of it promises returned by the body function are automatically awaited. The Catalyst version of it is fake-async, so awaiting the promise does not make sense; however it would be nice if Catalyst automatically flushed the promise to replicate the experience of using standard it. This would allow users to do the following:

```
it('should fail later', async () => {
  await new Promise(r => setTimeout(r));
  fail('failure');
});
```
> In Catalyst today the above test will pass. If this proposal to automatically flush the resulting promise were implemented it would fail.

Flushing after the tests complete has been the default behavior inside
Google since 2020. Very few tests remain that use the old behavior of
only flushing microtasks. The example above would actually fail with
`fakeAsync` due to the pending timer, but the argument still remains the
same. We might as well just flush if we're going to fail the test
anyways by throwing if there's no flush at the end.

BREAKING CHANGE: `fakeAsync` will now flush pending timers at the end of
the given function by default. To opt-out of this, you can use `{flush:
false}` in options parameter of `fakeAsync`
@atscott atscott added the target: minor This PR is targeted for the next minor release label Aug 5, 2024
@thePunderWoman thePunderWoman added the action: merge The PR is ready for merge by the caretaker label Aug 5, 2024
@thePunderWoman
Copy link
Copy Markdown
Contributor

This PR was merged into the repository by commit 70e8b40.

The changes were merged into the following branches: main

@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 Sep 5, 2024
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: zones Issues related to zone.js detected: breaking change PR contains a commit with a breaking change target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants