Skip to content

Commit

Permalink
Flake: List Add/Delete Unit Tests (#3627)
Browse files Browse the repository at this point in the history
Both unit tests had the same issue - under high load, the workqueue
could process the current data set that was batched for updates,
resulting in non-deterministic test state.

Moving the `sc.Run(...)` operation until only the moment where update
checks are required resolves this issue.

Fixes #3625
Fixes #3622
  • Loading branch information
markmandel authored Feb 1, 2024
1 parent e88f7c5 commit e0c8057
Showing 1 changed file with 20 additions and 22 deletions.
42 changes: 20 additions & 22 deletions pkg/sdkserver/sdkserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1437,16 +1437,8 @@ func TestSDKServerAddListValue(t *testing.T) {
require.NoError(t, err)
assert.NoError(t, sc.WaitForConnection(ctx))
sc.informerFactory.Start(ctx.Done())
assert.True(t, cache.WaitForCacheSync(ctx.Done(), sc.gameServerSynced))

wg := sync.WaitGroup{}
wg.Add(1)

go func() {
err = sc.Run(ctx)
assert.NoError(t, err)
wg.Done()
}()
require.True(t, cache.WaitForCacheSync(ctx.Done(), sc.gameServerSynced))
sc.gsWaitForSync.Done()

// check initial value comes through
require.Eventually(t, func() bool {
Expand All @@ -1469,6 +1461,14 @@ func TestSDKServerAddListValue(t *testing.T) {
assert.Equal(t, testCase.want.Values, got.Values)
assert.Equal(t, testCase.want.Capacity, got.Capacity)

// start workerqueue processing at this point, so there is no chance of processing the above updates
// earlier.
sc.gsWaitForSync.Add(1)
go func() {
err = sc.Run(ctx)
assert.NoError(t, err)
}()

// on an update, confirm that the update hits the K8s api
if testCase.updated {
select {
Expand All @@ -1487,7 +1487,6 @@ func TestSDKServerAddListValue(t *testing.T) {
assert.Equal(t, testCase.expectedUpdatesQueueLen, glu)

cancel()
wg.Wait()
})
}
}
Expand Down Expand Up @@ -1586,16 +1585,8 @@ func TestSDKServerRemoveListValue(t *testing.T) {
require.NoError(t, err)
assert.NoError(t, sc.WaitForConnection(ctx))
sc.informerFactory.Start(ctx.Done())
assert.True(t, cache.WaitForCacheSync(ctx.Done(), sc.gameServerSynced))

wg := sync.WaitGroup{}
wg.Add(1)

go func() {
err = sc.Run(ctx)
assert.NoError(t, err)
wg.Done()
}()
require.True(t, cache.WaitForCacheSync(ctx.Done(), sc.gameServerSynced))
sc.gsWaitForSync.Done()

// check initial value comes through
require.Eventually(t, func() bool {
Expand All @@ -1618,6 +1609,14 @@ func TestSDKServerRemoveListValue(t *testing.T) {
assert.Equal(t, testCase.want.Values, got.Values)
assert.Equal(t, testCase.want.Capacity, got.Capacity)

// start workerqueue processing at this point, so there is no chance of processing the above updates
// earlier.
sc.gsWaitForSync.Add(1)
go func() {
err = sc.Run(ctx)
assert.NoError(t, err)
}()

// on an update, confirm that the update hits the K8s api
if testCase.updated {
select {
Expand All @@ -1636,7 +1635,6 @@ func TestSDKServerRemoveListValue(t *testing.T) {
assert.Equal(t, testCase.expectedUpdatesQueueLen, glu)

cancel()
wg.Wait()
})
}
}
Expand Down

0 comments on commit e0c8057

Please sign in to comment.