Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/net/http2: flaky TestTransportReusesConn_RequestClose and TestTransportReusesConn_ConnClose #15612

Closed
bradfitz opened this issue May 9, 2016 · 4 comments
Closed

x/net/http2: flaky TestTransportReusesConn_RequestClose and TestTransportReusesConn_ConnClose #15612

bradfitz opened this issue May 9, 2016 · 4 comments
Labels
FrozenDueToAge Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Go1.8

Comments

@bradfitz
Copy link
Contributor

bradfitz commented May 9, 2016

http2 test flake seen on a trybot:

https://storage.googleapis.com/go-build-log/0b6e5e3d/freebsd-amd64-gce101_809acc80.log

--- FAIL: TestTransportReusesConn_RequestClose (0.01s)
    transport_test.go:145: first and second responses were not on different connections
--- FAIL: TestTransportReusesConn_ConnClose (0.01s)
    transport_test.go:151: first and second responses were not on different connections
FAIL
FAIL    golang.org/x/net/http2  4.436s
@bradfitz bradfitz added the Testing An issue that has been verified to require only test changes, not just a test failure. label May 9, 2016
@bradfitz bradfitz added this to the Go1.7Maybe milestone May 9, 2016
@bradfitz
Copy link
Contributor Author

This should be fixed by https://golang.org/cl/24810 at least for the Request.Close = true case. Maybe the "Connection: close" case needs attention too.

@gopherbot
Copy link

CL https://golang.org/cl/24982 mentions this issue.

gopherbot pushed a commit to golang/net that referenced this issue Jul 17, 2016
@bradfitz
http2: make Transport treat "Connection: close" the same as Request.C…
be28236 
…lose

Both "Connection: close" and Request.Close should mean the same thing:
to use the same connection.

The Request.Close case was fixed in https://golang.org/cl/24810 for
Request.Close but I forgot about "Connection: close". Unify the code.

Fixes flaky failures such as:
https://storage.googleapis.com/go-build-log/6adb97bd/freebsd-amd64-gce101_5c331c2a.log
(noticed by Ian on a https://golang.org/cl/24977 trybot run)

Updates golang/go#15612

Change-Id: I3ab5175b8bc528ee240370c906b00cc691651bb4
Reviewed-on: https://go-review.googlesource.com/24982
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@bradfitz bradfitz modified the milestones: Go1.8, Go1.7Maybe Jul 18, 2016
@bradfitz
Copy link
Contributor Author

Not going to bundle this into Go 1.7.

@bradfitz bradfitz self-assigned this Jul 18, 2016
@bradfitz
Copy link
Contributor Author

Actually since this is already fixed in x/net, I'll just close this. We'll bundle this before Go 1.8 via other things anyway.

@golang golang locked and limited conversation to collaborators Jul 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Go1.8
Development

No branches or pull requests
2 participants
@bradfitz @gopherbot