Skip to content

Commit

Permalink
net/http: deflake TestServerKeepAlivesEnabled_h{1,2}
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
neild committed Nov 2, 2021
1 parent 80a7968 commit 1011e26
Showing 1 changed file with 29 additions and 13 deletions.
42 changes: 29 additions & 13 deletions src/net/http/serve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"net"
. "net/http"
"net/http/httptest"
"net/http/httptrace"
"net/http/httputil"
"net/http/internal"
"net/http/internal/testcert"
Expand Down Expand Up @@ -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)
}
}
}

Expand Down

0 comments on commit 1011e26

Please sign in to comment.