Skip to content

Fix test imports, add !, and restore OnPush change detection#1481

Merged
josemontespg merged 9 commits into
google:mainfrom
josemontespg:web-core-tests-in-1p
May 26, 2026
Merged

Fix test imports, add !, and restore OnPush change detection#1481
josemontespg merged 9 commits into
google:mainfrom
josemontespg:web-core-tests-in-1p

Conversation

@josemontespg
Copy link
Copy Markdown
Collaborator

@josemontespg josemontespg commented May 21, 2026

Fixes test imports to use namespace imports, adds non-null assertions (!) to restore successful compilation and passing tests under 1P / google3 environment, and removes ChangeDetectionStrategy overrides in tests while restoring components/samples to OnPush.

Copy link
Copy Markdown
Contributor

@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 node:assert imports and adds non-null assertion operators in test files. The reviewer identified a violation of the repository's style guide regarding the PR description checklist and suggested a refactor to improve code consistency and readability.

Comment thread renderers/web_core/src/v0_8/data/guards.test.ts
Comment thread renderers/web_core/src/v0_8/data/model-processor.test.ts
@josemontespg josemontespg requested a review from gspencergoog May 21, 2026 22:56
@josemontespg josemontespg changed the title Fix test imports and add ! to fix 1P roll Fix test imports, add !, and restore OnPush change detection May 22, 2026
})
.overrideComponent(Card, {
set: {
changeDetection: ChangeDetectionStrategy.Default,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wasn't the problem of this change that the google3 defaults are different than the open source ones?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The problem is that ChangeDetectionStrategy.Default does not exist in google3. Only Eager and OnPush.
These tests don't really need to override the change detection, so we can just remove these.

@josemontespg josemontespg requested a review from ditman May 26, 2026 17:19
Copy link
Copy Markdown
Collaborator

@ditman ditman left a comment

Choose a reason for hiding this comment

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

LGTM! Good luck with the OnPush!

@josemontespg josemontespg merged commit 0e03757 into google:main May 26, 2026
27 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in A2UI May 26, 2026
@google google deleted a comment from sarashed731-prog May 26, 2026
suyangw-g pushed a commit to suyangw-g/A2UI that referenced this pull request May 26, 2026
…1481)

* Fix test imports and add ! to fix 1P roll

* Use ChangeDetectionStrategy.Eager in tests

* test(angular): remove ChangeDetectionStrategy overrides in tests and revert components/samples back to OnPush

* Fix failing PR actions: format Spec files after removing ChangeDetectionStrategy overrides
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants