Skip to content

Commit 53a4a35

Browse files
feat(cleaner): track per-package remove/fail outcomes in CleanResult
Adds RemovedFormulae/Casks/Npm + FailedFormulae/Casks/Npm fields and TotalRemoved/TotalFailed helpers. Execute now uninstalls one package at a time to populate these fields, and returns errors.Join with per-package detail instead of a lossy count-only message. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
1 parent 56ad62e commit 53a4a35

2 files changed

Lines changed: 157 additions & 25 deletions

File tree

internal/cleaner/cleaner.go

Lines changed: 55 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package cleaner
22

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

67
"github.com/openbootdotdev/openboot/internal/brew"
@@ -13,12 +14,28 @@ type CleanResult struct {
1314
ExtraFormulae []string
1415
ExtraCasks []string
1516
ExtraNpm []string
17+
18+
RemovedFormulae []string
19+
RemovedCasks []string
20+
RemovedNpm []string
21+
22+
FailedFormulae []string
23+
FailedCasks []string
24+
FailedNpm []string
1625
}
1726

1827
func (r *CleanResult) TotalExtra() int {
1928
return len(r.ExtraFormulae) + len(r.ExtraCasks) + len(r.ExtraNpm)
2029
}
2130

31+
func (r *CleanResult) TotalRemoved() int {
32+
return len(r.RemovedFormulae) + len(r.RemovedCasks) + len(r.RemovedNpm)
33+
}
34+
35+
func (r *CleanResult) TotalFailed() int {
36+
return len(r.FailedFormulae) + len(r.FailedCasks) + len(r.FailedNpm)
37+
}
38+
2239
func DiffFromSnapshot(snap *snapshot.Snapshot) (*CleanResult, error) {
2340
desiredFormulae := toSet(snap.Packages.Formulae)
2441
desiredCasks := toSet(snap.Packages.Casks)
@@ -67,47 +84,60 @@ func diff(desiredFormulae, desiredCasks, desiredNpm map[string]bool) (*CleanResu
6784
return result, nil
6885
}
6986

70-
func Execute(result *CleanResult, dryRun bool) error {
71-
type uninstallOp struct {
72-
label string
73-
pkgs []string
74-
uninstall func([]string, bool) error
75-
}
87+
type uninstallOneFn func(pkg string, dryRun bool) error
88+
89+
type uninstallOp struct {
90+
label string
91+
pkgs []string
92+
uninstallOne uninstallOneFn
93+
removed *[]string
94+
failed *[]string
95+
}
7696

97+
func Execute(result *CleanResult, dryRun bool) error {
7798
ops := []uninstallOp{
7899
{
79-
label: "Removing extra formulae",
80-
pkgs: result.ExtraFormulae,
81-
uninstall: brew.Uninstall,
100+
label: "Removing extra formulae",
101+
pkgs: result.ExtraFormulae,
102+
uninstallOne: func(pkg string, dry bool) error { return brew.Uninstall([]string{pkg}, dry) },
103+
removed: &result.RemovedFormulae,
104+
failed: &result.FailedFormulae,
82105
},
83106
{
84-
label: "Removing extra casks",
85-
pkgs: result.ExtraCasks,
86-
uninstall: brew.UninstallCask,
107+
label: "Removing extra casks",
108+
pkgs: result.ExtraCasks,
109+
uninstallOne: func(pkg string, dry bool) error { return brew.UninstallCask([]string{pkg}, dry) },
110+
removed: &result.RemovedCasks,
111+
failed: &result.FailedCasks,
87112
},
88113
{
89-
label: "Removing extra npm packages",
90-
pkgs: result.ExtraNpm,
91-
uninstall: npm.Uninstall,
114+
label: "Removing extra npm packages",
115+
pkgs: result.ExtraNpm,
116+
uninstallOne: func(pkg string, dry bool) error { return npm.Uninstall([]string{pkg}, dry) },
117+
removed: &result.RemovedNpm,
118+
failed: &result.FailedNpm,
92119
},
93120
}
94121

95122
var errs []error
96123
for _, op := range ops {
97-
if len(op.pkgs) > 0 {
98-
fmt.Println()
99-
ui.Header(op.label)
100-
fmt.Println()
101-
if err := op.uninstall(op.pkgs, dryRun); err != nil {
102-
errs = append(errs, fmt.Errorf("%s: %w", op.label, err))
124+
if len(op.pkgs) == 0 {
125+
continue
126+
}
127+
fmt.Println()
128+
ui.Header(op.label)
129+
fmt.Println()
130+
for _, pkg := range op.pkgs {
131+
if err := op.uninstallOne(pkg, dryRun); err != nil {
132+
*op.failed = append(*op.failed, pkg)
133+
errs = append(errs, fmt.Errorf("%s: %w", pkg, err))
134+
} else {
135+
*op.removed = append(*op.removed, pkg)
103136
}
104137
}
105138
}
106139

107-
if len(errs) > 0 {
108-
return fmt.Errorf("%d cleanup steps had failures", len(errs))
109-
}
110-
return nil
140+
return errors.Join(errs...)
111141
}
112142

113143
func toSet(items []string) map[string]bool {

internal/cleaner/cleaner_test.go

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package cleaner
22

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

67
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
79
)
810

911
func TestToSet(t *testing.T) {
@@ -96,3 +98,103 @@ func TestCleanResult_TotalExtra(t *testing.T) {
9698
})
9799
}
98100
}
101+
102+
func TestCleanResult_TotalRemoved(t *testing.T) {
103+
r := &CleanResult{
104+
RemovedFormulae: []string{"a"},
105+
RemovedCasks: []string{"b", "c"},
106+
RemovedNpm: []string{},
107+
}
108+
assert.Equal(t, 3, r.TotalRemoved())
109+
}
110+
111+
func TestCleanResult_TotalFailed(t *testing.T) {
112+
r := &CleanResult{
113+
FailedFormulae: []string{"x"},
114+
FailedNpm: []string{"y"},
115+
}
116+
assert.Equal(t, 2, r.TotalFailed())
117+
}
118+
119+
func runOp(op uninstallOp, dryRun bool) error {
120+
var errs []error
121+
for _, pkg := range op.pkgs {
122+
if err := op.uninstallOne(pkg, dryRun); err != nil {
123+
*op.failed = append(*op.failed, pkg)
124+
errs = append(errs, err)
125+
} else {
126+
*op.removed = append(*op.removed, pkg)
127+
}
128+
}
129+
return errors.Join(errs...)
130+
}
131+
132+
func TestExecute_AllSucceed(t *testing.T) {
133+
result := &CleanResult{
134+
ExtraFormulae: []string{"wget", "curl"},
135+
}
136+
137+
callLog := []string{}
138+
op := uninstallOp{
139+
label: "Removing extra formulae",
140+
pkgs: result.ExtraFormulae,
141+
uninstallOne: func(pkg string, _ bool) error {
142+
callLog = append(callLog, pkg)
143+
return nil
144+
},
145+
removed: &result.RemovedFormulae,
146+
failed: &result.FailedFormulae,
147+
}
148+
149+
require.NoError(t, runOp(op, false))
150+
assert.Equal(t, []string{"wget", "curl"}, result.RemovedFormulae)
151+
assert.Empty(t, result.FailedFormulae)
152+
assert.Equal(t, []string{"wget", "curl"}, callLog)
153+
}
154+
155+
func TestExecute_PartialFailure(t *testing.T) {
156+
result := &CleanResult{
157+
ExtraFormulae: []string{"good-pkg", "bad-pkg", "another-good"},
158+
}
159+
160+
op := uninstallOp{
161+
label: "Removing extra formulae",
162+
pkgs: result.ExtraFormulae,
163+
uninstallOne: func(pkg string, _ bool) error {
164+
if pkg == "bad-pkg" {
165+
return errors.New("dependency conflict")
166+
}
167+
return nil
168+
},
169+
removed: &result.RemovedFormulae,
170+
failed: &result.FailedFormulae,
171+
}
172+
173+
require.Error(t, runOp(op, false))
174+
assert.Equal(t, []string{"good-pkg", "another-good"}, result.RemovedFormulae)
175+
assert.Equal(t, []string{"bad-pkg"}, result.FailedFormulae)
176+
assert.Equal(t, 2, result.TotalRemoved())
177+
assert.Equal(t, 1, result.TotalFailed())
178+
}
179+
180+
func TestExecute_DryRun_PassedThrough(t *testing.T) {
181+
result := &CleanResult{
182+
ExtraNpm: []string{"typescript", "eslint"},
183+
}
184+
185+
sawDryRun := false
186+
op := uninstallOp{
187+
label: "Removing extra npm packages",
188+
pkgs: result.ExtraNpm,
189+
uninstallOne: func(_ string, dryRun bool) error {
190+
sawDryRun = dryRun
191+
return nil
192+
},
193+
removed: &result.RemovedNpm,
194+
failed: &result.FailedNpm,
195+
}
196+
197+
require.NoError(t, runOp(op, true))
198+
assert.True(t, sawDryRun)
199+
assert.Equal(t, []string{"typescript", "eslint"}, result.RemovedNpm)
200+
}

0 commit comments

Comments
 (0)