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

net/http/httptest: Recorder.Result().Status "OK" instead of "200 OK" #18438

Closed
msample opened this issue Dec 27, 2016 · 5 comments
Closed

net/http/httptest: Recorder.Result().Status "OK" instead of "200 OK" #18438

msample opened this issue Dec 27, 2016 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.9

Comments

@msample
Copy link

msample commented Dec 27, 2016 •
edited

What version of Go are you using (go version)?

go version go1.7.1 darwin/amd64

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.7.1/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.7.1/libexec/pkg/tool/darwin_amd64"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/5k/zbrj93rx1cz25qykggmbjjxw0000gp/T/go-build657987326=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"

What did you do?

Tests on a HTTP gateway project failed due to differences between the (weakly) documented definition of http.Response.Status and the Status the Recorder result held.

play.golang snippet

bug line in master src

What did you expect to see?

httptest.Recorder.Result().Status == "200 OK"

What did you see instead?

httptest.Recorder.Result().Status == "OK"
@bradfitz bradfitz added this to the Go1.9 milestone Dec 27, 2016
@bradfitz bradfitz added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Dec 27, 2016
@bradfitz
Copy link
Contributor

That's unfortunate we didn't get it right for Go 1.7, but the 1.7 tree is frozen and Go 1.8 is about to be cut, so it's also frozen. Because this isn't a regression from Go 1.7, it doesn't qualify for fixing in Go 1.8 this late in the game. We can consider it during the Go 1.9 cycle, opening early February.

@bradfitz bradfitz changed the title net/http/httptest Recorder.Result().Status "OK" instead of "200 OK" net/http/httptest: Recorder.Result().Status "OK" instead of "200 OK" Dec 27, 2016
@vcabbage
Copy link
Member

I wanted to point out that it's a better idea to check resp.StatusCode than resp.Status. A server can technically set the reason to whatever it wants, including omitting it altogether. (https://tools.ietf.org/html/rfc7230#section-3.1.2)

https://play.golang.org/p/kKo0VmT1jb

@bradfitz
Copy link
Contributor

Agreed. Also, the reason-phrase doesn't even exist in HTTP/2 so we just kinda fake it.

@msample
Copy link
Author

msample commented Dec 28, 2016

Thanks @vcabbage - in this case though I'm proxying back a a status-message from a HTTP-like protocol and I want to preserve them in case some human is tracing it. I encountered this minor bug when I was generating test responses using the recorder. Agree, other than creating the initial status string, code should not care about it.

@bradfitz bradfitz self-assigned this May 23, 2017
@bradfitz bradfitz added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels May 23, 2017
@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators May 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.9
Development

No branches or pull requests
4 participants
@bradfitz @msample @gopherbot @vcabbage