refactor: port openClose common test helper to browser mode
#13630
Conversation
github-actions
bot
added
the
refactor
jcfranco
added
the
skip visual snapshots
There was a problem hiding this comment.
Pull request overview
This PR migrates the openClose common test helper from Puppeteer-based E2E testing to Vitest browser mode, continuing the modernization effort outlined in issue #11268.
Key Changes
- Removed Puppeteer-based
openClose.tsimplementation - Added new browser-mode implementation in
open-close.ts - Updated export statements to reflect the migration
- Migrated test usage across 13 component test files from Puppeteer to browser mode
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/components/src/tests/commonTests/openClose.ts | Removed legacy Puppeteer implementation |
| packages/components/src/tests/commonTests/index.ts | Removed openClose export from legacy module |
| packages/components/src/tests/commonTests/browser/open-close.ts | New browser-mode implementation |
| packages/components/src/tests/commonTests/browser/index.ts | Added openClose export for browser module |
| packages/components/src/components/tooltip/*.tsx | Migrated tests from Puppeteer to browser mode |
| packages/components/src/components/sort-handle/*.tsx | Migrated tests from Puppeteer to browser mode |
| packages/components/src/components/sheet/*.tsx | Migrated tests from Puppeteer to browser mode |
| packages/components/src/components/popover/*.tsx | Migrated tests from Puppeteer to browser mode |
| packages/components/src/components/notice/*.tsx | Migrated tests from Puppeteer to browser mode |
| packages/components/src/components/input-time-zone/*.tsx | Migrated tests from Puppeteer to browser mode |
| packages/components/src/components/input-time-picker/*.tsx | Migrated tests from Puppeteer to browser mode |
| packages/components/src/components/input-date-picker/*.tsx | Migrated tests from Puppeteer to browser mode |
| packages/components/src/components/dropdown/*.tsx | Migrated tests from Puppeteer to browser mode |
| packages/components/src/components/dialog/*.tsx | Migrated tests from Puppeteer to browser mode |
| packages/components/src/components/combobox/*.tsx | Migrated tests from Puppeteer to browser mode |
| packages/components/src/components/block/*.tsx | Migrated tests from Puppeteer to browser mode |
| packages/components/src/components/autocomplete/*.tsx | Migrated tests from Puppeteer to browser mode |
| packages/components/src/components/alert/*.tsx | Migrated tests from Puppeteer to browser mode |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| try { | ||
| const renderResult = await setup(); | ||
| await setUpEventListeners(renderResult.el.localName as keyof DeclareElements); |
There was a problem hiding this comment.
The type casting to keyof DeclareElements could fail at runtime if renderResult.el.localName is not a recognized component tag. Add validation or use a safer type pattern to prevent invalid component tags from being passed to setUpEventListeners.
| if (startOpen) { | ||
| const component = document.createElement(tag); | ||
| component[openPropName] = true; | ||
|
|
||
| document.body.append(component); | ||
| } | ||
|
|
There was a problem hiding this comment.
When startOpen is true, this creates a duplicate element in addition to the one already created by setup(). The newly created component is appended to document.body but renderResult.el from the setup function is used for the test assertions, potentially causing test failures. Either remove this block or ensure only one element is created.
| if (startOpen) { | |
| const component = document.createElement(tag); | |
| component[openPropName] = true; | |
| document.body.append(component); | |
| } |
| await testOpenCloseEvents({ | ||
| animationsEnabled: !effectiveOptions.willUseFallback, | ||
| collapsedOnClose: effectiveOptions.collapsedOnClose, | ||
| openPropName: effectiveOptions.openPropName!, |
There was a problem hiding this comment.
The non-null assertion operator (!) assumes openPropName is always defined, but this relies on the defaultOptions merge. Consider explicitly checking this value or refining the type definition to make this guarantee explicit.
| describe("openClose", () => { | ||
| openClose(() => mount("calcite-notice")); |
There was a problem hiding this comment.
The migrated test no longer includes the collapsedOnClose: \"vertical\" option that was present in the Puppeteer version. This removes test coverage for vertical collapse behavior in the notice component.
|
This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions. |
github-actions
bot
added
the
Stale
Related Issue: #11268
Summary
Continues migration of Puppeteer-based common test helpers to Vitest browser mode.