Repository navigation
termtest: deliver the prompt result before closing the stdin pipe - #7506
Merged
Merged
Conversation
On Windows, bubbletea cannot cancel a read on a pipe, so closing the read end could block until the test closed the write end in cleanup, after Result had already timed out. Co-authored-by: Isaac <no-reply@databricks.com>
github-actions
Bot
added
the
test-only-fix
denik
marked this pull request as ready for review
November 11, 2026 20:26
denik
enabled auto-merge
November 11, 2026 21:48
pietern
approved these changes
Nov 12, 2026
pietern
disabled auto-merge
November 12, 2026 07:32
pietern
enabled auto-merge
November 12, 2026 07:32
Collaborator
✅ Integration test reportCommit: e6f2326
6 interesting tests: 5 flaky, 1 RECOVERED
Top 14 slowest tests (at least 2 minutes):
|
Collaborator
❌ Integration test reportCommit: e93a3ff
268 interesting tests: 158 MISS, 98 FAIL, 9 RECOVERED, 3 KNOWN
Top 50 slowest tests (at least 2 minutes):
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
In
termtest.Run, the goroutine running the cmdio call now sends(value, err)ondonebefore it closes the stdin pipe's read end. Before this change it closed the pipe first.Why
On Windows, bubbletea wraps a non-console input in cancelreader's fallback reader, and that reader cannot cancel a read in progress (
Cancel()returns false). After the prompt quits, bubbletea's read loop can still be blocked inReadFileon the pipe. Go's(*os.File).Closeon Windows callsCancelIoExon pipes, but it does nothing when the read loop hasn't enteredReadFileyet. Close then waits for the outstanding read, which only ends whenTerm.Close(run int.Cleanup) closes the write end. That happens only afterResult()has given up, so the test fails withcmdio call did not finish in 5s, even though the raw output shows bubbletea already restored the terminal. Whether it fails depends on goroutine scheduling, so on a retry it usually passes.Sending the result first lets
Result()return as soon as the call finishes.Closestill closes the write end and waits for the goroutine, so nothing leaks.Background:
cmd/authtests ontotermtest.Run, which is why the flake rate rose.bubbletea cannot cancel reads on non-console input on Windows). This PR does not change that.I decided not to raise
defaultTimeout, because the wait isn't caused by load: it lasts until cleanup, however long the timeout is.Tests
go test ./libs/cmdio/... ./cmd/auth/(Linux, also-race -count=5for cmdiotest);GOOS=windows go veton the same packages.Dashboard:
select_baseline_wrap_around_test.go:39: cmdio call did not finish in 5s; raw output:…\u001b[?2004l\u001b[?25h\u001b[?1002l\u001b[?1003l\u001b[?1006l/--- FAIL: TestSelectBaseline_WrapAround (5.05s)Affected (flaky on Windows, 59 tests), e.g. cmdiotest.TestSelectBaseline_Scroll, cmdiotest.TestPromptBaseline_AltKeyNoop, auth.TestPromptForWorkspaceSelection/id_beyond_first_fifty, auth.TestLoginDeviceCodeNewProfilePromptsForHost.
This PR was opened by an AI agent (harness_auto).
This pull request and its description were written by Isaac.