Attempt to reduce test flakiness on Windows (#1845)

## Changes

Test failures indicate that both stdout and stderr are consumed, yet the
content of stdout doesn't end up in the intended output. This can happen
if the goroutines responsible for writing to the combined output buffer
attempt to write to the same underlying buffer concurrently.

Example failure:
```
=== RUN   TestBackgroundCombinedOutput
    background_test.go:65: 
        	Error Trace:	D:/a/cli/cli/libs/process/background_test.go:65
        	Error:      	elements differ
        	            	
        	            	extra elements in list A:
        	            	([]interface {}) (len=1) {
        	            	 (string) (len=1) "2"
        	            	}
        	            	
        	            	
        	            	listA:
        	            	([]string) (len=2) {
        	            	 (string) (len=1) "1",
        	            	 (string) (len=1) "2"
        	            	}
        	            	
        	            	
        	            	listB:
        	            	([]string) (len=1) {
        	            	 (string) (len=1) "1"
        	            	}
        	Test:       	TestBackgroundCombinedOutput
```

With the test body:

ca45e53f42/libs/process/background_test.go (L48-L66)

With the implementation of `WithCombinedOutput`:

ca45e53f42/libs/process/opts.go (L72-L78)

Notice that `c.Stdout` does get the "2", or the test failure would have
included the relevant assertion error. This leads me to believe that
there is a race on writing to `buf` from the two goroutines writing to
`c.Stdout` and `c.Stderr`.

## Tests

The test passes. If this PR has the intended effect remains to be
seen...
This commit is contained in:
Pieter Noordhuis 2024-10-24 14:03:12 +02:00 committed by GitHub
parent ab622e65bb
commit eddaddaf8b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 17 additions and 2 deletions

View File

@ -6,6 +6,7 @@ import (
"fmt"
"io"
"os/exec"
"sync"
)
type execOption func(context.Context, *exec.Cmd) error
@ -69,10 +70,24 @@ func WithStdoutWriter(dst io.Writer) execOption {
}
}
// safeWriter is a writer that is safe to use concurrently.
// It serializes writes to the underlying writer.
type safeWriter struct {
w io.Writer
m sync.Mutex
}
func (s *safeWriter) Write(p []byte) (n int, err error) {
s.m.Lock()
defer s.m.Unlock()
return s.w.Write(p)
}
func WithCombinedOutput(buf *bytes.Buffer) execOption {
sw := &safeWriter{w: buf}
return func(_ context.Context, c *exec.Cmd) error {
c.Stdout = io.MultiWriter(buf, c.Stdout)
c.Stderr = io.MultiWriter(buf, c.Stderr)
c.Stdout = io.MultiWriter(sw, c.Stdout)
c.Stderr = io.MultiWriter(sw, c.Stderr)
return nil
}
}