rspec/rspec-rails

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

janko opened this issue ยท 6 comments

janko commented

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.

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

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

Yes, that's exactly it, I think these changes are all that's needed to fix the issue ๐Ÿ‘Œ๐Ÿป

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

PRs with the changes and accompanying tests would be accepted

janko commented

Just upgraded to 6.1.1, and the fix for namespaced model fixtures is working great ๐Ÿ‘Œ๐Ÿป I appreciate the fix and cutting the release โค๏ธ