Skip to content

Updated isolated tests to assert correct behavior. #2010

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

Merged
merged 10 commits into from
Dec 25, 2016
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ def render_with_jsonapi_renderer
render jsonapi: author
end

def render_with_jsonapi_fields
post = Post.new(params[:data][:attributes])
render jsonapi: post, fields: {posts: ['title']}
end

def parse
self.class.last_request_parameters = request.request_parameters
head :ok
Expand Down Expand Up @@ -58,21 +63,6 @@ def test_jsonapi_parser_not_registered
assert_nil parsers[Mime[:jsonapi]]
end

def test_jsonapi_renderer_not_registered
expected = {
'data' => {
'attributes' => {
'name' => 'Johnny Rico'
},
'type' => 'users'
}
}
payload = '{"data": {"attributes": {"name": "Johnny Rico"}, "type": "authors"}}'
headers = { 'CONTENT_TYPE' => 'application/vnd.api+json' }
post '/render_with_jsonapi_renderer', params: payload, headers: headers
assert expected, response.body
end

def test_jsonapi_parser
assert_parses(
{},
Expand Down Expand Up @@ -111,18 +101,20 @@ def test_jsonapi_parser_registered
end

def test_jsonapi_renderer_registered
expected = {
'data' => {
'attributes' => {
'name' => 'Johnny Rico'
},
'type' => 'users'
}
}
expected = {'name' => 'Johnny Rico'}

payload = '{"data": {"attributes": {"name": "Johnny Rico"}, "type": "authors"}}'
Copy link
Member

Choose a reason for hiding this comment

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

I guess we should include a uuid here. how about "id": ""36c9c04e-86b1-4636-a5b0-8616672d1765" will make the expectation simpler. JSON API recommends that if a client wants to create an id on post, to use a uuid.

You've been working on this PR a while now, and the main goal of it is done. Thanks! I can do some of the other things I'd like in a followup :)

headers = { 'CONTENT_TYPE' => 'application/vnd.api+json' }
post '/render_with_jsonapi_renderer', params: payload, headers: headers
assert expected, response.body
assert_equal expected, JSON.parse(response.body)["data"]["attributes"]
Copy link
Member

Choose a reason for hiding this comment

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

why not just assert_equal expected, JSON.parse(respond.body)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was because actual response has tons of other things added to it like meta, relationships which was not part of this test and would break anytime a new element is added.

end

def test_jsonapi_renderer_registered_withfields
expected = {'title' => 'This is a test' }
payload = '{"data": {"attributes": {"title": "This is a test","body": "Of an emergency alert system."}, "type": "posts"},"fields": { "posts" : ["title"] } }'
headers = { 'CONTENT_TYPE' => 'application/vnd.api+json' }
post '/render_with_jsonapi_fields?fields[posts][]=title', params: payload, headers: headers
assert_equal expected, JSON.parse(response.body)["data"]["attributes"]
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this test belongs here. (typo in name as well). This test appears to be testing usage of the jsonapi renderer, where all this file is doing is testing registering it, which is why it's isolated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point. I will remove it.

end

def test_jsonapi_parser
Expand Down