Skip to content

Commit a172479

Browse files
Fix flaky test TestActionDispatcher/Dispatch_multiple_events_returns_one_error (#5366)
I have introduced the following changes: - Removed the `time.Sleep` calls in favor of waiting on the completion of the `Dispatch` method. - Changed the handler mock to return two errors instead of one, and assert that only the second error is put on the errors channel. - Narrowed down `mock.Anything` parameters in the `Handle` method mock to specific variables `dispatchCtx`, `action1|2`, `ack`.
1 parent 4682a69 commit a172479

File tree

1 file changed

+24
-18
lines changed

1 file changed

+24
-18
lines changed

internal/pkg/agent/application/dispatcher/dispatcher_test.go

+24-18
Original file line numberDiff line numberDiff line change
@@ -357,43 +357,49 @@ func TestActionDispatcher(t *testing.T) {
357357
action.AssertExpectations(t)
358358
})
359359

360-
t.Run("Dispatch multiples events returns one error", func(t *testing.T) {
361-
def := &mockHandler{}
362-
def.On("Handle", mock.Anything, mock.Anything, mock.Anything).Return(errors.New("test error")).Once()
363-
def.On("Handle", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
364-
360+
t.Run("Dispatch multiple events returns one error", func(t *testing.T) {
365361
queue := &mockQueue{}
366362
queue.On("Save").Return(nil).Once()
367363
queue.On("DequeueActions").Return([]fleetapi.ScheduledAction{}).Once()
368364

369-
d, err := New(nil, t.TempDir(), def, queue)
370-
require.NoError(t, err)
371-
err = d.Register(&mockAction{}, def)
372-
require.NoError(t, err)
373-
374365
action1 := &mockAction{}
375366
action1.On("Type").Return("action")
376367
action1.On("ID").Return("id")
368+
377369
action2 := &mockAction{}
378370
action2.On("Type").Return("action")
379371
action2.On("ID").Return("id")
380372

381-
// Kind of a dirty work around to test an error return.
382-
// launch in another routing and sleep to check if an error is generated
383373
dispatchCtx, cancelFn := context.WithCancel(context.Background())
384374
defer cancelFn()
385-
go d.Dispatch(dispatchCtx, detailsSetter, ack, action1, action2)
386-
time.Sleep(time.Millisecond * 200)
375+
376+
def := &mockHandler{}
377+
def.On("Handle", dispatchCtx, action1, ack).Return(errors.New("first error")).Once()
378+
def.On("Handle", dispatchCtx, action2, ack).Return(errors.New("second error")).Once()
379+
380+
d, err := New(nil, t.TempDir(), def, queue)
381+
require.NoError(t, err)
382+
383+
dispatchCompleted := make(chan struct{})
384+
go func() {
385+
d.Dispatch(dispatchCtx, detailsSetter, ack, action1, action2)
386+
dispatchCompleted <- struct{}{}
387+
}()
388+
389+
// First, assert that the Dispatch method puts one error - the second one - on the error channel.
387390
select {
388-
case <-d.Errors():
389-
default:
391+
case err := <-d.Errors():
392+
assert.EqualError(t, err, "second error")
393+
case <-dispatchCompleted:
390394
t.Fatal("Expected error")
391395
}
392-
time.Sleep(time.Millisecond * 200)
396+
397+
// Second, assert that the Dispatch method completes without putting anything else on the error channel.
393398
select {
394399
case <-d.Errors():
395400
t.Fatal(err)
396-
default:
401+
case <-dispatchCompleted:
402+
// Expecting the dispatch to complete.
397403
}
398404

399405
def.AssertExpectations(t)

0 commit comments

Comments
 (0)