Fix flaky TestNewCmdTaskInstantStop test (#5743)

TestNewCmdTaskInstantStop is flaky: it closes the stop channel from
within start() and asserts the stopped task touched nothing. But Go's
select picks uniformly at random among ready cases, so when opts.Stop
and a data channel are both ready the loop can pick the data channel,
call beforeStart() (which clears the view) and write the prefix before
bailing. In production a task that's already been superseded thereby
clobbers the output the incoming task is about to render.

Check stop with a non-blocking select before each blocking select, so
the stop signal wins whenever it's already closed (Go has no built-in
priority select; this is the idiomatic substitute). The selects keep
their own stop case for liveness, to unblock when stop closes while
parked waiting for data.
This commit is contained in:
Stefan Haller 2026-06-30 17:40:06 +02:00 committed by GitHub
commit 760e9e587b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 25 additions and 0 deletions

View file

@ -63,6 +63,9 @@ while still being meaningful and self-contained.
preceding commit, by staging hunks or resetting and recommitting in order.
- **Do not use conventional commits** (no `feat:`/`fix:`/`chore:` prefixes).
Match the plain English imperative style of the existing history.
- **Wrap message body to 72 characters**. The subject is allowed to go up to 80
characters, or even a little more if needed to convey a good single-line
summary; the body should be wrapped at 72 exactly, no more, no less.
## Iterate with `fixup!` commits

View file

@ -248,8 +248,26 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p
}
}
// Go's select picks randomly among ready cases, so once opts.Stop is
// closed the selects below could still service a ready data channel
// instead of bailing. Check stop explicitly first to give it priority:
// a task that's been stopped (it's being replaced by a newer one) must
// not touch the view here — beforeStart clears it and the prefix gets
// written, clobbering what the incoming task is about to render.
stopped := func() bool {
select {
case <-opts.Stop:
return true
default:
return false
}
}
outer:
for {
if stopped() {
break outer
}
select {
case <-opts.Stop:
break outer
@ -260,6 +278,10 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p
}
}
for i := 0; linesToRead.Total == -1 || i < linesToRead.Total; i++ {
if stopped() {
callThen()
break outer
}
var ok bool
var line []byte
select {