Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NoMethodError: undefined method `session' for nil #207

Closed
yshmarov opened this issue Feb 10, 2024 · 12 comments · Fixed by #211
Closed

NoMethodError: undefined method `session' for nil #207

yshmarov opened this issue Feb 10, 2024 · 12 comments · Fixed by #211
Assignees

Comments

@yshmarov
Copy link
Contributor

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
@mikker
Copy link
Owner

mikker commented Feb 12, 2024

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)

@yshmarov
Copy link
Contributor Author

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

@mikker
Copy link
Owner

mikker commented Feb 13, 2024

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

@bcasci
Copy link
Contributor

bcasci commented Feb 28, 2024

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
Copy link
Contributor

bcasci commented Feb 28, 2024

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
Copy link
Contributor

bcasci commented Feb 28, 2024

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 "https://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.

ActiveSupport::TestCase.send(:include, ::Passwordless::TestHelpers::ControllerTestCase)

@mikker
Copy link
Owner

mikker commented Feb 28, 2024

Nice digging 👍 do you have an idea for a fix?

@bcasci
Copy link
Contributor

bcasci commented Feb 28, 2024

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
Copy link
Contributor

bcasci commented Feb 28, 2024

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
Copy link
Contributor

bcasci commented Feb 28, 2024

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.

@mikker
Copy link
Owner

mikker commented Feb 28, 2024

Appreciate it! 😅

@yshmarov
Copy link
Contributor Author

yshmarov commented Mar 9, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants