-
Notifications
You must be signed in to change notification settings - Fork 23
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
Provide default values when Content-Length is absent #70
Conversation
Based on the HAR spec, I think we want to put -1 if we don't know the length rather than 0
|
952c42e
to
de28038
Compare
119342c
to
8cbcadf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple small suggestions, otherwise LGTM
expect(response.body).to eq "" | ||
end | ||
|
||
it "returns an empty string when the body is an array of string" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The doc string is wrong here, this isn't an empty string
response = HttpResponse.from_parts( | ||
204, | ||
{"Content-Length" => 53}, | ||
nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put an actual body here so the test proves that it's the 204 status rather than the nil
causing the 0?
The Content-Length header is not included by default starting in Rails 5. This commit adds a default -1 value for the `Har::ResponseSerializer` for values that rely on the Content-Length header when it is absent. The instructions are also updated to mention the `Rack::ContentLength` middleware users can add to their application if they'd like to see that value in their ReadMe requests.
8cbcadf
to
f8978aa
Compare
…meio#70) Bumps [@readme/eslint-config](https://github.com/readmeio/eslint-config) from 2.0.4 to 2.0.6. - [Release notes](https://github.com/readmeio/eslint-config/releases) - [Changelog](https://github.com/readmeio/eslint-config/blob/master/CHANGELOG.md) - [Commits](readmeio/standards@2.0.4...2.0.6) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
🧰 What's being changed?
The Content-Length header is not included by default starting in Rails
5. This commit adds some new functionality to the HttpResponse class
so that we can calculate the content length if the header is not present.
Commit that removes the middleware: rails/rails@5690358
🧪 Testing
Automated tests were added to cover this new behavior.