net/http: deflake TestRetryIdempotentRequestsOnError

The test was previously an integration test, relying on luck and many
goroutines and lots of time to hit the path to be tested.

Instead, rewrite the test to exactly hit the path to be tested, in one
try, in one goroutine.

Fixes #18205

Change-Id: I63cd513316344bfd7375dcc452c1c396dec0e49f
Reviewed-on: https://go-review.googlesource.com/35107
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/transport_test.go b/src/net/http/transport_test.go
index d5ddf6a1232a019e97d1d48f9f423d58d8af7dc0..a58b1839cc6e09e30d5f1a975a6f8394008dae98 100644 (file)
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -36,6 +36,7 @@ import (
        "strconv"
        "strings"
        "sync"
+       "sync/atomic"
        "testing"
        "time"
 )
@@ -2545,6 +2546,13 @@ type closerFunc func() error
 
 func (f closerFunc) Close() error { return f() }
 
+type writerFuncConn struct {
+       net.Conn
+       write func(p []byte) (n int, err error)
+}
+
+func (c writerFuncConn) Write(p []byte) (n int, err error) { return c.write(p) }
+
 // Issue 4677. If we try to reuse a connection that the server is in the
 // process of closing, we may end up successfully writing out our request (or a
 // portion of our request) only to find a connection error when we try to read
@@ -2557,66 +2565,78 @@ func (f closerFunc) Close() error { return f() }
 func TestRetryIdempotentRequestsOnError(t *testing.T) {
        defer afterTest(t)
 
+       var (
+               mu     sync.Mutex
+               logbuf bytes.Buffer
+       )
+       logf := func(format string, args ...interface{}) {
+               mu.Lock()
+               defer mu.Unlock()
+               fmt.Fprintf(&logbuf, format, args...)
+               logbuf.WriteByte('\n')
+       }
+
        ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+               logf("Handler")
+               w.Header().Set("X-Status", "ok")
        }))
        defer ts.Close()
 
-       tr := &Transport{}
+       var writeNumAtomic int32
+       tr := &Transport{
+               Dial: func(network, addr string) (net.Conn, error) {
+                       logf("Dial")
+                       c, err := net.Dial(network, ts.Listener.Addr().String())
+                       if err != nil {
+                               logf("Dial error: %v", err)
+                               return nil, err
+                       }
+                       return &writerFuncConn{
+                               Conn: c,
+                               write: func(p []byte) (n int, err error) {
+                                       if atomic.AddInt32(&writeNumAtomic, 1) == 2 {
+                                               logf("intentional write failure")
+                                               return 0, errors.New("second write fails")
+                                       }
+                                       logf("Write(%q)", p)
+                                       return c.Write(p)
+                               },
+                       }, nil
+               },
+       }
+       defer tr.CloseIdleConnections()
        c := &Client{Transport: tr}
 
-       const N = 2
-       retryc := make(chan struct{}, N)
        SetRoundTripRetried(func() {
-               retryc <- struct{}{}
+               logf("Retried.")
        })
        defer SetRoundTripRetried(nil)
 
-       for n := 0; n < 100; n++ {
-               // open 2 conns
-               errc := make(chan error, N)
-               for i := 0; i < N; i++ {
-                       // start goroutines, send on errc
-                       go func() {
-                               res, err := c.Get(ts.URL)
-                               if err == nil {
-                                       res.Body.Close()
-                               }
-                               errc <- err
-                       }()
-               }
-               for i := 0; i < N; i++ {
-                       if err := <-errc; err != nil {
-                               t.Fatal(err)
-                       }
-               }
-
-               ts.CloseClientConnections()
-               for i := 0; i < N; i++ {
-                       go func() {
-                               res, err := c.Get(ts.URL)
-                               if err == nil {
-                                       res.Body.Close()
-                               }
-                               errc <- err
-                       }()
+       for i := 0; i < 3; i++ {
+               res, err := c.Get("http://fake.golang/")
+               if err != nil {
+                       t.Fatalf("i=%d: Get = %v", i, err)
                }
+               res.Body.Close()
+       }
 
-               for i := 0; i < N; i++ {
-                       if err := <-errc; err != nil {
-                               t.Fatal(err)
-                       }
-               }
-               for i := 0; i < N; i++ {
-                       select {
-                       case <-retryc:
-                               // we triggered a retry, test was successful
-                               t.Logf("finished after %d runs\n", n)
-                               return
-                       default:
-                       }
-               }
+       mu.Lock()
+       got := logbuf.String()
+       mu.Unlock()
+       const want = `Dial
+Write("GET / HTTP/1.1\r\nHost: fake.golang\r\nUser-Agent: Go-http-client/1.1\r\nAccept-Encoding: gzip\r\n\r\n")
+Handler
+intentional write failure
+Retried.
+Dial
+Write("GET / HTTP/1.1\r\nHost: fake.golang\r\nUser-Agent: Go-http-client/1.1\r\nAccept-Encoding: gzip\r\n\r\n")
+Handler
+Write("GET / HTTP/1.1\r\nHost: fake.golang\r\nUser-Agent: Go-http-client/1.1\r\nAccept-Encoding: gzip\r\n\r\n")
+Handler
+`
+       if got != want {
+               t.Errorf("Log of events differs. Got:\n%s\nWant:\n%s", got, want)
        }
-       t.Fatal("did not trigger any retries")
 }
 
 // Issue 6981