From 747e1961e95c2eb3df62e045b90b111c2ceea337 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Mon, 3 Oct 2022 16:07:48 -0700 Subject: net/http: refactor tests to run most in HTTP/1 and HTTP/2 modes Replace the ad-hoc approach to running tests in HTTP/1 and HTTP/2 modes with a 'run' function that executes a test in various modes. By default, these modes are HTTP/1 and HTTP/2, but tests can opt-in to HTTPS/1 as well. The 'run' function also takes care of post-test cleanup (running the afterTest function). The 'run' function runs tests in parallel by default. Tests which can't run in parallel (generally because they use global test hooks) pass a testNotParallel option to disable parallelism. Update clientServerTest to use t.Cleanup to clean up after itself, rather than leaving this up to tests to handle. Drop an unnecessary mutex in SetReadLoopBeforeNextReadHook. Test hooks can't be set in parallel, and we want the race detector to notify us if two simultaneous tests try to set a hook. Fixes #56032 Change-Id: I16be64913c426fc93d84abc6ad85dbd3bc191224 Reviewed-on: https://go-review.googlesource.com/c/go/+/438137 TryBot-Result: Gopher Robot Run-TryBot: Damien Neil Reviewed-by: Brad Fitzpatrick Reviewed-by: David Chase --- src/net/http/request_test.go | 60 ++++++++++++-------------------------------- 1 file changed, 16 insertions(+), 44 deletions(-) (limited to 'src/net/http/request_test.go') diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go index 27e9eb30ee..686a8699fb 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -15,7 +15,6 @@ import ( "math" "mime/multipart" . "net/http" - "net/http/httptest" "net/url" "os" "reflect" @@ -289,10 +288,11 @@ Content-Type: text/plain // the payload size and the internal leeway buffer size of 10MiB overflows, that we // correctly return an error. func TestMaxInt64ForMultipartFormMaxMemoryOverflow(t *testing.T) { - defer afterTest(t) - + run(t, testMaxInt64ForMultipartFormMaxMemoryOverflow) +} +func testMaxInt64ForMultipartFormMaxMemoryOverflow(t *testing.T, mode testMode) { payloadSize := 1 << 10 - cst := httptest.NewServer(HandlerFunc(func(rw ResponseWriter, req *Request) { + cst := newClientServerTest(t, mode, HandlerFunc(func(rw ResponseWriter, req *Request) { // The combination of: // MaxInt64 + payloadSize + (internal spare of 10MiB) // triggers the overflow. See issue https://golang.org/issue/40430/ @@ -300,8 +300,7 @@ func TestMaxInt64ForMultipartFormMaxMemoryOverflow(t *testing.T) { Error(rw, err.Error(), StatusBadRequest) return } - })) - defer cst.Close() + })).ts fBuf := new(bytes.Buffer) mw := multipart.NewWriter(fBuf) mf, err := mw.CreateFormFile("file", "myfile.txt") @@ -329,11 +328,9 @@ func TestMaxInt64ForMultipartFormMaxMemoryOverflow(t *testing.T) { } } -func TestRedirect_h1(t *testing.T) { testRedirect(t, h1Mode) } -func TestRedirect_h2(t *testing.T) { testRedirect(t, h2Mode) } -func testRedirect(t *testing.T, h2 bool) { - defer afterTest(t) - cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) { +func TestRequestRedirect(t *testing.T) { run(t, testRequestRedirect) } +func testRequestRedirect(t *testing.T, mode testMode) { + cst := newClientServerTest(t, mode, HandlerFunc(func(w ResponseWriter, r *Request) { switch r.URL.Path { case "/": w.Header().Set("Location", "/foo/") @@ -344,7 +341,6 @@ func testRedirect(t *testing.T, h2 bool) { w.WriteHeader(StatusBadRequest) } })) - defer cst.close() var end = regexp.MustCompile("/foo/$") r, err := cst.c.Get(cst.ts.URL) @@ -1035,19 +1031,10 @@ func TestRequestCloneTransferEncoding(t *testing.T) { } } -func TestNoPanicOnRoundTripWithBasicAuth_h1(t *testing.T) { - testNoPanicWithBasicAuth(t, h1Mode) -} - -func TestNoPanicOnRoundTripWithBasicAuth_h2(t *testing.T) { - testNoPanicWithBasicAuth(t, h2Mode) -} - // Issue 34878: verify we don't panic when including basic auth (Go 1.13 regression) -func testNoPanicWithBasicAuth(t *testing.T, h2 bool) { - defer afterTest(t) - cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {})) - defer cst.close() +func TestNoPanicOnRoundTripWithBasicAuth(t *testing.T) { run(t, testNoPanicWithBasicAuth) } +func testNoPanicWithBasicAuth(t *testing.T, mode testMode) { + cst := newClientServerTest(t, mode, HandlerFunc(func(w ResponseWriter, r *Request) {})) u, err := url.Parse(cst.ts.URL) if err != nil { @@ -1328,11 +1315,6 @@ Host: localhost:8080 `) } -const ( - withTLS = true - noTLS = false -) - func BenchmarkFileAndServer_1KB(b *testing.B) { benchmarkFileAndServer(b, 1<<10) } @@ -1360,16 +1342,12 @@ func benchmarkFileAndServer(b *testing.B, n int64) { b.Fatalf("Failed to copy %d bytes: %v", n, err) } - b.Run("NoTLS", func(b *testing.B) { - runFileAndServerBenchmarks(b, noTLS, f, n) - }) - - b.Run("TLS", func(b *testing.B) { - runFileAndServerBenchmarks(b, withTLS, f, n) - }) + run(b, func(b *testing.B, mode testMode) { + runFileAndServerBenchmarks(b, mode, f, n) + }, []testMode{http1Mode, https1Mode, http2Mode}) } -func runFileAndServerBenchmarks(b *testing.B, tlsOption bool, f *os.File, n int64) { +func runFileAndServerBenchmarks(b *testing.B, mode testMode, f *os.File, n int64) { handler := HandlerFunc(func(rw ResponseWriter, req *Request) { defer req.Body.Close() nc, err := io.Copy(io.Discard, req.Body) @@ -1382,14 +1360,8 @@ func runFileAndServerBenchmarks(b *testing.B, tlsOption bool, f *os.File, n int6 } }) - var cst *httptest.Server - if tlsOption == withTLS { - cst = httptest.NewTLSServer(handler) - } else { - cst = httptest.NewServer(handler) - } + cst := newClientServerTest(b, mode, handler).ts - defer cst.Close() b.ResetTimer() for i := 0; i < b.N; i++ { // Perform some setup. -- cgit v1.3