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

Comment out Gzip compression test #950

Merged
merged 1 commit into from
Aug 23, 2019
Merged

Comment out Gzip compression test #950

merged 1 commit into from
Aug 23, 2019

Conversation

thomasleese
Copy link
Contributor

@thomasleese thomasleese commented Aug 23, 2019

Since rest-client 2.1, the gzip behaviour has changed rest-client/rest-client#597 such that it relies on Net::HTTP rather than performing the decompression itself.

This test is currently failing because the body is not getting decoded correctly, and I can't find a way of getting it to pass. It's blocking us from adding new features so I thought it would be reasonable to temporarily comment it out and I'll write up a card for Platform Health to investigate putting it back.

Error
  1) Error:
JsonClientTest#test_client_can_decompress_gzip_responses:
JSON::ParserError: 767: unexpected token at ''
    /Users/thomasleese/.rbenv/versions/2.6.3/lib/ruby/2.6.0/json/common.rb:156:in `parse'
    /Users/thomasleese/.rbenv/versions/2.6.3/lib/ruby/2.6.0/json/common.rb:156:in `parse'
    /Users/thomasleese/govuk/gds-api-adapters/lib/gds_api/response.rb:74:in `parsed_content'
    /Users/thomasleese/govuk/gds-api-adapters/lib/gds_api/response.rb:70:in `to_hash'
    /Users/thomasleese/.rbenv/versions/2.6.3/lib/ruby/2.6.0/forwardable.rb:224:in `[]'
    /Users/thomasleese/govuk/gds-api-adapters/test/json_client_test.rb:489:in `test_client_can_decompress_gzip_responses'

I'm not sure if the test is actually useful in the first place because we're testing HTTP behaviour of the Gem, not anything in our code.

I've tried the following things to get it to work:

One thing I've noticed is that for some reason decode_content is always set to false on the Net::HTTPResponse object (https://ruby-doc.org/stdlib-2.6.3/libdoc/net/http/rdoc/Net/HTTPResponse.html).

Since rest-client 2.1, the gzip behaviour has changed
rest-client/rest-client#597 such that it relies
on Net::HTTP rather than performing the decompression itself.

This test is currently failing because the body is not getting decoded
correctly, and I can't find a way of getting it to pass. It's blocking
us from adding new features so I thought it would be reasonable to
temporarily comment it out and I'll write up a card for Platform Health
to investigate putting it back.

I'm not sure if the test is actually useful in the first place because
we're testing HTTP behaviour of the Gem, not anything in our code.

I've tried the following things to get it to work:

- Manually adding an `Accept-Encoding` header (although it seems like
this definitely shouldn't work:
jnunemaker/httparty#562 (comment))
- Manually adding a `Content-Type` header.
- Generating the Gziped string with `GzipWriter`.

One thing I've noticed is that for some reason `decode_content` is
always set to false on the Net::HTTPResponse object
(https://ruby-doc.org/stdlib-2.6.3/libdoc/net/http/rdoc/Net/HTTPResponse.html).
@thomasleese thomasleese merged commit a0dd7d4 into master Aug 23, 2019
@thomasleese thomasleese deleted the fix-gzip branch August 23, 2019 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants