detached c7c30b4

fix(review): address copilot PR review feedback

commit
c7c30b45db7de117293419b0b804ca866f17df59
author
bnema · · 2025-12-25 19:06 UTC
parent
c7808b4
signature
GPG Unverified2B82207D370127A0 The key is unknown
3 files · +229 -4
Unified
UnifiedSplit
A internal/application/usecase/cleanup_sessions_test.go +224 -0
@@ -0,0 +1,224 @@
1+package usecase_test
2+
3+import (
4+ "errors"
5+ "testing"
6+ "time"
7+
8+ "github.com/bnema/dumber/internal/application/usecase"
9+ repomocks "github.com/bnema/dumber/internal/domain/repository/mocks"
10+ "github.com/stretchr/testify/assert"
11+ "github.com/stretchr/testify/mock"
12+ "github.com/stretchr/testify/require"
13+)
14+
15+func TestCleanupSessionsUseCase_Execute_AgeAndCountCleanup(t *testing.T) {
16+ ctx := testContext()
17+
18+ sessionRepo := repomocks.NewMockSessionRepository(t)
19+
20+ // Expect age-based deletion
21+ sessionRepo.EXPECT().
22+ DeleteExitedBefore(ctx, mock.MatchedBy(func(cutoff time.Time) bool {
23+ // Should be approximately 7 days ago
24+ expected := time.Now().AddDate(0, 0, -7)
25+ diff := expected.Sub(cutoff)
26+ return diff > -time.Minute && diff < time.Minute
27+ })).
28+ Return(int64(3), nil)
29+
30+ // Expect count-based deletion
31+ sessionRepo.EXPECT().
32+ DeleteOldestExited(ctx, 50).
33+ Return(int64(2), nil)
34+
35+ uc := usecase.NewCleanupSessionsUseCase(sessionRepo)
36+
37+ output, err := uc.Execute(ctx, usecase.CleanupSessionsInput{
38+ MaxExitedSessions: 50,
39+ MaxExitedSessionAgeDays: 7,
40+ })
41+
42+ require.NoError(t, err)
43+ assert.Equal(t, int64(3), output.DeletedByAge)
44+ assert.Equal(t, int64(2), output.DeletedByCount)
45+ assert.Equal(t, int64(5), output.TotalDeleted)
46+}
47+
48+func TestCleanupSessionsUseCase_Execute_AgeOnlyCleanup(t *testing.T) {
49+ ctx := testContext()
50+
51+ sessionRepo := repomocks.NewMockSessionRepository(t)
52+
53+ // Expect age-based deletion
54+ sessionRepo.EXPECT().
55+ DeleteExitedBefore(ctx, mock.Anything).
56+ Return(int64(5), nil)
57+
58+ // Count-based with -1 should be skipped (disabled)
59+ // MaxExitedSessions >= 0 triggers count cleanup, so use -1 to disable
60+ uc := usecase.NewCleanupSessionsUseCase(sessionRepo)
61+
62+ output, err := uc.Execute(ctx, usecase.CleanupSessionsInput{
63+ MaxExitedSessions: -1, // Disabled
64+ MaxExitedSessionAgeDays: 30,
65+ })
66+
67+ require.NoError(t, err)
68+ assert.Equal(t, int64(5), output.DeletedByAge)
69+ assert.Equal(t, int64(0), output.DeletedByCount)
70+ assert.Equal(t, int64(5), output.TotalDeleted)
71+}
72+
73+func TestCleanupSessionsUseCase_Execute_CountOnlyCleanup(t *testing.T) {
74+ ctx := testContext()
75+
76+ sessionRepo := repomocks.NewMockSessionRepository(t)
77+
78+ // Count-based deletion only (age disabled with 0)
79+ sessionRepo.EXPECT().
80+ DeleteOldestExited(ctx, 10).
81+ Return(int64(4), nil)
82+
83+ uc := usecase.NewCleanupSessionsUseCase(sessionRepo)
84+
85+ output, err := uc.Execute(ctx, usecase.CleanupSessionsInput{
86+ MaxExitedSessions: 10,
87+ MaxExitedSessionAgeDays: 0, // Disabled
88+ })
89+
90+ require.NoError(t, err)
91+ assert.Equal(t, int64(0), output.DeletedByAge)
92+ assert.Equal(t, int64(4), output.DeletedByCount)
93+ assert.Equal(t, int64(4), output.TotalDeleted)
94+}
95+
96+func TestCleanupSessionsUseCase_Execute_NoCleanupNeeded(t *testing.T) {
97+ ctx := testContext()
98+
99+ sessionRepo := repomocks.NewMockSessionRepository(t)
100+
101+ // Nothing to delete
102+ sessionRepo.EXPECT().
103+ DeleteExitedBefore(ctx, mock.Anything).
104+ Return(int64(0), nil)
105+
106+ sessionRepo.EXPECT().
107+ DeleteOldestExited(ctx, 50).
108+ Return(int64(0), nil)
109+
110+ uc := usecase.NewCleanupSessionsUseCase(sessionRepo)
111+
112+ output, err := uc.Execute(ctx, usecase.CleanupSessionsInput{
113+ MaxExitedSessions: 50,
114+ MaxExitedSessionAgeDays: 7,
115+ })
116+
117+ require.NoError(t, err)
118+ assert.Equal(t, int64(0), output.DeletedByAge)
119+ assert.Equal(t, int64(0), output.DeletedByCount)
120+ assert.Equal(t, int64(0), output.TotalDeleted)
121+}
122+
123+func TestCleanupSessionsUseCase_Execute_AgeDeleteError_ContinuesToCount(t *testing.T) {
124+ ctx := testContext()
125+
126+ sessionRepo := repomocks.NewMockSessionRepository(t)
127+
128+ // Age-based deletion fails
129+ sessionRepo.EXPECT().
130+ DeleteExitedBefore(ctx, mock.Anything).
131+ Return(int64(0), errors.New("db error"))
132+
133+ // Should still attempt count-based deletion
134+ sessionRepo.EXPECT().
135+ DeleteOldestExited(ctx, 50).
136+ Return(int64(3), nil)
137+
138+ uc := usecase.NewCleanupSessionsUseCase(sessionRepo)
139+
140+ output, err := uc.Execute(ctx, usecase.CleanupSessionsInput{
141+ MaxExitedSessions: 50,
142+ MaxExitedSessionAgeDays: 7,
143+ })
144+
145+ // Should not return error, just log warning
146+ require.NoError(t, err)
147+ assert.Equal(t, int64(0), output.DeletedByAge)
148+ assert.Equal(t, int64(3), output.DeletedByCount)
149+ assert.Equal(t, int64(3), output.TotalDeleted)
150+}
151+
152+func TestCleanupSessionsUseCase_Execute_CountDeleteError(t *testing.T) {
153+ ctx := testContext()
154+
155+ sessionRepo := repomocks.NewMockSessionRepository(t)
156+
157+ // Age-based deletion succeeds
158+ sessionRepo.EXPECT().
159+ DeleteExitedBefore(ctx, mock.Anything).
160+ Return(int64(2), nil)
161+
162+ // Count-based deletion fails
163+ sessionRepo.EXPECT().
164+ DeleteOldestExited(ctx, 50).
165+ Return(int64(0), errors.New("db error"))
166+
167+ uc := usecase.NewCleanupSessionsUseCase(sessionRepo)
168+
169+ output, err := uc.Execute(ctx, usecase.CleanupSessionsInput{
170+ MaxExitedSessions: 50,
171+ MaxExitedSessionAgeDays: 7,
172+ })
173+
174+ // Should not return error, just log warning
175+ require.NoError(t, err)
176+ assert.Equal(t, int64(2), output.DeletedByAge)
177+ assert.Equal(t, int64(0), output.DeletedByCount)
178+ assert.Equal(t, int64(2), output.TotalDeleted)
179+}
180+
181+func TestCleanupSessionsUseCase_Execute_BothDisabled(t *testing.T) {
182+ ctx := testContext()
183+
184+ sessionRepo := repomocks.NewMockSessionRepository(t)
185+
186+ // No expectations - nothing should be called
187+ // Actually, count with 0 means "keep 0" which will trigger cleanup
188+ // So we need -1 for count and 0 for age to fully disable
189+
190+ uc := usecase.NewCleanupSessionsUseCase(sessionRepo)
191+
192+ output, err := uc.Execute(ctx, usecase.CleanupSessionsInput{
193+ MaxExitedSessions: -1, // Disabled
194+ MaxExitedSessionAgeDays: 0, // Disabled
195+ })
196+
197+ require.NoError(t, err)
198+ assert.Equal(t, int64(0), output.DeletedByAge)
199+ assert.Equal(t, int64(0), output.DeletedByCount)
200+ assert.Equal(t, int64(0), output.TotalDeleted)
201+}
202+
203+func TestCleanupSessionsUseCase_Execute_ZeroMaxSessions(t *testing.T) {
204+ ctx := testContext()
205+
206+ sessionRepo := repomocks.NewMockSessionRepository(t)
207+
208+ // MaxExitedSessions = 0 means keep 0 sessions (delete all)
209+ sessionRepo.EXPECT().
210+ DeleteOldestExited(ctx, 0).
211+ Return(int64(10), nil)
212+
213+ uc := usecase.NewCleanupSessionsUseCase(sessionRepo)
214+
215+ output, err := uc.Execute(ctx, usecase.CleanupSessionsInput{
216+ MaxExitedSessions: 0, // Keep 0 = delete all
217+ MaxExitedSessionAgeDays: 0, // Disabled
218+ })
219+
220+ require.NoError(t, err)
221+ assert.Equal(t, int64(0), output.DeletedByAge)
222+ assert.Equal(t, int64(10), output.DeletedByCount)
223+ assert.Equal(t, int64(10), output.TotalDeleted)
224+}
@@ -0,0 +1,224 @@
1+package usecase_test
2+
3+import (
4+ "errors"
5+ "testing"
6+ "time"
7+
8+ "github.com/bnema/dumber/internal/application/usecase"
9+ repomocks "github.com/bnema/dumber/internal/domain/repository/mocks"
10+ "github.com/stretchr/testify/assert"
11+ "github.com/stretchr/testify/mock"
12+ "github.com/stretchr/testify/require"
13+)
14+
15+func TestCleanupSessionsUseCase_Execute_AgeAndCountCleanup(t *testing.T) {
16+ ctx := testContext()
17+
18+ sessionRepo := repomocks.NewMockSessionRepository(t)
19+
20+ // Expect age-based deletion
21+ sessionRepo.EXPECT().
22+ DeleteExitedBefore(ctx, mock.MatchedBy(func(cutoff time.Time) bool {
23+ // Should be approximately 7 days ago
24+ expected := time.Now().AddDate(0, 0, -7)
25+ diff := expected.Sub(cutoff)
26+ return diff > -time.Minute && diff < time.Minute
27+ })).
28+ Return(int64(3), nil)
29+
30+ // Expect count-based deletion
31+ sessionRepo.EXPECT().
32+ DeleteOldestExited(ctx, 50).
33+ Return(int64(2), nil)
34+
35+ uc := usecase.NewCleanupSessionsUseCase(sessionRepo)
36+
37+ output, err := uc.Execute(ctx, usecase.CleanupSessionsInput{
38+ MaxExitedSessions: 50,
39+ MaxExitedSessionAgeDays: 7,
40+ })
41+
42+ require.NoError(t, err)
43+ assert.Equal(t, int64(3), output.DeletedByAge)
44+ assert.Equal(t, int64(2), output.DeletedByCount)
45+ assert.Equal(t, int64(5), output.TotalDeleted)
46+}
47+
48+func TestCleanupSessionsUseCase_Execute_AgeOnlyCleanup(t *testing.T) {
49+ ctx := testContext()
50+
51+ sessionRepo := repomocks.NewMockSessionRepository(t)
52+
53+ // Expect age-based deletion
54+ sessionRepo.EXPECT().
55+ DeleteExitedBefore(ctx, mock.Anything).
56+ Return(int64(5), nil)
57+
58+ // Count-based with -1 should be skipped (disabled)
59+ // MaxExitedSessions >= 0 triggers count cleanup, so use -1 to disable
60+ uc := usecase.NewCleanupSessionsUseCase(sessionRepo)
61+
62+ output, err := uc.Execute(ctx, usecase.CleanupSessionsInput{
63+ MaxExitedSessions: -1, // Disabled
64+ MaxExitedSessionAgeDays: 30,
65+ })
66+
67+ require.NoError(t, err)
68+ assert.Equal(t, int64(5), output.DeletedByAge)
69+ assert.Equal(t, int64(0), output.DeletedByCount)
70+ assert.Equal(t, int64(5), output.TotalDeleted)
71+}
72+
73+func TestCleanupSessionsUseCase_Execute_CountOnlyCleanup(t *testing.T) {
74+ ctx := testContext()
75+
76+ sessionRepo := repomocks.NewMockSessionRepository(t)
77+
78+ // Count-based deletion only (age disabled with 0)
79+ sessionRepo.EXPECT().
80+ DeleteOldestExited(ctx, 10).
81+ Return(int64(4), nil)
82+
83+ uc := usecase.NewCleanupSessionsUseCase(sessionRepo)
84+
85+ output, err := uc.Execute(ctx, usecase.CleanupSessionsInput{
86+ MaxExitedSessions: 10,
87+ MaxExitedSessionAgeDays: 0, // Disabled
88+ })
89+
90+ require.NoError(t, err)
91+ assert.Equal(t, int64(0), output.DeletedByAge)
92+ assert.Equal(t, int64(4), output.DeletedByCount)
93+ assert.Equal(t, int64(4), output.TotalDeleted)
94+}
95+
96+func TestCleanupSessionsUseCase_Execute_NoCleanupNeeded(t *testing.T) {
97+ ctx := testContext()
98+
99+ sessionRepo := repomocks.NewMockSessionRepository(t)
100+
101+ // Nothing to delete
102+ sessionRepo.EXPECT().
103+ DeleteExitedBefore(ctx, mock.Anything).
104+ Return(int64(0), nil)
105+
106+ sessionRepo.EXPECT().
107+ DeleteOldestExited(ctx, 50).
108+ Return(int64(0), nil)
109+
110+ uc := usecase.NewCleanupSessionsUseCase(sessionRepo)
111+
112+ output, err := uc.Execute(ctx, usecase.CleanupSessionsInput{
113+ MaxExitedSessions: 50,
114+ MaxExitedSessionAgeDays: 7,
115+ })
116+
117+ require.NoError(t, err)
118+ assert.Equal(t, int64(0), output.DeletedByAge)
119+ assert.Equal(t, int64(0), output.DeletedByCount)
120+ assert.Equal(t, int64(0), output.TotalDeleted)
121+}
122+
123+func TestCleanupSessionsUseCase_Execute_AgeDeleteError_ContinuesToCount(t *testing.T) {
124+ ctx := testContext()
125+
126+ sessionRepo := repomocks.NewMockSessionRepository(t)
127+
128+ // Age-based deletion fails
129+ sessionRepo.EXPECT().
130+ DeleteExitedBefore(ctx, mock.Anything).
131+ Return(int64(0), errors.New("db error"))
132+
133+ // Should still attempt count-based deletion
134+ sessionRepo.EXPECT().
135+ DeleteOldestExited(ctx, 50).
136+ Return(int64(3), nil)
137+
138+ uc := usecase.NewCleanupSessionsUseCase(sessionRepo)
139+
140+ output, err := uc.Execute(ctx, usecase.CleanupSessionsInput{
141+ MaxExitedSessions: 50,
142+ MaxExitedSessionAgeDays: 7,
143+ })
144+
145+ // Should not return error, just log warning
146+ require.NoError(t, err)
147+ assert.Equal(t, int64(0), output.DeletedByAge)
148+ assert.Equal(t, int64(3), output.DeletedByCount)
149+ assert.Equal(t, int64(3), output.TotalDeleted)
150+}
151+
152+func TestCleanupSessionsUseCase_Execute_CountDeleteError(t *testing.T) {
153+ ctx := testContext()
154+
155+ sessionRepo := repomocks.NewMockSessionRepository(t)
156+
157+ // Age-based deletion succeeds
158+ sessionRepo.EXPECT().
159+ DeleteExitedBefore(ctx, mock.Anything).
160+ Return(int64(2), nil)
161+
162+ // Count-based deletion fails
163+ sessionRepo.EXPECT().
164+ DeleteOldestExited(ctx, 50).
165+ Return(int64(0), errors.New("db error"))
166+
167+ uc := usecase.NewCleanupSessionsUseCase(sessionRepo)
168+
169+ output, err := uc.Execute(ctx, usecase.CleanupSessionsInput{
170+ MaxExitedSessions: 50,
171+ MaxExitedSessionAgeDays: 7,
172+ })
173+
174+ // Should not return error, just log warning
175+ require.NoError(t, err)
176+ assert.Equal(t, int64(2), output.DeletedByAge)
177+ assert.Equal(t, int64(0), output.DeletedByCount)
178+ assert.Equal(t, int64(2), output.TotalDeleted)
179+}
180+
181+func TestCleanupSessionsUseCase_Execute_BothDisabled(t *testing.T) {
182+ ctx := testContext()
183+
184+ sessionRepo := repomocks.NewMockSessionRepository(t)
185+
186+ // No expectations - nothing should be called
187+ // Actually, count with 0 means "keep 0" which will trigger cleanup
188+ // So we need -1 for count and 0 for age to fully disable
189+
190+ uc := usecase.NewCleanupSessionsUseCase(sessionRepo)
191+
192+ output, err := uc.Execute(ctx, usecase.CleanupSessionsInput{
193+ MaxExitedSessions: -1, // Disabled
194+ MaxExitedSessionAgeDays: 0, // Disabled
195+ })
196+
197+ require.NoError(t, err)
198+ assert.Equal(t, int64(0), output.DeletedByAge)
199+ assert.Equal(t, int64(0), output.DeletedByCount)
200+ assert.Equal(t, int64(0), output.TotalDeleted)
201+}
202+
203+func TestCleanupSessionsUseCase_Execute_ZeroMaxSessions(t *testing.T) {
204+ ctx := testContext()
205+
206+ sessionRepo := repomocks.NewMockSessionRepository(t)
207+
208+ // MaxExitedSessions = 0 means keep 0 sessions (delete all)
209+ sessionRepo.EXPECT().
210+ DeleteOldestExited(ctx, 0).
211+ Return(int64(10), nil)
212+
213+ uc := usecase.NewCleanupSessionsUseCase(sessionRepo)
214+
215+ output, err := uc.Execute(ctx, usecase.CleanupSessionsInput{
216+ MaxExitedSessions: 0, // Keep 0 = delete all
217+ MaxExitedSessionAgeDays: 0, // Disabled
218+ })
219+
220+ require.NoError(t, err)
221+ assert.Equal(t, int64(0), output.DeletedByAge)
222+ assert.Equal(t, int64(10), output.DeletedByCount)
223+ assert.Equal(t, int64(10), output.TotalDeleted)
224+}
M internal/cli/styles/purge.go +1 -1
@@ -383,7 +383,7 @@ func (m PurgeModel) renderSessionsItem(isCursor bool) string {
383383 }
384384
385385 // Format: Sessions (5 inactive, 12.3 MB) or (included with Data)
386- label := fmt.Sprintf("%s Sessions", IconLogs)
386+ label := fmt.Sprintf("%s Sessions", IconSession)
387387 var detail string
388388 if enabled {
389389 detail = fmt.Sprintf("(%d inactive, %s)", len(m.Sessions), formatSize(m.SessionsSize))
@@ -383,7 +383,7 @@ func (m PurgeModel) renderSessionsItem(isCursor bool) string {
383 }383 }
384 384
385 // Format: Sessions (5 inactive, 12.3 MB) or (included with Data)385 // Format: Sessions (5 inactive, 12.3 MB) or (included with Data)
386- label := fmt.Sprintf("%s Sessions", IconLogs)386+ label := fmt.Sprintf("%s Sessions", IconSession)
387 var detail string387 var detail string
388 if enabled {388 if enabled {
389 detail = fmt.Sprintf("(%d inactive, %s)", len(m.Sessions), formatSize(m.SessionsSize))389 detail = fmt.Sprintf("(%d inactive, %s)", len(m.Sessions), formatSize(m.SessionsSize))
M internal/ui/window/main_window.go +4 -3
@@ -176,10 +176,11 @@ func (mw *MainWindow) SetTitle(title string) {
176176 if mw.window == nil {
177177 return
178178 }
179- // Truncate title to 255 characters max
179+ // Truncate title to 255 characters max (by runes, to avoid breaking UTF-8)
180180 const maxTitleLen = 255
181- if len(title) > maxTitleLen {
182- title = title[:maxTitleLen-3] + "..."
181+ runes := []rune(title)
182+ if len(runes) > maxTitleLen {
183+ title = string(runes[:maxTitleLen-3]) + "..."
183184 }
184185 mw.window.SetTitle(&title)
185186 }
@@ -176,10 +176,11 @@ func (mw *MainWindow) SetTitle(title string) {
176 if mw.window == nil {176 if mw.window == nil {
177 return177 return
178 }178 }
179- // Truncate title to 255 characters max179+ // Truncate title to 255 characters max (by runes, to avoid breaking UTF-8)
180 const maxTitleLen = 255180 const maxTitleLen = 255
181- if len(title) > maxTitleLen {181+ runes := []rune(title)
182- title = title[:maxTitleLen-3] + "..."182+ if len(runes) > maxTitleLen {
183+ title = string(runes[:maxTitleLen-3]) + "..."
183 }184 }
184 mw.window.SetTitle(&title)185 mw.window.SetTitle(&title)
185 }186 }

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 8

Tags 42

↑↓ moveenter openesc close