From 8111b43b104ebbbffc4a29cd446ef5c15081d860 Mon Sep 17 00:00:00 2001 From: Tim Ramlot <42113979+inteon@users.noreply.github.com> Date: Wed, 3 Jan 2024 14:05:13 +0100 Subject: [PATCH] stop relying on context.DeadlineExceeded error in tests Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com> --- test/e2e/suite/issuers/acme/issuer.go | 14 +++++++++++--- test/integration/acme/orders_controller_test.go | 14 +++++++++++--- .../certificates/trigger_controller_test.go | 14 +++++++++++--- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/test/e2e/suite/issuers/acme/issuer.go b/test/e2e/suite/issuers/acme/issuer.go index 39d6a2de6..5b4f32846 100644 --- a/test/e2e/suite/issuers/acme/issuer.go +++ b/test/e2e/suite/issuers/acme/issuer.go @@ -321,7 +321,15 @@ var _ = framework.CertManagerDescribe("ACME Issuer", func() { // TODO: we should use observedGeneration here, but currently it won't // be incremented correctly in this scenario. // Verify that Issuer's Ready condition remains True for 5 seconds. - err = wait.PollUntilContextTimeout(context.TODO(), time.Millisecond*200, time.Second*5, true, func(ctx context.Context) (bool, error) { + startTime := time.Now() + successful := false + err = wait.PollUntilContextCancel(context.TODO(), time.Millisecond*200, true, func(ctx context.Context) (bool, error) { + // Check if issuer has been ready for 5s + if time.Since(startTime) > time.Second*5 { + successful = true + return true, nil + } + iss, err := f.CertManagerClientSet.CertmanagerV1().Issuers(f.Namespace.Name).Get(ctx, issuerName, metav1.GetOptions{}) if err != nil { return false, err @@ -335,7 +343,7 @@ var _ = framework.CertManagerDescribe("ACME Issuer", func() { // keep polling return false, nil }) - Expect(err).To(HaveOccurred()) - Expect(err).To(MatchError(context.DeadlineExceeded)) + Expect(err).NotTo(HaveOccurred()) + Expect(successful).To(BeTrue()) }) }) diff --git a/test/integration/acme/orders_controller_test.go b/test/integration/acme/orders_controller_test.go index 3dd6fef08..9d1f6528e 100644 --- a/test/integration/acme/orders_controller_test.go +++ b/test/integration/acme/orders_controller_test.go @@ -260,7 +260,15 @@ func TestAcmeOrdersController(t *testing.T) { // Reason field on Order's status. Change this test once we are setting // Reasons on intermittent Order states. var pendingOrder *cmacme.Order - err = wait.PollUntilContextTimeout(ctx, time.Millisecond*200, acmeorders.RequeuePeriod, true, func(ctx context.Context) (bool, error) { + startTime := time.Now() + successful := false + err = wait.PollUntilContextCancel(ctx, time.Millisecond*200, true, func(ctx context.Context) (bool, error) { + // Check if order has been pending for 2s (requeue period) + if time.Since(startTime) > acmeorders.RequeuePeriod { + successful = true + return true, nil + } + pendingOrder, err = cmCl.AcmeV1().Orders(testName).Get(ctx, testName, metav1.GetOptions{}) if err != nil { return false, err @@ -271,9 +279,9 @@ func TestAcmeOrdersController(t *testing.T) { return false, nil }) switch { - case err == nil: + case err == nil && !successful: t.Fatalf("Expected Order to have pending status instead got: %v", pendingOrder.Status.State) - case err == context.DeadlineExceeded: + case err == nil && successful: // this is the expected 'happy case' default: t.Fatal(err) diff --git a/test/integration/certificates/trigger_controller_test.go b/test/integration/certificates/trigger_controller_test.go index ca9432cd0..10dc9c526 100644 --- a/test/integration/certificates/trigger_controller_test.go +++ b/test/integration/certificates/trigger_controller_test.go @@ -345,7 +345,15 @@ func TestTriggerController_ExpBackoff(t *testing.T) { func ensureCertificateDoesNotHaveIssuingCondition(t *testing.T, ctx context.Context, cmCl cmclient.Interface, namespace, name string) { t.Helper() - err := wait.PollUntilContextTimeout(ctx, time.Millisecond*200, time.Second*2, true, func(ctx context.Context) (bool, error) { + startTime := time.Now() + successful := false + err := wait.PollUntilContextCancel(ctx, time.Millisecond*200, true, func(ctx context.Context) (bool, error) { + // Check if certificate has not had condition for 2s + if time.Since(startTime) > time.Second*2 { + successful = true + return true, nil + } + c, err := cmCl.CertmanagerV1().Certificates(namespace).Get(ctx, name, metav1.GetOptions{}) if err != nil { return false, err @@ -360,9 +368,9 @@ func ensureCertificateDoesNotHaveIssuingCondition(t *testing.T, ctx context.Cont return false, nil }) switch { - case err == nil: + case err == nil && !successful: t.Fatal("expected Certificate to not have the Issuing condition") - case err == context.DeadlineExceeded: + case err == nil && successful: // this is the expected 'happy case' default: t.Fatal(err)