Add login_as(user) testing helper when generating authentication by gobijan · Pull Request #53708 · rails/rails

gobijan · 2024-11-22T13:05:10Z

Motivation / Background

This Pull Request has been created because the generated authentication logic does not offer a way to test when a user is authenticated. There is a discussion going in here I initialized: #53207
So this PR adds a SessionTestHelper module with sign_in_as(user) and sign_out test helpers when
running rails g authentication. This simplifies authentication in integration tests.

Detail

This Pull Request changes the generator for authentication to add described test helpers.

Additional information

There were a few discussions how to do it best and for now I settle with the approach from the PR.

Checklist

Before submitting the PR make sure the following are checked:

This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
Tests are added or updated if you fix a bug or add a feature.
CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

stevepolitodesign · 2024-12-04T21:52:27Z

Would there be any value in adding a complimentary logout helper too?

stevepolitodesign · 2024-12-04T21:58:12Z

+    def login_as(user)
+      session = user.sessions.create!
+      Current.session = session
+      request = ActionDispatch::Request.new(Rails.application.env_config)
+      cookies = request.cookie_jar
+      cookies.signed[:session_id] = { value: session.id, httponly: true, same_site: :lax }
+    end


I had success with logout locally.

Suggested change

def login_as(user)

session = user.sessions.create!

Current.session = session

request = ActionDispatch::Request.new(Rails.application.env_config)

cookies = request.cookie_jar

cookies.signed[:session_id] = { value: session.id, httponly: true, same_site: :lax }

end

def login_as(user)

session = user.sessions.create!

Current.session = session

request = ActionDispatch::Request.new(Rails.application.env_config)

cookies = request.cookie_jar

cookies.signed[:session_id] = { value: session.id, httponly: true, same_site: :lax }

end

def logout

Current.session.destroy

request = ActionDispatch::Request.new(Rails.application.env_config)

cookies = request.cookie_jar

cookies.delete(:session_id)

end

dhh · 2024-12-09T21:46:31Z

Yes, this is good. We have a helper like this in our projects as well, which is very similar. But let's name these "sign_in_as" and "sign_out". And then let's instantiate the cookie jar directly. Here's our method:

def sign_in_as(user)
  Current.session = user.sessions.create!

  ActionDispatch::TestRequest.create.cookie_jar.tap do |cookie_jar|
    cookie_jar.signed[:session_id] = Current.session.id
    cookies[:session_id] = cookie_jar[:session_id]
  end
end

dhh · 2024-12-09T21:50:01Z

Also, I would actually recommend that we add this in a dedicated file called SessionTestHelper and then include that via:

class ActionDispatch::IntegrationTest
  include SessionTestHelper
end

Listed below the ActiveSupport::TestCase recommendation.

MatheusRich · 2025-01-17T15:23:43Z

@gobijan do you still plan to work on this? I can take over this task and co-author you

gobijan · 2025-01-17T15:55:25Z

I had serious problems getting the test suite to run tbh. Locally didn't work. The devcontainer environment was bugged and I had to rely on Github PR which was kind of unsatisfactory.
Would you mind pointing me in the direction why my tests are failing? I'd love to collaborate on this and need help with the rails test suite.

MatheusRich · 2025-01-17T16:25:23Z

@gobijan It should be pretty straightforward:

cd railties
bundle
bin/test

If that doesn't work, please ask for help in the Discord server.

gobijan · 2025-01-18T11:33:14Z

I just refactored the PR according to @dhh

The change:

Extract test helpers from test_helper.rb into SessionTestHelper module
Rename login_as to sign_in_as for consistent terminology
Add sign_out helper method for testing user logout flows
Update authentication generator and tests accordingly

Another note:
I used require_relative "helpers/session_test_helper". I was wondering if it would be better to add config.autoload_paths += %w[ test/helpers ] to test.rb
Any opinion on that?

flavorjones

I made some comments that should get you to green in CI

jeromedalbert · 2025-01-18T21:29:47Z

Another note:
I used require_relative "helpers/session_test_helper". I was wondering if it would be better to add config.autoload_paths += %w[ test/helpers ] to test.rb
Any opinion on that?

I think this would be nice as it would cut down on boilerplate requires whenever a test needs to sign in or sign out. In this PR I also used autoload_paths, which I believe was inspired by code from 37signals' Writebook. Feel free to take anything from that PR if needed.

flavorjones · 2025-01-19T03:25:16Z

I used require_relative "helpers/session_test_helper". I was wondering if it would be better to add config.autoload_paths += %w[ test/helpers ] to test.rb
Any opinion on that?

I think this would be nice as it would cut down on boilerplate requires

I agree it would be nice, but I also don't think it needs to be part of this PR.

The failure in CI is unrelated, I'm tagging this for review by a core team member.

zzak · 2025-01-19T08:39:00Z

Please add a changelog and squash your commits.

MatheusRich · 2025-01-19T15:24:41Z

+
+    ActionDispatch::TestRequest.create.cookie_jar.tap do |cookie_jar|
+      cookie_jar.signed[:session_id] = Current.session.id
+      cookies[:session_id] = cookie_jar[:session_id]


Is this supposed to be added o system tests or request tests? I don't remember if system tests can access cookies

This is for IntegrationTests (ActionDispatch::IntegrationTest).

Should we have one for system tests too? I know @dhh is not a big fan of them, but for smoke tests, a set of system tests are useful. Should we let that for each app to implement? Maybe a initial implementation on the docs could be an alternative, if we don't want to ship it in the framework.

Well good question. I think system tests are probably way simpler. These helpers here were not trivial because of the cookie jar problem mentioned in the referenced discussion.

