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.

Sign up for GitHub

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

Jump to bottom

CustomSelectControlV2: Stabilize tests #60133

Merged
merged 2 commits into from Mar 24, 2024
Merged

CustomSelectControlV2: Stabilize tests #60133

merged 2 commits into from Mar 24, 2024

Conversation

mirka
Copy link
Member

@mirka mirka commented Mar 22, 2024

What?

Stabilizes the flakiness we've been seeing in the unit tests for CustomSelectControlV2.

How I tested

✅ Consistently passes in chunks of 50 repeated isolated tests (Whereas previously, it would consistently fail)

@mirka mirka added [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Package] Components /packages/components labels Mar 22, 2024
@mirka mirka self-assigned this Mar 22, 2024
@mirka mirka requested a review from ajitbohra as a code owner March 22, 2024 18:44
mirka
mirka commented Mar 22, 2024
View reviewed changes
packages/components/src/custom-select-control-v2/legacy-component/test/index.tsx

await type( 'aq' );
Copy link
Member Author

Choose a reason for hiding this comment

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

Apparently the multi-character typeahead is causing the flakiness. I think it'd be an acceptable compromise to test only a single-character typeahead.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth adding an inline comment here, and in the non-legacy component test, flagging this as deliberately avoiding the multi-character typeahead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, will do 👍

@mirka mirka requested a review from a team March 22, 2024 18:49
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 22, 2024 •
edited

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@mirka mirka requested a review from Mamaduka March 22, 2024 18:49
@mirka mirka mentioned this pull request Mar 22, 2024
Merged
tyxla
tyxla approved these changes Mar 22, 2024
View reviewed changes
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.

Looks good, thanks 🚀

aaronrobertshaw
aaronrobertshaw approved these changes Mar 24, 2024
View reviewed changes
Copy link
Contributor

@aaronrobertshaw aaronrobertshaw left a comment

Choose a reason for hiding this comment

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

The approach here makes sense and tests well. LGTM 👍

packages/components/src/custom-select-control-v2/legacy-component/test/index.tsx

await type( 'aq' );
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth adding an inline comment here, and in the non-legacy component test, flagging this as deliberately avoiding the multi-character typeahead?

@mirka mirka enabled auto-merge (squash) March 24, 2024 20:19
@mirka mirka merged commit c2d3978 into trunk Mar 24, 2024
57 checks passed
@mirka mirka deleted the fix/custom-select-tests branch March 24, 2024 20:50
@github-actions github-actions bot added this to the Gutenberg 18.1 milestone Mar 24, 2024
carstingaxion pushed a commit to carstingaxion/gutenberg that referenced this pull request Mar 27, 2024
@mirka @tyxla @carstingaxion
CustomSelectControlV2: Stabilize tests (WordPress#60133)
27fdbc3 
* CustomSelectControlV2: Stabilize tests

* Add comments

Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
cbravobernal pushed a commit to garridinsi/gutenberg that referenced this pull request Apr 9, 2024
@mirka @tyxla @cbravobernal
CustomSelectControlV2: Stabilize tests (WordPress#60133)
ba5c806 
* CustomSelectControlV2: Stabilize tests

* Add comments

Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tyxla tyxla tyxla approved these changes

@aaronrobertshaw aaronrobertshaw aaronrobertshaw approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

@Mamaduka Mamaduka Awaiting requested review from Mamaduka

Assignees

@mirka mirka

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
Milestone
Gutenberg 18.1
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mirka @tyxla @aaronrobertshaw