From: Brad Fitzpatrick Date: Mon, 23 Jul 2018 23:42:10 +0000 (+0000) Subject: net/http: deflake TestRetryRequestsOnError X-Git-Tag: go1.11beta3~96 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=416676f4d9bb2e14bce4e396c2ce67d091264751;p=gostls13.git net/http: deflake TestRetryRequestsOnError There's a 50ms threshold in net/http.Transport that this test sometimes hitting on slower devices. That was unrelated to what this test was trying to test. So instead just t.Skip on RoundTrip errors unless the failure was quick (under 25ms), in which case the error must've been about something else. Our fast machines should catch regressions there. Fixes #25366 Change-Id: Ibe8e2716a5b68558b57d0b8b5c46f38e46a2cba2 Reviewed-on: https://go-review.googlesource.com/125555 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- diff --git a/src/net/http/export_test.go b/src/net/http/export_test.go index 2c606a45a3..5ff85bc7c8 100644 --- a/src/net/http/export_test.go +++ b/src/net/http/export_test.go @@ -33,6 +33,8 @@ var ( Export_writeStatusLine = writeStatusLine ) +const MaxWriteWaitBeforeConnReuse = maxWriteWaitBeforeConnReuse + func init() { // We only want to pay for this cost during testing. // When not under test, these values are always nil diff --git a/src/net/http/transport.go b/src/net/http/transport.go index 4e2dd3beb5..28469f2d82 100644 --- a/src/net/http/transport.go +++ b/src/net/http/transport.go @@ -1918,6 +1918,11 @@ func (pc *persistConn) writeLoop() { } } +// maxWriteWaitBeforeConnReuse is how long the a Transport RoundTrip +// will wait to see the Request's Body.Write result after getting a +// response from the server. See comments in (*persistConn).wroteRequest. +const maxWriteWaitBeforeConnReuse = 50 * time.Millisecond + // wroteRequest is a check before recycling a connection that the previous write // (from writeLoop above) happened and was successful. func (pc *persistConn) wroteRequest() bool { @@ -1940,7 +1945,7 @@ func (pc *persistConn) wroteRequest() bool { select { case err := <-pc.writeErrCh: return err == nil - case <-time.After(50 * time.Millisecond): + case <-time.After(maxWriteWaitBeforeConnReuse): return false } } diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index d1efa73cd9..aa8beb9357 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -3050,9 +3050,16 @@ func TestRetryRequestsOnError(t *testing.T) { defer SetRoundTripRetried(nil) for i := 0; i < 3; i++ { + t0 := time.Now() res, err := c.Do(tc.req()) if err != nil { - t.Fatalf("i=%d: Do = %v", i, err) + if time.Since(t0) < MaxWriteWaitBeforeConnReuse/2 { + mu.Lock() + got := logbuf.String() + mu.Unlock() + t.Fatalf("i=%d: Do = %v; log:\n%s", i, err, got) + } + t.Skipf("connection likely wasn't recycled within %d, interfering with actual test; skipping", MaxWriteWaitBeforeConnReuse) } res.Body.Close() }