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

net/http/httptest: something about the ResponseRecorder change #14928

Closed
rogpeppe opened this issue Mar 23, 2016 · 4 comments
Closed

net/http/httptest: something about the ResponseRecorder change #14928

rogpeppe opened this issue Mar 23, 2016 · 4 comments

Comments

@rogpeppe
Copy link
Contributor

This seems like a significant addition that should be mentioned.

Associated with this is a change to net/http/httptest.ResponseRecorder
that ended up breaking our tests because for historical reasons
we were constructing the ResponseRecorder directly with the HeaderMap
field, expecting that that be reflected in result of the Header method,
but that's no longer the case.

It was an easy fix and we shouldn't have been doing that anyway, but
a reminder that it's easy to break things even when keeping the
exposed API ostensibly the same.

@bradfitz
Copy link
Contributor

Go has supported Trailers in net/http for ages. Which particular change are you talking about?

@cmarcelo
Copy link
Contributor

I'm guessing the change is c69e686 but that isn't in Go 1.6.

Before that change it was possible for the handler to access the values in the (previously set) HeaderMap. Should we re-enable this use case? @bradfitz

@rogpeppe
Copy link
Contributor Author

I'm sorry - I hadn't noticed the Trailer stuff before and a brief git blame
made me think it was new. c69e686 is indeed the change I'm talking
about, and yes, it's not in Go 1.6. It did break our tests, but it's
arguable whether that's a problem (FWIW we were creating the
ResponseRecorder directly to remain backwardly compatible with an old
API that used it, even though we changed all our tests to use httptest.NewServer
instead for better correspondence of tests to real world behaviour).

@bradfitz bradfitz modified the milestones: Go1.7, Go1.6.1 Mar 26, 2016
@bradfitz bradfitz changed the title doc: Go 1.6 release notes don't mention Trailer support in net/http net/http/httptest: something about the ResponseRecorder change Mar 26, 2016
@bradfitz
Copy link
Contributor

How were you creating the ResponseRecorder? With a HeaderMap already made? Can you post a minimal test that passed before and fails now?

@golang golang locked and limited conversation to collaborators Apr 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants