Skip to content

Commit

Permalink
Fix termination tests randomly timing out
Browse files Browse the repository at this point in the history
Unit tests are run in random order. Termination tests also verify the
polling logic of the respective code, by atomically incrementing a
counter in each call of the HTTP handler. The issues fixed with these
changes are:

1. counter var not being reset after each termination test using polling
2. using the polling iterations loop in termination tests that were not
   using the HTTP handler that increments the counter
3. polling iterations loop not using sleep between iterations, which
   spikes CPU usage

The first two issues led to the termination tests sometimes passing,
depending on which termination test run first. If it was a test that
incremented the polling counter, then all other test would be successful
as well. If it was a test that didn't increment the polling counter,
then the termination tests would be stuck in an endless loop, until the
unit tests reached a timeout.

Signed-off-by: Michail Resvanis <mresvani@redhat.com>
  • Loading branch information
mresvanis authored and RadekManak committed Mar 5, 2024
1 parent 5c01e37 commit 0fa855e
Showing 1 changed file with 18 additions and 7 deletions.
25 changes: 18 additions & 7 deletions pkg/termination/termination_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ var _ = Describe("Handler Suite", func() {
var counter int32

BeforeEach(func() {
counter = 0

// Ensure the polling logic is excercised in tests
httpHandler = newMockHTTPHandler(func(rw http.ResponseWriter, req *http.Request) {
if atomic.LoadInt32(&counter) == 4 {
Expand All @@ -135,15 +137,15 @@ var _ = Describe("Handler Suite", func() {
})
})

JustBeforeEach(func() {
// Ensure the polling logic is excercised in tests
for atomic.LoadInt32(&counter) < 4 {
continue
}
})

Context("and the handler is stopped", func() {
JustBeforeEach(func() {
// Ensure the polling logic is tested as well
for atomic.LoadInt32(&counter) < 4 {
// use 50ms since polling is set to 100ms
time.Sleep(50 * time.Millisecond)
continue
}

close(stop)
})

Expand All @@ -157,6 +159,15 @@ var _ = Describe("Handler Suite", func() {
})

Context("and the instance termination notice is fulfilled", func() {
JustBeforeEach(func() {
// Ensure the polling logic is tested as well
for atomic.LoadInt32(&counter) < 4 {
// use 50ms since polling is set to 100ms
time.Sleep(50 * time.Millisecond)
continue
}
})

It("should mark the node for deletion", func() {
Eventually(nodeMarkedForDeletion(testNode.Name)).Should(BeTrue())
})
Expand Down

0 comments on commit 0fa855e

Please sign in to comment.