passwordless icon indicating copy to clipboard operation
passwordless copied to clipboard

NoMethodError: undefined method `session' for nil

Open yshmarov opened this issue 1 year ago β€’ 3 comments

When running passwordless_sign_in(@user), I get the error NoMethodError: undefined method session' for nil. It breaks [here](https://github.com/mikker/passwordless//blob/3b12614dd0ca377e7dc8c57ba9221a56538e5816/lib/passwordless/test_helpers.rb#L14-L15). It looks like @requestisnil`

rails 7.1.3
ruby 3.3.0
passwordless 1.4

curiously, I am having errors in different versions of passwordless

Screenshot 2024-02-10 at 17 41 36

yshmarov avatar Feb 10 '24 16:02 yshmarov

These are your system tests? (I see /integration/ in the path) Can you make sure the right test helper is being included? We want it to be these https://github.com/mikker/passwordless/blob/master/lib/passwordless/test_helpers.rb#L46-L65 however it looks like your tests are calling the controller ones (as they're the only ones referencing @request, https://github.com/mikker/passwordless/blob/master/lib/passwordless/test_helpers.rb#L15)

mikker avatar Feb 12 '24 11:02 mikker

The inegrations path has the controller, not system tests.

I am including 'passwordless/test_helpers'.

When I run a controller test within ActionDispatch::IntegrationTest:

  • Passwordless::TestHelpers::SystemTestCase is defined
  • Passwordless::TestHelpers::ControllerTestCase is not defined I think it should be vice versa?

Somewhere around here https://github.com/mikker/passwordless/blob/master/lib/passwordless/test_helpers.rb#L71

Screenshot 2024-02-12 at 17 23 28

yshmarov avatar Feb 12 '24 16:02 yshmarov

Does your project include https://github.com/rails/rails-controller-testing ? Maybe that adds something?

mikker avatar Feb 13 '24 08:02 mikker

I'm having this issue in a new Rails 7.1 project as well. Though I am not in a 7.0 project that also uses the passwords gem. I've only just started looking into it. Maybe it's an issue brought on by a newer version or Rails.

bcasci avatar Feb 28 '24 04:02 bcasci

For the record, I tried downgrading to several lower version of the passwordless gem, but that hasn't changed the behavior. The code breaks because as @mikker pointed out, this like is being executed:

https://github.com/mikker/passwordless/blob/master/lib/passwordless/test_helpers.rb#L15


require 'passwordless/test_helpers'

class AlbumsControllerTest < ActionDispatch::IntegrationTest
  test 'this is what breaks' do
     passwordless_sign_in(users(:org_one_admin))
  end
end

bcasci avatar Feb 28 '24 04:02 bcasci

Here is a bit more information about this error, specific to ActionDispatch::IntegrationTest. It doesn't seem to be an issue with controller tests, which you can still do with Rspec or the spec flavor of Minitest, but they have been phased out by the Rails team for a while. So if you're working with a fairly default test environment, you will be ActionDispatch::IntegrationTest when testing controllers.

require 'test_helper'

module Manage
  class AlbumsControllerTest < ActionDispatch::IntegrationTest

    test 'this fails' do
      # @request is nil until a request is made, or alternatively until @request is manually set.
      # This is normal behavior for ActionDispatch::IntegrationTest, a request must be made in order
      # for @request to be set to an instance of ActionDispatch::Request
      # So you will always get a an error when ControllerHelpers.passwordless_sign_in is called  first, because @request is nil
      
      get "/" 

      # At this point @request is set to #<ActionDispatch::Request GET "http://www.example.com/" for 127.0.0.1> 
      
      # The passwordless helper can be called without error now,
      # however what it does will not do you any good with ActionDispatch::IntegrationTest
      
      passwordless_sign_in(users(:org_one_admin))
      
      get new_manage_album_url(subdomain: organizations(:one).subdomain)
      
      assert_response :success # fails because it's a 303 to the sign_in page
    end
    
    test 'this passes' do
    
      def passwordless_sign_in(resource)
        # Taken from Passwordless::TestHelpers::RequestTestCase
          session = Passwordless::Session.create!(authenticatable: resource)

          magic_link = Passwordless.context.path_for(
            session,
            action: "confirm",
            id: session.to_param,
            token: session.token
          )

          get(magic_link)
          follow_redirect!
          
           get new_manage_album_url(subdomain: organizations(:one).subdomain)
      
           assert_response :success # passes 200
      end
    end

I think its possible that including the Passwordless::TestHelpers::ControllerTestCase on ActiveSupport::TestCase might not work for some recent versions of Rails. https://github.com/mikker/passwordless/blob/09ed49293bef7e993bec7c0b205a82d224075f33/lib/passwordless/test_helpers.rb#L71

bcasci avatar Feb 28 '24 07:02 bcasci

Nice digging πŸ‘ do you have an idea for a fix?

mikker avatar Feb 28 '24 10:02 mikker

I’m not sure what the best approach is. Maybe:

  • add logic to include the correct methods on Actiondispatch::IntegrationTest what that is defined
  • instead of include use meta programming or reflection to figure out which login helper to user after when a test calls the login helper (ugh)
  • Don’t clause anything, and let the developer handle that in their own test helpers
  • Something else?

It would be interesting to run this situation by Copilot for an opinion.

bcasci avatar Feb 28 '24 12:02 bcasci

The issue was resolved when I added this to my test_helper.rb

if defined?(ActionDispatch::IntegrationTest)
  ActionDispatch::IntegrationTest.send(:include, ::Passwordless::TestHelpers::RequestTestCase)
end

So I guess that's the only change the gem source needs for it to work with a newer default rails configuration. Again, anyone using Rspec probably wouldn't encounter this issue, or people using the minitest DSL with describe(MyController) instead of the class form, because I believe minitest is using ActionDispatch::TestCase behind the DSL.

bcasci avatar Feb 28 '24 14:02 bcasci

I could submit a change after I get to a stopping point on my current project....you could assign this to me if you want so I won't forget.

bcasci avatar Feb 28 '24 18:02 bcasci

Appreciate it! πŸ˜…

mikker avatar Feb 28 '24 20:02 mikker

The issue was resolved when I added this to my test_helper.rb

if defined?(ActionDispatch::IntegrationTest)
  ActionDispatch::IntegrationTest.send(:include, ::Passwordless::TestHelpers::RequestTestCase)
end

So I guess that's the only change the gem source needs for it to work with a newer default rails configuration. Again, anyone using Rspec probably wouldn't encounter this issue, or people using the minitest DSL with describe(MyController) instead of the class form, because I believe minitest is using ActionDispatch::TestCase behind the DSL.

this workaround works for me

yshmarov avatar Mar 09 '24 21:03 yshmarov