Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tabs: Implement ariakit/test in unit tests #56835

Merged
merged 1 commit into from
Dec 7, 2023
Merged

Conversation

chad1008
Copy link
Contributor

@chad1008 chad1008 commented Dec 6, 2023

What?

Uses @ariakit/test utils instead of @testing-library's userEvent utils

Why?

While working on these tests I've encountered a number of timing issues while using userEvent utils, at times finding that assertions would be checked before the component had time to fully update and/or rerender. Most recently, while adding new unit tests in #56658, it appeared data was leaking between tests in ways I couldn't nail down.

How?

All keyboard and mouse interactions that were previously simulated using userEvent are now performed via @ariakit/test's press and click utils.

This made a some of our existing waitFor() calls unnecessary, due to how these new utilities perform and time these interactions.

Testing Instructions

All Tabs unit tests should pass consistently

cc @diegohaz, in case this is of interest

@chad1008 chad1008 added the [Package] Components /packages/components label Dec 6, 2023
@chad1008 chad1008 requested a review from a team December 6, 2023 17:09
@chad1008 chad1008 self-assigned this Dec 6, 2023
@chad1008 chad1008 requested a review from ajitbohra as a code owner December 6, 2023 17:09
@chad1008 chad1008 added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label Dec 6, 2023
Copy link

github-actions bot commented Dec 6, 2023

Flaky tests detected in 6b7e8ac.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7117835562
📝 Reported issues:

Copy link
Contributor

@brookewp brookewp left a comment

Choose a reason for hiding this comment

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

Tests here are passing consistently! ✅

I don't see flakiness with the tests in trunk at the moment, but I did see it in your other PR #56658. I'm interested to see how that goes once this change is implemented there!

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

Really cool to see a better way to handle the ariakit store setup update than our prior hacks!

Copy link
Member

@diegohaz diegohaz left a comment

Choose a reason for hiding this comment

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

Great! Although Ariakit Test doesn't decrease flakiness to the same extent as synchronous tests, it should perform better than User Event.

@diegohaz
Copy link
Member

diegohaz commented Dec 7, 2023

Really cool to see a better way to handle the ariakit store setup update than our prior hacks!

I'd argue it's not just better for Ariakit, but also for most front-end libraries that can be tested with JSDOM. We've run tests for Framer Motion, React Router, and even Radix UI.

@chad1008 chad1008 merged commit ee9baf2 into trunk Dec 7, 2023
53 of 56 checks passed
@chad1008 chad1008 deleted the tabs-ariakit-test branch December 7, 2023 16:01
@github-actions github-actions bot added this to the Gutenberg 17.3 milestone Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants