git » go-net » commit e90d6d0

http2: fix flaky TestTransportResPattern_* tests

author Brad Fitzpatrick
2016-07-15 16:59:30 UTC
committer Brad Fitzpatrick
2016-07-15 18:41:38 UTC
parent a728288923b47049b2ce791836767ffbe964a5bd

http2: fix flaky TestTransportResPattern_* tests

The test's server was replying with the stream closure before reading
the test client's request body. This should in theory be handled
better (and golang/go#16029 will be kept open to track it), but that's not
what this test was trying to test anyway.

Instead, make the test do things in the expected order so it can
continue to test its original subject reliably. (It was trying to test
all the orders of optional & optionally fragmented headers in responses.)

Updates golang/go#16029 (flaky http2 TestTransportResPattern_*)
Updates golang/go#11811 (subrepos need to be green)

Change-Id: I631730fce5dad598120bb2d1ea8895e39255d711
Reviewed-on: https://go-review.googlesource.com/24970
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

http2/transport_test.go +38 -33

diff --git a/http2/transport_test.go b/http2/transport_test.go
index 6009cef..5887714 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -1017,41 +1017,38 @@ func testTransportResPattern(t *testing.T, expect100Continue, resHeader headerTy
 			if err != nil {
 				return err
 			}
+			endStream := false
+			send := func(mode headerType) {
+				hbf := buf.Bytes()
+				switch mode {
+				case oneHeader:
+					ct.fr.WriteHeaders(HeadersFrameParam{
+						StreamID:      f.Header().StreamID,
+						EndHeaders:    true,
+						EndStream:     endStream,
+						BlockFragment: hbf,
+					})
+				case splitHeader:
+					if len(hbf) < 2 {
+						panic("too small")
+					}
+					ct.fr.WriteHeaders(HeadersFrameParam{
+						StreamID:      f.Header().StreamID,
+						EndHeaders:    false,
+						EndStream:     endStream,
+						BlockFragment: hbf[:1],
+					})
+					ct.fr.WriteContinuation(f.Header().StreamID, true, hbf[1:])
+				default:
+					panic("bogus mode")
+				}
+			}
 			switch f := f.(type) {
 			case *WindowUpdateFrame, *SettingsFrame:
 			case *DataFrame:
-				// ignore for now.
-			case *HeadersFrame:
-				endStream := false
-				send := func(mode headerType) {
-					hbf := buf.Bytes()
-					switch mode {
-					case oneHeader:
-						ct.fr.WriteHeaders(HeadersFrameParam{
-							StreamID:      f.StreamID,
-							EndHeaders:    true,
-							EndStream:     endStream,
-							BlockFragment: hbf,
-						})
-					case splitHeader:
-						if len(hbf) < 2 {
-							panic("too small")
-						}
-						ct.fr.WriteHeaders(HeadersFrameParam{
-							StreamID:      f.StreamID,
-							EndHeaders:    false,
-							EndStream:     endStream,
-							BlockFragment: hbf[:1],
-						})
-						ct.fr.WriteContinuation(f.StreamID, true, hbf[1:])
-					default:
-						panic("bogus mode")
-					}
-				}
-				if expect100Continue != noHeader {
-					buf.Reset()
-					enc.WriteField(hpack.HeaderField{Name: ":status", Value: "100"})
-					send(expect100Continue)
+				if !f.StreamEnded() {
+					// No need to send flow control tokens. The test request body is tiny.
+					continue
 				}
 				// Response headers (1+ frames; 1 or 2 in this test, but never 0)
 				{
@@ -1075,7 +1072,15 @@ func testTransportResPattern(t *testing.T, expect100Continue, resHeader headerTy
 					enc.WriteField(hpack.HeaderField{Name: "some-trailer", Value: "some-value"})
 					send(trailers)
 				}
-				return nil
+				if endStream {
+					return nil
+				}
+			case *HeadersFrame:
+				if expect100Continue != noHeader {
+					buf.Reset()
+					enc.WriteField(hpack.HeaderField{Name: ":status", Value: "100"})
+					send(expect100Continue)
+				}
 			}
 		}
 	}