Skip to content

Refactor user controller tests - #449

Merged
mackenziehicks merged 1 commit into
developfrom
443-refactor-user-controller-tests
Nov 11, 2016
Merged

mackenziehicks merged 1 commit into
developfrom
443-refactor-user-controller-tests

Conversation

@mackenziehicks

Copy link
Copy Markdown

What's in this PR?

Added helpers from api_case.ex to the tests for User Controller.

References

Fixes #443

Progress on: #413

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.01%) to 93.851% when pulling ad1594a on 443-refactor-user-controller-tests into e492da7 on develop.

assert user.last_name == "User"
assert user.website == "http://www.example.com"
assert user.biography == "Just a test user"
assert conn |> authenticate(user) |> put(path, params) |> json_response(200)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Got a bad indent here

@begedin

begedin commented Nov 8, 2016

Copy link
Copy Markdown
Contributor

Tests seem to pass here. What's the reason for "help wanted". Anything I can help with?

@sbatson5 sbatson5 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is looking good to me 👍

[user_1, user_2] = insert_pair(:user)

conn
|> request_index

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think the patter is not to indent here.

|> json_response(200)

assert json["data"]["attributes"]["email"] == user.email
assert json["data"]["attributes"]["email"] == current_user.email

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do we need to assert that the email is serialized correctly? Does the json_response returning 200 cover us?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@sbatson5 I initially thought that we did need to test this here, but we do have a way of testing this at the level of the view test. In that test we do conn = Phoenix.ConnTest.build_conn |> assign(:current_user, auth_user) in order to check that the user only see their email when authorized.

We can remove that assertion from here then @mackenziehicks. Asserting 200 is sufficient.

@mackenziehicks mackenziehicks changed the title 443 refactor user controller tests Refactor user controller tests Nov 11, 2016
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.01%) to 93.851% when pulling a2989a9 on 443-refactor-user-controller-tests into e492da7 on develop.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.01%) to 93.851% when pulling a2989a9 on 443-refactor-user-controller-tests into e492da7 on develop.

@begedin begedin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@mackenziehicks
mackenziehicks force-pushed the 443-refactor-user-controller-tests branch from a2989a9 to 72a5974 Compare November 11, 2016 21:14
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 93.7% when pulling 72a5974 on 443-refactor-user-controller-tests into 1928206 on develop.

@mackenziehicks
mackenziehicks merged commit 672d48b into develop Nov 11, 2016
@mackenziehicks
mackenziehicks deleted the 443-refactor-user-controller-tests branch November 11, 2016 21:32
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.

5 participants