Repository navigation
test: use playwright browser locators in Button and TreeView tests - #8527
tay1orjones wants to merge 3 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3051e4e7-3e74-4dcd-b1f4-1314e3b9b56e
|
github-actions
Bot
added
the
integration-tests: recommended
|
Restore the programmatic click for the pointer-only TreeView chevron. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3051e4e7-3e74-4dcd-b1f4-1314e3b9b56e
Keep RTL rendering and scoped act boundaries for manual updates. Await Vitest browser interactions directly without forcing act mode for the entire browser suite. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3051e4e7-3e74-4dcd-b1f4-1314e3b9b56e
|
|
||
| // @ts-expect-error this is needed for act() from React | ||
| // @see https://react.dev/reference/react/act#error-the-current-testing-environment-is-not-configured-to-support-act | ||
| globalThis.IS_REACT_ACT_ENVIRONMENT = true |
There was a problem hiding this comment.
Removing this override lets Button and TreeView tests await browser interactions directly. The eight test suites unrelated to those (DataTableGrouping, UnderlinePanels, etc) still perform manual timer, hydration, or observer updates. RTL’s act enables the environment for those operations and restores it afterward, whereas React’s raw act doesn’t.
tay1orjones
changed the title
|
My understanding is that integration tests aren't needed when changing only test files, but please correct me if I'm wrong here. |
tay1orjones
added
integration-tests: skipped manually
joshblack
left a comment
joshblack
left a comment
There was a problem hiding this comment.
Looks great! Just have a couple comments/questions. If you're interested, would be great to have a testing skill that loads testing.md (or I guess translate testing.md into the skill plus references format we've been doing) but that could totally be in a separate PR
| ### Storybook Interaction Tests | ||
|
|
||
| We use [Storybook interactions tests](https://storybook.js.org/docs/react/writing-tests/interaction-testing) to simulate and test some complex user interactions. They are particularly useful for cases where writing unit tests is not practical due to the limitation of the mock browser functionalities of JSDOM. For example testing out a component's overflow behaviour whose responsiveness is managed by its own dynamic width. | ||
| We use [Storybook interactions tests](https://storybook.js.org/docs/react/writing-tests/interaction-testing) to simulate and test complex user interactions in story scenarios. For example, testing a component's overflow behaviour whose responsiveness is managed by its own dynamic width. | ||
|
|
||
| Storybook tests are authored within the components's source directory with the file name of `interactions.stories.tsx` |
There was a problem hiding this comment.
Honestly I think we can remove this whole section 😅 lol. These should likely use playwright instead
There was a problem hiding this comment.
omg I agree! Agents seem to always want to put things into play functions on stories where they should actually be in playwright tests. I bet this is why. Will update.
There was a problem hiding this comment.
Do you think it'd be worth rolling this into a testing skill similar to what we do for style-guide or not really?
There was a problem hiding this comment.
Yeah I'm happy to try it out in a follow up
This branch was successfully deployed
2 participants
Related to github/primer#6369.
Use Button and TreeView to establish a bounded locator migration pattern before converting more browser tests. Keep RTL rendering and lifecycle APIs, along with the existing synchronous event, focus, and timer contracts.
Changelog
New
Changed
Removed
Rollout strategy
Testing & Reviewing
Run
CI=true VITEST_FAIL_ON_CONSOLE=true npm test -- --run --project=@primer/react packages/react/src/Button/__tests__/Button.test.tsx packages/react/src/TreeView/TreeView.test.tsx. Review the exact-name selectors after expansion, detached-node checks, intentional synthetic events, and fake-timer boundaries against their original assertions. Browser interactions use awaited Reactactboundaries; the pointer-only TreeView chevron retains its programmatic click.On the original pilot commit
e8d2a7b69, 20 local before/after runs measured median suite times of 60.3 -> 200.2 ms for Button and 311.8 -> 950.5 ms for TreeView, excluding startup. These measurements precede the CI fixes; they don't measure the current revision or establish a flakiness improvement.Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com