Skip to content

feat(@schematics/angular): migrate fakeAsync's flush behavior when used in beforeEach#33135

Open
yjaaidi wants to merge 1 commit intoangular:mainfrom
yjaaidi:feat/migrate-fake-async-flush-behavior
Open

feat(@schematics/angular): migrate fakeAsync's flush behavior when used in beforeEach#33135
yjaaidi wants to merge 1 commit intoangular:mainfrom
yjaaidi:feat/migrate-fake-async-flush-behavior

Conversation

@yjaaidi
Copy link
Copy Markdown
Contributor

@yjaaidi yjaaidi commented May 6, 2026

PR Checklist

Please check to confirm your PR fulfills the following requirements:

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
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@angular-robot angular-robot Bot added detected: feature PR contains a feature commit area: @schematics/angular labels May 6, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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`',
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I split the different hooks in different tests as beforeEach diverges from others.

let count = 0;
beforeEach(async () => {
setTimeout(() => ++count, 100);
await vi.runOnlyPendingTimersAsync();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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',
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep the migration simple, we only detect object expression with false as a literal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant