From 1011e26b9cec8b5e7b0b827805b2fe079904521a Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Tue, 2 Nov 2021 11:52:36 -0700
Subject: [PATCH] net/http: deflake TestServerKeepAlivesEnabled_h{1,2}

This test assumes that two successive TCP connections will use different
source ports. This does not appear to be a universally safe assumption.

Rewrite the test to use httptrace to detect connection reuse instead.

Fixes #46707

Change-Id: Iebfbdfdeb77a1e6663a0c654dc847cc270c5d54d
Reviewed-on: https://go-review.googlesource.com/c/go/+/360854
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
---
 src/net/http/serve_test.go | 42 ++++++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index 6394da3bb7cf6..a98d6c313f86c 100644
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -23,6 +23,7 @@ import (
 	"net"
 	. "net/http"
 	"net/http/httptest"
+	"net/http/httptrace"
 	"net/http/httputil"
 	"net/http/internal"
 	"net/http/internal/testcert"
@@ -5689,22 +5690,37 @@ func testServerKeepAlivesEnabled(t *testing.T, h2 bool) {
 	}
 	// Not parallel: messes with global variable. (http2goAwayTimeout)
 	defer afterTest(t)
-	cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {
-		fmt.Fprintf(w, "%v", r.RemoteAddr)
-	}))
+	cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {}))
 	defer cst.close()
 	srv := cst.ts.Config
 	srv.SetKeepAlivesEnabled(false)
-	a := cst.getURL(cst.ts.URL)
-	if !waitCondition(2*time.Second, 10*time.Millisecond, srv.ExportAllConnsIdle) {
-		t.Fatalf("test server has active conns")
-	}
-	b := cst.getURL(cst.ts.URL)
-	if a == b {
-		t.Errorf("got same connection between first and second requests")
-	}
-	if !waitCondition(2*time.Second, 10*time.Millisecond, srv.ExportAllConnsIdle) {
-		t.Fatalf("test server has active conns")
+	for try := 0; try < 2; try++ {
+		if !waitCondition(2*time.Second, 10*time.Millisecond, srv.ExportAllConnsIdle) {
+			t.Fatalf("request %v: test server has active conns", try)
+		}
+		conns := 0
+		var info httptrace.GotConnInfo
+		ctx := httptrace.WithClientTrace(context.Background(), &httptrace.ClientTrace{
+			GotConn: func(v httptrace.GotConnInfo) {
+				conns++
+				info = v
+			},
+		})
+		req, err := NewRequestWithContext(ctx, "GET", cst.ts.URL, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		res, err := cst.c.Do(req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		res.Body.Close()
+		if conns != 1 {
+			t.Fatalf("request %v: got %v conns, want 1", try, conns)
+		}
+		if info.Reused || info.WasIdle {
+			t.Fatalf("request %v: Reused=%v (want false), WasIdle=%v (want false)", try, info.Reused, info.WasIdle)
+		}
 	}
 }