refactor(test): replace ad-hoc history fake with mockery mock
Replace the 100+ line historyWorkerRegressionRepo with a MockHistoryRepository using RunAndReturn closures over a shared map. The re-enqueue-during-Save callback pattern works cleanly with mockery's Run/RunAndReturn API + sync.Once.
- commit
- cba20902b0747ae5069dd79c5854d6b537f2898e
- author
- brice · · 2026-03-21 20:24 UTC
- committer
- brice · · 2026-03-28 04:38 UTC
- parent
- 9ad3737
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
1 file · +45 -123
M internal/application/usecase/navigate_test.go +45 -123
| @@ -8,7 +8,6 @@ import ( | |||
| 8 | 8 | "time" | |
| 9 | 9 | ||
| 10 | 10 | "github.com/bnema/dumber/internal/domain/entity" | |
| 11 | - | "github.com/bnema/dumber/internal/domain/repository" | |
| 12 | 11 | repomocks "github.com/bnema/dumber/internal/domain/repository/mocks" | |
| 13 | 12 | "github.com/bnema/dumber/internal/infrastructure/persistence/sqlite" | |
| 14 | 13 | "github.com/stretchr/testify/mock" | |
| @@ -94,138 +93,61 @@ func TestUpdateHistoryTitle_UsesMetadataUpdateWithoutIncrementingVisits(t *testi | |||
| 94 | 93 | ||
| 95 | 94 | func TestHistoryWorker_ReenqueueDuringFlushIsPersistedOnShutdown(t *testing.T) { | |
| 96 | 95 | ctx := context.Background() | |
| 97 | - | repo := newHistoryWorkerRegressionRepo() | |
| 98 | 96 | ||
| 97 | + | // Stateful store — Save writes here, FindByURL reads from here. | |
| 98 | + | var mu sync.Mutex | |
| 99 | + | store := make(map[string]*entity.HistoryEntry) | |
| 100 | + | ||
| 101 | + | // Fires once during the first Save to re-enqueue a title update while | |
| 102 | + | // the flush is in progress. This is the exact race that caused the | |
| 103 | + | // concurrent map crash (fixed by swapping maps in flushPending). | |
| 104 | + | var saveOnce sync.Once | |
| 99 | 105 | var uc *NavigateUseCase | |
| 100 | - | repo.onSave = func(entry *entity.HistoryEntry) { | |
| 101 | - | if entry.URL != "https://example.com/article" { | |
| 102 | - | return | |
| 103 | - | } | |
| 104 | - | uc.UpdateHistoryTitle(ctx, entry.URL, "Queued title") | |
| 105 | - | } | |
| 106 | + | ||
| 107 | + | repo := repomocks.NewMockHistoryRepository(t) | |
| 108 | + | ||
| 109 | + | repo.EXPECT().FindByURL(mock.Anything, mock.Anything).RunAndReturn( | |
| 110 | + | func(_ context.Context, url string) (*entity.HistoryEntry, error) { | |
| 111 | + | mu.Lock() | |
| 112 | + | defer mu.Unlock() | |
| 113 | + | e := store[url] | |
| 114 | + | if e == nil { | |
| 115 | + | return nil, nil | |
| 116 | + | } | |
| 117 | + | clone := *e | |
| 118 | + | return &clone, nil | |
| 119 | + | }, | |
| 120 | + | ).Maybe() | |
| 121 | + | ||
| 122 | + | repo.EXPECT().Save(mock.Anything, mock.Anything).RunAndReturn( | |
| 123 | + | func(_ context.Context, entry *entity.HistoryEntry) error { | |
| 124 | + | clone := *entry | |
| 125 | + | mu.Lock() | |
| 126 | + | store[entry.URL] = &clone | |
| 127 | + | mu.Unlock() | |
| 128 | + | ||
| 129 | + | // Re-enqueue a title update during the first Save. | |
| 130 | + | saveOnce.Do(func() { | |
| 131 | + | if entry.URL == "https://example.com/article" { | |
| 132 | + | uc.UpdateHistoryTitle(ctx, entry.URL, "Queued title") | |
| 133 | + | } | |
| 134 | + | }) | |
| 135 | + | return nil | |
| 136 | + | }, | |
| 137 | + | ).Maybe() | |
| 106 | 138 | ||
| 107 | 139 | uc = NewNavigateUseCase(repo, nil, entity.ZoomDefault) | |
| 108 | 140 | uc.RecordHistory(ctx, "pane-1", "https://example.com/article") | |
| 109 | 141 | ||
| 110 | - | // The history worker flushes on a timer; there is no exported sync/flush | |
| 111 | - | // mechanism we can hook into, so we sleep for 3× the flush interval to | |
| 112 | - | // give it a comfortable margin before shutting down. | |
| 142 | + | // The history worker flushes on a timer; there is no exported sync | |
| 143 | + | // mechanism, so sleep 3× the flush interval before shutting down. | |
| 113 | 144 | time.Sleep(historyWorkerFlushInterval * 3) | |
| 114 | 145 | uc.Close() | |
| 115 | 146 | ||
| 116 | - | entry, err := repo.FindByURL(ctx, "https://example.com/article") | |
| 117 | - | require.NoError(t, err) | |
| 147 | + | mu.Lock() | |
| 148 | + | entry := store["https://example.com/article"] | |
| 149 | + | mu.Unlock() | |
| 118 | 150 | require.NotNil(t, entry) | |
| 119 | 151 | require.Equal(t, int64(1), entry.VisitCount) | |
| 120 | 152 | require.Equal(t, "Queued title", entry.Title) | |
| 121 | 153 | } | |
| 122 | - | ||
| 123 | - | // historyWorkerRegressionRepo is a hand-rolled fake instead of a mockery | |
| 124 | - | // mock because mockery cannot express the re-enqueue-during-Save callback | |
| 125 | - | // pattern needed for this regression test (onSave fires a second write | |
| 126 | - | // back into the use-case while the first Save is still in progress). | |
| 127 | - | type historyWorkerRegressionRepo struct { | |
| 128 | - | mu sync.Mutex | |
| 129 | - | byURL map[string]*entity.HistoryEntry | |
| 130 | - | saveMu sync.Once | |
| 131 | - | onSave func(*entity.HistoryEntry) | |
| 132 | - | } | |
| 133 | - | ||
| 134 | - | func newHistoryWorkerRegressionRepo() *historyWorkerRegressionRepo { | |
| 135 | - | return &historyWorkerRegressionRepo{ | |
| 136 | - | byURL: make(map[string]*entity.HistoryEntry), | |
| 137 | - | } | |
| 138 | - | } | |
| 139 | - | ||
| 140 | - | func (r *historyWorkerRegressionRepo) Save(_ context.Context, entry *entity.HistoryEntry) error { | |
| 141 | - | clone := *entry | |
| 142 | - | ||
| 143 | - | r.mu.Lock() | |
| 144 | - | r.byURL[entry.URL] = &clone | |
| 145 | - | r.mu.Unlock() | |
| 146 | - | ||
| 147 | - | if r.onSave != nil { | |
| 148 | - | r.saveMu.Do(func() { | |
| 149 | - | r.onSave(&clone) | |
| 150 | - | }) | |
| 151 | - | } | |
| 152 | - | return nil | |
| 153 | - | } | |
| 154 | - | ||
| 155 | - | func (r *historyWorkerRegressionRepo) FindByURL(_ context.Context, url string) (*entity.HistoryEntry, error) { | |
| 156 | - | r.mu.Lock() | |
| 157 | - | defer r.mu.Unlock() | |
| 158 | - | entry := r.byURL[url] | |
| 159 | - | if entry == nil { | |
| 160 | - | return nil, nil | |
| 161 | - | } | |
| 162 | - | clone := *entry | |
| 163 | - | return &clone, nil | |
| 164 | - | } | |
| 165 | - | ||
| 166 | - | func (*historyWorkerRegressionRepo) Search(context.Context, string, int) ([]entity.HistoryMatch, error) { | |
| 167 | - | return nil, nil | |
| 168 | - | } | |
| 169 | - | ||
| 170 | - | func (*historyWorkerRegressionRepo) GetRecent(context.Context, int, int) ([]*entity.HistoryEntry, error) { | |
| 171 | - | return nil, nil | |
| 172 | - | } | |
| 173 | - | ||
| 174 | - | func (*historyWorkerRegressionRepo) GetRecentSince(context.Context, int) ([]*entity.HistoryEntry, error) { | |
| 175 | - | return nil, nil | |
| 176 | - | } | |
| 177 | - | ||
| 178 | - | func (*historyWorkerRegressionRepo) GetMostVisited(context.Context, int) ([]*entity.HistoryEntry, error) { | |
| 179 | - | return nil, nil | |
| 180 | - | } | |
| 181 | - | ||
| 182 | - | func (*historyWorkerRegressionRepo) GetAllRecentHistory(context.Context) ([]*entity.HistoryEntry, error) { | |
| 183 | - | return nil, nil | |
| 184 | - | } | |
| 185 | - | ||
| 186 | - | func (*historyWorkerRegressionRepo) GetAllMostVisited(context.Context) ([]*entity.HistoryEntry, error) { | |
| 187 | - | return nil, nil | |
| 188 | - | } | |
| 189 | - | ||
| 190 | - | func (r *historyWorkerRegressionRepo) IncrementVisitCount(_ context.Context, url string) error { | |
| 191 | - | r.mu.Lock() | |
| 192 | - | defer r.mu.Unlock() | |
| 193 | - | if entry := r.byURL[url]; entry != nil { | |
| 194 | - | entry.VisitCount++ | |
| 195 | - | } | |
| 196 | - | return nil | |
| 197 | - | } | |
| 198 | - | ||
| 199 | - | func (*historyWorkerRegressionRepo) Delete(context.Context, int64) error { | |
| 200 | - | return nil | |
| 201 | - | } | |
| 202 | - | ||
| 203 | - | func (*historyWorkerRegressionRepo) DeleteOlderThan(context.Context, time.Time) error { | |
| 204 | - | return nil | |
| 205 | - | } | |
| 206 | - | ||
| 207 | - | func (*historyWorkerRegressionRepo) DeleteAll(context.Context) error { | |
| 208 | - | return nil | |
| 209 | - | } | |
| 210 | - | ||
| 211 | - | func (*historyWorkerRegressionRepo) DeleteByDomain(context.Context, string) error { | |
| 212 | - | return nil | |
| 213 | - | } | |
| 214 | - | ||
| 215 | - | func (*historyWorkerRegressionRepo) GetStats(context.Context) (*entity.HistoryStats, error) { | |
| 216 | - | return nil, nil | |
| 217 | - | } | |
| 218 | - | ||
| 219 | - | func (*historyWorkerRegressionRepo) GetDomainStats(context.Context, int) ([]*entity.DomainStat, error) { | |
| 220 | - | return nil, nil | |
| 221 | - | } | |
| 222 | - | ||
| 223 | - | func (*historyWorkerRegressionRepo) GetHourlyDistribution(context.Context) ([]*entity.HourlyDistribution, error) { | |
| 224 | - | return nil, nil | |
| 225 | - | } | |
| 226 | - | ||
| 227 | - | func (*historyWorkerRegressionRepo) GetDailyVisitCount(context.Context, string) ([]*entity.DailyVisitCount, error) { | |
| 228 | - | return nil, nil | |
| 229 | - | } | |
| 230 | - | ||
| 231 | - | var _ repository.HistoryRepository = (*historyWorkerRegressionRepo)(nil) | |
| @@ -8,7 +8,6 @@ import ( | |||||
| 8 | "time" | 8 | "time" | ||
| 9 | 9 | ||||
| 10 | "github.com/bnema/dumber/internal/domain/entity" | 10 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 11 | - | "github.com/bnema/dumber/internal/domain/repository" | |||
| 12 | repomocks "github.com/bnema/dumber/internal/domain/repository/mocks" | 11 | repomocks "github.com/bnema/dumber/internal/domain/repository/mocks" | ||
| 13 | "github.com/bnema/dumber/internal/infrastructure/persistence/sqlite" | 12 | "github.com/bnema/dumber/internal/infrastructure/persistence/sqlite" | ||
| 14 | "github.com/stretchr/testify/mock" | 13 | "github.com/stretchr/testify/mock" | ||
| @@ -94,138 +93,61 @@ func TestUpdateHistoryTitle_UsesMetadataUpdateWithoutIncrementingVisits(t *testi | |||||
| 94 | 93 | ||||
| 95 | func TestHistoryWorker_ReenqueueDuringFlushIsPersistedOnShutdown(t *testing.T) { | 94 | func TestHistoryWorker_ReenqueueDuringFlushIsPersistedOnShutdown(t *testing.T) { | ||
| 96 | ctx := context.Background() | 95 | ctx := context.Background() | ||
| 97 | - | repo := newHistoryWorkerRegressionRepo() | |||
| 98 | 96 | ||||
| 97 | + | // Stateful store — Save writes here, FindByURL reads from here. | |||
| 98 | + | var mu sync.Mutex | |||
| 99 | + | store := make(map[string]*entity.HistoryEntry) | |||
| 100 | + | ||||
| 101 | + | // Fires once during the first Save to re-enqueue a title update while | |||
| 102 | + | // the flush is in progress. This is the exact race that caused the | |||
| 103 | + | // concurrent map crash (fixed by swapping maps in flushPending). | |||
| 104 | + | var saveOnce sync.Once | |||
| 99 | var uc *NavigateUseCase | 105 | var uc *NavigateUseCase | ||
| 100 | - | repo.onSave = func(entry *entity.HistoryEntry) { | 106 | + | |
| 101 | - | if entry.URL != "https://example.com/article" { | 107 | + | repo := repomocks.NewMockHistoryRepository(t) |
| 102 | - | return | 108 | + | |
| 103 | - | } | 109 | + | repo.EXPECT().FindByURL(mock.Anything, mock.Anything).RunAndReturn( |
| 104 | - | uc.UpdateHistoryTitle(ctx, entry.URL, "Queued title") | 110 | + | func(_ context.Context, url string) (*entity.HistoryEntry, error) { |
| 105 | - | } | 111 | + | mu.Lock() |
| 112 | + | defer mu.Unlock() | |||
| 113 | + | e := store[url] | |||
| 114 | + | if e == nil { | |||
| 115 | + | return nil, nil | |||
| 116 | + | } | |||
| 117 | + | clone := *e | |||
| 118 | + | return &clone, nil | |||
| 119 | + | }, | |||
| 120 | + | ).Maybe() | |||
| 121 | + | ||||
| 122 | + | repo.EXPECT().Save(mock.Anything, mock.Anything).RunAndReturn( | |||
| 123 | + | func(_ context.Context, entry *entity.HistoryEntry) error { | |||
| 124 | + | clone := *entry | |||
| 125 | + | mu.Lock() | |||
| 126 | + | store[entry.URL] = &clone | |||
| 127 | + | mu.Unlock() | |||
| 128 | + | ||||
| 129 | + | // Re-enqueue a title update during the first Save. | |||
| 130 | + | saveOnce.Do(func() { | |||
| 131 | + | if entry.URL == "https://example.com/article" { | |||
| 132 | + | uc.UpdateHistoryTitle(ctx, entry.URL, "Queued title") | |||
| 133 | + | } | |||
| 134 | + | }) | |||
| 135 | + | return nil | |||
| 136 | + | }, | |||
| 137 | + | ).Maybe() | |||
| 106 | 138 | ||||
| 107 | uc = NewNavigateUseCase(repo, nil, entity.ZoomDefault) | 139 | uc = NewNavigateUseCase(repo, nil, entity.ZoomDefault) | ||
| 108 | uc.RecordHistory(ctx, "pane-1", "https://example.com/article") | 140 | uc.RecordHistory(ctx, "pane-1", "https://example.com/article") | ||
| 109 | 141 | ||||
| 110 | - | // The history worker flushes on a timer; there is no exported sync/flush | 142 | + | // The history worker flushes on a timer; there is no exported sync |
| 111 | - | // mechanism we can hook into, so we sleep for 3× the flush interval to | 143 | + | // mechanism, so sleep 3× the flush interval before shutting down. |
| 112 | - | // give it a comfortable margin before shutting down. | |||
| 113 | time.Sleep(historyWorkerFlushInterval * 3) | 144 | time.Sleep(historyWorkerFlushInterval * 3) | ||
| 114 | uc.Close() | 145 | uc.Close() | ||
| 115 | 146 | ||||
| 116 | - | entry, err := repo.FindByURL(ctx, "https://example.com/article") | 147 | + | mu.Lock() |
| 117 | - | require.NoError(t, err) | 148 | + | entry := store["https://example.com/article"] |
| 149 | + | mu.Unlock() | |||
| 118 | require.NotNil(t, entry) | 150 | require.NotNil(t, entry) | ||
| 119 | require.Equal(t, int64(1), entry.VisitCount) | 151 | require.Equal(t, int64(1), entry.VisitCount) | ||
| 120 | require.Equal(t, "Queued title", entry.Title) | 152 | require.Equal(t, "Queued title", entry.Title) | ||
| 121 | } | 153 | } | ||
| 122 | - | ||||
| 123 | - | // historyWorkerRegressionRepo is a hand-rolled fake instead of a mockery | |||
| 124 | - | // mock because mockery cannot express the re-enqueue-during-Save callback | |||
| 125 | - | // pattern needed for this regression test (onSave fires a second write | |||
| 126 | - | // back into the use-case while the first Save is still in progress). | |||
| 127 | - | type historyWorkerRegressionRepo struct { | |||
| 128 | - | mu sync.Mutex | |||
| 129 | - | byURL map[string]*entity.HistoryEntry | |||
| 130 | - | saveMu sync.Once | |||
| 131 | - | onSave func(*entity.HistoryEntry) | |||
| 132 | - | } | |||
| 133 | - | ||||
| 134 | - | func newHistoryWorkerRegressionRepo() *historyWorkerRegressionRepo { | |||
| 135 | - | return &historyWorkerRegressionRepo{ | |||
| 136 | - | byURL: make(map[string]*entity.HistoryEntry), | |||
| 137 | - | } | |||
| 138 | - | } | |||
| 139 | - | ||||
| 140 | - | func (r *historyWorkerRegressionRepo) Save(_ context.Context, entry *entity.HistoryEntry) error { | |||
| 141 | - | clone := *entry | |||
| 142 | - | ||||
| 143 | - | r.mu.Lock() | |||
| 144 | - | r.byURL[entry.URL] = &clone | |||
| 145 | - | r.mu.Unlock() | |||
| 146 | - | ||||
| 147 | - | if r.onSave != nil { | |||
| 148 | - | r.saveMu.Do(func() { | |||
| 149 | - | r.onSave(&clone) | |||
| 150 | - | }) | |||
| 151 | - | } | |||
| 152 | - | return nil | |||
| 153 | - | } | |||
| 154 | - | ||||
| 155 | - | func (r *historyWorkerRegressionRepo) FindByURL(_ context.Context, url string) (*entity.HistoryEntry, error) { | |||
| 156 | - | r.mu.Lock() | |||
| 157 | - | defer r.mu.Unlock() | |||
| 158 | - | entry := r.byURL[url] | |||
| 159 | - | if entry == nil { | |||
| 160 | - | return nil, nil | |||
| 161 | - | } | |||
| 162 | - | clone := *entry | |||
| 163 | - | return &clone, nil | |||
| 164 | - | } | |||
| 165 | - | ||||
| 166 | - | func (*historyWorkerRegressionRepo) Search(context.Context, string, int) ([]entity.HistoryMatch, error) { | |||
| 167 | - | return nil, nil | |||
| 168 | - | } | |||
| 169 | - | ||||
| 170 | - | func (*historyWorkerRegressionRepo) GetRecent(context.Context, int, int) ([]*entity.HistoryEntry, error) { | |||
| 171 | - | return nil, nil | |||
| 172 | - | } | |||
| 173 | - | ||||
| 174 | - | func (*historyWorkerRegressionRepo) GetRecentSince(context.Context, int) ([]*entity.HistoryEntry, error) { | |||
| 175 | - | return nil, nil | |||
| 176 | - | } | |||
| 177 | - | ||||
| 178 | - | func (*historyWorkerRegressionRepo) GetMostVisited(context.Context, int) ([]*entity.HistoryEntry, error) { | |||
| 179 | - | return nil, nil | |||
| 180 | - | } | |||
| 181 | - | ||||
| 182 | - | func (*historyWorkerRegressionRepo) GetAllRecentHistory(context.Context) ([]*entity.HistoryEntry, error) { | |||
| 183 | - | return nil, nil | |||
| 184 | - | } | |||
| 185 | - | ||||
| 186 | - | func (*historyWorkerRegressionRepo) GetAllMostVisited(context.Context) ([]*entity.HistoryEntry, error) { | |||
| 187 | - | return nil, nil | |||
| 188 | - | } | |||
| 189 | - | ||||
| 190 | - | func (r *historyWorkerRegressionRepo) IncrementVisitCount(_ context.Context, url string) error { | |||
| 191 | - | r.mu.Lock() | |||
| 192 | - | defer r.mu.Unlock() | |||
| 193 | - | if entry := r.byURL[url]; entry != nil { | |||
| 194 | - | entry.VisitCount++ | |||
| 195 | - | } | |||
| 196 | - | return nil | |||
| 197 | - | } | |||
| 198 | - | ||||
| 199 | - | func (*historyWorkerRegressionRepo) Delete(context.Context, int64) error { | |||
| 200 | - | return nil | |||
| 201 | - | } | |||
| 202 | - | ||||
| 203 | - | func (*historyWorkerRegressionRepo) DeleteOlderThan(context.Context, time.Time) error { | |||
| 204 | - | return nil | |||
| 205 | - | } | |||
| 206 | - | ||||
| 207 | - | func (*historyWorkerRegressionRepo) DeleteAll(context.Context) error { | |||
| 208 | - | return nil | |||
| 209 | - | } | |||
| 210 | - | ||||
| 211 | - | func (*historyWorkerRegressionRepo) DeleteByDomain(context.Context, string) error { | |||
| 212 | - | return nil | |||
| 213 | - | } | |||
| 214 | - | ||||
| 215 | - | func (*historyWorkerRegressionRepo) GetStats(context.Context) (*entity.HistoryStats, error) { | |||
| 216 | - | return nil, nil | |||
| 217 | - | } | |||
| 218 | - | ||||
| 219 | - | func (*historyWorkerRegressionRepo) GetDomainStats(context.Context, int) ([]*entity.DomainStat, error) { | |||
| 220 | - | return nil, nil | |||
| 221 | - | } | |||
| 222 | - | ||||
| 223 | - | func (*historyWorkerRegressionRepo) GetHourlyDistribution(context.Context) ([]*entity.HourlyDistribution, error) { | |||
| 224 | - | return nil, nil | |||
| 225 | - | } | |||
| 226 | - | ||||
| 227 | - | func (*historyWorkerRegressionRepo) GetDailyVisitCount(context.Context, string) ([]*entity.DailyVisitCount, error) { | |||
| 228 | - | return nil, nil | |||
| 229 | - | } | |||
| 230 | - | ||||
| 231 | - | var _ repository.HistoryRepository = (*historyWorkerRegressionRepo)(nil) | |||