Skip to content

httptest: guarantee ResponseRecorder.Result returns a non-nil body #26453

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

Closed
wants to merge 1 commit into from

Conversation

jackxbritton
Copy link
Contributor

The doc for ResponseRecorder.Result guarantees that the body of the returned
http.Response will be non-nil, but this only holds true if the caller's body is
non-nil. With this change, if the caller's body is nil then the returned
response's body will be an empty io.ReadCloser.

Fixes #26442

@googlebot googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Jul 18, 2018
@gopherbot
Copy link
Contributor

Message from Gerrit User 5065:

Patch Set 1:

(2 comments)

Could you also add/update a test?


Please don’t reply on this GitHub thread. Visit golang.org/cl/124875.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 3: New patch set was added with same tree, parent, and commit message as Patch Set 2.


Please don’t reply on this GitHub thread. Visit golang.org/cl/124875.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 28384:

Patch Set 3:

Patch Set 1:

(2 comments)

Could you also add/update a test?

Not sure of the best way to create a test for when rec.Body == nil, take a look at the new code. I could just add some code at the end of TestRecorder.. thoughts?


Please don’t reply on this GitHub thread. Visit golang.org/cl/124875.
After addressing review feedback, remember to publish your drafts!

@gopherbot gopherbot force-pushed the master branch 7 times, most recently from d278f09 to eec9a89 Compare July 19, 2018 05:45
…l body

The doc for ResponseRecorder.Result guarantees that the body of the returned
http.Response will be non-nil, but this is only true if the caller's body is
non-nil. With this change, if the caller's body is nil then the returned
response's body will be an empty io.ReadCloser.
@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 5: New patch set was added with same tree, parent, and commit message as Patch Set 4.


Please don’t reply on this GitHub thread. Visit golang.org/cl/124875.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 5065:

Patch Set 5:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/124875.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 5065:

Patch Set 5:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/124875.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Jul 20, 2018
…l body

The doc for ResponseRecorder.Result guarantees that the body of the returned
http.Response will be non-nil, but this only holds true if the caller's body is
non-nil. With this change, if the caller's body is nil then the returned
response's body will be an empty io.ReadCloser.

Fixes #26442

Change-Id: I3b2fe4a2541caf9997dbb8978bbaf1f58cd1f471
GitHub-Last-Rev: d802967
GitHub-Pull-Request: #26453
Reviewed-on: https://go-review.googlesource.com/124875
Reviewed-by: Brad Fitzpatrick <[email protected]>
Run-TryBot: Brad Fitzpatrick <[email protected]>
@gopherbot gopherbot force-pushed the master branch 5 times, most recently from 0090c13 to 8fbbf63 Compare July 28, 2018 01:16
@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/124875 has been merged.

@gopherbot gopherbot closed this Jul 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants