Skip to content

Commit

Permalink
feat(core): Add 'flush' parameter option to fakeAsync to flush after …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
atscott committed Aug 2, 2024
1 parent c2779eb commit 08469aa
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 28 deletions.
4 changes: 3 additions & 1 deletion goldens/public-api/core/testing/index.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ export { DeferBlockState }
export function discardPeriodicTasks(): void;

// @public
export function fakeAsync(fn: Function): (...args: any[]) => any;
export function fakeAsync(fn: Function, options?: {
flush?: boolean;
}): (...args: any[]) => any;

// @public
export function flush(maxTurns?: number): number;
Expand Down
2 changes: 1 addition & 1 deletion integration/typings_test_ts55/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"@types/jasmine": "file:../../node_modules/@types/jasmine",
"rxjs": "file:../../node_modules/rxjs",
"typescript": "5.5.2",
"zone.js": "0.14.0"
"zone.js": "0.15.0"
},
"scripts": {
"test": "tsc"
Expand Down
2 changes: 1 addition & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
},
"peerDependencies": {
"rxjs": "^6.5.3 || ^7.4.0",
"zone.js": "~0.14.0"
"zone.js": "~0.15.0"
},
"repository": {
"type": "git",
Expand Down
18 changes: 12 additions & 6 deletions packages/core/test/fake_async_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,17 +204,23 @@ describe('fake async', () => {

it('should throw an error on dangling timers', () => {
expect(() => {
fakeAsync(() => {
setTimeout(() => {}, 10);
})();
fakeAsync(
() => {
setTimeout(() => {}, 10);
},
{flush: false},
)();
}).toThrowError('1 timer(s) still in the queue.');
});

it('should throw an error on dangling periodic timers', () => {
expect(() => {
fakeAsync(() => {
setInterval(() => {}, 10);
})();
fakeAsync(
() => {
setInterval(() => {}, 10);
},
{flush: false},
)();
}).toThrowError('1 periodic timer(s) still in the queue.');
});

Expand Down
9 changes: 5 additions & 4 deletions packages/core/testing/src/fake_async.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,12 @@ export function resetFakeAsyncZoneIfExists(): void {
* - Microtasks are manually executed by calling `flushMicrotasks()`.
* - Timers are synchronous; `tick()` simulates the asynchronous passage of time.
*
* If there are any pending timers at the end of the function, an exception is thrown.
*
* Can be used to wrap `inject()` calls.
*
* @param fn The function that you want to wrap in the `fakeAsync` zone.
* @param options
* - flush: When true, will drain the macrotask queue after the test function completes.
* When false, will throw an exception at the end of the function if there are pending timers.
*
* @usageNotes
* ### Example
Expand All @@ -53,9 +54,9 @@ export function resetFakeAsyncZoneIfExists(): void {
*
* @publicApi
*/
export function fakeAsync(fn: Function): (...args: any[]) => any {
export function fakeAsync(fn: Function, options?: {flush?: boolean}): (...args: any[]) => any {
if (fakeAsyncTestModule) {
return fakeAsyncTestModule.fakeAsync(fn);
return fakeAsyncTestModule.fakeAsync(fn, options);
}
throw new Error(fakeAsyncTestModuleNotLoadedErrorMessage);
}
Expand Down
40 changes: 25 additions & 15 deletions packages/zone.js/lib/zone-spec/fake-async-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ class FakeAsyncTestZoneSpec implements ZoneSpec {
}
}

let _fakeAsyncTestZoneSpec: any = null;
let _fakeAsyncTestZoneSpec: FakeAsyncTestZoneSpec | null = null;

type ProxyZoneSpecType = {
setDelegate(delegateSpec: ZoneSpec): void;
Expand Down Expand Up @@ -816,7 +816,8 @@ export function resetFakeAsyncZone() {
* - microtasks are manually executed by calling `flushMicrotasks()`,
* - timers are synchronous, `tick()` simulates the asynchronous passage of time.
*
* If there are any pending timers at the end of the function, an exception will be thrown.
* When flush is `false`, if there are any pending timers at the end of the function,
* an exception will be thrown.
*
* Can be used to wrap inject() calls.
*
Expand All @@ -825,11 +826,14 @@ export function resetFakeAsyncZone() {
* {@example core/testing/ts/fake_async.ts region='basic'}
*
* @param fn
* @param options
* flush: when true, will drain the macrotask queue after the test function completes.
* @returns The function wrapped to be executed in the fakeAsync zone
*
* @experimental
*/
export function fakeAsync(fn: Function): (...args: any[]) => any {
export function fakeAsync(fn: Function, options: {flush?: boolean} = {}): (...args: any[]) => any {
const {flush = false} = options;
// Not using an arrow function to preserve context passed from call site
const fakeAsyncFn: any = function (this: unknown, ...args: any[]) {
const ProxyZoneSpec = getProxyZoneSpec();
Expand All @@ -851,7 +855,7 @@ export function fakeAsync(fn: Function): (...args: any[]) => any {
throw new Error('fakeAsync() calls can not be nested');
}

_fakeAsyncTestZoneSpec = new FakeAsyncTestZoneSpec();
_fakeAsyncTestZoneSpec = new FakeAsyncTestZoneSpec() as FakeAsyncTestZoneSpec;
}

let res: any;
Expand All @@ -860,22 +864,28 @@ export function fakeAsync(fn: Function): (...args: any[]) => any {
_fakeAsyncTestZoneSpec.lockDatePatch();
try {
res = fn.apply(this, args);
flushMicrotasks();
if (flush) {
_fakeAsyncTestZoneSpec.flush(20, true);
} else {
flushMicrotasks();
}
} finally {
proxyZoneSpec.setDelegate(lastProxyZoneSpec);
}

if (_fakeAsyncTestZoneSpec.pendingPeriodicTimers.length > 0) {
throw new Error(
`${_fakeAsyncTestZoneSpec.pendingPeriodicTimers.length} ` +
`periodic timer(s) still in the queue.`,
);
}
if (!flush) {
if (_fakeAsyncTestZoneSpec.pendingPeriodicTimers.length > 0) {
throw new Error(
`${_fakeAsyncTestZoneSpec.pendingPeriodicTimers.length} ` +
`periodic timer(s) still in the queue.`,
);
}

if (_fakeAsyncTestZoneSpec.pendingTimers.length > 0) {
throw new Error(
`${_fakeAsyncTestZoneSpec.pendingTimers.length} timer(s) still in the queue.`,
);
if (_fakeAsyncTestZoneSpec.pendingTimers.length > 0) {
throw new Error(
`${_fakeAsyncTestZoneSpec.pendingTimers.length} timer(s) still in the queue.`,
);
}
}
return res;
} finally {
Expand Down

0 comments on commit 08469aa

Please sign in to comment.