diff --git a/pkg/prog/progtest/progtest.go b/pkg/prog/progtest/progtest.go index d2b4fe25..dfc32679 100644 --- a/pkg/prog/progtest/progtest.go +++ b/pkg/prog/progtest/progtest.go @@ -19,11 +19,20 @@ type Fixture struct { dirCleanup func() } +func captureOutput(p *pipe) { + b, err := ioutil.ReadAll(p.r) + if err != nil { + panic(err) + } + p.output <- b +} + // Setup sets up a test fixture. The caller is responsible for calling the // Cleanup method of the returned Fixture. func Setup() *Fixture { _, dirCleanup := util.InTestDir() - return &Fixture{[3]*pipe{makePipe(), makePipe(), makePipe()}, dirCleanup} + pipes := [3]*pipe{makePipe(false), makePipe(true), makePipe(true)} + return &Fixture{pipes, dirCleanup} } // Cleanup cleans up the test fixture. @@ -46,6 +55,7 @@ func (f *Fixture) FeedIn(s string) { panic(err) } f.pipes[0].w.Close() + f.pipes[0].wClosed = true } // TestOut tests that the output on the given FD matches the given text. @@ -68,29 +78,33 @@ type pipe struct { r, w *os.File rClosed, wClosed bool saved string + output chan []byte } -func makePipe() *pipe { +func makePipe(capture bool) *pipe { r, w, err := os.Pipe() if err != nil { panic(err) } - return &pipe{r: r, w: w} + if !capture { + return &pipe{r: r, w: w} + } + output := make(chan []byte, 1) + p := pipe{r: r, w: w, output: output} + go captureOutput(&p) + return &p } func (p *pipe) get() string { - if p.rClosed { - return p.saved + if !p.wClosed { + // Close the write side so captureOutput goroutine sees EOF and + // terminates allowing us to capture and cache the output. + p.w.Close() + p.wClosed = true + if p.output != nil { + p.saved = string(<-p.output) + } } - p.w.Close() - p.wClosed = true - b, err := ioutil.ReadAll(p.r) - if err != nil { - panic(err) - } - p.r.Close() - p.rClosed = true - p.saved = string(b) return p.saved } @@ -98,11 +112,18 @@ func (p *pipe) close() { if !p.wClosed { p.w.Close() p.wClosed = true + if p.output != nil { + p.saved = string(<-p.output) + } } if !p.rClosed { p.r.Close() p.rClosed = true } + if p.output != nil { + close(p.output) + p.output = nil + } } // MustWriteFile writes a file with the given name and content. It panics if the diff --git a/pkg/prog/progtest/progtest_test.go b/pkg/prog/progtest/progtest_test.go new file mode 100644 index 00000000..79543f0e --- /dev/null +++ b/pkg/prog/progtest/progtest_test.go @@ -0,0 +1,23 @@ +package progtest + +import ( + "testing" +) + +// Verify we don't deadlock if more output is written to stdout than can be +// buffered by a pipe. +func TestOutputCaptureDoesNotDeadlock(t *testing.T) { + t.Helper() + f := Setup() + + // We need enough data to verify whether we're likely to deadlock due to + // filling the pipe before the test completes. Pipes typically buffer 8 to + // 128 KiB. + bytes := [16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16} + for i := 0; i < 128*1024/len(bytes); i += 1 { + f.pipes[1].w.Write(bytes[:]) + } + f.pipes[1].w.WriteString("hello\n") + f.TestOutSnippet(t, 1, "hello") + f.TestOut(t, 2, "") +}