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

Add response.parsed_body to Integration Tests #23597

Merged
merged 2 commits into from
Feb 10, 2016

Conversation

kaspth
Copy link
Contributor

@kaspth kaspth commented Feb 10, 2016

Follow up to #21671

Fixes #23594

When testing:

```ruby
post articles_path, params: { article: { title: 'Ahoy!' } }, as: :json
```

It's common to want to make assertions on the response body. Perhaps the
server responded with JSON, so you write `JSON.parse(response.body)`.
But that gets tedious real quick.

Instead add `parsed_body` which will automatically parse the reponse
body as what the last request was encoded `as`.
Forgot to add this in the original pull request. No biggie, just show
some examples.
@kaspth kaspth force-pushed the integration-response-parsing branch from d31dc69 to c85b177 Compare February 10, 2016 21:05
@kaspth kaspth added this to the 5.0.0 milestone Feb 10, 2016
@rafaelfranca
Copy link
Member

:shipit:

@kaspth
Copy link
Contributor Author

kaspth commented Feb 10, 2016

Passed on this earlier build, https://travis-ci.org/rails/rails/builds/108383081, before I force pushed some changelog changes.

kaspth added a commit that referenced this pull request Feb 10, 2016
Add `response.parsed_body` to Integration Tests
@kaspth kaspth merged commit 33257d1 into rails:master Feb 10, 2016
@kaspth kaspth deleted the integration-response-parsing branch February 10, 2016 21:39
@@ -422,17 +424,21 @@ def encode_params(params)
@param_encoder.call(params)
end

def parse_body(body)
@response_parser.call(body)
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice if this was memoized. Frequent to have a bunch of checks against response.parsed_body['key1'], response.parsed_body['key2'].

@dhh
Copy link
Member

dhh commented Feb 11, 2016

We should extend this to work for the get case as well and whenever format is set:

  get circle_people_url(format: :json)
  assert_equal "something", response.parsed_body['key'] 

Right now it doesn't as format isn't used to detect the mime.

class ApiTest < ActionDispatch::IntegrationTest
test 'creates articles' do
assert_difference -> { Article.count } do
post articles_path, { article: { title: 'Ahoy!' } }, as: :json
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be: post articles_path, params: { article: { title: 'Ahoy!' } }, as: :json

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doh! 😁

@kaspth
Copy link
Contributor Author

kaspth commented Feb 11, 2016

@dhh ah, you'd like us to replace as with just format: :json on the URL? Don't know whether or not that introduces edge cases because we'd have to parse the returned URL string a bit.

@dhh
Copy link
Member

dhh commented Feb 11, 2016

I'd like for response.parsed_body to work if the response is encoded as
JSON. Shouldn't be dependent on as: :json.

On Thu, Feb 11, 2016 at 2:00 PM, Kasper Timm Hansen <
notifications@github.com> wrote:

@dhh https://github.com/dhh ah, you'd like us to replace as with just format:
:json on the URL? Don't know whether or not that introduces edge cases
because we'd have to parse the returned URL string a bit.


Reply to this email directly or view it on GitHub
#23597 (comment).

@matthewd
Copy link
Member

Yeah, this shouldn't need to be based on the request encoding at all: the response headers tell us whether the response is JSON.

@kaspth
Copy link
Contributor Author

kaspth commented Feb 11, 2016

@matthewd ah, of course 👍. Will patch this up tonight.

@kaspth
Copy link
Contributor Author

kaspth commented Feb 11, 2016

@dhh @matthewd fixed your suggestions here f9f98b7...036a7a0

@dhh
Copy link
Member

dhh commented Feb 12, 2016

Super!

On Feb 11, 2016, at 22:51, Kasper Timm Hansen notifications@github.com wrote:

@dhh @matthewd fixed your suggestions here f9f98b7...036a7a0


Reply to this email directly or view it on GitHub.

@kaspth
Copy link
Contributor Author

kaspth commented Jul 15, 2016

@dhh wups! Just noticed this doesn't work with format: :json calls. I'll look into it.

@f3ndot
Copy link
Contributor

f3ndot commented Jun 1, 2020

I noticed this still doesn't work with RSpec unit testing controllers for v5.2.4.3:

RSpec.describe Api::MyApi, type: :controller do
  controller do
    def create
      render json: { foo: 'bar' }
    end
  end

  it do
    # trying to do all forms of informing the testing framework that we're a JSON request
    @request.headers['Accept'] = 'application/json'
    @request.headers['Content-Type'] = 'application/json'
    post :create, as: :json, format: :json
    expect(response.parsed_body).to eq( 'foo' => 'bar' ) # fails
  end
end

Inspecting the code, it looks like this is because the @request is instantiated long before post() is invoked (during ActionController::TestCase::Behavior#setup_controller_request_and_response) where content-type is nil and thus the IdentityEncoder is used, not the json parsing lambda.

As far as I can see, additional work is required to change the @response_parser in TestResponse to match whatever the latest assigned content_type is on time of controller execution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide @response.parsed_body to work together with post ..., as: :json for testing
5 participants