From: Bryan C. Mills <bcmills@google.com>
Date: Fri, 27 Mar 2020 16:11:21 +0000 (-0400)
Subject: net/http: remove arbitrary timeouts from TestIdentityResponse and TestTLSHandshakeTimeout
X-Git-Tag: go1.15beta1~735
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=a5d1a9df812706708da1940898cf8cad8d5f9de2;p=gostls13.git

net/http: remove arbitrary timeouts from TestIdentityResponse and TestTLSHandshakeTimeout

These hard-coded timeouts make the tests flaky on slow builders (such
as solaris-amd64-oraclerel), and make test failures harder to diagnose
anyway (by replacing dumps of the stuck goroutine stacks with failure
messages that do not describe the stuck goroutines). Eliminate them
and simplify the tests.

Fixes #37327
Fixes #38112

Change-Id: Id40febe349d134ef53c702e36199bfbf2b6468ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/225977
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
---

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index 21ee7f33c8..9488821466 100644
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -1057,16 +1057,13 @@ func TestIdentityResponse(t *testing.T) {
 		t.Fatalf("error writing: %v", err)
 	}
 
-	// The ReadAll will hang for a failing test, so use a Timer to
-	// fail explicitly.
-	goTimeout(t, 2*time.Second, func() {
-		got, _ := ioutil.ReadAll(conn)
-		expectedSuffix := "\r\n\r\ntoo short"
-		if !strings.HasSuffix(string(got), expectedSuffix) {
-			t.Errorf("Expected output to end with %q; got response body %q",
-				expectedSuffix, string(got))
-		}
-	})
+	// The ReadAll will hang for a failing test.
+	got, _ := ioutil.ReadAll(conn)
+	expectedSuffix := "\r\n\r\ntoo short"
+	if !strings.HasSuffix(string(got), expectedSuffix) {
+		t.Errorf("Expected output to end with %q; got response body %q",
+			expectedSuffix, string(got))
+	}
 }
 
 func testTCPConnectionCloses(t *testing.T, req string, h Handler) {
@@ -1438,13 +1435,13 @@ func TestTLSHandshakeTimeout(t *testing.T) {
 		t.Fatalf("Dial: %v", err)
 	}
 	defer conn.Close()
-	goTimeout(t, 10*time.Second, func() {
-		var buf [1]byte
-		n, err := conn.Read(buf[:])
-		if err == nil || n != 0 {
-			t.Errorf("Read = %d, %v; want an error and no bytes", n, err)
-		}
-	})
+
+	var buf [1]byte
+	n, err := conn.Read(buf[:])
+	if err == nil || n != 0 {
+		t.Errorf("Read = %d, %v; want an error and no bytes", n, err)
+	}
+
 	select {
 	case v := <-errc:
 		if !strings.Contains(v, "timeout") && !strings.Contains(v, "TLS handshake") {
@@ -1479,30 +1476,29 @@ func TestTLSServer(t *testing.T) {
 		t.Fatalf("Dial: %v", err)
 	}
 	defer idleConn.Close()
-	goTimeout(t, 10*time.Second, func() {
-		if !strings.HasPrefix(ts.URL, "https://") {
-			t.Errorf("expected test TLS server to start with https://, got %q", ts.URL)
-			return
-		}
-		client := ts.Client()
-		res, err := client.Get(ts.URL)
-		if err != nil {
-			t.Error(err)
-			return
-		}
-		if res == nil {
-			t.Errorf("got nil Response")
-			return
-		}
-		defer res.Body.Close()
-		if res.Header.Get("X-TLS-Set") != "true" {
-			t.Errorf("expected X-TLS-Set response header")
-			return
-		}
-		if res.Header.Get("X-TLS-HandshakeComplete") != "true" {
-			t.Errorf("expected X-TLS-HandshakeComplete header")
-		}
-	})
+
+	if !strings.HasPrefix(ts.URL, "https://") {
+		t.Errorf("expected test TLS server to start with https://, got %q", ts.URL)
+		return
+	}
+	client := ts.Client()
+	res, err := client.Get(ts.URL)
+	if err != nil {
+		t.Error(err)
+		return
+	}
+	if res == nil {
+		t.Errorf("got nil Response")
+		return
+	}
+	defer res.Body.Close()
+	if res.Header.Get("X-TLS-Set") != "true" {
+		t.Errorf("expected X-TLS-Set response header")
+		return
+	}
+	if res.Header.Get("X-TLS-HandshakeComplete") != "true" {
+		t.Errorf("expected X-TLS-HandshakeComplete header")
+	}
 }
 
 func TestServeTLS(t *testing.T) {
@@ -3629,21 +3625,6 @@ func TestHeaderToWire(t *testing.T) {
 	}
 }
 
-// goTimeout runs f, failing t if f takes more than ns to complete.
-func goTimeout(t *testing.T, d time.Duration, f func()) {
-	ch := make(chan bool, 2)
-	timer := time.AfterFunc(d, func() {
-		t.Errorf("Timeout expired after %v", d)
-		ch <- true
-	})
-	defer timer.Stop()
-	go func() {
-		defer func() { ch <- true }()
-		f()
-	}()
-	<-ch
-}
-
 type errorListener struct {
 	errs []error
 }