Skip to content

Commit faf2384

Browse files
robbytrobbyt
authored
fix mock assertions (#87)
* add mock AssertExpectations calls * remove the .Maybe() mock calls, and clean up several unused mock assertions
1 parent ff30697 commit faf2384

File tree

6 files changed

+29
-79
lines changed

6 files changed

+29
-79
lines changed

‎runnables/composite/integration_race_test.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,10 @@ func testCompositeRaceCondition(t *testing.T) {
120120
mock1.AssertCalled(t, "Stop")
121121
mock2.AssertCalled(t, "Stop")
122122
mock3.AssertCalled(t, "Stop")
123+
124+
mock1.AssertExpectations(t)
125+
mock2.AssertExpectations(t)
126+
mock3.AssertExpectations(t)
123127
})
124128
}
125129

@@ -198,5 +202,8 @@ func TestIntegration_CompositeFullLifecycle(t *testing.T) {
198202

199203
mock1.AssertCalled(t, "Stop")
200204
mock2.AssertCalled(t, "Stop")
205+
206+
mock1.AssertExpectations(t)
207+
mock2.AssertExpectations(t)
201208
})
202209
}

‎runnables/composite/runner_test.go‎

Lines changed: 15 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,7 @@ func TestNewRunner(t *testing.T) {
9797
func TestCompositeRunner_String(t *testing.T) {
9898
t.Parallel()
9999
mockRunnable1 := mocks.NewMockRunnable()
100-
mockRunnable1.On("String").Return("runnable1").Maybe()
101-
mockRunnable1.On("Stop").Maybe()
102-
mockRunnable1.On("Run", mock.Anything).Return(nil).Maybe()
103-
104100
mockRunnable2 := mocks.NewMockRunnable()
105-
mockRunnable2.On("String").Return("runnable2").Maybe()
106-
mockRunnable2.On("Stop").Maybe()
107-
mockRunnable2.On("Run", mock.Anything).Return(nil).Maybe()
108101

109102
entries := []RunnableEntry[*mocks.Runnable]{
110103
{Runnable: mockRunnable1, Config: nil},
@@ -130,15 +123,15 @@ func TestCompositeRunner_Run(t *testing.T) {
130123
t.Run("successful run and graceful shutdown", func(t *testing.T) {
131124
// Setup mock runnables
132125
mockRunnable1 := mocks.NewMockRunnable()
133-
mockRunnable1.On("String").Return("runnable1").Maybe()
126+
mockRunnable1.On("String").Return("runnable1")
134127
mockRunnable1.On("Run", mock.Anything).Return(nil).Run(func(args mock.Arguments) {
135128
ctx := args.Get(0).(context.Context)
136129
<-ctx.Done() // Block until cancelled like a real service
137130
})
138131
mockRunnable1.On("Stop").Once()
139132

140133
mockRunnable2 := mocks.NewMockRunnable()
141-
mockRunnable2.On("String").Return("runnable2").Maybe()
134+
mockRunnable2.On("String").Return("runnable2")
142135
mockRunnable2.On("Run", mock.Anything).Return(nil).Run(func(args mock.Arguments) {
143136
ctx := args.Get(0).(context.Context)
144137
<-ctx.Done() // Block until cancelled like a real service
@@ -196,7 +189,7 @@ func TestCompositeRunner_Run(t *testing.T) {
196189
t.Run("runnable fails during execution", func(t *testing.T) {
197190
// Setup mock runnables
198191
mockRunnable1 := mocks.NewMockRunnable()
199-
mockRunnable1.On("String").Return("runnable1").Maybe()
192+
mockRunnable1.On("String").Return("runnable1")
200193

201194
var capturedRunner *Runner[*mocks.Runnable]
202195
mockRunnable1.On("Run", mock.Anything).Run(func(args mock.Arguments) {
@@ -211,7 +204,6 @@ func TestCompositeRunner_Run(t *testing.T) {
211204
// Wait until context is cancelled
212205
<-args.Get(0).(context.Context).Done()
213206
}).Return(nil)
214-
mockRunnable1.On("Stop").Maybe()
215207

216208
// Create entries
217209
entries := []RunnableEntry[*mocks.Runnable]{
@@ -242,14 +234,12 @@ func TestCompositeRunner_Run(t *testing.T) {
242234
t.Run("runnable fails during startup", func(t *testing.T) {
243235
// Setup mock runnables
244236
mockRunnable1 := mocks.NewMockRunnable()
245-
mockRunnable1.On("String").Return("runnable1").Maybe()
246-
mockRunnable1.On("Run", mock.Anything).Return(nil).Maybe()
247-
mockRunnable1.On("Stop").Maybe()
237+
mockRunnable1.On("String").Return("runnable1")
238+
mockRunnable1.On("Run", mock.Anything).Return(nil)
248239

249240
mockRunnable2 := mocks.NewMockRunnable()
250-
mockRunnable2.On("String").Return("runnable2").Maybe()
241+
mockRunnable2.On("String").Return("runnable2")
251242
mockRunnable2.On("Run", mock.Anything).Return(errors.New("failed to start"))
252-
mockRunnable2.On("Stop").Maybe()
253243

254244
// Create entries
255245
entries := []RunnableEntry[*mocks.Runnable]{
@@ -350,13 +340,13 @@ func TestCompositeRunner_Run(t *testing.T) {
350340

351341
// Setup mock runnable for reload
352342
mockRunnable := mocks.NewMockRunnable()
353-
mockRunnable.On("String").Return("runnable1").Maybe()
343+
mockRunnable.On("String").Return("runnable1")
354344
mockRunnable.On("Run", mock.Anything).Return(nil).Run(func(args mock.Arguments) {
355345
close(runStarted) // Signal that Run was called
356346
ctx := args.Get(0).(context.Context)
357347
<-ctx.Done() // Block until cancelled like a real service
358348
})
359-
mockRunnable.On("Stop").Maybe()
349+
mockRunnable.On("Stop").Once()
360350

361351
// Initially empty, later populated
362352
var useUpdatedEntries atomic.Bool
@@ -450,18 +440,9 @@ func TestCompositeRunner_Stop(t *testing.T) {
450440

451441
// Create reusable mock setup function
452442
setupMocksAndConfig := func() (entries []RunnableEntry[*mocks.Runnable], configFunc func() (*Config[*mocks.Runnable], error)) {
453-
// Setup mock runnables
454443
mockRunnable1 := mocks.NewMockRunnable()
455-
mockRunnable1.On("String").Return("runnable1").Maybe()
456-
mockRunnable1.On("Run", mock.Anything).Return(nil).Maybe()
457-
mockRunnable1.On("Stop").Maybe()
458-
459444
mockRunnable2 := mocks.NewMockRunnable()
460-
mockRunnable2.On("String").Return("runnable2").Maybe()
461-
mockRunnable2.On("Run", mock.Anything).Return(nil).Maybe()
462-
mockRunnable2.On("Stop").Maybe()
463445

464-
// Create entries
465446
entries = []RunnableEntry[*mocks.Runnable]{
466447
{Runnable: mockRunnable1, Config: nil},
467448
{Runnable: mockRunnable2, Config: nil},
@@ -537,24 +518,21 @@ func TestCompositeRunner_MultipleChildFailures(t *testing.T) {
537518
started := make(chan struct{})
538519

539520
mockRunnable1 := mocks.NewMockRunnable()
540-
mockRunnable1.On("String").Return("failer1").Maybe()
541-
mockRunnable1.On("Stop").Maybe()
521+
mockRunnable1.On("String").Return("failer1")
542522
mockRunnable1.On("Run", mock.Anything).Run(func(args mock.Arguments) {
543523
started <- struct{}{}
544524
time.Sleep(20 * time.Millisecond)
545525
}).Return(failErr)
546526

547527
mockRunnable2 := mocks.NewMockRunnable()
548-
mockRunnable2.On("String").Return("failer2").Maybe()
549-
mockRunnable2.On("Stop").Maybe()
528+
mockRunnable2.On("String").Return("failer2")
550529
mockRunnable2.On("Run", mock.Anything).Run(func(args mock.Arguments) {
551530
started <- struct{}{}
552531
time.Sleep(20 * time.Millisecond)
553532
}).Return(failErr)
554533

555534
mockRunnable3 := mocks.NewMockRunnable()
556-
mockRunnable3.On("String").Return("failer3").Maybe()
557-
mockRunnable3.On("Stop").Maybe()
535+
mockRunnable3.On("String").Return("failer3")
558536
mockRunnable3.On("Run", mock.Anything).Run(func(args mock.Arguments) {
559537
started <- struct{}{}
560538
time.Sleep(20 * time.Millisecond)
@@ -601,5 +579,9 @@ func TestCompositeRunner_MultipleChildFailures(t *testing.T) {
601579
default:
602580
t.Fatal("runner should return an error from failing children")
603581
}
582+
583+
mockRunnable1.AssertExpectations(t)
584+
mockRunnable2.AssertExpectations(t)
585+
mockRunnable3.AssertExpectations(t)
604586
})
605587
}

‎supervisor/reload_test.go‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,11 @@ func TestPIDZero_ReloadManager(t *testing.T) {
2121
// Setup mock
2222
sender := mocks.NewMockRunnableWithReloadSender()
2323
reloadTrigger := make(chan struct{})
24-
stateChan := make(chan string)
2524

2625
sender.On("GetReloadTrigger").Return(reloadTrigger)
2726
sender.On("Run", mock.Anything).Return(nil)
2827
sender.On("Reload").Return().Once()
2928
sender.On("Stop").Return()
30-
sender.On("GetState").Return("running").Maybe()
31-
sender.On("GetStateChan", mock.Anything).Return(stateChan).Maybe()
3229

3330
p, err := New(WithContext(context.Background()), WithRunnables(sender))
3431
require.NoError(t, err)
@@ -59,8 +56,6 @@ func TestPIDZero_ReloadManager(t *testing.T) {
5956

6057
reloadTrigger1 := make(chan struct{})
6158
reloadTrigger2 := make(chan struct{})
62-
stateChan1 := make(chan string)
63-
stateChan2 := make(chan string)
6459

6560
sender1.On("GetReloadTrigger").Return(reloadTrigger1)
6661
sender2.On("GetReloadTrigger").Return(reloadTrigger2)
@@ -74,11 +69,6 @@ func TestPIDZero_ReloadManager(t *testing.T) {
7469
sender1.On("Stop").Return()
7570
sender2.On("Stop").Return()
7671

77-
sender1.On("GetState").Return("running").Maybe()
78-
sender2.On("GetState").Return("running").Maybe()
79-
sender1.On("GetStateChan", mock.Anything).Return(stateChan1).Maybe()
80-
sender2.On("GetStateChan", mock.Anything).Return(stateChan2).Maybe()
81-
8272
p, err := New(WithContext(context.Background()), WithRunnables(sender1, sender2))
8373
require.NoError(t, err)
8474

@@ -109,13 +99,10 @@ func TestPIDZero_ReloadManager(t *testing.T) {
10999

110100
sender := mocks.NewMockRunnableWithReloadSender()
111101
reloadTrigger := make(chan struct{})
112-
stateChan := make(chan string)
113102

114103
sender.On("GetReloadTrigger").Return(reloadTrigger)
115104
sender.On("Run", mock.Anything).Return(nil)
116105
sender.On("Stop").Return()
117-
sender.On("GetState").Return("running").Maybe()
118-
sender.On("GetStateChan", mock.Anything).Return(stateChan).Maybe()
119106

120107
p, err := New(WithContext(ctx), WithRunnables(sender))
121108
require.NoError(t, err)
@@ -147,9 +134,6 @@ func TestPIDZero_ReloadManager(t *testing.T) {
147134
mockService1 := mocks.NewMockRunnable()
148135
mockService2 := mocks.NewMockRunnable()
149136

150-
mockService1.On("String").Return("ReloadableService1").Maybe()
151-
mockService2.On("String").Return("ReloadableService2").Maybe()
152-
153137
mockService1.On("Reload").Once()
154138
mockService2.On("Reload").Once()
155139

@@ -203,8 +187,6 @@ func TestPIDZero_ReloadManager(t *testing.T) {
203187

204188
reloadTrigger1 := make(chan struct{})
205189
reloadTrigger2 := make(chan struct{})
206-
stateChan1 := make(chan string)
207-
stateChan2 := make(chan string)
208190

209191
var reloadCount atomic.Int32
210192
sender1.On("GetReloadTrigger").Return(reloadTrigger1)
@@ -215,10 +197,6 @@ func TestPIDZero_ReloadManager(t *testing.T) {
215197
sender2.On("Reload").Run(func(_ mock.Arguments) { reloadCount.Add(1) }).Return()
216198
sender1.On("Stop").Return()
217199
sender2.On("Stop").Return()
218-
sender1.On("GetState").Return("running").Maybe()
219-
sender2.On("GetState").Return("running").Maybe()
220-
sender1.On("GetStateChan", mock.Anything).Return(stateChan1).Maybe()
221-
sender2.On("GetStateChan", mock.Anything).Return(stateChan2).Maybe()
222200

223201
p, err := New(WithContext(context.Background()), WithRunnables(sender1, sender2))
224202
require.NoError(t, err)
@@ -260,7 +238,6 @@ func TestPIDZero_ReloadManager(t *testing.T) {
260238

261239
t.Run("SIGTERM handled while reload is in progress", func(t *testing.T) {
262240
mockService := mocks.NewMockRunnable()
263-
mockService.On("String").Return("SlowReloader").Maybe()
264241
mockService.On("Run", mock.Anything).Return(nil)
265242
mockService.On("Stop").Return()
266243
mockService.DelayReload = 500 * time.Millisecond
@@ -298,7 +275,6 @@ func TestPIDZero_ReloadManager(t *testing.T) {
298275

299276
t.Run("concurrent ReloadAll is safe under race detector", func(t *testing.T) {
300277
mockService := mocks.NewMockRunnable()
301-
mockService.On("String").Return("ConcurrentReloader").Maybe()
302278
mockService.On("Run", mock.Anything).Return(nil)
303279
mockService.On("Stop").Return()
304280
mockService.On("Reload").Return()

‎supervisor/shutdown_test.go‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ func TestPIDZero_StartShutdownManager_TriggersShutdown(t *testing.T) {
2727
shutdownChan := make(chan struct{}, 1)
2828

2929
mockService.On("GetShutdownTrigger").Return(shutdownChan).Once()
30-
mockService.On("String").Return("mockShutdownService").Maybe()
31-
mockService.On("Stop").Return().Maybe()
30+
mockService.On("String").Return("mockShutdownService")
31+
mockService.On("Stop").Return().Once()
3232

3333
pidZero, err := New(WithContext(supervisorCtx), WithRunnables(mockService))
3434
require.NoError(t, err)
@@ -58,8 +58,6 @@ func TestPIDZero_StartShutdownManager_ContextCancel(t *testing.T) {
5858
shutdownChan := make(chan struct{})
5959

6060
mockService.On("GetShutdownTrigger").Return(shutdownChan).Once()
61-
mockService.On("String").Return("mockShutdownService").Maybe()
62-
mockService.On("Stop").Return().Maybe()
6361

6462
pidZero, err := New(WithContext(ctx), WithRunnables(mockService))
6563
require.NoError(t, err)
@@ -82,9 +80,6 @@ func TestPIDZero_StartShutdownManager_NoSenders(t *testing.T) {
8280
defer cancel()
8381

8482
nonSenderRunnable := mocks.NewMockRunnable()
85-
nonSenderRunnable.On("Run", mock.Anything).Return(nil).Maybe()
86-
nonSenderRunnable.On("Stop").Maybe()
87-
nonSenderRunnable.On("String").Return("simpleRunnable").Maybe()
8883

8984
pidZero, err := New(WithContext(ctx), WithRunnables(nonSenderRunnable))
9085
require.NoError(t, err)
@@ -118,8 +113,6 @@ func TestPIDZero_Shutdown_WithTimeoutNotExceeded(t *testing.T) {
118113
close(stopCalled)
119114
})
120115

121-
runnable.On("String").Return("blockingRunnable").Maybe()
122-
123116
pidZero, err := New(
124117
WithRunnables(runnable),
125118
WithShutdownTimeout(2*time.Second),
@@ -178,8 +171,6 @@ func TestPIDZero_Shutdown_WithTimeoutExceeded(t *testing.T) {
178171
time.Sleep(50 * time.Millisecond)
179172
})
180173

181-
runnable.On("String").Return("stuckRunnable").Maybe()
182-
183174
pidZero, err := New(
184175
WithRunnables(runnable),
185176
WithShutdownTimeout(200*time.Millisecond),

‎supervisor/state_deduplication_test.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ func TestStateDeduplication(t *testing.T) {
2424
runnable := mocks.NewMockRunnableWithStateable()
2525
runnable.On("String").Return("test-runnable")
2626
runnable.On("GetStateChan", mock.Anything).Return(stateChan)
27-
runnable.On("GetState").Return("initial")
2827

2928
pidZero, err := New(WithContext(ctx), WithRunnables(runnable))
3029
require.NoError(t, err)
@@ -68,7 +67,6 @@ func TestStateDeduplication(t *testing.T) {
6867
stateChan <- "initial"
6968

7069
t.Log("Sending 'running' state")
71-
runnable.On("GetState").Return("running")
7270
stateChan <- "running"
7371

7472
t.Log("Sending 'running' state again (should be ignored)")
@@ -78,14 +76,12 @@ func TestStateDeduplication(t *testing.T) {
7876
stateChan <- "running"
7977

8078
t.Log("Sending 'stopped' state")
81-
runnable.On("GetState").Return("stopped")
8279
stateChan <- "stopped"
8380

8481
t.Log("Sending 'stopped' state again (should be ignored)")
8582
stateChan <- "stopped"
8683

8784
t.Log("Sending 'error' state")
88-
runnable.On("GetState").Return("error")
8985
stateChan <- "error"
9086

9187
synctest.Wait()
@@ -109,5 +105,7 @@ func TestStateDeduplication(t *testing.T) {
109105
t, 1, statesReceived["error"],
110106
"Should receive exactly one 'error' state broadcast",
111107
)
108+
109+
runnable.AssertExpectations(t)
112110
})
113111
}

‎supervisor/state_monitoring_test.go‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,11 @@ func TestPIDZero_StartStateMonitor(t *testing.T) {
1818
t.Parallel()
1919

2020
mockStateable := mocks.NewMockRunnableWithStateable()
21-
mockStateable.On("String").Return("stateable-runnable").Maybe()
21+
mockStateable.On("String").Return("stateable-runnable")
2222
mockStateable.On("Run", mock.Anything).Return(nil)
2323
mockStateable.On("Stop").Once()
2424
mockStateable.On("GetState").Return("initial").Once()
25-
mockStateable.On("GetState").Return("running").Maybe()
25+
mockStateable.On("GetState").Return("running")
2626

2727
stateChan := make(chan string, 5)
2828
mockStateable.On("GetStateChan", mock.Anything).Return(stateChan).Once()
@@ -83,11 +83,7 @@ func TestPIDZero_SubscribeStateChanges(t *testing.T) {
8383
mockService := mocks.NewMockRunnableWithStateable()
8484
stateChan := make(chan string, 2)
8585
mockService.On("GetStateChan", mock.Anything).Return(stateChan).Once()
86-
mockService.On("String").Return("mock-service").Maybe()
87-
mockService.On("Run", mock.Anything).Return(nil).Maybe()
88-
mockService.On("Stop").Maybe()
89-
mockService.On("GetState").Return("initial").Maybe()
90-
mockService.On("IsRunning").Return(true).Maybe()
86+
mockService.On("String").Return("mock-service")
9187

9288
pid0, err := New(WithContext(ctx), WithRunnables(mockService))
9389
require.NoError(t, err)

0 commit comments

Comments
 (0)