Skip to content

Commit 96140bd

Browse files
andrewandrew
committed
Run then: chains from GenericManager
BuildCommands and Command.Then existed but every GenericManager operation called BuildCommand (singular), so gomod add never ran `go mod tidy` and renv add/remove/update never ran `renv::snapshot`. Route all operations except Path through a run() helper that executes the full chain, stopping on the first non-zero exit. On success the result is the primary command's; on failure it is the failing command's so callers see what broke. Fixes #39
1 parent d963aa9 commit 96140bd

2 files changed

Lines changed: 98 additions & 99 deletions

File tree

‎generic_manager.go‎

Lines changed: 36 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -32,123 +32,80 @@ func (m *GenericManager) Warnings() []string {
3232
return m.warnings
3333
}
3434

35-
func (m *GenericManager) Init(ctx context.Context) (*Result, error) {
36-
input := CommandInput{
37-
Args: map[string]string{},
38-
Flags: map[string]any{},
39-
}
40-
41-
cmd, err := m.translator.BuildCommand(m.def.Name, "init", input)
35+
// run builds the command chain for an operation (base command plus any
36+
// then: entries) and executes them in order. It returns the first
37+
// command's result when the whole chain succeeds. When a command fails,
38+
// either as a Go error or a non-zero exit, execution stops and that
39+
// command's result is returned so callers see what failed.
40+
func (m *GenericManager) run(ctx context.Context, operation string, input CommandInput) (*Result, error) {
41+
cmds, err := m.translator.BuildCommands(m.def.Name, operation, input)
4242
if err != nil {
4343
return nil, err
4444
}
45+
var first *Result
46+
for i, cmd := range cmds {
47+
res, err := m.runner.Run(ctx, m.dir, cmd...)
48+
if err != nil {
49+
return res, err
50+
}
51+
if i == 0 {
52+
first = res
53+
}
54+
if !res.Success() {
55+
return res, nil
56+
}
57+
}
58+
return first, nil
59+
}
4560

46-
return m.runner.Run(ctx, m.dir, cmd...)
61+
func (m *GenericManager) Init(ctx context.Context) (*Result, error) {
62+
return m.run(ctx, "init", CommandInput{})
4763
}
4864

4965
func (m *GenericManager) Install(ctx context.Context, opts InstallOptions) (*Result, error) {
50-
input := CommandInput{
51-
Args: map[string]string{},
66+
return m.run(ctx, "install", CommandInput{
5267
Flags: map[string]any{
5368
"frozen": opts.Frozen,
5469
"clean": opts.Clean,
5570
"production": opts.Production,
5671
},
57-
}
58-
59-
cmd, err := m.translator.BuildCommand(m.def.Name, "install", input)
60-
if err != nil {
61-
return nil, err
62-
}
63-
64-
return m.runner.Run(ctx, m.dir, cmd...)
72+
})
6573
}
6674

6775
func (m *GenericManager) Add(ctx context.Context, pkg string, opts AddOptions) (*Result, error) {
6876
input := CommandInput{
69-
Args: map[string]string{
70-
argPackage: pkg,
71-
},
77+
Args: map[string]string{argPackage: pkg},
7278
Flags: map[string]any{
7379
"dev": opts.Dev,
7480
"optional": opts.Optional,
7581
"exact": opts.Exact,
7682
"workspace": opts.Workspace,
7783
},
7884
}
79-
8085
if opts.Version != "" {
8186
input.Args["version"] = opts.Version
8287
}
83-
84-
cmd, err := m.translator.BuildCommand(m.def.Name, "add", input)
85-
if err != nil {
86-
return nil, err
87-
}
88-
89-
return m.runner.Run(ctx, m.dir, cmd...)
88+
return m.run(ctx, "add", input)
9089
}
9190

9291
func (m *GenericManager) Remove(ctx context.Context, pkg string) (*Result, error) {
93-
input := CommandInput{
94-
Args: map[string]string{
95-
argPackage: pkg,
96-
},
97-
Flags: map[string]any{},
98-
}
99-
100-
cmd, err := m.translator.BuildCommand(m.def.Name, "remove", input)
101-
if err != nil {
102-
return nil, err
103-
}
104-
105-
return m.runner.Run(ctx, m.dir, cmd...)
92+
return m.run(ctx, "remove", CommandInput{Args: map[string]string{argPackage: pkg}})
10693
}
10794

10895
func (m *GenericManager) List(ctx context.Context) (*Result, error) {
109-
input := CommandInput{
110-
Args: map[string]string{},
111-
Flags: map[string]any{},
112-
}
113-
114-
cmd, err := m.translator.BuildCommand(m.def.Name, "list", input)
115-
if err != nil {
116-
return nil, err
117-
}
118-
119-
return m.runner.Run(ctx, m.dir, cmd...)
96+
return m.run(ctx, "list", CommandInput{})
12097
}
12198

12299
func (m *GenericManager) Outdated(ctx context.Context) (*Result, error) {
123-
input := CommandInput{
124-
Args: map[string]string{},
125-
Flags: map[string]any{},
126-
}
127-
128-
cmd, err := m.translator.BuildCommand(m.def.Name, "outdated", input)
129-
if err != nil {
130-
return nil, err
131-
}
132-
133-
return m.runner.Run(ctx, m.dir, cmd...)
100+
return m.run(ctx, "outdated", CommandInput{})
134101
}
135102

136103
func (m *GenericManager) Update(ctx context.Context, pkg string) (*Result, error) {
137-
input := CommandInput{
138-
Args: map[string]string{},
139-
Flags: map[string]any{},
140-
}
141-
104+
input := CommandInput{}
142105
if pkg != "" {
143-
input.Args[argPackage] = pkg
106+
input.Args = map[string]string{argPackage: pkg}
144107
}
145-
146-
cmd, err := m.translator.BuildCommand(m.def.Name, "update", input)
147-
if err != nil {
148-
return nil, err
149-
}
150-
151-
return m.runner.Run(ctx, m.dir, cmd...)
108+
return m.run(ctx, "update", input)
152109
}
153110

154111
func (m *GenericManager) Supports(cap Capability) bool {
@@ -172,31 +129,11 @@ func (m *GenericManager) Capabilities() []Capability {
172129
}
173130

174131
func (m *GenericManager) Vendor(ctx context.Context) (*Result, error) {
175-
input := CommandInput{
176-
Args: map[string]string{},
177-
Flags: map[string]any{},
178-
}
179-
180-
cmd, err := m.translator.BuildCommand(m.def.Name, "vendor", input)
181-
if err != nil {
182-
return nil, err
183-
}
184-
185-
return m.runner.Run(ctx, m.dir, cmd...)
132+
return m.run(ctx, "vendor", CommandInput{})
186133
}
187134

188135
func (m *GenericManager) Resolve(ctx context.Context) (*Result, error) {
189-
input := CommandInput{
190-
Args: map[string]string{},
191-
Flags: map[string]any{},
192-
}
193-
194-
cmd, err := m.translator.BuildCommand(m.def.Name, "resolve", input)
195-
if err != nil {
196-
return nil, err
197-
}
198-
199-
return m.runner.Run(ctx, m.dir, cmd...)
136+
return m.run(ctx, "resolve", CommandInput{})
200137
}
201138

202139
func (m *GenericManager) Path(ctx context.Context, pkg string) (*PathResult, error) {

‎generic_manager_test.go‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,68 @@ func TestGenericManager_Add_CargoVersion(t *testing.T) {
6161
}
6262
}
6363

64+
func TestGenericManager_Add_RunsThenChain(t *testing.T) {
65+
runner := NewMockRunner()
66+
mgr := newTestManager(embeddedDef(t, "gomod"), runner)
67+
res, err := mgr.Add(context.Background(), "github.com/pkg/errors", AddOptions{})
68+
if err != nil {
69+
t.Fatalf("Add: %v", err)
70+
}
71+
if len(runner.Captured) != 2 {
72+
t.Fatalf("captured %d commands, want 2 (go get + go mod tidy)", len(runner.Captured))
73+
}
74+
if !slicesEqual(runner.Captured[0], []string{"go", "get", "github.com/pkg/errors"}) {
75+
t.Errorf("cmd[0] = %v", runner.Captured[0])
76+
}
77+
if !slicesEqual(runner.Captured[1], []string{"go", "mod", "tidy"}) {
78+
t.Errorf("cmd[1] = %v", runner.Captured[1])
79+
}
80+
// Result reflects the primary command on success.
81+
if !slicesEqual(res.Command, []string{"go", "get", "github.com/pkg/errors"}) {
82+
t.Errorf("res.Command = %v, want the primary go get", res.Command)
83+
}
84+
}
85+
86+
func TestGenericManager_Add_ChainStopsOnFailure(t *testing.T) {
87+
runner := NewMockRunner()
88+
runner.Results = []*Result{
89+
{Command: []string{"go", "get", "x"}, ExitCode: 1, Stderr: "boom"},
90+
}
91+
mgr := newTestManager(embeddedDef(t, "gomod"), runner)
92+
res, err := mgr.Add(context.Background(), "example.com/x", AddOptions{})
93+
if err != nil {
94+
t.Fatalf("Add: %v", err)
95+
}
96+
if res.Success() {
97+
t.Error("Success() should be false when first command exits non-zero")
98+
}
99+
if len(runner.Captured) != 1 {
100+
t.Errorf("captured %d commands, want 1 (chain should stop)", len(runner.Captured))
101+
}
102+
}
103+
104+
func TestGenericManager_Add_ChainReturnsFailingThen(t *testing.T) {
105+
runner := NewMockRunner()
106+
runner.Results = []*Result{
107+
{Command: []string{"go", "get", "x"}, ExitCode: 0},
108+
{Command: []string{"go", "mod", "tidy"}, ExitCode: 1, Stderr: "tidy failed"},
109+
}
110+
mgr := newTestManager(embeddedDef(t, "gomod"), runner)
111+
res, err := mgr.Add(context.Background(), "example.com/x", AddOptions{})
112+
if err != nil {
113+
t.Fatalf("Add: %v", err)
114+
}
115+
if res.Success() {
116+
t.Error("Success() should be false when a then: command exits non-zero")
117+
}
118+
if !slicesEqual(res.Command, []string{"go", "mod", "tidy"}) {
119+
t.Errorf("res.Command = %v, want the failing then: command", res.Command)
120+
}
121+
if res.Stderr != "tidy failed" {
122+
t.Errorf("res.Stderr = %q", res.Stderr)
123+
}
124+
}
125+
64126
func TestGenericManager_Path_Raw(t *testing.T) {
65127
def := &definitions.Definition{
66128
Name: "testpkg",

0 commit comments

Comments
 (0)