From d8870b0bf2f2426fc8d19a9332f652da5c25418f Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Mon, 18 Mar 2024 13:51:41 -0700
Subject: [PATCH] http2: use synthetic time in TestIdleConnTimeout

Rewrite TestIdleConnTimeout to use the new synthetic time and
synchronization test facilities, rather than using real time
and sleeps.

Reduces the test time from 20 seconds to 0.
Reduces all package tests on my laptop from 32 seconds to 12.

Change-Id: I33838488168450a7acd6a462777b5a4caf7f5307
Reviewed-on: https://go-review.googlesource.com/c/net/+/572379
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
---
 http2/transport.go      |  4 +-
 http2/transport_test.go | 90 +++++++++++++++++++++++++----------------
 2 files changed, 57 insertions(+), 37 deletions(-)

diff --git a/http2/transport.go b/http2/transport.go
index ba0956e22..ce375c8c7 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -310,7 +310,7 @@ type ClientConn struct {
 	readerErr  error         // set before readerDone is closed
 
 	idleTimeout time.Duration // or 0 for never
-	idleTimer   *time.Timer
+	idleTimer   timer
 
 	mu              sync.Mutex // guards following
 	cond            *sync.Cond // hold mu; broadcast on flow/closed changes
@@ -828,7 +828,7 @@ func (t *Transport) newClientConn(c net.Conn, singleUse bool, hooks *testSyncHoo
 	}
 	if d := t.idleConnTimeout(); d != 0 {
 		cc.idleTimeout = d
-		cc.idleTimer = time.AfterFunc(d, cc.onIdleTimeout)
+		cc.idleTimer = cc.afterFunc(d, cc.onIdleTimeout)
 	}
 	if VerboseLogs {
 		t.vlogf("http2: Transport creating client conn %p to %v", cc, c.RemoteAddr())
diff --git a/http2/transport_test.go b/http2/transport_test.go
index 5226a61f7..18d4db3ed 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -97,63 +97,83 @@ func startH2cServer(t *testing.T) net.Listener {
 
 func TestIdleConnTimeout(t *testing.T) {
 	for _, test := range []struct {
+		name            string
 		idleConnTimeout time.Duration
 		wait            time.Duration
 		baseTransport   *http.Transport
-		wantConns       int32
+		wantNewConn     bool
 	}{{
+		name:            "NoExpiry",
 		idleConnTimeout: 2 * time.Second,
 		wait:            1 * time.Second,
 		baseTransport:   nil,
-		wantConns:       1,
+		wantNewConn:     false,
 	}, {
+		name:            "H2TransportTimeoutExpires",
 		idleConnTimeout: 1 * time.Second,
 		wait:            2 * time.Second,
 		baseTransport:   nil,
-		wantConns:       5,
+		wantNewConn:     true,
 	}, {
+		name:            "H1TransportTimeoutExpires",
 		idleConnTimeout: 0 * time.Second,
 		wait:            1 * time.Second,
 		baseTransport: &http.Transport{
 			IdleConnTimeout: 2 * time.Second,
 		},
-		wantConns: 1,
+		wantNewConn: false,
 	}} {
-		var gotConns int32
-
-		st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
-			io.WriteString(w, r.RemoteAddr)
-		}, optOnlyServer)
-		defer st.Close()
+		t.Run(test.name, func(t *testing.T) {
+			tt := newTestTransport(t, func(tr *Transport) {
+				tr.IdleConnTimeout = test.idleConnTimeout
+			})
+			var tc *testClientConn
+			for i := 0; i < 3; i++ {
+				req, _ := http.NewRequest("GET", "https://dummy.tld/", nil)
+				rt := tt.roundTrip(req)
+
+				// This request happens on a new conn if it's the first request
+				// (and there is no cached conn), or if the test timeout is long
+				// enough that old conns are being closed.
+				wantConn := i == 0 || test.wantNewConn
+				if has := tt.hasConn(); has != wantConn {
+					t.Fatalf("request %v: hasConn=%v, want %v", i, has, wantConn)
+				}
+				if wantConn {
+					tc = tt.getConn()
+					// Read client's SETTINGS and first WINDOW_UPDATE,
+					// send our SETTINGS.
+					tc.wantFrameType(FrameSettings)
+					tc.wantFrameType(FrameWindowUpdate)
+					tc.writeSettings()
+				}
+				if tt.hasConn() {
+					t.Fatalf("request %v: Transport has more than one conn", i)
+				}
 
-		tr := &Transport{
-			IdleConnTimeout: test.idleConnTimeout,
-			TLSClientConfig: tlsConfigInsecure,
-		}
-		defer tr.CloseIdleConnections()
+				// Respond to the client's request.
+				hf := testClientConnReadFrame[*MetaHeadersFrame](tc)
+				tc.writeHeaders(HeadersFrameParam{
+					StreamID:   hf.StreamID,
+					EndHeaders: true,
+					EndStream:  true,
+					BlockFragment: tc.makeHeaderBlockFragment(
+						":status", "200",
+					),
+				})
+				rt.wantStatus(200)
 
-		for i := 0; i < 5; i++ {
-			req, _ := http.NewRequest("GET", st.ts.URL, http.NoBody)
-			trace := &httptrace.ClientTrace{
-				GotConn: func(connInfo httptrace.GotConnInfo) {
-					if !connInfo.Reused {
-						atomic.AddInt32(&gotConns, 1)
-					}
-				},
-			}
-			req = req.WithContext(httptrace.WithClientTrace(req.Context(), trace))
+				// If this was a newly-accepted conn, read the SETTINGS ACK.
+				if wantConn {
+					tc.wantFrameType(FrameSettings) // ACK to our settings
+				}
 
-			_, err := tr.RoundTrip(req)
-			if err != nil {
-				t.Fatalf("%v", err)
+				tt.advance(test.wait)
+				if got, want := tc.netConnClosed, test.wantNewConn; got != want {
+					t.Fatalf("after waiting %v, conn closed=%v; want %v", test.wait, got, want)
+				}
 			}
-
-			<-time.After(test.wait)
-		}
-
-		if gotConns != test.wantConns {
-			t.Errorf("incorrect gotConns: %d != %d", gotConns, test.wantConns)
-		}
+		})
 	}
 }