[skip-changelog] Proper handling of stdout/stderr copy in subprocess in integration tests (arduino#1862)

* Proper handling of stdout/stderr copy in subprocess

* Apply suggestions from code review

Co-authored-by: Umberto Baldi <[email protected]>

Co-authored-by: Umberto Baldi <[email protected]>

Author: cmaglie

Diff for: executils/process.go

@@ -86,12 +86,22 @@ func (p *Process) StdinPipe() (io.WriteCloser, error) {
 
 // StdoutPipe returns a pipe that will be connected to the command's standard
 // output when the command starts.
+//
+// Wait will close the pipe after seeing the command exit, so most callers
+// don't need to close the pipe themselves. It is thus incorrect to call Wait
+// before all reads from the pipe have completed.
+// For the same reason, it is incorrect to call Run when using StdoutPipe.
 func (p *Process) StdoutPipe() (io.ReadCloser, error) {
 	return p.cmd.StdoutPipe()
 }
 
 // StderrPipe returns a pipe that will be connected to the command's standard
 // error when the command starts.
+//
+// Wait will close the pipe after seeing the command exit, so most callers
+// don't need to close the pipe themselves. It is thus incorrect to call Wait
+// before all reads from the pipe have completed.
+// For the same reason, it is incorrect to use Run when using StderrPipe.
 func (p *Process) StderrPipe() (io.ReadCloser, error) {
 	return p.cmd.StderrPipe()
 }

Diff for: internal/integrationtest/arduino-cli.go

@@ -143,19 +143,22 @@ func (cli *ArduinoCLI) Run(args ...string) ([]byte, []byte, error) {
 	cli.t.NoError(cliProc.Start())
 
 	var stdoutBuf, stderrBuf bytes.Buffer
-	stdoutCtx, stdoutCancel := context.WithCancel(context.Background())
-	stderrCtx, stderrCancel := context.WithCancel(context.Background())
+	var wg sync.WaitGroup
+	wg.Add(2)
 	go func() {
-		io.Copy(&stdoutBuf, io.TeeReader(stdout, os.Stdout))
-		stdoutCancel()
+		defer wg.Done()
+		if _, err := io.Copy(&stdoutBuf, io.TeeReader(stdout, os.Stdout)); err != nil {
+			fmt.Println(color.HiBlackString("<<< stdout copy error:"), err)
+		}
 	}()
 	go func() {
-		io.Copy(&stderrBuf, io.TeeReader(stderr, os.Stderr))
-		stderrCancel()
+		defer wg.Done()
+		if _, err := io.Copy(&stderrBuf, io.TeeReader(stderr, os.Stderr)); err != nil {
+			fmt.Println(color.HiBlackString("<<< stderr copy error:"), err)
+		}
 	}()
+	wg.Wait()
 	cliErr := cliProc.Wait()
-	<-stdoutCtx.Done()
-	<-stderrCtx.Done()
 	fmt.Println(color.HiBlackString("<<< Run completed (err = %v)", cliErr))
 
 	return stdoutBuf.Bytes(), stderrBuf.Bytes(), cliErr