From 3630328fd98896996fe3bf5248bb1efbca269a89 Mon Sep 17 00:00:00 2001 From: Danielle Lancashire Date: Tue, 8 Feb 2022 16:54:50 +0100 Subject: [PATCH] eviction: Deflake TestStart TestStart was previously flaky. In approx 100_000 local runs, it failed about 70% of the time, and has been mentioned as a flaky unit test in the past. This flake was due to a race condition with the logic as written and the go scheduler. UpdateThreshold calls `notifier.Start(events)` in a new Go Routine, which is not guarunteed to be called immediately. This meant that if `m.Start()` was called before `notifier.Start()`, the test would fail, as the notifier would not have been started before the 4 events were processed and lock released. Here, we update the test to more closely match the intended application behaviour, and have events passed to the channel when `Start` is called on the notifier. This ensures that -Start gets called and additionally validates that the correct channel is provided to the notifier. Stop was never called previously, as it only gets called on a subsequent call to UpdateThreshold. `AnyTimes()` hid that this did not occur. --- pkg/kubelet/eviction/memory_threshold_notifier_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/kubelet/eviction/memory_threshold_notifier_test.go b/pkg/kubelet/eviction/memory_threshold_notifier_test.go index 41063cace44..a8a39a56395 100644 --- a/pkg/kubelet/eviction/memory_threshold_notifier_test.go +++ b/pkg/kubelet/eviction/memory_threshold_notifier_test.go @@ -174,8 +174,11 @@ func TestStart(t *testing.T) { notifierFactory.EXPECT().NewCgroupNotifier(testCgroupPath, memoryUsageAttribute, int64(0)).Return(notifier, nil).Times(1) var events chan<- struct{} = m.events - notifier.EXPECT().Start(events).Return() - notifier.EXPECT().Stop().Return().AnyTimes() + notifier.EXPECT().Start(events).DoAndReturn(func(events chan<- struct{}) { + for i := 0; i < 4; i++ { + events <- struct{}{} + } + }) err := m.UpdateThreshold(nodeSummary(noResources, noResources, noResources, isAllocatableEvictionThreshold(threshold))) if err != nil { @@ -184,9 +187,6 @@ func TestStart(t *testing.T) { go m.Start() - for i := 0; i < 4; i++ { - m.events <- struct{}{} - } wg.Wait() }