Skip to content
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

(core/testing) flush() not working when requestAnimationFrame contexts #43875

Open
ver-1000000 opened this issue Oct 18, 2021 · 6 comments
Open
Assignees
Labels
area: testing Issues related to Angular testing features, such as TestBed area: zones P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Milestone

Comments

@ver-1000000
Copy link

ver-1000000 commented Oct 18, 2021

Which @angular/* package(s) are the source of the bug?

core

Is this a regression?

No

Description

The requestAnimationFrame is one of the same macrotasks as setTimeout, but it is not drained by the flush() function.

20211018211946

Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/node-qvczbt?file=src/app/app.component.spec.ts

Please provide the exception or error you saw

Chrome 94.0.4606.81 (Linux x86_64) AppComponent should increased number after drawn FAILED

Please provide the environment you discovered this bug in

❯ ng version

     _                      _                 ____ _     ___
    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/
    

Angular CLI: 12.2.10
Node: 14.16.0
Package Manager: npm 7.17.0
OS: linux x64

Angular: 12.2.10
... animations, cli, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1202.10
@angular-devkit/build-angular   12.2.10
@angular-devkit/core            12.2.10
@angular-devkit/schematics      12.2.10
@schematics/angular             12.2.10
rxjs                            6.6.7
typescript                      4.3.5
    

~/projects/node-qvczbt

from stackblitz WebContainer.

Anything else?

Work tick(16), but flush() should also work.

related

@petebacondarwin
Copy link
Member

I believe this is by design:

// flush only non-periodic timers.
// If the only remaining tasks are periodic(or requestAnimationFrame), finish flushing.
if (this._schedulerQueue.filter(task => !task.isPeriodic && !task.isRequestAnimationFrame)
.length === 0) {
break;
}

If you want to cause non-timer macrotasks to resolve then you can do tick(), I believe.

@JiaLiPassion JiaLiPassion self-assigned this Oct 18, 2021
@ver-1000000
Copy link
Author

@petebacondarwin

Oh, I see...
I was a bit confused.

I guess users don't know that you need to use tick(16) when you use rAF?

@petebacondarwin
Copy link
Member

You are correct that the API docs do not mention this aspect. By the way do you have to do tick(16) or is tick() enough?

@AndrewKushnir AndrewKushnir added area: core Issues related to the framework runtime comp: docs area: testing Issues related to Angular testing features, such as TestBed labels Oct 18, 2021
@ngbot ngbot bot modified the milestone: needsTriage Oct 18, 2021
@ver-1000000
Copy link
Author

By the way do you have to do tick(16) or is tick() enough?

In order for rAF to pass the test, tick(16) is essential, not tick().

I believe this is because rAF is executed 60 times per second (i.e. it takes 16.66ms).

(If I could, I would like to see an API that can solve asynchronous processing without relying on such magic-numbers.)

tick(16) passed image
tick() failed image

@petebacondarwin
Copy link
Member

So it is the case that when you request an animation frame from within fakeAsync then a timer is scheduled to resolve in 16ms time.

case 'requestAnimationFrame':
case 'webkitRequestAnimationFrame':
case 'mozRequestAnimationFrame':
// Simulate a requestAnimationFrame by using a setTimeout with 16 ms.
// (60 frames per second)
task.data!['handleId'] = this._setTimeout(
task.invoke, 16, (task.data as any)['args'],
this.trackPendingRequestAnimationFrame);

Further since rAF are treated diifferently to setTimeout they cannot be triggered simply by a call to flush() as discussed above.

@pkozlowski-opensource pkozlowski-opensource added area: zones and removed area: core Issues related to the framework runtime labels Oct 27, 2021
@JiaLiPassion
Copy link
Contributor

I think this is a bug, since the current logic is not consistent and not good for performance either.
tick(16) will trigger both macroTask and requestAnimationFrame, but flush() is not, also flush() will not trigger period macroTask such as setInterval which is also not a correct behavior and inconsistent with tick(). I will create a PR to fix this one.

@alxhub alxhub added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label Nov 16, 2022
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: testing Issues related to Angular testing features, such as TestBed area: zones P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants