[net] http2: Move most tests from the http2 package to the http2_test package.

0 views

Skip to first unread message

Gopher Robot (Gerrit)

unread,

Mar 3, 2026, 8:16:59 PM (2 days ago) Mar 3

to Damien Neil, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go LUCI, Nicholas Husin, Nicholas Husin, golang-co...@googlegroups.com

Gopher Robot submitted the change with unreviewed changes

Unreviewed changes

2 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: http2/server_test.go
Insertions: 0, Deletions: 8.

@@ -394,14 +394,6 @@
 	return "dummy.tld"
 }
 
-/*
-func (st *serverTester) closeConn() {
-	st.scMu.Lock()
-	defer st.scMu.Unlock()
-	st.sc.conn.Close()
-}
-*/
-
 func (st *serverTester) addLogFilter(phrase string) {
 	st.logFilter = append(st.logFilter, phrase)
 }
```
```
The name of the file: http2/transport_test.go
Insertions: 1, Deletions: 0.

@@ -1454,6 +1454,7 @@
 		if err != nil {
 			t.Fatal(err)
 		}
+		fmt.Println(size)
 		return size
 	}
 	// Create a new Request for each test, rather than reusing the
@@ -4561,15 +4562,15 @@
 	}
 }
 
-func TestTransportRetriesOnStreamProtocolError(t *testing.T) {
-	synctestTest(t, testTransportRetriesOnStreamProtocolError)
+func TestTransportNoRetryOnStreamProtocolError(t *testing.T) {
+	synctestTest(t, testTransportNoRetryOnStreamProtocolError)
 }
-func testTransportRetriesOnStreamProtocolError(t testing.TB) {
-	// This test verifies that
+func testTransportNoRetryOnStreamProtocolError(t testing.TB) {
+	// This test verifies that:
+	//   - a request that fails with ErrCodeProtocol is not retried. See
+	//     go.dev/issue/77843.
 	//   - receiving a protocol error on a connection does not interfere with
-	//     other requests in flight on that connection;
-	//   - the connection is not reused for further requests; and
-	//   - the failed request is retried on a new connecection.
+	//     other requests in flight on that connection.
 	tt := newTestTransport(t)
 
 	// Start two requests. The first is a long request
@@ -4589,7 +4590,7 @@
 	tc1.writeSettings()
 	tc1.wantFrameType(FrameSettings) // settings ACK
 
-	// Request #2(a): The short request.
+	// Request #2: The short request.
 	req2, _ := http.NewRequest("GET", "https://dummy.tld/", nil)
 	rt2 := tt.roundTrip(req2)
 	tc1.wantHeaders(wantHeader{
@@ -4597,36 +4598,18 @@
 		endStream: true,
 	})
 
-	// Request #2(a) fails with ErrCodeProtocol.
+	// Request #2 fails with ErrCodeProtocol.
 	tc1.writeRSTStream(3, ErrCodeProtocol)
 	if rt1.done() {
 		t.Fatalf("After protocol error on RoundTrip #2, RoundTrip #1 is done; want still in progress")
 	}
-	if rt2.done() {
-		t.Fatalf("After protocol error on RoundTrip #2, RoundTrip #2 is done; want still in progress")
+	if !rt2.done() {
+		t.Fatalf("After protocol error on RoundTrip #2, RoundTrip #2 is in progress; want done")
 	}
-
-	// Request #2(b): The short request is retried on a new connection.
-	tc2 := tt.getConn()
-	tc2.wantFrameType(FrameSettings)
-	tc2.wantFrameType(FrameWindowUpdate)
-	tc2.wantHeaders(wantHeader{
-		streamID:  1,
-		endStream: true,
-	})
-	tc2.writeSettings()
-	tc2.wantFrameType(FrameSettings) // settings ACK
-
-	// Request #2(b) succeeds.
-	tc2.writeHeaders(HeadersFrameParam{
-		StreamID:   1,
-		EndHeaders: true,
-		EndStream:  true,
-		BlockFragment: tc1.makeHeaderBlockFragment(
-			":status", "201",
-		),
-	})
-	rt2.wantStatus(201)
+	// Request #2 should not be retried.
+	if tt.hasConn() {
+		t.Fatalf("After protocol error on RoundTrip #2, RoundTrip #2 is unexpectedly retried")
+	}
 
 	// Request #1 succeeds.
 	tc1.writeHeaders(HeadersFrameParam{
```

Change information

Commit message:

http2: Move most tests from the http2 package to the http2_test package.

This change makes it easier to move x/net/http2 into std.
Moving the http2 package into std and importing it from net/http
(rather than bundling it as net/http/h2_bundle.go) requires
removing the http2->net/http dependency. Moving tests into
the http2_test package allows them to continue importing net/http
without creating a cycle.

Change-Id: If0799a94a6d2c90f02d7f391e352e14e6a6a6964

Reviewed-on: https://go-review.googlesource.com/c/net/+/749280

Auto-Submit: Damien Neil <dn...@google.com>

Reviewed-by: Nicholas Husin <hu...@google.com>

LUCI-TryBot-Result: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>

Reviewed-by: Nicholas Husin <n...@golang.org>

Files: