Skip to content

Commit 4682a69

Browse files
Fix flaky test TestActionDispatcher/Cancel_queued_action (#5364)
* Fix flaky test `TestActionDispatcher/Cancel_queued_action` Here's the reason the test was flaky. The flaky `queueAssertExpectations(t)` call asserts that the queue's functions `DequeueActions` and `Save` functions are called. These functions are indeed called during the `Dispatch` function call, but only after cancel actions are dispatched. The test structure was the following: 1. Call the manager's `Dispatch` method in a goroutine. 2. Wait for the registered handler's `Handle` method to be called. 3. Assert that the queue's methods have been called. Since the queue's methods are called during the `Dispatch` method AFTER the `Handle` method is called for the Cancel action, the queue assertions may or may not succeed, depending on their timing. To fix this, I have restructured the test to: 1. Call the `Dispatch` method in a goroutine. 2. Wait for the `Dispatch` method to end by waiting on the `dispatchCompleted` channel. 3. Confirm that the `Dispatch` method didn't instead block on the errors channel, which would indicate that the dispatcher dispatched an action other than the Cancel action (dispatcher never puts anything on the errors channel for dispatched Cancel actions). 4. Assert that `Handle` has been called and that the queue's methods have been called. At this point, the queue assertions are not prone to timing, as we have waited for the `Dispatch` method to complete. * Remove unnecessary handler registration This handler is already registered as default. * Narrow down mock parameters Replaces `mock.Anything` parameters with specific variables.
1 parent 0efab75 commit 4682a69

File tree

1 file changed

+17
-20
lines changed

1 file changed

+17
-20
lines changed

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

+17-20
Original file line numberDiff line numberDiff line change
@@ -231,41 +231,38 @@ func TestActionDispatcher(t *testing.T) {
231231
})
232232

233233
t.Run("Cancel queued action", func(t *testing.T) {
234-
def := &mockHandler{}
235-
calledCh := make(chan bool)
236-
call := def.On("Handle", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
237-
call.RunFn = func(_ mock.Arguments) {
238-
calledCh <- true
239-
}
240-
241234
queue := &mockQueue{}
242235
queue.On("Save").Return(nil).Once()
243236
queue.On("DequeueActions").Return([]fleetapi.ScheduledAction{}).Once()
244237

245-
d, err := New(nil, t.TempDir(), def, queue)
246-
require.NoError(t, err)
247-
err = d.Register(&mockAction{}, def)
248-
require.NoError(t, err)
249-
250238
action := &mockAction{}
251239
action.On("Type").Return(fleetapi.ActionTypeCancel)
252240
action.On("ID").Return("id")
253241

254242
dispatchCtx, cancelFn := context.WithCancel(context.Background())
255243
defer cancelFn()
256-
go d.Dispatch(dispatchCtx, detailsSetter, ack, action)
244+
245+
def := &mockHandler{}
246+
def.On("Handle", dispatchCtx, action, ack).Return(nil).Once()
247+
248+
d, err := New(nil, t.TempDir(), def, queue)
249+
require.NoError(t, err)
250+
251+
dispatchCompleted := make(chan struct{})
252+
go func() {
253+
d.Dispatch(dispatchCtx, detailsSetter, ack, action)
254+
dispatchCompleted <- struct{}{}
255+
}()
256+
257257
select {
258258
case err := <-d.Errors():
259259
t.Fatalf("Unexpected error: %v", err)
260-
case <-calledCh:
261-
// Handle was called, expected
262-
case <-time.After(1 * time.Second):
263-
t.Fatal("mock Handle never called")
260+
case <-dispatchCompleted:
261+
// OK, expected to complete the dispatch without blocking on the errors channel
264262
}
263+
265264
def.AssertExpectations(t)
266-
// Flaky assertion: https://github.com/elastic/elastic-agent/issues/3137
267-
// TODO: re-enabled when fixed
268-
// queue.AssertExpectations(t)
265+
queue.AssertExpectations(t)
269266
})
270267

271268
t.Run("Retrieve actions from queue", func(t *testing.T) {

0 commit comments

Comments
 (0)