git: capture Git STDERR log in Go error

In a609c0b (git: refactor into interface, 2023-02-24), the function
'GitCommandWithStdin()' was replaced with 'gitCommandWithStdin()'. In that
update, the STDERR log that was previously written to the error message was
removed. Downstream functions (namely 'CreateIncrementalBundle()') relied on
the content of that STDERR message to determine behavior. Add it back to the
error message to correct the behavor of 'CreateIncrementalBundle()'.

Additionally, add unit tests covering 'CreateIncrementalBundle()'.

Signed-off-by: Victoria Dye <[email protected]>

Author: vdye

internal/git/git.go

@@ -51,17 +51,19 @@ func (g *gitHelper) gitCommandWithStdin(ctx context.Context, stdinLines []string
 	for line := range stdinLines {
 		buffer.Write([]byte(stdinLines[line] + "\n"))
 	}
+
+	stderr := bytes.Buffer{}
 	exitCode, err := g.cmdExec.Run(ctx, "git", args,
 		cmd.Stdin(&buffer),
 		cmd.Stdout(os.Stdout),
-		cmd.Stderr(os.Stderr),
+		cmd.Stderr(&stderr),
 		cmd.Env([]string{"LC_CTYPE=C"}),
 	)
 
 	if err != nil {
 		return g.logger.Error(ctx, err)
 	} else if exitCode != 0 {
-		return g.logger.Errorf(ctx, "'git' exited with status %d", exitCode)
+		return g.logger.Errorf(ctx, "'git' exited with status %d\n%s", exitCode, stderr.String())
 	}
 
 	return nil

internal/git/git_test.go

@@ -0,0 +1,125 @@
+package git_test
+
+import (
+	"context"
+	"io"
+	"testing"
+
+	"github.com/github/git-bundle-server/internal/cmd"
+	"github.com/github/git-bundle-server/internal/git"
+	. "github.com/github/git-bundle-server/internal/testhelpers"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+)
+
+var createIncrementalBundleTests = []struct {
+	title string
+
+	// Inputs
+	repoDir  string
+	filename string
+	prereqs  []string
+
+	// Mocked responses
+	bundleCreate       Pair[int, error]
+	bundleCreateStderr string
+
+	// Expected values
+	expectedBundleCreated bool
+	expectErr             bool
+}{
+	{
+		"Successful bundle creation",
+
+		"/test/home/git-bundle-server/git/test/myrepo/",
+		"/test/home/git-bundle-server/www/test/myrepo/bundle-1234.bundle",
+		[]string{"^018d4b8a"},
+
+		NewPair[int, error](0, nil),
+		"",
+
+		true,
+		false,
+	},
+	{
+		"Successful no-op (empty bundle)",
+
+		"/test/home/git-bundle-server/git/test/myrepo/",
+		"/test/home/git-bundle-server/www/test/myrepo/bundle-5678.bundle",
+		[]string{"^0793b0ce", "^3649daa0"},
+
+		NewPair[int, error](128, nil),
+		"fatal: Refusing to create empty bundle",
+
+		false,
+		false,
+	},
+}
+
+func TestGit_CreateIncrementalBundle(t *testing.T) {
+	// Set up mocks
+	testLogger := &MockTraceLogger{}
+	testCommandExecutor := &MockCommandExecutor{}
+
+	gitHelper := git.NewGitHelper(testLogger, testCommandExecutor)
+
+	for _, tt := range createIncrementalBundleTests {
+		t.Run(tt.title, func(t *testing.T) {
+			var stdin io.Reader
+			var stdout io.Writer
+
+			// Mock responses
+			testCommandExecutor.On("Run",
+				mock.Anything,
+				"git",
+				[]string{"-C", tt.repoDir, "bundle", "create", tt.filename, "--stdin", "--branches"},
+				mock.MatchedBy(func(settings []cmd.Setting) bool {
+					var ok bool
+					stdin = nil
+					stdout = nil
+					for _, setting := range settings {
+						switch setting.Key {
+						case cmd.StdinKey:
+							stdin, ok = setting.Value.(io.Reader)
+							if !ok {
+								return false
+							}
+						case cmd.StderrKey:
+							stdout, ok = setting.Value.(io.Writer)
+							if !ok {
+								return false
+							}
+						}
+					}
+					return stdin != nil && stdout != nil
+				}),
+			).Run(func(mock.Arguments) {
+				stdout.Write([]byte(tt.bundleCreateStderr))
+			}).Return(tt.bundleCreate.First, tt.bundleCreate.Second)
+
+			// Run 'CreateIncrementalBundle()'
+			actualBundleCreated, err := gitHelper.CreateIncrementalBundle(context.Background(), tt.repoDir, tt.filename, tt.prereqs)
+
+			// Assert on expected values
+			assert.Equal(t, tt.expectedBundleCreated, actualBundleCreated)
+			if tt.expectErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+			mock.AssertExpectationsForObjects(t, testCommandExecutor)
+
+			// Check the content of stdin
+			expectedStdin := ConcatLines(tt.prereqs)
+			expectedStdinLen := len(expectedStdin)
+			stdinBytes := make([]byte, expectedStdinLen+1)
+			numRead, err := stdin.Read(stdinBytes)
+			assert.NoError(t, err)
+			assert.Equal(t, expectedStdinLen, numRead)
+			assert.Equal(t, expectedStdin, string(stdinBytes[:expectedStdinLen]))
+
+			// Reset mocks
+			testCommandExecutor.Mock = mock.Mock{}
+		})
+	}
+}

internal/testhelpers/funcs.go

@@ -1,9 +1,17 @@
 package testhelpers
 
+import "github.com/github/git-bundle-server/internal/utils"
+
 /********************************************/
 /************* Helper Functions *************/
 /********************************************/
 
 func PtrTo[T any](val T) *T {
 	return &val
 }
+
+func ConcatLines(lines []string) string {
+	return utils.Reduce(lines, "", func(line string, out string) string {
+		return out + line + "\n"
+	})
+}