rspec / rspec-rails

RSpec for Rails 7+
https://rspec.info
MIT License
5.19k stars 1.04k forks source link

Fixtures for namespaced models don't seem to work in rspec-rails 6.1.0 #2715

Closed janko closed 10 months ago

janko commented 1 year ago

What Ruby, Rails and RSpec versions are you using?

Ruby version: 3.2.2 Rails version: 7.1.1 RSpec version: 6.1.0

Observed behaviour

We have some namespaced Active Record models, e.g. Device and Device::ModelNumber. Fixtures for these are in directories matching the namespace, e.g. spec/fixtures/device/model_numbers.yml.

On rspec-rails 6.1.0, I'm getting undefined method '[]' for nil errors when trying to call these fixture methods, e.g. device_model_numbers(:some_name). Looking at ActiveRecord::TestFixtures#method_missing, it appears that Active Record stores fixtures as device/model_numbers, and keeps fixture_sets hash that maps device_model_numbers to device/model_numbers, which is used for converting the method name to the fixture name.

Expected behaviour

I expected rspec-rails 6.1.0 to keep handling fixtures for namespaced Active Record models like it did before.

Can you provide an example app?

I can create one if this is not enough information.

JonRowe commented 1 year ago

Can you provide an example snippet of what doesn't work that did before? (Ideally not a whole app)

benedikt commented 1 year ago

The problem seems to be related to this change in fixture_support.rb.

The fixtures method is only looking at the keys of fixture sets, but I think it also needs to look at the values. The keys seem to represent the method name, and the values represent the fixture names.

It probably works fine for not namespaced fixtures, but the values are different for namespaced ones:

{
  "accounts"=>"accounts",
  "users"=>"users",
  "oauth2_access_tokens"=>"oauth2/access_tokens",
  "oauth2_refresh_tokens"=>"oauth2/refresh_tokens",   
  "oauth2_applications"=>"oauth2/applications",
}

Calling access_fixture with the method name raises the error described above, because the fixture cache is based on the fixture name (with / in the values).

Changing the implementation to use both method_name and fixture_name fixes the issue:

def fixtures(*args)
  super.tap do
    fixture_sets.each_pair do |method_name, fixture_name|
      proxy_method_warning_if_called_in_before_context_scope(method_name, fixture_name)
    end
  end
end

def proxy_method_warning_if_called_in_before_context_scope(method_name, fixture_name)
  define_method(method_name) do |*args, **kwargs, &blk|
    if RSpec.current_scope == :before_context_hook
      RSpec.warn_with("Calling fixture method in before :context ")
    else
      access_fixture(fixture_name, *args, **kwargs, &blk)
    end
  end
end
janko commented 1 year ago

Yes, that's exactly it, I think these changes are all that's needed to fix the issue 👌🏻

dbackeus commented 10 months ago

Cutting a release for this would be nice. I wasn't able to use the git: "https://github.com/rspec/rspec-rails" approach due to a dependency to a non existing rspec-core prerelease so for now I added this monkey patch to my rails_helper.rb:

# Monkey patch to fix Rails 7.1 compatibility with rspec-rails. Fetched from:
# https://github.com/rspec/rspec-rails/pull/2664/files
#
# NOTE: Because of weird dependency configurations using rspec-rails from GitHub wasn't an option.
# TODO: Remove this when upgrading to rspec-rails > 6.1.0
module RSpec
  module Rails
    module FixtureSupport
      module Fixtures
        class_methods do
          def fixtures(*args)
            super.tap do
              fixture_sets.each_pair do |method_name, fixture_name|
                proxy_method_warning_if_called_in_before_context_scope(method_name, fixture_name)
              end
            end
          end

          def proxy_method_warning_if_called_in_before_context_scope(method_name, fixture_name)
            define_method(method_name) do |*args, **kwargs, &blk|
              if RSpec.current_scope == :before_context_hook
                RSpec.warn_with("Calling fixture method in before :context ")
              else
                access_fixture(fixture_name, *args, **kwargs, &blk)
              end
            end
          end
        end
      end
    end
  end
end
JonRowe commented 10 months ago

PRs with the changes and accompanying tests would be accepted

janko commented 10 months ago

Just upgraded to 6.1.1, and the fix for namespaced model fixtures is working great 👌🏻 I appreciate the fix and cutting the release ❤️