Skip to content

Commit f495dc3

Browse files
Bryan C. Millsgopherbot
Bryan C. Mills
authored and
gopherbot
committed
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 <[email protected]> Run-TryBot: Bryan Mills <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]> Auto-Submit: Bryan Mills <[email protected]>
1 parent eb2c406 commit f495dc3

File tree

3 files changed

+50
-97
lines changed

3 files changed

+50
-97
lines changed

‎acme/acme_test.go

+9-22
Original file line numberDiff line numberDiff line change
@@ -354,11 +354,12 @@ func TestWaitAuthorization(t *testing.T) {
354354
})
355355
}
356356
t.Run("context cancel", func(t *testing.T) {
357-
ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond)
357+
ctx, cancel := context.WithCancel(context.Background())
358358
defer cancel()
359359
_, err := runWaitAuthorization(ctx, t, func(w http.ResponseWriter, r *http.Request) {
360360
w.Header().Set("Retry-After", "60")
361361
fmt.Fprintf(w, `{"status":"pending"}`)
362+
time.AfterFunc(1*time.Millisecond, cancel)
362363
})
363364
if err == nil {
364365
t.Error("err is nil")
@@ -373,28 +374,14 @@ func runWaitAuthorization(ctx context.Context, t *testing.T, h http.HandlerFunc)
373374
h(w, r)
374375
}))
375376
defer ts.Close()
376-
type res struct {
377-
authz *Authorization
378-
err error
379-
}
380-
ch := make(chan res, 1)
381-
go func() {
382-
client := &Client{
383-
Key: testKey,
384-
DirectoryURL: ts.URL,
385-
dir: &Directory{},
386-
KID: "some-key-id", // set to avoid lookup attempt
387-
}
388-
a, err := client.WaitAuthorization(ctx, ts.URL)
389-
ch <- res{a, err}
390-
}()
391-
select {
392-
case <-time.After(3 * time.Second):
393-
t.Fatal("WaitAuthorization took too long to return")
394-
case v := <-ch:
395-
return v.authz, v.err
377+
378+
client := &Client{
379+
Key: testKey,
380+
DirectoryURL: ts.URL,
381+
dir: &Directory{},
382+
KID: "some-key-id", // set to avoid lookup attempt
396383
}
397-
panic("runWaitAuthorization: out of select")
384+
return client.WaitAuthorization(ctx, ts.URL)
398385
}
399386

