Fix flaky TestHuhPrompterMultiSelectWithSearchPersistence on slow architectures by pdostal · Pull Request #13675 · cli/cli · GitHub
Skip to content

Fix flaky TestHuhPrompterMultiSelectWithSearchPersistence on slow architectures#13675

Merged
BagToad merged 1 commit into
cli:trunkfrom
pdostal:fix/multi-select-search-race-s390x
Jun 25, 2026
Merged

Fix flaky TestHuhPrompterMultiSelectWithSearchPersistence on slow architectures#13675
BagToad merged 1 commit into
cli:trunkfrom
pdostal:fix/multi-select-search-race-s390x

Conversation

@pdostal

@pdostal pdostal commented Jun 17, 2026 •
edited by BagToad
Loading

Copy link
Copy Markdown
Contributor

Fixes #13727

The test was using a fixed 50ms sleep (waitForOptions()) to wait for an async bubbletea search goroutine before sending the next keystroke. On slow architectures such as s390x under QEMU emulation, the goroutine scheduling and event loop round-trip can exceed 50ms, causing the enter key to arrive while m.loading is still true. In that state the field silently drops all keystrokes, the form never advances, and the 5s overall deadline fires with "form.Run() did not complete in time".

The fix replaces the blind sleep with proper channel-based synchronization. An onSearchDone func() hook is added to multiSelectSearchField (nil in production, set only in tests) and called at the end of applySearchResult. A new test helper waitForSearch(field) wires a one-shot sync.Once-guarded channel into that hook and blocks until it fires - guaranteeing the search is truly complete before the next step is sent, regardless of scheduler timing or machine speed.

Error log

[  770s] --- FAIL: TestHuhPrompterMultiSelectWithSearchPersistence (5.42s)
[  770s]     --- FAIL: TestHuhPrompterMultiSelectWithSearchPersistence/selections_persist_after_changing_search_query (5.31s)                                                                                                      Context
[  770s]         huh_prompter_test.go:478: form.Run() did not complete in time                                                                                                                                                     54,494 tokens
[  770s] FAIL                                                                                                                                                                                                                      5% used
[  770s] FAIL  github.com/cli/cli/v2/internal/prompter  11.841s

@pdostal pdostal requested a review from a team as a code owner June 17, 2026 13:44
@pdostal pdostal requested review from BagToad and Copilot June 17, 2026 13:44
@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown

@github-actions github-actions Bot added external pull request originating outside of the CLI core team needs-triage needs to be reviewed labels Jun 17, 2026
Copilot AI reviewed Jun 17, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR improves the reliability of Bubble Tea-based prompt tests by replacing fixed sleeps with a deterministic synchronization hook that waits for async search completion.

Changes:

  • Add a test-only onSearchDone callback hook to multiSelectSearchField, invoked when async search results are applied.
  • Extend the test interaction harness to support blocking “wait” steps (waitFn) in addition to time-based delays.
  • Update the multi-select-with-search persistence test to wait for async search completion before submitting.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
internal/prompter/multi_select_with_search.go Adds a callback hook fired on search completion to enable deterministic test synchronization.
internal/prompter/huh_prompter_test.go Adds wait-capable interaction steps and uses them to remove timing flakiness in async search tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/prompter/multi_select_with_search.go Outdated
Comment thread internal/prompter/multi_select_with_search.go Outdated
Comment thread internal/prompter/huh_prompter_test.go
Comment thread internal/prompter/multi_select_with_search.go Outdated
@pdostal pdostal force-pushed the fix/multi-select-search-race-s390x branch from 58f5399 to 5c33128 Compare June 17, 2026 13:56
@pdostal
test(prompter): fix flaky MultiSelectWithSearch test on slow architec…
300cae9 
…tures

Replace fixed-duration sleep with a channel-based synchronization hook
so the test waits for the async search to genuinely complete before
sending the next keystroke. The previous 50ms waitForOptions() was too
short on slow architectures such as s390x under QEMU emulation, causing
the enter key to be dropped while the field was still loading.
@pdostal pdostal force-pushed the fix/multi-select-search-race-s390x branch from 5c33128 to 300cae9 Compare June 17, 2026 14:03
@pdostal

pdostal commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Hello @williammartin, I remember we already did similar fixes together, can you help here?

@github-actions github-actions Bot removed the needs-triage needs to be reviewed label Jun 22, 2026
@github-actions github-actions Bot closed this Jun 22, 2026
@pdostal pdostal mentioned this pull request Jun 25, 2026
Closed
@BagToad BagToad reopened this Jun 25, 2026
@github-actions github-actions Bot added needs-triage needs to be reviewed and removed needs-triage needs to be reviewed labels Jun 25, 2026
@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown

BagToad
BagToad approved these changes Jun 25, 2026
View reviewed changes

@BagToad BagToad left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for bumping this with an issue @pdostal - I think that's the best way to contribute (and follows our documented process) since we don't have bandwidth to review many PRs at the moment, the ones we do review come from well-reasoned issues right now. Thank you for making this better!

@BagToad BagToad merged commit 954ffc3 into cli:trunk Jun 25, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@BagToad BagToad BagToad approved these changes

Assignees

No one assigned

Labels

external pull request originating outside of the CLI core team unmet-requirements

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Flaky TestHuhPrompterMultiSelectWithSearchPersistence on slow architectures

3 participants

@pdostal @BagToad