Synchonize captureFifoToFile's goroutine with its test

Instead of waiting for 250ms, we should explicitly synchronize
the goroutine and the test, to be safer.

Signed-off-by: Kazuyoshi Kato <[email protected]>

Author: kzys

machine.go

@@ -584,6 +584,10 @@ func (m *Machine) setupLogging(ctx context.Context) error {
 }
 
 func (m *Machine) captureFifoToFile(ctx context.Context, logger *log.Entry, fifoPath string, w io.Writer) error {
+	return m.captureFifoToFileWithChannel(ctx, logger, fifoPath, w, make(chan error, 1))
+}
+
+func (m *Machine) captureFifoToFileWithChannel(ctx context.Context, logger *log.Entry, fifoPath string, w io.Writer, done chan error) error {
 	// open the fifo pipe which will be used
 	// to write its contents to a file.
 	fifoPipe, err := fifo.OpenFifo(ctx, fifoPath, syscall.O_RDONLY|syscall.O_NONBLOCK, 0600)
@@ -620,7 +624,10 @@ func (m *Machine) captureFifoToFile(ctx context.Context, logger *log.Entry, fifo
 
 		if _, err := io.Copy(w, fifoPipe); err != nil {
 			logger.WithError(err).Warn("io.Copy failed to copy contents of fifo pipe")
+			done <- err
 		}
+
+		close(done)
 	}()
 
 	return nil

machine_test.go

@@ -1073,20 +1073,19 @@ func TestCaptureFifoToFile_leak(t *testing.T) {
 	logger := fctesting.NewLogEntry(t)
 	logger.Logger.Level = logrus.WarnLevel
 	logger.Logger.Out = loggerBuffer
-	err = m.captureFifoToFile(context.Background(), logger, fifoPath, buf)
+
+	done := make(chan error)
+	err = m.captureFifoToFileWithChannel(context.Background(), logger, fifoPath, buf, done)
 	assert.NoError(t, err, "failed to capture fifo to file")
+
+	// Stopping the machine will close the FIFO
 	close(m.exitCh)
 
-	// wait sometime for the logs to populate
-	time.Sleep(250 * time.Millisecond)
-	expectedClosedFifo := `reading from a closed fifo`
-	expectedClosedFile := `file already closed`
-	logs := loggerBuffer.String()
-	if !(strings.Contains(logs, expectedClosedFifo) ||
-		strings.Contains(logs, expectedClosedFile)) {
+	// Waiting the channel to make sure that the contents of the FIFO has been copied
+	copyErr := <-done
 
-		t.Errorf("logs did not container a closed fifo error or closed file")
-	}
+	assert.Contains(t, copyErr.Error(), `file already closed`, "error")
+	assert.Contains(t, loggerBuffer.String(), `file already closed`, "log")
 }
 
 func TestWaitWithKill(t *testing.T) {