janko / rodauth-rails

Rails integration for Rodauth authentication framework
https://github.com/jeremyevans/rodauth
MIT License
565 stars 40 forks source link

Suggestion for adjusting the Testing section of documentation #178

Closed soulcutter closed 1 year ago

soulcutter commented 1 year ago

Problem Statement

There is a large area of the Testing section dedicated to describing how to test using ActionController::TestCase

That form of Controller test is deprecated in favor of ActionDispatch::IntegrationTest-based tests. The default Rails generator will create tests using that base class, and the official Rails documentation now discourages using the ActionController::TestCase form.

Proposal

I would propose that the ActionController::Base section of the roadauth-rails README be extracted into its own wiki page, and that the large area currently in the readme be replaced by a short example of how a rodauth user might author their test helper for ActionDispatch::IntegrationTests:

  def login(email: "user@example.com", password: "secret")
    post "/login", as: :json, params: { login: email, password: password }
  end

  def logout
    post "/logout", as: :json, headers: { "Authorization" => headers["Authorization"] }
  end

I acknowledge that there exists a Testing wiki page, though I still imagine this being a separate page given the deprecation. One of the reasons for writing this issue is to look for agreement on wiki structure before going further.

I'd be willing to author a PR to rejigger of the README testing section if this approach works for you.

janko commented 1 year ago

Thanks for bringing this to my attention. Yes, I agree with replacing the ActionController::TestCase instructions with an ActionDispatch::IntegrationTest example. Would it make sense to show HTML requests in the README, and keep showing the JSON/JWT example in the wiki page, since the former is probably more common?

I think it would make sense to move controller tests into the existing wiki page, even though they're deprecated, to have all test types in one place. Someone had asked me for controller test support couple of months ago, so those still seem to be used.

soulcutter commented 1 year ago

I contributed to the wiki page, adding a "Controller test (ActionController::TestCase)" section - how does that look to you?

https://github.com/janko/rodauth-rails/wiki/Testing#controller-tests-actioncontrollertestcase

soulcutter commented 1 year ago

I might be over-analyzing what you wrote, but upon re-reading your reply, this stuck out to me:

Someone had asked me for controller test support couple of months ago, so those still seem to be used.

Rails generates test/controllers/posts_controller_test.rb with ActionDispatch::IntegrationTest as a base class. It is confusing! One of the reasons I ended up here to suggest a clarification was that I tried setting the session in my Rails-generated controller test to follow the pattern that was documented, and when it didn't work I came to realize that it was using a different base class. (toot)

I blame Rails for this confusion 😠

janko commented 1 year ago

The wiki page change looks great, thanks!

I now looked at the PR where I added support for controller tests, and remembered it was mainly due to rspec-rails, which uses ActionController::TestCase for controller specs.

But I agree README shouldn't be be showing something that's deprecated. Showing a JSON/JWT example with ActionDispatch::IntegrationTest that you mentioned sounds good 👍