diff options
author | Bryan C. Mills <bcmills@google.com> | 2022-12-13 14:19:59 -0500 |
---|---|---|
committer | Gopher Robot <gobot@golang.org> | 2023-01-19 20:45:34 +0000 |
commit | f2884bf42317011371440d90805e63248d94c45d (patch) | |
tree | 26a929ec5f321fcce33b8c307ac49df9235f38b7 /src/os | |
parent | 3bb8864e9f5d4505c792528c9460fced4c0e7346 (diff) | |
download | go-git-f2884bf42317011371440d90805e63248d94c45d.tar.gz |
os: deflake TestPipeEOF and TestFifoEOF
- Consolidate the two test bodies as one helper function.
- Eliminate arbitrary timeout.
- Shorten arbitrary sleeps in short mode.
- Simplify goroutines.
- Mark the tests as parallel.
Fixes #36107.
Updates #24164.
Change-Id: I14fe4395963a7256cb6d2d743d348a1ade077d5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/457336
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Diffstat (limited to 'src/os')
-rw-r--r-- | src/os/fifo_test.go | 96 | ||||
-rw-r--r-- | src/os/pipe_test.go | 92 |
2 files changed, 75 insertions, 113 deletions
diff --git a/src/os/fifo_test.go b/src/os/fifo_test.go index de70927961..2f0e06bc52 100644 --- a/src/os/fifo_test.go +++ b/src/os/fifo_test.go @@ -2,30 +2,19 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build darwin || dragonfly || freebsd || linux || netbsd || openbsd +//go:build darwin || dragonfly || freebsd || (linux && !android) || netbsd || openbsd package os_test import ( - "bufio" - "bytes" - "fmt" - "io" "os" "path/filepath" - "runtime" - "sync" "syscall" "testing" - "time" ) -// Issue 24164. func TestFifoEOF(t *testing.T) { - switch runtime.GOOS { - case "android": - t.Skip("skipping on Android; mkfifo syscall not available") - } + t.Parallel() dir := t.TempDir() fifoName := filepath.Join(dir, "fifo") @@ -33,71 +22,40 @@ func TestFifoEOF(t *testing.T) { t.Fatal(err) } - var wg sync.WaitGroup - wg.Add(1) + // Per https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html#tag_16_357_03: + // + // - “If O_NONBLOCK is clear, an open() for reading-only shall block the + // calling thread until a thread opens the file for writing. An open() for + // writing-only shall block the calling thread until a thread opens the file + // for reading.” + // + // In order to unblock both open calls, we open the two ends of the FIFO + // simultaneously in separate goroutines. + + rc := make(chan *os.File, 1) go func() { - defer wg.Done() - - w, err := os.OpenFile(fifoName, os.O_WRONLY, 0) + r, err := os.Open(fifoName) if err != nil { t.Error(err) - return } - - defer func() { - if err := w.Close(); err != nil { - t.Errorf("error closing writer: %v", err) - } - }() - - for i := 0; i < 3; i++ { - time.Sleep(10 * time.Millisecond) - _, err := fmt.Fprintf(w, "line %d\n", i) - if err != nil { - t.Errorf("error writing to fifo: %v", err) - return - } - } - time.Sleep(10 * time.Millisecond) + rc <- r }() - defer wg.Wait() - - r, err := os.Open(fifoName) + w, err := os.OpenFile(fifoName, os.O_WRONLY, 0) if err != nil { - t.Fatal(err) + t.Error(err) } - done := make(chan bool) - go func() { - defer close(done) - - defer func() { - if err := r.Close(); err != nil { - t.Errorf("error closing reader: %v", err) - } - }() - - rbuf := bufio.NewReader(r) - for { - b, err := rbuf.ReadBytes('\n') - if err == io.EOF { - break - } - if err != nil { - t.Error(err) - return - } - t.Logf("%s\n", bytes.TrimSpace(b)) + r := <-rc + if t.Failed() { + if r != nil { + r.Close() } - }() - - select { - case <-done: - // Test succeeded. - case <-time.After(time.Second): - t.Error("timed out waiting for read") - // Close the reader to force the read to complete. - r.Close() + if w != nil { + w.Close() + } + return } + + testPipeEOF(t, r, w) } diff --git a/src/os/pipe_test.go b/src/os/pipe_test.go index 26565853e1..a20a12aac4 100644 --- a/src/os/pipe_test.go +++ b/src/os/pipe_test.go @@ -339,68 +339,72 @@ func testCloseWithBlockingRead(t *testing.T, r, w *os.File) { wg.Wait() } -// Issue 24164, for pipes. func TestPipeEOF(t *testing.T) { + t.Parallel() + r, w, err := os.Pipe() if err != nil { t.Fatal(err) } - var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() - - defer func() { - if err := w.Close(); err != nil { - t.Errorf("error closing writer: %v", err) - } - }() + testPipeEOF(t, r, w) +} - for i := 0; i < 3; i++ { - time.Sleep(10 * time.Millisecond) - _, err := fmt.Fprintf(w, "line %d\n", i) - if err != nil { - t.Errorf("error writing to fifo: %v", err) - return - } +// testPipeEOF tests that when the write side of a pipe or FIFO is closed, +// a blocked Read call on the reader side returns io.EOF. +// +// This scenario previously failed to unblock the Read call on darwin. +// (See https://go.dev/issue/24164.) +func testPipeEOF(t *testing.T, r io.ReadCloser, w io.WriteCloser) { + // parkDelay is an arbitrary delay we wait for a pipe-reader goroutine to park + // before issuing the corresponding write. The test should pass no matter what + // delay we use, but with a longer delay is has a higher chance of detecting + // poller bugs. + parkDelay := 10 * time.Millisecond + if testing.Short() { + parkDelay = 100 * time.Microsecond + } + writerDone := make(chan struct{}) + defer func() { + if err := r.Close(); err != nil { + t.Errorf("error closing reader: %v", err) } - time.Sleep(10 * time.Millisecond) + <-writerDone }() - defer wg.Wait() - - done := make(chan bool) + write := make(chan int, 1) go func() { - defer close(done) - - defer func() { - if err := r.Close(); err != nil { - t.Errorf("error closing reader: %v", err) - } - }() + defer close(writerDone) - rbuf := bufio.NewReader(r) - for { - b, err := rbuf.ReadBytes('\n') - if err == io.EOF { - break - } + for i := range write { + time.Sleep(parkDelay) + _, err := fmt.Fprintf(w, "line %d\n", i) if err != nil { - t.Error(err) + t.Errorf("error writing to fifo: %v", err) return } - t.Logf("%s\n", bytes.TrimSpace(b)) + } + + time.Sleep(parkDelay) + if err := w.Close(); err != nil { + t.Errorf("error closing writer: %v", err) } }() - select { - case <-done: - // Test succeeded. - case <-time.After(time.Second): - t.Error("timed out waiting for read") - // Close the reader to force the read to complete. - r.Close() + rbuf := bufio.NewReader(r) + for i := 0; i < 3; i++ { + write <- i + b, err := rbuf.ReadBytes('\n') + if err != nil { + t.Fatal(err) + } + t.Logf("%s\n", bytes.TrimSpace(b)) + } + + close(write) + b, err := rbuf.ReadBytes('\n') + if err != io.EOF || len(b) != 0 { + t.Errorf(`ReadBytes: %q, %v; want "", io.EOF`, b, err) } } |