Skip to content

Commit a863be0

Browse files
aledbfclaude
andcommitted
test(lifecycle): expect executor calls with a gomock mock
Four hand-written CommandExecutor doubles recorded commands in slices and the tests read them back; one of them raced once an object hook ran its commands in parallel. MockCommandExecutor (go.uber.org/mock, generated with `go generate ./internal/lifecycle`) states the commands each test expects instead, in order where the lifecycle runs them in order, with no state shared between goroutines. Tests that counted commands now name them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent aeaad24 commit a863be0

8 files changed

Lines changed: 175 additions & 123 deletions

File tree

‎go.mod‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ require (
88
github.com/Masterminds/semver/v3 v3.5.0
99
github.com/iancoleman/orderedmap v0.3.0
1010
github.com/moby/buildkit v0.34.1
11+
github.com/moby/docker-image-spec v1.3.1
1112
github.com/moby/moby/api v1.56.1
1213
github.com/moby/moby/client v0.6.2
1314
github.com/opencontainers/go-digest v1.0.0
@@ -17,6 +18,7 @@ require (
1718
github.com/spin-stack/go-tools v0.0.0-20261025134518-0ee222410bc5
1819
github.com/tailscale/hujson v0.0.0-20260727124030-b80ff77dac4f
1920
github.com/testcontainers/testcontainers-go v0.44.0
21+
go.uber.org/mock v0.6.0
2022
golang.org/x/crypto v0.58.0
2123
golang.org/x/net v0.61.0
2224
gopkg.in/yaml.v3 v3.0.1
@@ -127,7 +129,6 @@ require (
127129
github.com/mattn/go-runewidth v0.0.27 // indirect
128130
github.com/mitchellh/go-homedir v1.1.0 // indirect
129131
github.com/mitchellh/hashstructure/v2 v2.0.2 // indirect
130-
github.com/moby/docker-image-spec v1.3.1 // indirect
131132
github.com/moby/go-archive v0.3.0 // indirect
132133
github.com/moby/patternmatcher v0.6.1 // indirect
133134
github.com/moby/sys/sequential v0.7.0 // indirect
@@ -197,5 +198,6 @@ tool (
197198
github.com/spin-stack/go-tools/cmd/testquality
198199
github.com/spin-stack/go-tools/cmd/versions
199200
github.com/spin-stack/go-tools/cmd/vulngate
201+
go.uber.org/mock/mockgen
200202
golang.org/x/tools/cmd/deadcode
201203
)

‎go.sum‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,8 @@ go.opentelemetry.io/otel/sdk/metric v1.46.0 h1:0piZ26EG4RBfebb2jhDH6ERCYHoVWduc3
361361
go.opentelemetry.io/otel/sdk/metric v1.46.0/go.mod h1:I1PbKrdVc8Qu8HYVDNtqVIwLwjNrhsV/uFuxfwg8mO4=
362362
go.opentelemetry.io/otel/trace v1.46.0 h1:OULy7ccdJnZtJ0UDYFOIGaCmiWzJ8Vi2G/Rsu60qs1c=
363363
go.opentelemetry.io/otel/trace v1.46.0/go.mod h1:J7GAXweO77XSFkB/rmAqk9D6ihszhFjLU+d9WuUxDLI=
364+
go.uber.org/mock v0.6.0 h1:hyF9dfmbgIX5EfOdasqLsWD6xqpNZlXblLB/Dbnwv3Y=
365+
go.uber.org/mock v0.6.0/go.mod h1:KiVJ4BqZJaMj4svdfmHM0AUx4NJYO8ZNpPnZn1Z+BBU=
364366
go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg=
365367
go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw=
366368
go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg=

‎internal/lifecycle/dotfiles_held_test.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"strings"
55
"testing"
66

7+
"go.uber.org/mock/gomock"
8+
79
"github.com/devcontainers/cli/internal/log"
810
)
911

@@ -18,12 +20,12 @@ func TestInstallDotfilesPrefixesOnlyGitHubShorthand(t *testing.T) {
1820
"../dots": "../dots",
1921
"git@example.com:o/dots": "git@example.com:o/dots",
2022
} {
21-
exec := &mockExecutor{}
23+
exec := NewMockCommandExecutor(gomock.NewController(t))
24+
exec.EXPECT().Exec(gomock.Cond(func(command string) bool {
25+
return strings.Contains(command, "git clone --depth 1 "+want+" /t ")
26+
}))
2227
if err := InstallDotfiles(log.Null, exec, DotfilesConfig{Repository: repo, TargetPath: "/t"}); err != nil {
2328
t.Fatal(err)
2429
}
25-
if len(exec.commands) != 1 || !strings.Contains(exec.commands[0], "git clone --depth 1 "+want+" /t ") {
26-
t.Errorf("%s: commands = %q, want a clone of %s", repo, exec.commands, want)
27-
}
2830
}
2931
}

‎internal/lifecycle/hooks_held_test.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
package lifecycle
22

33
import (
4+
"errors"
45
"testing"
56

7+
"go.uber.org/mock/gomock"
8+
69
"github.com/devcontainers/cli/internal/imagemeta"
710
"github.com/devcontainers/cli/internal/log"
811
)
@@ -15,8 +18,13 @@ func TestRunHooksErrorNamesTheFailedCommand(t *testing.T) {
1518
"postCreateCommand failed: command failed: bad": "bad",
1619
"postCreateCommand failed: b: command failed: bad": map[string]any{"a": "good", "b": "bad"},
1720
} {
21+
exec := NewMockCommandExecutor(gomock.NewController(t))
22+
if _, ok := command.(map[string]any); ok {
23+
exec.EXPECT().Exec("good")
24+
}
25+
exec.EXPECT().Exec("bad").Return(errors.New("command failed: bad"))
1826
merged := &imagemeta.MergedConfig{PostCreateCommands: []any{command}}
19-
err := RunHooks(log.Null, &mockExecutor{failOn: "bad"}, merged, RunOptions{})
27+
err := RunHooks(log.Null, exec, merged, RunOptions{})
2028
if err == nil || err.Error() != want {
2129
t.Errorf("RunHooks(%v) = %v, want %q", command, err, want)
2230
}

‎internal/lifecycle/hooks_nodocker_held_test.go‎

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"sync"
77
"testing"
88

9+
"go.uber.org/mock/gomock"
10+
911
"github.com/devcontainers/cli/internal/imagemeta"
1012
"github.com/devcontainers/cli/internal/log"
1113
)
@@ -29,15 +31,6 @@ func (l *lockedLog) has(s string) bool {
2931
return slices.Contains(l.lines, s)
3032
}
3133

32-
// orderExecutor records every command, and every AfterPostCreate call as
33-
// "<dotfiles>", in the order they happen.
34-
type orderExecutor struct{ seen []string }
35-
36-
func (o *orderExecutor) Exec(command string) error {
37-
o.seen = append(o.seen, command)
38-
return nil
39-
}
40-
4134
// skipPostAttach skips postAttachCommand alone, and the dotfiles hook runs once,
4235
// right after postCreateCommand (TS runLifecycleHooks / installDotfiles).
4336
func TestRunHooksSkipsOnlyPostAttachAndInstallsDotfilesAfterPostCreate(t *testing.T) {
@@ -48,14 +41,27 @@ func TestRunHooksSkipsOnlyPostAttachAndInstallsDotfilesAfterPostCreate(t *testin
4841
PostStartCommands: []any{"postStart"},
4942
PostAttachCommands: []any{"postAttach"},
5043
}
51-
exec := &orderExecutor{}
52-
opts := RunOptions{SkipPostAttach: true, AfterPostCreate: func() { exec.seen = append(exec.seen, "<dotfiles>") }}
44+
dotfiles := 0
45+
dotfilesRanBefore := func(want int) func(string) {
46+
return func(command string) {
47+
if dotfiles != want {
48+
t.Errorf("%s ran after %d dotfiles installs, want %d", command, dotfiles, want)
49+
}
50+
}
51+
}
52+
exec := NewMockCommandExecutor(gomock.NewController(t))
53+
gomock.InOrder(
54+
exec.EXPECT().Exec("onCreate"),
55+
exec.EXPECT().Exec("updateContent"),
56+
exec.EXPECT().Exec("postCreate").Do(dotfilesRanBefore(0)),
57+
exec.EXPECT().Exec("postStart").Do(dotfilesRanBefore(1)),
58+
)
59+
opts := RunOptions{SkipPostAttach: true, AfterPostCreate: func() { dotfiles++ }}
5360
if err := RunHooks(log.Null, exec, merged, opts); err != nil {
5461
t.Fatal(err)
5562
}
56-
want := []string{"onCreate", "updateContent", "postCreate", "<dotfiles>", "postStart"}
57-
if !slices.Equal(exec.seen, want) {
58-
t.Errorf("ran %v, want %v", exec.seen, want)
63+
if dotfiles != 1 {
64+
t.Errorf("dotfiles installed %d times, want once", dotfiles)
5965
}
6066
}
6167

@@ -66,7 +72,10 @@ func TestRunHooksSkipsOnlyPostAttachAndInstallsDotfilesAfterPostCreate(t *testin
6672
func TestParallelCommandHeaderOnlyForSuccess(t *testing.T) {
6773
l := &lockedLog{Logger: log.Null}
6874
cmd := map[string]any{"a": "ok-a", "b": "fails-b"}
69-
if err := executeCommand(&cmdErrExecutor{failOn: "fails"}, l, PhasePostCreate, cmd, ""); err == nil {
75+
exec := NewMockCommandExecutor(gomock.NewController(t))
76+
exec.EXPECT().Exec("ok-a")
77+
exec.EXPECT().Exec("fails-b").Return(exitedOne("fails-b"))
78+
if err := executeCommand(exec, l, PhasePostCreate, cmd, ""); err == nil {
7079
t.Fatal("expected b's failure")
7180
}
7281
if !l.has("Running a of postCreateCommand from devcontainer.json...") {
@@ -80,24 +89,22 @@ func TestParallelCommandHeaderOnlyForSuccess(t *testing.T) {
8089
}
8190
}
8291

83-
// codeExecutor fails every command with the given exit code.
84-
type codeExecutor struct{ code int }
85-
86-
func (c codeExecutor) Exec(command string) error {
87-
return &CommandError{Command: command, ExitCode: c.code}
88-
}
89-
9092
// TS runLifecycleCommand writes "failed with exit code N" only when the error
9193
// carries a code (`if (err?.code)`): exit code 1 is reported, 0 is not.
9294
func TestFailureLineNeedsANonZeroExitCode(t *testing.T) {
9395
const line = "postStartCommand from devcontainer.json failed with exit code 1. Skipping any further user-provided commands."
96+
exitsWith := func(code int) CommandExecutor {
97+
exec := NewMockCommandExecutor(gomock.NewController(t))
98+
exec.EXPECT().Exec("x").Return(&CommandError{Command: "x", ExitCode: code})
99+
return exec
100+
}
94101
l := &lockedLog{Logger: log.Null}
95-
_ = executeCommand(codeExecutor{code: 1}, l, PhasePostStart, "x", "")
102+
_ = executeCommand(exitsWith(1), l, PhasePostStart, "x", "")
96103
if !l.has(line) {
97104
t.Errorf("exit code 1: lines = %q, want %q", l.lines, line)
98105
}
99106
l = &lockedLog{Logger: log.Null}
100-
_ = executeCommand(codeExecutor{code: 0}, l, PhasePostStart, "x", "")
107+
_ = executeCommand(exitsWith(0), l, PhasePostStart, "x", "")
101108
for _, got := range l.lines {
102109
if strings.Contains(got, "failed with exit code") {
103110
t.Errorf("exit code 0 reported a failure line: %q", got)

0 commit comments

Comments
 (0)