400387
func TestRevokeAuthorization(t *testing.T) {

‎acme/autocert/autocert_test.go

+16-25
Original file line numberDiff line numberDiff line change
@@ -427,18 +427,7 @@ func TestGetCertificate(t *testing.T) {
427427

428428
man.Client = &acme.Client{DirectoryURL: s.URL()}
429429

430-
var tlscert *tls.Certificate
431-
var err error
432-
done := make(chan struct{})
433-
go func() {
434-
tlscert, err = man.GetCertificate(tt.hello)
435-
close(done)
436-
}()
437-
select {
438-
case <-time.After(time.Minute):
439-
t.Fatal("man.GetCertificate took too long to return")
440-
case <-done:
441-
}
430+
tlscert, err := man.GetCertificate(tt.hello)
442431
if tt.expectError != "" {
443432
if err == nil {
444433
t.Fatal("expected error, got certificate")
@@ -515,15 +504,12 @@ func TestGetCertificate_failedAttempt(t *testing.T) {
515504
if _, err := man.GetCertificate(hello); err == nil {
516505
t.Error("GetCertificate: err is nil")
517506
}
518-
select {
519-
case <-time.After(5 * time.Second):
520-
t.Errorf("took too long to remove the %q state", exampleCertKey)
521-
case <-done:
522-
man.stateMu.Lock()
523-
defer man.stateMu.Unlock()
524-
if v, exist := man.state[exampleCertKey]; exist {
525-
t.Errorf("state exists for %v: %+v", exampleCertKey, v)
526-
}
507+
508+
<-done
509+
man.stateMu.Lock()
510+
defer man.stateMu.Unlock()
511+
if v, exist := man.state[exampleCertKey]; exist {
512+
t.Errorf("state exists for %v: %+v", exampleCertKey, v)
527513
}
528514
}
529515

@@ -542,19 +528,24 @@ func TestRevokeFailedAuthz(t *testing.T) {
542528
t.Fatal("expected GetCertificate to fail")
543529
}
544530

545-
start := time.Now()
546-
for time.Since(start) < 3*time.Second {
531+
logTicker := time.NewTicker(3 * time.Second)
532+
defer logTicker.Stop()
533+
for {
547534
authz, err := m.Client.GetAuthorization(context.Background(), ca.URL()+"/authz/0")
548535
if err != nil {
549536
t.Fatal(err)
550537
}
551538
if authz.Status == acme.StatusDeactivated {
552539
return
553540
}
541+
542+
select {
543+
case <-logTicker.C:
544+
t.Logf("still waiting on revocations")
545+
default:
546+
}
554547
time.Sleep(50 * time.Millisecond)
555548
}
556-
t.Error("revocations took too long")
557-
558549
}
559550

560551
func TestHTTPHandlerDefaultFallback(t *testing.T) {

‎acme/rfc8555_test.go

+25-50
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,7 @@ func TestRFC_postKID(t *testing.T) {
177177
}))
178178
defer ts.Close()
179179

180-
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
181-
defer cancel()
180+
ctx := context.Background()
182181
cl := &Client{
183182
Key: testKey,
184183
DirectoryURL: ts.URL,
@@ -316,8 +315,7 @@ func TestRFC_Register(t *testing.T) {
316315
s.start()
317316
defer s.close()
318317

319-
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
320-
defer cancel()
318+
ctx := context.Background()
321319
cl := &Client{
322320
Key: testKeyEC,
323321
DirectoryURL: s.url("/"),
@@ -454,8 +452,7 @@ func TestRFC_RegisterExternalAccountBinding(t *testing.T) {
454452
s.start()
455453
defer s.close()
456454

457-
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
458-
defer cancel()
455+
ctx := context.Background()
459456
cl := &Client{
460457
Key: testKeyEC,
461458
DirectoryURL: s.url("/"),
@@ -867,24 +864,13 @@ func testWaitOrderStatus(t *testing.T, okStatus string) {
867864
s.start()
868865
defer s.close()
869866

870-
var order *Order
871-
var err error
872-
done := make(chan struct{})
873-
go func() {
874-
cl := &Client{Key: testKeyEC, DirectoryURL: s.url("/")}
875-
order, err = cl.WaitOrder(context.Background(), s.url("/orders/1"))
876-
close(done)
877-
}()
878-
select {
879-
case <-time.After(3 * time.Second):
880-
t.Fatal("WaitOrder took too long to return")
881-
case <-done:
882-
if err != nil {
883-
t.Fatalf("WaitOrder: %v", err)
884-
}
885-
if order.Status != okStatus {
886-
t.Errorf("order.Status = %q; want %q", order.Status, okStatus)
887-
}
867+
cl := &Client{Key: testKeyEC, DirectoryURL: s.url("/")}
868+
order, err := cl.WaitOrder(context.Background(), s.url("/orders/1"))
869+
if err != nil {
870+
t.Fatalf("WaitOrder: %v", err)
871+
}
872+
if order.Status != okStatus {
873+
t.Errorf("order.Status = %q; want %q", order.Status, okStatus)
888874
}
889875
}
890876

@@ -909,30 +895,20 @@ func TestRFC_WaitOrderError(t *testing.T) {
909895
s.start()
910896
defer s.close()
911897

912-
var err error
913-
done := make(chan struct{})
914-
go func() {
915-
cl := &Client{Key: testKeyEC, DirectoryURL: s.url("/")}
916-
_, err = cl.WaitOrder(context.Background(), s.url("/orders/1"))
917-
close(done)
918-
}()
919-
select {
920-
case <-time.After(3 * time.Second):
921-
t.Fatal("WaitOrder took too long to return")
922-
case <-done:
923-
if err == nil {
924-
t.Fatal("WaitOrder returned nil error")
925-
}
926-
e, ok := err.(*OrderError)
927-
if !ok {
928-
t.Fatalf("err = %v (%T); want OrderError", err, err)
929-
}
930-
if e.OrderURL != s.url("/orders/1") {
931-
t.Errorf("e.OrderURL = %q; want %q", e.OrderURL, s.url("/orders/1"))
932-
}
933-
if e.Status != StatusInvalid {
934-
t.Errorf("e.Status = %q; want %q", e.Status, StatusInvalid)
935-
}
898+
cl := &Client{Key: testKeyEC, DirectoryURL: s.url("/")}
899+
_, err := cl.WaitOrder(context.Background(), s.url("/orders/1"))
900+
if err == nil {
901+
t.Fatal("WaitOrder returned nil error")
902+
}
903+
e, ok := err.(*OrderError)
904+
if !ok {
905+
t.Fatalf("err = %v (%T); want OrderError", err, err)
906+
}
907+
if e.OrderURL != s.url("/orders/1") {
908+
t.Errorf("e.OrderURL = %q; want %q", e.OrderURL, s.url("/orders/1"))
909+
}
910+
if e.Status != StatusInvalid {
911+
t.Errorf("e.Status = %q; want %q", e.Status, StatusInvalid)
936912
}
937913
}
938914

@@ -972,8 +948,7 @@ func TestRFC_CreateOrderCert(t *testing.T) {
972948
})
973949
s.start()
974950
defer s.close()
975-
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
976-
defer cancel()
951+
ctx := context.Background()
977952

978953
cl := &Client{Key: testKeyEC, DirectoryURL: s.url("/")}
979954
cert, curl, err := cl.CreateOrderCert(ctx, s.url("/pleaseissue"), csr, true)

0 commit comments

Comments
 (0)