Skip to content

test(element): migrate element-ng to vitest part 2#1700

Merged
spike-rabbit merged 1 commit intomainfrom
test/migrate-vitest-part2
Mar 20, 2026
Merged

test(element): migrate element-ng to vitest part 2#1700
spike-rabbit merged 1 commit intomainfrom
test/migrate-vitest-part2

Conversation

@spliffone
Copy link
Member

@spliffone spliffone commented Mar 20, 2026

@spliffone spliffone requested review from a team as code owners March 20, 2026 10:11
Copy link
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 continues the migration of tests from Karma/Jasmine to Vitest for the element-ng project, focusing on components in directories starting with 'd'. The changes correctly update test configurations, replace Jasmine APIs with their Vitest counterparts, and refactor some test logic for stability. The file structure is also improved by moving a test helper. The migration is well-executed, but I have one suggestion to improve test code maintainability by addressing a redundant change detection call.

@spliffone spliffone force-pushed the test/migrate-vitest-part2 branch 2 times, most recently from 42ec329 to 5adb6f2 Compare March 20, 2026 10:26
Copy link
Member

@spike-rabbit spike-rabbit left a comment

Choose a reason for hiding this comment

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

Please double check all new fixture.detectChanges / fixture.whenStable.

I prefer new signals for components instead.

@spliffone spliffone force-pushed the test/migrate-vitest-part2 branch 3 times, most recently from a493714 to 71466db Compare March 20, 2026 13:54
@spliffone spliffone force-pushed the test/migrate-vitest-part2 branch from 71466db to 2600250 Compare March 20, 2026 14:08
Copy link
Member

@spike-rabbit spike-rabbit left a comment

Choose a reason for hiding this comment

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

👍

@spike-rabbit spike-rabbit enabled auto-merge March 20, 2026 14:12
@spike-rabbit spike-rabbit added this pull request to the merge queue Mar 20, 2026
Merged via the queue into main with commit a92d1c5 Mar 20, 2026
11 checks passed
@spike-rabbit spike-rabbit deleted the test/migrate-vitest-part2 branch March 20, 2026 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants