From f495dc37d5f5cc59ca73af6acbbe0a741559792b Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Thu, 8 Dec 2022 12:40:34 -0500 Subject: [PATCH] acme: eliminate arbitrary timeouts in tests Fixes golang/go#57107. Change-Id: I20b1f6ca85170c6b4731d7c7ea06f4db742526cc Reviewed-on: https://go-review.googlesource.com/c/crypto/+/456123 TryBot-Result: Gopher Robot Run-TryBot: Bryan Mills Reviewed-by: Roland Shoemaker Auto-Submit: Bryan Mills --- acme/acme_test.go | 31 ++++---------- acme/autocert/autocert_test.go | 41 ++++++++----------- acme/rfc8555_test.go | 75 ++++++++++++---------------------- 3 files changed, 50 insertions(+), 97 deletions(-) diff --git a/acme/acme_test.go b/acme/acme_test.go index a748d88ffd..3f6e2748f3 100644 --- a/acme/acme_test.go +++ b/acme/acme_test.go @@ -354,11 +354,12 @@ func TestWaitAuthorization(t *testing.T) { }) } t.Run("context cancel", func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + ctx, cancel := context.WithCancel(context.Background()) defer cancel() _, err := runWaitAuthorization(ctx, t, func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Retry-After", "60") fmt.Fprintf(w, `{"status":"pending"}`) + time.AfterFunc(1*time.Millisecond, cancel) }) if err == nil { t.Error("err is nil") @@ -373,28 +374,14 @@ func runWaitAuthorization(ctx context.Context, t *testing.T, h http.HandlerFunc) h(w, r) })) defer ts.Close() - type res struct { - authz *Authorization - err error - } - ch := make(chan res, 1) - go func() { - client := &Client{ - Key: testKey, - DirectoryURL: ts.URL, - dir: &Directory{}, - KID: "some-key-id", // set to avoid lookup attempt - } - a, err := client.WaitAuthorization(ctx, ts.URL) - ch <- res{a, err} - }() - select { - case <-time.After(3 * time.Second): - t.Fatal("WaitAuthorization took too long to return") - case v := <-ch: - return v.authz, v.err + + client := &Client{ + Key: testKey, + DirectoryURL: ts.URL, + dir: &Directory{}, + KID: "some-key-id", // set to avoid lookup attempt } - panic("runWaitAuthorization: out of select") + return client.WaitAuthorization(ctx, ts.URL) } func TestRevokeAuthorization(t *testing.T) { diff --git a/acme/autocert/autocert_test.go b/acme/autocert/autocert_test.go index 789a44147b..725677574b 100644 --- a/acme/autocert/autocert_test.go +++ b/acme/autocert/autocert_test.go @@ -427,18 +427,7 @@ func TestGetCertificate(t *testing.T) { man.Client = &acme.Client{DirectoryURL: s.URL()} - var tlscert *tls.Certificate - var err error - done := make(chan struct{}) - go func() { - tlscert, err = man.GetCertificate(tt.hello) - close(done) - }() - select { - case <-time.After(time.Minute): - t.Fatal("man.GetCertificate took too long to return") - case <-done: - } + tlscert, err := man.GetCertificate(tt.hello) if tt.expectError != "" { if err == nil { t.Fatal("expected error, got certificate") @@ -515,15 +504,12 @@ func TestGetCertificate_failedAttempt(t *testing.T) { if _, err := man.GetCertificate(hello); err == nil { t.Error("GetCertificate: err is nil") } - select { - case <-time.After(5 * time.Second): - t.Errorf("took too long to remove the %q state", exampleCertKey) - case <-done: - man.stateMu.Lock() - defer man.stateMu.Unlock() - if v, exist := man.state[exampleCertKey]; exist { - t.Errorf("state exists for %v: %+v", exampleCertKey, v) - } + + <-done + man.stateMu.Lock() + defer man.stateMu.Unlock() + if v, exist := man.state[exampleCertKey]; exist { + t.Errorf("state exists for %v: %+v", exampleCertKey, v) } } @@ -542,8 +528,9 @@ func TestRevokeFailedAuthz(t *testing.T) { t.Fatal("expected GetCertificate to fail") } - start := time.Now() - for time.Since(start) < 3*time.Second { + logTicker := time.NewTicker(3 * time.Second) + defer logTicker.Stop() + for { authz, err := m.Client.GetAuthorization(context.Background(), ca.URL()+"/authz/0") if err != nil { t.Fatal(err) @@ -551,10 +538,14 @@ func TestRevokeFailedAuthz(t *testing.T) { if authz.Status == acme.StatusDeactivated { return } + + select { + case <-logTicker.C: + t.Logf("still waiting on revocations") + default: + } time.Sleep(50 * time.Millisecond) } - t.Error("revocations took too long") - } func TestHTTPHandlerDefaultFallback(t *testing.T) { diff --git a/acme/rfc8555_test.go b/acme/rfc8555_test.go index e92f6583e6..d65720a356 100644 --- a/acme/rfc8555_test.go +++ b/acme/rfc8555_test.go @@ -177,8 +177,7 @@ func TestRFC_postKID(t *testing.T) { })) defer ts.Close() - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() + ctx := context.Background() cl := &Client{ Key: testKey, DirectoryURL: ts.URL, @@ -316,8 +315,7 @@ func TestRFC_Register(t *testing.T) { s.start() defer s.close() - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() + ctx := context.Background() cl := &Client{ Key: testKeyEC, DirectoryURL: s.url("/"), @@ -454,8 +452,7 @@ func TestRFC_RegisterExternalAccountBinding(t *testing.T) { s.start() defer s.close() - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() + ctx := context.Background() cl := &Client{ Key: testKeyEC, DirectoryURL: s.url("/"), @@ -867,24 +864,13 @@ func testWaitOrderStatus(t *testing.T, okStatus string) { s.start() defer s.close() - var order *Order - var err error - done := make(chan struct{}) - go func() { - cl := &Client{Key: testKeyEC, DirectoryURL: s.url("/")} - order, err = cl.WaitOrder(context.Background(), s.url("/orders/1")) - close(done) - }() - select { - case <-time.After(3 * time.Second): - t.Fatal("WaitOrder took too long to return") - case <-done: - if err != nil { - t.Fatalf("WaitOrder: %v", err) - } - if order.Status != okStatus { - t.Errorf("order.Status = %q; want %q", order.Status, okStatus) - } + cl := &Client{Key: testKeyEC, DirectoryURL: s.url("/")} + order, err := cl.WaitOrder(context.Background(), s.url("/orders/1")) + if err != nil { + t.Fatalf("WaitOrder: %v", err) + } + if order.Status != okStatus { + t.Errorf("order.Status = %q; want %q", order.Status, okStatus) } } @@ -909,30 +895,20 @@ func TestRFC_WaitOrderError(t *testing.T) { s.start() defer s.close() - var err error - done := make(chan struct{}) - go func() { - cl := &Client{Key: testKeyEC, DirectoryURL: s.url("/")} - _, err = cl.WaitOrder(context.Background(), s.url("/orders/1")) - close(done) - }() - select { - case <-time.After(3 * time.Second): - t.Fatal("WaitOrder took too long to return") - case <-done: - if err == nil { - t.Fatal("WaitOrder returned nil error") - } - e, ok := err.(*OrderError) - if !ok { - t.Fatalf("err = %v (%T); want OrderError", err, err) - } - if e.OrderURL != s.url("/orders/1") { - t.Errorf("e.OrderURL = %q; want %q", e.OrderURL, s.url("/orders/1")) - } - if e.Status != StatusInvalid { - t.Errorf("e.Status = %q; want %q", e.Status, StatusInvalid) - } + cl := &Client{Key: testKeyEC, DirectoryURL: s.url("/")} + _, err := cl.WaitOrder(context.Background(), s.url("/orders/1")) + if err == nil { + t.Fatal("WaitOrder returned nil error") + } + e, ok := err.(*OrderError) + if !ok { + t.Fatalf("err = %v (%T); want OrderError", err, err) + } + if e.OrderURL != s.url("/orders/1") { + t.Errorf("e.OrderURL = %q; want %q", e.OrderURL, s.url("/orders/1")) + } + if e.Status != StatusInvalid { + t.Errorf("e.Status = %q; want %q", e.Status, StatusInvalid) } } @@ -972,8 +948,7 @@ func TestRFC_CreateOrderCert(t *testing.T) { }) s.start() defer s.close() - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) - defer cancel() + ctx := context.Background() cl := &Client{Key: testKeyEC, DirectoryURL: s.url("/")} cert, curl, err := cl.CreateOrderCert(ctx, s.url("/pleaseissue"), csr, true)