Skip to content

Commit d225e55

Browse files
akoclaude
andcommitted
fix(#424): pin diff-local git-error propagation with regression tests
The bug (exit 0 when git diff fails) was introduced in the original implementation and was already fixed by the executor refactoring, but with no test coverage to prevent it from regressing. Makes execCommand a package-level var so tests can inject a stub that simulates git exit-128 failures without spawning a real git process. Adds two regression tests: - GitError_ReturnsError: git fails → diffLocal returns non-nil error - NoChanges_ReturnsNil: git succeeds with no output → nil (exit 0) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 01c2d77 commit d225e55

2 files changed

Lines changed: 82 additions & 2 deletions

File tree

mdl/executor/cmd_diff_local.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -753,8 +753,10 @@ func parseRefRange(ref string) (base, target string, isRange bool) {
753753
return "", ref, false
754754
}
755755

756-
// execCommand creates an exec.Cmd for running git commands
757-
func execCommand(name string, args ...string) *exec.Cmd {
756+
// execCommand creates an exec.Cmd for running git commands.
757+
// It is a package-level variable so tests can replace it with a stub that
758+
// simulates git failures without actually invoking git.
759+
var execCommand = func(name string, args ...string) *exec.Cmd {
758760
return exec.Command(name, args...)
759761
}
760762

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
3+
package executor
4+
5+
import (
6+
"bytes"
7+
"context"
8+
"os/exec"
9+
"strings"
10+
"testing"
11+
12+
"github.com/mendixlabs/mxcli/mdl/backend/mock"
13+
"github.com/mendixlabs/mxcli/mdl/types"
14+
)
15+
16+
// gitFailBackend returns an ExecContext backed by a MockBackend that reports
17+
// MPR v2 with a non-empty ContentsDir — enough to reach findChangedMxunitFiles.
18+
func gitFailBackend(contentsDir string) *ExecContext {
19+
mb := &mock.MockBackend{
20+
IsConnectedFunc: func() bool { return true },
21+
VersionFunc: func() types.MPRVersion { return 2 },
22+
ContentsDirFunc: func() string { return contentsDir },
23+
}
24+
return &ExecContext{
25+
Context: context.Background(),
26+
Backend: mb,
27+
Output: &bytes.Buffer{},
28+
Cache: &executorCache{},
29+
}
30+
}
31+
32+
// TestDiffLocal_GitError_ReturnsError is a regression test for issue #424:
33+
// when git is not available or the project is not in a git repo, DiffLocal
34+
// must return a non-nil error so the CLI can exit with code 1.
35+
//
36+
// The test replaces execCommand with a stub that simulates "git not a repo"
37+
// (exit 128) so no real git process is spawned.
38+
func TestDiffLocal_GitError_ReturnsError(t *testing.T) {
39+
orig := execCommand
40+
defer func() { execCommand = orig }()
41+
42+
// Simulate `git diff` failing with exit 128 (not a git repo).
43+
execCommand = func(name string, args ...string) *exec.Cmd {
44+
// Use the "false" command (always exits 1) or a helper binary that
45+
// exits with a specific code. exec.Command("sh", "-c", "exit 128")
46+
// produces the same *ExitError that real git produces.
47+
return exec.Command("sh", "-c", "exit 128")
48+
}
49+
50+
ctx := gitFailBackend("/tmp/mprcontents")
51+
err := diffLocal(ctx, "HEAD", DiffOptions{})
52+
if err == nil {
53+
t.Fatal("diffLocal must return a non-nil error when git fails (issue #424: exit 0 on git error)")
54+
}
55+
56+
// Error message should mention "git diff" so callers can surface a useful message.
57+
if !strings.Contains(err.Error(), "git diff") {
58+
t.Errorf("error message should mention 'git diff', got: %q", err.Error())
59+
}
60+
}
61+
62+
// TestDiffLocal_NoChanges_ReturnsNil verifies that when git succeeds but
63+
// reports no changed files, DiffLocal returns nil (exit 0 is correct).
64+
func TestDiffLocal_NoChanges_ReturnsNil(t *testing.T) {
65+
orig := execCommand
66+
defer func() { execCommand = orig }()
67+
68+
// Simulate `git diff` succeeding with empty output (no changes).
69+
execCommand = func(name string, args ...string) *exec.Cmd {
70+
return exec.Command("sh", "-c", "exit 0")
71+
}
72+
73+
ctx := gitFailBackend("/tmp/mprcontents")
74+
err := diffLocal(ctx, "HEAD", DiffOptions{})
75+
if err != nil {
76+
t.Errorf("diffLocal must return nil when git reports no changes; got: %v", err)
77+
}
78+
}

0 commit comments

Comments
 (0)