Improve our integration test "Eventually" assertions.

This fixes some rare test flakes caused by a data race inherent in the way we use `assert.Eventually()` with extra variables for followup assertions. This function is tricky to use correctly because it runs the passed function in a separate goroutine, and you have no guarantee that any shared variables are in a coherent state when the `assert.Eventually()` call returns. Even if you add manual mutexes, it's tricky to get the semantics right. This has been a recurring pain point and the cause of several test flakes.

This change introduces a new `library.RequireEventually()` that works by internally constructing a per-loop `*require.Assertions` and running everything on a single goroutine (using `wait.PollImmediate()`). This makes it very easy to write eventual assertions.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
This commit is contained in:
Matt Moyer
2021-06-16 17:51:23 -05:00
parent 6a9eb87c35
commit 3efa7bdcc2
14 changed files with 318 additions and 323 deletions

View File

@@ -9,7 +9,6 @@ import (
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/square/go-jose.v2"
corev1 "k8s.io/api/core/v1"
@@ -76,16 +75,15 @@ func TestSupervisorSecrets(t *testing.T) {
t.Run(test.name, func(t *testing.T) {
// Ensure a secret is created with the FederationDomain's JWKS.
var updatedFederationDomain *configv1alpha1.FederationDomain
var err error
assert.Eventually(t, func() bool {
updatedFederationDomain, err = supervisorClient.
library.RequireEventually(t, func(requireEventually *require.Assertions) {
resp, err := supervisorClient.
ConfigV1alpha1().
FederationDomains(env.SupervisorNamespace).
Get(ctx, federationDomain.Name, metav1.GetOptions{})
return err == nil && test.secretName(updatedFederationDomain) != ""
requireEventually.NoError(err)
requireEventually.NotEmpty(test.secretName(resp))
updatedFederationDomain = resp
}, time.Second*10, time.Millisecond*500)
require.NoError(t, err)
require.NotEmpty(t, test.secretName(updatedFederationDomain))
// Ensure the secret actually exists.
secret, err := kubeClient.
@@ -109,14 +107,14 @@ func TestSupervisorSecrets(t *testing.T) {
Secrets(env.SupervisorNamespace).
Delete(ctx, test.secretName(updatedFederationDomain), metav1.DeleteOptions{})
require.NoError(t, err)
assert.Eventually(t, func() bool {
library.RequireEventually(t, func(requireEventually *require.Assertions) {
var err error
secret, err = kubeClient.
CoreV1().
Secrets(env.SupervisorNamespace).
Get(ctx, test.secretName(updatedFederationDomain), metav1.GetOptions{})
return err == nil
requireEventually.NoError(err)
}, time.Second*10, time.Millisecond*500)
require.NoError(t, err)
// Ensure that the new secret is valid.
test.ensureValid(t, secret)