feat(@schematics/angular): migrate fakeAsync's flush behavior when used in beforeEach#33135
Conversation
angular-robot
Bot
added
detected: feature
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the fakeAsync transformer to mimic Angular's flush behavior in beforeEach hooks by appending await vi.runOnlyPendingTimersAsync(). It also adds logic to detect and respect the { flush: false } option. Feedback was provided to improve the robustness of the AST parsing by avoiding getText() when checking for the flush property, suggesting the use of the text property and ts.SyntaxKind instead.
| }, | ||
| { | ||
| description: | ||
| 'should transform fakeAsync test to `vi.useFakeTimers()` in `beforeEach`, `afterEach`, `beforeAll`, `afterAll`', |
There was a problem hiding this comment.
I split the different hooks in different tests as beforeEach diverges from others.
| let count = 0; | ||
| beforeEach(async () => { | ||
| setTimeout(() => ++count, 100); | ||
| await vi.runOnlyPendingTimersAsync(); |
There was a problem hiding this comment.
This is what mimics best fakeAsync's flush option behavior.
I've only enabled it for beforeEach as it's the hook that is most likely to break tests if timers are not flushed.
I've decided not to apply it to tests and other hooks to reduce code pollution. On the other hand, it is also possible that some afterEach hooks make assertions that assume timers are flushed in tests.
While writing this, I realize that it would be more consistent to flush pending timers everywhere and not just in beforeEach hook.
@atscott @clydin what do you think?
| }, | ||
| { | ||
| description: | ||
| 'should not append `vi.runOnlyPendingTimersAsync()` if `flush` option is set to false', |
There was a problem hiding this comment.
To keep the migration simple, we only detect object expression with false as a literal.
clydin
added
the
action: review
clydin
added
action: merge
|
This PR was merged into the repository. The changes were merged into the following branches:
|
3 participants
PR Checklist
Please check to confirm your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information