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 <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/net/http/export_test.go b/src/net/http/export_test.go
index 2c606a45a3ff3f2130ba5cf67417910247c857ef..5ff85bc7c8ce9dfd021a301e5e464cdf76825137 100644 (file)
--- 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 4e2dd3beb5737f89c7fb877552501dc52ddabf3f..28469f2d82ca83dac9132e5ecf91d1f9e8fb70f8 100644 (file)
--- 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 d1efa73cd92ec4bc7b959c0d1eba7984b71d4b57..aa8beb9357c8831fc515061662c36580f3282183 100644 (file)
--- 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()
                        }