Leave stderr alone when passed into command

3 files changed, 49 insertions, 6 deletions

diff --git a/changelogs/unreleased/jc-leave-stderr-pipe-alone.yml b/changelogs/unreleased/jc-leave-stderr-pipe-alone.yml
new file mode 100644
index 000000000..ca9fa4502
--- /dev/null
+++ b/changelogs/unreleased/jc-leave-stderr-pipe-alone.yml
@@ -0,0 +1,5 @@
+---
+title: Leave stderr alone when passed into command
+merge_request: 1456
+author:
+type: fixed
diff --git a/internal/command/command.go b/internal/command/command.go
index 7e7fbdaa3..a89be98cf 100644
--- a/internal/command/command.go
+++ b/internal/command/command.go
@@ -232,7 +232,8 @@ func New(ctx context.Context, cmd *exec.Cmd, stdin io.Reader, stdout, stderr io.
 	}
 
 	if stderr != nil {
-		command.stderrCloser = escapeNewlineWriter(&noopWriteCloser{stderr}, command.stderrDone, MaxStderrBytes)
+		command.stderrCloser = &noopWriteCloser{stderr}
+		close(command.stderrDone)
 	} else {
 		command.stderrCloser = escapeNewlineWriter(grpc_logrus.Extract(ctx).WriterLevel(log.ErrorLevel), command.stderrDone, MaxStderrBytes)
 	}
diff --git a/internal/command/command_test.go b/internal/command/command_test.go
index a4eecebca..dbf241bd7 100644
--- a/internal/command/command_test.go
+++ b/internal/command/command_test.go
@@ -11,6 +11,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -195,11 +197,18 @@ func TestCommandStdErr(t *testing.T) {
 
 	var stdout, stderr bytes.Buffer
 
-	cmd, err := New(ctx, exec.Command("./testdata/stderr_script.sh"), nil, &stdout, &stderr)
+	logger := logrus.New()
+	logger.SetOutput(&stderr)
+
+	ctx = ctxlogrus.ToContext(ctx, logrus.NewEntry(logger))
+
+	cmd, err := New(ctx, exec.Command("./testdata/stderr_script.sh"), nil, &stdout, nil)
 	require.NoError(t, err)
 
 	require.Error(t, cmd.Wait())
 	assert.Empty(t, stdout.Bytes())
+
+	logger.Exit(0)
 	assert.Equal(t, `hello world\nhello world\nhello world\nhello world\nhello world\n`, stderr.String())
 }
 
@@ -209,11 +218,18 @@ func TestCommandStdErrLargeOutput(t *testing.T) {
 
 	var stdout, stderr bytes.Buffer
 
-	cmd, err := New(ctx, exec.Command("./testdata/stderr_many_lines.sh"), nil, &stdout, &stderr)
+	logger := logrus.New()
+	logger.SetOutput(&stderr)
+
+	ctx = ctxlogrus.ToContext(ctx, logrus.NewEntry(logger))
+
+	cmd, err := New(ctx, exec.Command("./testdata/stderr_many_lines.sh"), nil, &stdout, nil)
 	require.NoError(t, err)
 
 	require.Error(t, cmd.Wait())
 	assert.Empty(t, stdout.Bytes())
+
+	logger.Exit(0)
 	assert.True(t, stderr.Len() <= MaxStderrBytes)
 }
 
@@ -223,11 +239,18 @@ func TestCommandStdErrBinaryNullBytes(t *testing.T) {
 
 	var stdout, stderr bytes.Buffer
 
-	cmd, err := New(ctx, exec.Command("./testdata/stderr_binary_null.sh"), nil, &stdout, &stderr)
+	logger := logrus.New()
+	logger.SetOutput(&stderr)
+
+	ctx = ctxlogrus.ToContext(ctx, logrus.NewEntry(logger))
+
+	cmd, err := New(ctx, exec.Command("./testdata/stderr_binary_null.sh"), nil, &stdout, nil)
 	require.NoError(t, err)
 
 	require.Error(t, cmd.Wait())
 	assert.Empty(t, stdout.Bytes())
+
+	logger.Exit(0)
 	assert.NotEmpty(t, stderr.Bytes())
 }
 
@@ -237,11 +260,18 @@ func TestCommandStdErrLongLine(t *testing.T) {
 
 	var stdout, stderr bytes.Buffer
 
-	cmd, err := New(ctx, exec.Command("./testdata/stderr_repeat_a.sh"), nil, &stdout, &stderr)
+	logger := logrus.New()
+	logger.SetOutput(&stderr)
+
+	ctx = ctxlogrus.ToContext(ctx, logrus.NewEntry(logger))
+
+	cmd, err := New(ctx, exec.Command("./testdata/stderr_repeat_a.sh"), nil, &stdout, nil)
 	require.NoError(t, err)
 
 	require.Error(t, cmd.Wait())
 	assert.Empty(t, stdout.Bytes())
+
+	logger.Exit(0)
 	assert.NotEmpty(t, stderr.Bytes())
 	assert.Equal(t, fmt.Sprintf("%s\\n%s", strings.Repeat("a", StderrBufferSize), strings.Repeat("b", StderrBufferSize)), stderr.String())
 }
@@ -252,10 +282,17 @@ func TestCommandStdErrMaxBytes(t *testing.T) {
 
 	var stdout, stderr bytes.Buffer
 
-	cmd, err := New(ctx, exec.Command("./testdata/stderr_max_bytes_edge_case.sh"), nil, &stdout, &stderr)
+	logger := logrus.New()
+	logger.SetOutput(&stderr)
+
+	ctx = ctxlogrus.ToContext(ctx, logrus.NewEntry(logger))
+
+	cmd, err := New(ctx, exec.Command("./testdata/stderr_max_bytes_edge_case.sh"), nil, &stdout, nil)
 	require.NoError(t, err)
 
 	require.Error(t, cmd.Wait())
 	assert.Empty(t, stdout.Bytes())
+
+	logger.Exit(0)
 	assert.NotEmpty(t, stderr.Bytes())
 }