For system tests it's pretty straightforward I guess. Something like:

visit new_session_url fill_in "Email", with: "test@example.com" fill_in "Password", with: "password123" click_on "Sign in"

Where should logic for this go? Directly in the ApplicationSystemTestCase class?
Anyways I needed the IntegrationTest functionality as I was repeating it in all my projects. I let you guys decide how to move forward.

I wonder if the system specs should actually login, or set cookies more "manually" too.

I'll defer to @dhh to say if this is necessary or not.

For system tests, I needed to add in the following to get that solution to work:

page.instance_variable_set(:@touched, false) page.reset!

However, this is pretty hacky, and I'm pretty sure there's a better solution for system tests

I had something created for system specs before this PR:

def sign_in(user = users(:john)) session = user.sessions.create! Current.session = session request = ActionDispatch::Request.new(Rails.application.env_config) cookies = request.cookie_jar cookies.signed[:session_id] = { value: session.id, httponly: true, same_site: :lax } end

Hope that helps

Just an update, the solution in #53207 (comment) works better for me than the page resetting! I think there was a significant amount of flake introduced by using stock capybara fill_in to initiate the session

Hah, @MatheusRich, your comment didn't seem to load for me in time, but I think we came to the same conclusion 😄

If it is for ActionDispatch::IntegrationTest why is is being added to ActiveSupport::TestCase

…t` test helpers when running `rails g authentication`. Simplifies authentication in integration tests.

dhh · 2025-02-27T14:17:35Z

Nice work, all! This is a good upgrade 🎉

…ls#53708) * Add a `SessionTestHelper` module with `sign_in_as(user)` and `sign_out` test helpers when running `rails g authentication`. Simplifies authentication in integration tests. * Style --------- Co-authored-by: David Heinemeier Hansson <david@hey.com>

dwightwatson · 2025-03-04T10:43:00Z

Should the template go into the test/test_helpers directory, not the test/helpers directory (which is for testing application helpers).

test_helpers is referenced here: https://guides.rubyonrails.org/testing.html#using-separate-files

dhh · 2025-03-04T10:58:52Z

Wups, good catch @dwightwatson. We do need to use test/test_helpers. If you or anyone wants to fix that, cc me on the PR ✌️

gobijan · 2025-03-04T23:15:37Z

Wups, good catch @dwightwatson. We do need to use test/test_helpers. If you or anyone wants to fix that, cc me on the PR ✌️

Of course I can step in and fix it. Will look into it tomorrow :)

rails-bot Bot added the railties label Nov 22, 2024

stevepolitodesign reviewed Dec 4, 2024

View reviewed changes

rafaelfranca requested a review from dhh December 9, 2024 20:43

stevepolitodesign mentioned this pull request Dec 10, 2024

[RF-DOCS] Document Rails 8 Authentication generator feature [ci-skip] #53802

Merged

darronschall mentioned this pull request Dec 13, 2024

Generate session controller tests for auth generator #53726

Merged

4 tasks done

gobijan force-pushed the auth_testing branch from 58d389a to 446f79b Compare January 18, 2025 11:32

flavorjones requested changes Jan 18, 2025

View reviewed changes

Comment thread railties/test/generators/authentication_generator_test.rb Outdated

Comment thread railties/test/generators/authentication_generator_test.rb

flavorjones approved these changes Jan 19, 2025

View reviewed changes

flavorjones added the ready PRs ready to merge label Jan 19, 2025

MatheusRich reviewed Jan 19, 2025

View reviewed changes

Add a SessionTestHelper module with sign_in_as(user) and `sign_ou…

4a82ba4

…t` test helpers when running `rails g authentication`. Simplifies authentication in integration tests.

gobijan force-pushed the auth_testing branch from 1f3cead to 4a82ba4 Compare January 20, 2025 08:54

Merge branch 'main' into auth_testing

783fe82

miguno mentioned this pull request Jan 28, 2025

[Docs] Getting Started guide: add instructions to update tests after authentication is added #54382

Closed

dhh added 2 commits February 27, 2025 15:15

Style

56d4e1a

Merge branch 'main' into auth_testing

91679e7

dhh merged commit 8009769 into rails:main Feb 27, 2025

dwightwatson mentioned this pull request Mar 4, 2025

Move authentication test helpers into test_helpers #54688

Merged

4 tasks done

partydrone mentioned this pull request Apr 19, 2025

Controller testing new authentication generator. #53207

Closed

excid3 mentioned this pull request May 27, 2025

Add System test support in session test helper #55124

Closed

2 of 4 tasks

tsvallender mentioned this pull request Jul 3, 2025

rename schema method tsvallender/rails#3

Closed

4 tasks

Conversation

gobijan commented Nov 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation / Background

Detail

Additional information

Checklist

Uh oh!

stevepolitodesign commented Dec 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhh commented Dec 9, 2024

Uh oh!

dhh commented Dec 9, 2024

Uh oh!

MatheusRich commented Jan 17, 2025

Uh oh!

gobijan commented Jan 17, 2025

Uh oh!

MatheusRich commented Jan 17, 2025

Uh oh!

gobijan commented Jan 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flavorjones left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jeromedalbert commented Jan 18, 2025

Uh oh!

flavorjones commented Jan 19, 2025

Uh oh!

zzak commented Jan 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhh commented Feb 27, 2025

Uh oh!

dwightwatson commented Mar 4, 2025

Uh oh!

dhh commented Mar 4, 2025

Uh oh!

gobijan commented Mar 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

gobijan commented Nov 22, 2024 •

edited

Loading

stevepolitodesign commented Dec 4, 2024 •

edited

Loading

gobijan commented Jan 18, 2025 •

edited

Loading