Skip to content

Commit 523e8dd

Browse files
jbrand-dspjbrand-dsp
committed
improved tests
1 parent 52939f6 commit 523e8dd

File tree

2 files changed

+139
-60
lines changed

2 files changed

+139
-60
lines changed

‎cmd/clean.go‎

Lines changed: 83 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,24 @@ import (
1414
"github.com/bruin-data/bruin/pkg/git"
1515
"github.com/bruin-data/bruin/pkg/telemetry"
1616
"github.com/bruin-data/bruin/pkg/user"
17+
"github.com/fatih/color"
1718
"github.com/pkg/errors"
1819
"github.com/spf13/afero"
1920
"github.com/urfave/cli/v3"
2021
)
2122

23+
type colorPrinter struct {
24+
c *color.Color
25+
}
26+
27+
func (p *colorPrinter) Printf(format string, args ...interface{}) {
28+
p.c.Printf(format, args...) // ignore return values
29+
}
30+
31+
func (p *colorPrinter) Println(args ...interface{}) {
32+
p.c.Println(args...)
33+
}
34+
2235
func CleanCmd() *cli.Command {
2336
return &cli.Command{
2437
Name: "clean",
@@ -37,87 +50,118 @@ func CleanCmd() *cli.Command {
3750
inputPath = "."
3851
}
3952

40-
r := CleanCommand{
41-
infoPrinter: infoPrinter,
42-
errorPrinter: errorPrinter,
53+
r := NewCleanCommand(
54+
user.NewConfigManager(afero.NewOsFs()), // cm
55+
&git.RepoFinder{}, // gitFinder
56+
afero.NewOsFs(), // fs
57+
&colorPrinter{c: color.New(color.FgGreen)}, // infoPrinter
58+
&colorPrinter{c: color.New(color.FgRed)}, // errorPrinter
59+
)
60+
61+
err := r.Run(inputPath, c.Bool("uv-cache"))
62+
if err != nil {
63+
return cli.Exit("", 1)
4364
}
44-
45-
return r.Run(inputPath, c.Bool("uv-cache"))
65+
return nil
4666
},
4767
Before: telemetry.BeforeCommand,
4868
After: telemetry.AfterCommand,
4969
}
5070
}
5171

72+
type ConfigManager interface {
73+
EnsureAndGetBruinHomeDir() (string, error)
74+
RecreateHomeDir() error
75+
}
76+
77+
type GitFinder interface {
78+
Repo(path string) (*git.Repo, error)
79+
}
80+
81+
type Printer interface {
82+
Printf(format string, a ...interface{})
83+
Println(a ...interface{})
84+
}
85+
5286
type CleanCommand struct {
53-
cm user.ConfigManager
54-
fs afero.Fs
55-
infoPrinter printer
56-
errorPrinter printer
87+
cm ConfigManager
88+
gitFinder GitFinder
89+
fs afero.Fs
90+
infoPrinter Printer
91+
errorPrinter Printer
5792
}
5893

59-
func NewCleanCommand(
60-
cm user.ConfigManager,
61-
fs afero.Fs,
62-
infoPrinter printer,
63-
errorPrinter printer,
64-
) *CleanCommand {
94+
func NewCleanCommand(cm ConfigManager, gitFinder GitFinder, fs afero.Fs, info Printer, errPrinter Printer) *CleanCommand {
6595
return &CleanCommand{
6696
cm: cm,
97+
gitFinder: gitFinder,
6798
fs: fs,
68-
infoPrinter: infoPrinter,
69-
errorPrinter: errorPrinter,
99+
infoPrinter: info,
100+
errorPrinter: errPrinter,
70101
}
71102
}
72103

73-
74104
func (r *CleanCommand) Run(inputPath string, cleanUvCache bool) error {
75-
bruinHomeDirAbsPath, err := r.cm.EnsureAndGetBruinHomeDir()
105+
bruinHomeDir, err := r.cm.EnsureAndGetBruinHomeDir()
76106
if err != nil {
77107
return errors.Wrap(err, "failed to get bruin home directory")
78108
}
79109

80-
// Clean uv caches if requested
81110
if cleanUvCache {
82-
if err := r.cleanUvCache(bruinHomeDirAbsPath); err != nil {
111+
if err := r.cleanUvCache(bruinHomeDir); err != nil {
83112
return err
84113
}
85114
}
86115

87-
err = r.cm.RecreateHomeDir()
88-
if err != nil {
116+
if err := r.cm.RecreateHomeDir(); err != nil {
89117
return errors.Wrap(err, "failed to recreate the home directory")
90118
}
91119

92-
repoRoot, err := git.FindRepoFromPath(inputPath)
120+
repoRoot, err := r.gitFinder.Repo(inputPath)
93121
if err != nil {
94122
r.errorPrinter.Printf("Failed to find the git repository root: %v\n", err)
95-
return cli.Exit("", 1)
123+
return errors.Wrap(err, "failed to find the git repository root")
96124
}
97125

98126
logsFolder := path.Join(repoRoot.Path, LogsFolder)
99127

100-
contents, err := filepath.Glob(logsFolder + "/*.log")
128+
// Check if logs folder exists
129+
exists, err := afero.Exists(r.fs, logsFolder)
101130
if err != nil {
102-
return errors.Wrap(err, "failed to find the logs folder")
131+
return errors.Wrap(err, "failed to check logs folder")
103132
}
104133

105-
if len(contents) == 0 {
134+
if !exists {
106135
r.infoPrinter.Println("No log files found, nothing to clean up...")
107136
return nil
108137
}
109138

110-
r.infoPrinter.Printf("Found %d log files, cleaning them up...\n", len(contents))
139+
// Read directory contents
140+
entries, err := afero.ReadDir(r.fs, logsFolder)
141+
if err != nil {
142+
return errors.Wrap(err, "failed to read logs folder")
143+
}
111144

112-
for _, f := range contents {
113-
err := os.Remove(f)
114-
if err != nil {
115-
return errors.Wrapf(err, "failed to remove file: %s", f)
145+
// Filter for .log files
146+
var logFiles []string
147+
for _, entry := range entries {
148+
if !entry.IsDir() && strings.HasSuffix(entry.Name(), ".log") {
149+
logFiles = append(logFiles, path.Join(logsFolder, entry.Name()))
116150
}
117151
}
118152

119-
r.infoPrinter.Printf("Successfully removed %d log files.\n", len(contents))
153+
if len(logFiles) == 0 {
154+
r.infoPrinter.Println("No log files found, nothing to clean up...")
155+
return nil
156+
}
120157

158+
r.infoPrinter.Printf("Found %d log files, cleaning them up...\n", len(logFiles))
159+
for _, f := range logFiles {
160+
if err := r.fs.Remove(f); err != nil {
161+
return errors.Wrapf(err, "failed to remove file: %s", f)
162+
}
163+
}
164+
r.infoPrinter.Printf("Successfully removed %d log files.\n", len(logFiles))
121165
return nil
122166
}
123167

@@ -132,7 +176,7 @@ func (r *CleanCommand) cleanUvCache(bruinHomeDirAbsPath string) error {
132176

133177
// Check if uv binary exists
134178
if _, err := os.Stat(uvBinaryPath); os.IsNotExist(err) {
135-
infoPrinter.Println("UV is not installed yet. Nothing to clean.")
179+
r.infoPrinter.Println("UV is not installed yet. Nothing to clean.")
136180
return nil
137181
}
138182

@@ -144,19 +188,19 @@ func (r *CleanCommand) cleanUvCache(bruinHomeDirAbsPath string) error {
144188

145189
// Prompt user for confirmation
146190
if !r.confirmUvCacheClean() {
147-
infoPrinter.Println("UV cache cleaning cancelled by user.")
191+
r.infoPrinter.Println("UV cache cleaning cancelled by user.")
148192
return nil
149193
}
150194

151-
infoPrinter.Println("Cleaning uv caches...")
195+
r.infoPrinter.Println("Cleaning uv caches...")
152196

153197
cleanCmd := exec.Command(uvBinaryPath, "cache", "clean")
154198
output, err := cleanCmd.CombinedOutput()
155199
if err != nil {
156200
return errors.Wrapf(err, "failed to clean uv cache: %s", string(output))
157201
}
158202

159-
infoPrinter.Println("Successfully cleaned uv caches.")
203+
r.infoPrinter.Println("Successfully cleaned uv caches.")
160204
return nil
161205
}
162206

@@ -166,7 +210,7 @@ func (r *CleanCommand) confirmUvCacheClean() bool {
166210

167211
response, err := reader.ReadString('\n')
168212
if err != nil {
169-
errorPrinter.Printf("Error reading input: %v\n", err)
213+
r.errorPrinter.Printf("Error reading input: %v\n", err)
170214
return false
171215
}
172216

‎cmd/clean_test.go‎

Lines changed: 56 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"path"
77
"testing"
88

9+
"github.com/bruin-data/bruin/pkg/git"
910
"github.com/spf13/afero"
1011
"github.com/stretchr/testify/assert"
1112
"github.com/stretchr/testify/require"
@@ -26,6 +27,16 @@ func (m *mockConfigManager) RecreateHomeDir() error {
2627
return m.recreateErr
2728
}
2829

30+
// --- Mock GitFinder ---
31+
type mockGitFinder struct {
32+
repo *git.Repo
33+
err error
34+
}
35+
36+
func (m *mockGitFinder) Repo(path string) (*git.Repo, error) {
37+
return m.repo, m.err
38+
}
39+
2940
// --- Mock filesystem that fails on Remove ---
3041
type mockFailingFs struct {
3142
afero.Fs
@@ -56,61 +67,65 @@ func TestCleanCommand_Run(t *testing.T) {
5667
name string
5768
inputPath string
5869
cleanUvCache bool
59-
setupMocks func(t *testing.T) (afero.Fs, *mockConfigManager, string)
70+
setupMocks func(t *testing.T) (afero.Fs, *mockConfigManager, *mockGitFinder, string)
6071
expectedErr string
6172
expectedOutput []string
6273
}{
6374
{
6475
name: "config manager error - failed to get bruin home dir",
6576
inputPath: ".",
66-
setupMocks: func(t *testing.T) (afero.Fs, *mockConfigManager, string) {
77+
setupMocks: func(t *testing.T) (afero.Fs, *mockConfigManager, *mockGitFinder, string) {
6778
return afero.NewMemMapFs(), &mockConfigManager{
6879
homeDirErr: errors.New("failed to get bruin home directory"),
69-
}, "."
80+
}, &mockGitFinder{}, "."
7081
},
7182
expectedErr: "failed to get bruin home directory",
7283
},
7384
{
7485
name: "config manager error - failed to recreate home dir",
7586
inputPath: ".",
76-
setupMocks: func(t *testing.T) (afero.Fs, *mockConfigManager, string) {
87+
setupMocks: func(t *testing.T) (afero.Fs, *mockConfigManager, *mockGitFinder, string) {
7788
fs := afero.NewMemMapFs()
7889
repoRoot := "/test-repo"
7990
fs.MkdirAll(path.Join(repoRoot, LogsFolder), 0755)
8091
return fs, &mockConfigManager{
8192
bruinHomeDir: "/test-bruin-home",
8293
recreateErr: errors.New("failed to recreate the home directory"),
83-
}, repoRoot
94+
}, &mockGitFinder{}, repoRoot
8495
},
8596
expectedErr: "failed to recreate the home directory",
8697
},
8798
{
8899
name: "git repo not found",
89100
inputPath: "/nonexistent-path",
90-
setupMocks: func(t *testing.T) (afero.Fs, *mockConfigManager, string) {
101+
setupMocks: func(t *testing.T) (afero.Fs, *mockConfigManager, *mockGitFinder, string) {
91102
return afero.NewMemMapFs(), &mockConfigManager{
92-
bruinHomeDir: "/test-bruin-home",
93-
}, "/nonexistent-path"
103+
bruinHomeDir: "/test-bruin-home",
104+
}, &mockGitFinder{
105+
err: errors.New("no git repository found"),
106+
}, "/nonexistent-path"
94107
},
95-
expectedErr: "Failed to find the git repository root",
108+
expectedErr: "failed to find the git repository root",
96109
},
97110
{
98111
name: "no log files found",
99112
inputPath: ".",
100-
setupMocks: func(t *testing.T) (afero.Fs, *mockConfigManager, string) {
113+
setupMocks: func(t *testing.T) (afero.Fs, *mockConfigManager, *mockGitFinder, string) {
101114
fs := afero.NewMemMapFs()
102115
repoRoot := "/test-repo"
103116
fs.MkdirAll(path.Join(repoRoot, LogsFolder), 0755)
104117
return fs, &mockConfigManager{
105-
bruinHomeDir: "/test-bruin-home",
106-
}, repoRoot
118+
bruinHomeDir: "/test-bruin-home",
119+
}, &mockGitFinder{
120+
repo: &git.Repo{Path: repoRoot},
121+
}, repoRoot
107122
},
108123
expectedOutput: []string{"No log files found, nothing to clean up..."},
109124
},
110125
{
111126
name: "successful cleanup with log files",
112127
inputPath: ".",
113-
setupMocks: func(t *testing.T) (afero.Fs, *mockConfigManager, string) {
128+
setupMocks: func(t *testing.T) (afero.Fs, *mockConfigManager, *mockGitFinder, string) {
114129
fs := afero.NewMemMapFs()
115130
repoRoot := "/test-repo"
116131
logsFolder := path.Join(repoRoot, LogsFolder)
@@ -122,18 +137,20 @@ func TestCleanCommand_Run(t *testing.T) {
122137
file.Close()
123138
}
124139
return fs, &mockConfigManager{
125-
bruinHomeDir: "/test-bruin-home",
126-
}, repoRoot
140+
bruinHomeDir: "/test-bruin-home",
141+
}, &mockGitFinder{
142+
repo: &git.Repo{Path: repoRoot},
143+
}, repoRoot
127144
},
128145
expectedOutput: []string{
129-
"Found 3 log files, cleaning them up...",
130-
"Successfully removed 3 log files.",
146+
"Found 3 log files, cleaning them up...\n",
147+
"Successfully removed 3 log files.\n",
131148
},
132149
},
133150
{
134151
name: "file removal error",
135152
inputPath: ".",
136-
setupMocks: func(t *testing.T) (afero.Fs, *mockConfigManager, string) {
153+
setupMocks: func(t *testing.T) (afero.Fs, *mockConfigManager, *mockGitFinder, string) {
137154
fs := &mockFailingFs{
138155
Fs: afero.NewMemMapFs(),
139156
removeErr: errors.New("permission denied"),
@@ -144,21 +161,39 @@ func TestCleanCommand_Run(t *testing.T) {
144161
file, _ := fs.Create(path.Join(logsFolder, "test1.log"))
145162
file.WriteString("test log content")
146163
file.Close()
164+
return fs, &mockConfigManager{
165+
bruinHomeDir: "/test-bruin-home",
166+
}, &mockGitFinder{
167+
repo: &git.Repo{Path: repoRoot},
168+
}, repoRoot
169+
},
170+
expectedErr: "failed to remove file",
171+
},
172+
{
173+
name: "successful cleanup with uv cache",
174+
inputPath: ".",
175+
cleanUvCache: true,
176+
setupMocks: func(t *testing.T) (afero.Fs, *mockConfigManager, *mockGitFinder, string) {
177+
fs := afero.NewMemMapFs()
178+
repoRoot := "/test-repo"
179+
fs.MkdirAll(path.Join(repoRoot, LogsFolder), 0755)
147180
return fs, &mockConfigManager{
148181
bruinHomeDir: "/test-bruin-home",
182+
}, &mockGitFinder{
183+
repo: &git.Repo{Path: repoRoot},
149184
}, repoRoot
150185
},
151-
expectedErr: "failed to remove file",
186+
expectedOutput: []string{"No log files found, nothing to clean up..."},
152187
},
153188
}
154189

155190
for _, tt := range tests {
156191
t.Run(tt.name, func(t *testing.T) {
157-
fs, mockCM, repoRoot := tt.setupMocks(t)
192+
fs, mockCM, mockGF, repoRoot := tt.setupMocks(t)
158193

159194
output := []string{}
160195
mockPrinter := &mockOutputPrinter{output: &output}
161-
cmd := NewCleanCommand(mockCM, fs, mockPrinter, mockPrinter)
196+
cmd := NewCleanCommand(mockCM, mockGF, fs, mockPrinter, mockPrinter)
162197

163198
err := cmd.Run(repoRoot, tt.cleanUvCache)
164199

0 commit comments

Comments
 (0)