Skip to content

refactor(core): Migrate more tests off fakeAsync - #69956

Draft
JeanMeche wants to merge 1 commit into
angular:mainfrom
JeanMeche:fake-async
Draft

refactor(core): Migrate more tests off fakeAsync#69956
JeanMeche wants to merge 1 commit into
angular:mainfrom
JeanMeche:fake-async

Conversation

@JeanMeche

Copy link
Copy Markdown
Member

This will prevent to polute the agent context with outdated/bad practices.

@JeanMeche
JeanMeche marked this pull request as ready for review July 26, 2026 22:02
@JeanMeche
JeanMeche marked this pull request as draft July 26, 2026 22:02
@angular-robot angular-robot Bot added the area: core Issues related to the framework runtime label Jul 26, 2026
@ngbot ngbot Bot added this to the Backlog milestone Jul 26, 2026
);
fixture.detectChanges();
tick();
await fixture.whenStable();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Q: Is detectChanges still necessary at this point since we are adding whenStable?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I was debating with myself what to do with those animation related tests. ATM the changes were purely migrated by Gemini.

@SkyZeroZx SkyZeroZx Jul 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we try to clean it up? We can do it later to improve the output for LLMs (even asking GTP 5.6, it has sometimes written to me using detectChanges instead of whenStable if I don't specify or it doesn't read the most modern tests generated in the project or Angular itself)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah I think cleaning up our unit tests would improve the agent outputs. Looking at some of the tests, not every detectChanges should be removed though we should be a bit careful when migrating them.

@JeanMeche
JeanMeche force-pushed the fake-async branch 4 times, most recently from 7256586 to 6e7c035 Compare July 26, 2026 22:23
@JeanMeche
JeanMeche requested a review from csmick July 26, 2026 22:25
This will prevent to polute the agent context with outdated/bad practices.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: core Issues related to the framework runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants