Add fixture_paths configuration setting

[jguecaimburu]

ActiveRecord::TestFixtures#fixture_path was deprecated by Rails in 7.1 and will be removed from version 7.2.

#2664 fixed the deprecation message. Here a new setting is added to the config to allow multiple paths.

[JonRowe]

Thanks for this, I'd like to see the config conditionally defined, and the old option should use the new one and issue a deprecation via our mechanism (this is configurable as to errors and silencing) this would also then simplify the usage.

[jguecaimburu]

Thanks for the suggestions @JonRowe! I wasn't aware of the deprecation mechanism.
Fixed all the issues and added tests for the new behaviour.

Some tests are failing in main as I never touched this file: https://github.com/rspec/rspec-rails/blob/main/lib/rspec/rails/fixture_file_upload_support.rb
I'm working on it.

EDIT: It failed in the CI as well, not sure why it's green: https://github.com/rspec/rspec-rails/actions/runs/4763638250/jobs/8468212157?pr=2673

[jguecaimburu]

Fixed the failing spec and added new checks here: b64a9a4
Not a 100% sure which is the purpose of RailsFixtureFileWrapper but I wanted to keep existing behavior here.

I tried removing the :fixture_path accessor from it and not a single test failed. Except for some cases of the concern test file, the complete suite passed if I changed the concern to this:

module RSpec
  module Rails
    # @private
    module FixtureFileUploadSupport
      include ActionDispatch::TestProcess if defined?(ActionDispatch::TestProcess)
    end
  end
end

My understanding is that FileFixtureSupport sets file_fixture_path to the one defined in the RSpec config setting, that has a default of 'spec/fixtures/files'. The wrapper handles the case where that value could be set manually to nil or am I missing something else?

For reference, Rails sets the default file fixture path here: https://github.com/rails/rails/blob/6f6a04f8781d03261b5cbc1d8b65ebb43311f2a4/railties/lib/rails/test_help.rb#L28

[pirj]

Is it possible that respond_to?(:file_fixture_path) is false in Rails 7.1+?

[jguecaimburu]

I don't think so. This module is included in TestCase: https://github.com/rails/rails/blob/b72b7c35dd9506b08ed040594bbd4536139e6d3f/activesupport/lib/active_support/testing/file_fixtures.rb#L16

But it could be set to nil explicitly if somebody decides to do Rspec.configure { |config| config.file_fixture_path = nil } or self.file_fixture_path = nil in a test case. But in that case, I'm not sure why the wrapper should return a default value. Maybe I'm missing something here.

[JonRowe]

Does Rails not support multiple paths here? Seems odd they would allow multiple paths for fixtures but not for files?

[jguecaimburu]

It is weird indeed, but I could not find any reference to a plural method in their codebase. Here the author of the change mentions that it is something that they might wanna do in the future.

[JonRowe]

Does Rails solely configure this with file_fixture_path directly?

[jguecaimburu]

I think so: https://github.com/search?q=repo%3Arails%2Frails%20file_fixture_path&type=code

It defaults to "#{Rails.root}/test/fixtures/files" most of the time and you can overwrite that config. Rails does not use any of the configured fixture_paths to build the file_fixture_path. Althought the first of the fixture paths is "#{Rails.root}/test/fixtures" by default, I think that relation is not enforced.

[pirj]

Looks great, thanks!

[jguecaimburu]

@pirj @JonRowe made the last couple of fixes. I believe this is ready to merge.

Would you consider a PR to simplify FileFixtureUploadSupport as mentioned here #2673 (comment)?

[pirj]

simplify FileFixtureUploadSupport

It looks sleek. WDYT of doing this in a separate PR?

Changes addressed

[jguecaimburu]

simplify FileFixtureUploadSupport

It looks sleek. WDYT of doing this in a separate PR?

I believe that change is not completely related to the Rails deprecation addressed here so better to keep it separately and merge this one as it is. But I can add an extra commit here as well. I'll follow your advice on this.

[jguecaimburu]

@JonRowe @pirj please let me know if there's anything missing here

[JonRowe]

Thanks for the work on this and apologies for the delay, I've merged this manually 🎉

[JonRowe]

Released in 6.1.0

[benoittgt]

Hello

I noticed a side effect with that. I you still have

  config.fixture_path  = ["#{::Rails.root}/spec/fixtures"]

or

  config.fixture_path  = "#{::Rails.root}/spec/fixtures"

In my case I add duplicated rspec output. At the beginning

Randomized with seed 6205

Randomized with seed 6205

At the end

Finished in 3.35 seconds (files took 3.31 seconds to load)
1 example, 0 failures

Finished in 3.35 seconds (files took 3.31 seconds to load)
1 example, 0 failures

Randomized with seed 6205

Randomized with seed 6205

I had not this in 6.0.4.

I think the deprecation should mention that we should use fixture_paths with an "s" and the attribute as an array.