From 88622f0cb1bcb38bb86de8a45df993b35c28c8e5 Mon Sep 17 00:00:00 2001
From: "Luis Gustavo S. Barreto" <gustavosbarreto@gmail.com>
Date: Wed, 3 Apr 2024 06:49:55 -0300
Subject: [PATCH] fix(exec): updates the `Multiplexed` opt to combine stdout
 and stderr (#2452)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix(exec): updates the `Multiplexed` opt to combine stdout and stderr

Updates the `Multiplexed` option to combine the output of stdout and stderr,
changing the previous behavior where stderr was preferred over stdout.
Now, both streams are properly combined into a single stream.

Additionally, this commit enhance and adjust existing test cases for
the `Multiplexed` option:

* Modifies `TestExecWithMultiplexedResponse` to ensure that stdout and
  stderr are on the same stream, testing the new behavior of the
  `Multiplexed` option.
* Refactors `TestExecWithNonMultiplexedResponse` to use `stdcopy.StdCopy`
  for ensuring that `io.Reader` contains a separated stdout and stderr.

* chore: no need to pass the provider type

---------

Co-authored-by: Manuel de la Peña <mdelapenya@gmail.com>
---
 docker_exec_test.go | 59 +++++++++++++--------------------------------
 exec/processor.go   |  6 +----
 2 files changed, 18 insertions(+), 47 deletions(-)

diff --git a/docker_exec_test.go b/docker_exec_test.go
index 7e1cd4bc2f..4716c493ab 100644
--- a/docker_exec_test.go
+++ b/docker_exec_test.go
@@ -1,44 +1,16 @@
 package testcontainers
 
 import (
+	"bytes"
 	"context"
 	"io"
-	"strings"
 	"testing"
 
+	"github.com/docker/docker/pkg/stdcopy"
 	"github.com/stretchr/testify/require"
-
 	tcexec "github.com/testcontainers/testcontainers-go/exec"
 )
 
-func TestExecWithMultiplexedResponse(t *testing.T) {
-	ctx := context.Background()
-	req := ContainerRequest{
-		Image: nginxAlpineImage,
-	}
-
-	container, err := GenericContainer(ctx, GenericContainerRequest{
-		ProviderType:     providerType,
-		ContainerRequest: req,
-		Started:          true,
-	})
-
-	require.NoError(t, err)
-	terminateContainerOnEnd(t, ctx, container)
-
-	code, reader, err := container.Exec(ctx, []string{"ls", "/usr/share/nginx"}, tcexec.Multiplexed())
-	require.NoError(t, err)
-	require.Zero(t, code)
-	require.NotNil(t, reader)
-
-	b, err := io.ReadAll(reader)
-	require.NoError(t, err)
-	require.NotNil(t, b)
-
-	str := string(b)
-	require.Equal(t, "html\n", str)
-}
-
 func TestExecWithOptions(t *testing.T) {
 	tests := []struct {
 		name string
@@ -79,7 +51,6 @@ func TestExecWithOptions(t *testing.T) {
 			}
 
 			container, err := GenericContainer(ctx, GenericContainerRequest{
-				ProviderType:     providerType,
 				ContainerRequest: req,
 				Started:          true,
 			})
@@ -106,14 +77,13 @@ func TestExecWithOptions(t *testing.T) {
 	}
 }
 
-func TestExecWithMultiplexedStderrResponse(t *testing.T) {
+func TestExecWithMultiplexedResponse(t *testing.T) {
 	ctx := context.Background()
 	req := ContainerRequest{
 		Image: nginxAlpineImage,
 	}
 
 	container, err := GenericContainer(ctx, GenericContainerRequest{
-		ProviderType:     providerType,
 		ContainerRequest: req,
 		Started:          true,
 	})
@@ -121,9 +91,9 @@ func TestExecWithMultiplexedStderrResponse(t *testing.T) {
 	require.NoError(t, err)
 	terminateContainerOnEnd(t, ctx, container)
 
-	code, reader, err := container.Exec(ctx, []string{"ls", "/non-existing-directory"}, tcexec.Multiplexed())
+	code, reader, err := container.Exec(ctx, []string{"sh", "-c", "echo stdout; echo stderr >&2"}, tcexec.Multiplexed())
 	require.NoError(t, err)
-	require.NotZero(t, code)
+	require.Zero(t, code)
 	require.NotNil(t, reader)
 
 	b, err := io.ReadAll(reader)
@@ -131,7 +101,8 @@ func TestExecWithMultiplexedStderrResponse(t *testing.T) {
 	require.NotNil(t, b)
 
 	str := string(b)
-	require.Contains(t, str, "No such file or directory")
+	require.Contains(t, str, "stdout")
+	require.Contains(t, str, "stderr")
 }
 
 func TestExecWithNonMultiplexedResponse(t *testing.T) {
@@ -141,7 +112,6 @@ func TestExecWithNonMultiplexedResponse(t *testing.T) {
 	}
 
 	container, err := GenericContainer(ctx, GenericContainerRequest{
-		ProviderType:     providerType,
 		ContainerRequest: req,
 		Started:          true,
 	})
@@ -149,15 +119,20 @@ func TestExecWithNonMultiplexedResponse(t *testing.T) {
 	require.NoError(t, err)
 	terminateContainerOnEnd(t, ctx, container)
 
-	code, reader, err := container.Exec(ctx, []string{"ls", "/usr/share/nginx"})
+	code, reader, err := container.Exec(ctx, []string{"sh", "-c", "echo stdout; echo stderr >&2"})
 	require.NoError(t, err)
 	require.Zero(t, code)
 	require.NotNil(t, reader)
 
-	b, err := io.ReadAll(reader)
+	var stdout bytes.Buffer
+	var stderr bytes.Buffer
+
+	written, err := stdcopy.StdCopy(&stdout, &stderr, reader)
 	require.NoError(t, err)
-	require.NotNil(t, b)
+	require.NotZero(t, written)
+	require.NotNil(t, stdout)
+	require.NotNil(t, stderr)
 
-	str := string(b)
-	require.True(t, strings.HasSuffix(str, "html\n"))
+	require.Equal(t, stdout.String(), "stdout\n")
+	require.Equal(t, stderr.String(), "stderr\n")
 }
diff --git a/exec/processor.go b/exec/processor.go
index c4d40f1e73..c4f9e1b100 100644
--- a/exec/processor.go
+++ b/exec/processor.go
@@ -82,10 +82,6 @@ func Multiplexed() ProcessOption {
 
 		<-done
 
-		if errBuff.Bytes() != nil {
-			opts.Reader = &errBuff
-		} else {
-			opts.Reader = &outBuff
-		}
+		opts.Reader = io.MultiReader(&outBuff, &errBuff)
 	})
 }