riverrun / phauxth

Not actively maintained - Authentication library for Phoenix, and other Plug-based, web applications
409 stars 20 forks source link

Why add session in tests? #57

Closed antalvarenga closed 6 years ago

antalvarenga commented 6 years ago

Hi @riverrun, and thanks again for the Phauxth library, it really "just works". I noted when testing user controller actions, you create a session to test the actions that need a logged-in user. Regarding this, the authors in Programming Phoenix say:

You might be tempted to place the user_id in the session for the Auth plug to pick up:

​  conn() ​  |> fetch_session() ​  |> put_session(​:user_id​, user.id) ​  |> get(​"​​/videos"​)

This approach is a little messy. We don’t want to store anything directly in the session, because we don’t want to leak implementation details.

The way they do it is they just put a user in conn.assigns and add a condition to the auth plug to first check if there is already a user in conn.assigns. The auth plug is tested in isolation.

I don't know if your familiar with this, but I would like to know the reason behind the decision to do that this way. ( I must say, however, I honestly don't know what they mean with "we don't want to leak implementation details").

riverrun commented 6 years ago

Sorry for the delay in getting back to you. I can understand their point of view - they want to narrow the focus of the tests, and there is a lot to be said for that opinion. In my case, as I'm maintaining a library that sets the current_user in the router module, I want to make sure that I'm testing the current_user being set as well :)

Having said that, when I get time to work on version 2, I might well implement the tests, in the example and installer, the way they suggested.

By the way, thanks for raising the issue.

antalvarenga commented 6 years ago

Thanks for the feedback. Yeah I guess it's a matter of better/worse test scopes. Works fine like this, though. Closing.