gittest: Fix `ExecOpts()` with explicit stdout writer

The `ExecOpts()` function accepts a configuration that allows the caller
to set various options like the standard streams. As it turns out though
setting `Stdout` in that configuration is broken and will cause us to
fail immediately. The root cause is that we call `exec.Cmd.Output()`,
which is not allowed when you set the command's `Stdout` variable.

Fix this by calling `Run()` instead when `Stdout` is passed by the
caller. Add tests to catch any future regressions.

2 files changed, 65 insertions, 0 deletions

diff --git a/internal/git/gittest/command.go b/internal/git/gittest/command.go
index b31e57cd3..997e2905f 100644
--- a/internal/git/gittest/command.go
+++ b/internal/git/gittest/command.go
@@ -35,6 +35,20 @@ func ExecOpts(t testing.TB, cfg config.Cfg, execCfg ExecConfig, args ...string)
 
 	cmd := createCommand(t, cfg, execCfg, args...)
 
+	// If the caller has passed an stdout writer to us we cannot use `cmd.Output()`. So
+	// we detect this case and call `cmd.Run()` instead.
+	if execCfg.Stdout != nil {
+		if err := cmd.Run(); err != nil {
+			t.Log(cfg.Git.BinPath, args)
+			if ee, ok := err.(*exec.ExitError); ok {
+				t.Logf("%s\n", ee.Stderr)
+			}
+			t.Fatal(err)
+		}
+
+		return nil
+	}
+
 	output, err := cmd.Output()
 	if err != nil {
 		t.Log(cfg.Git.BinPath, args)
diff --git a/internal/git/gittest/command_test.go b/internal/git/gittest/command_test.go
new file mode 100644
index 000000000..2cbe8dae3
--- /dev/null
+++ b/internal/git/gittest/command_test.go
@@ -0,0 +1,51 @@
+package gittest
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/text"
+)
+
+func TestExecOpts(t *testing.T) {
+	cfg, _, repoPath := setup(t)
+
+	t.Run("default config", func(t *testing.T) {
+		toplevel := ExecOpts(t, cfg, ExecConfig{}, "-C", repoPath, "rev-parse", "--path-format=absolute", "--git-dir")
+		require.Equal(t, repoPath, text.ChompBytes(toplevel))
+	})
+
+	t.Run("env", func(t *testing.T) {
+		configValue := ExecOpts(t, cfg, ExecConfig{
+			Env: []string{
+				"GIT_CONFIG_COUNT=1",
+				"GIT_CONFIG_KEY_0=injected.env-config",
+				"GIT_CONFIG_VALUE_0=some value",
+			},
+		}, "-C", repoPath, "config", "injected.env-config")
+
+		require.Equal(t, "some value", text.ChompBytes(configValue))
+	})
+
+	t.Run("stdin", func(t *testing.T) {
+		blobID := ExecOpts(t, cfg, ExecConfig{
+			Stdin: strings.NewReader("blob contents"),
+		}, "-C", repoPath, "hash-object", "-w", "--stdin")
+
+		require.Equal(t,
+			"blob contents",
+			string(Exec(t, cfg, "-C", repoPath, "cat-file", "-p", text.ChompBytes(blobID))),
+		)
+	})
+
+	t.Run("stdout", func(t *testing.T) {
+		var buf bytes.Buffer
+		ExecOpts(t, cfg, ExecConfig{
+			Stdout: &buf,
+		}, "-C", repoPath, "rev-parse", "--path-format=absolute", "--git-dir")
+
+		require.Equal(t, repoPath, text.ChompBytes(buf.Bytes()))
+	})
+}