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

let(:name) will conflict with Rails internals for fixtures #2451

Closed
grekko opened this issue Feb 1, 2021 · 15 comments
Closed

let(:name) will conflict with Rails internals for fixtures #2451

grekko opened this issue Feb 1, 2021 · 15 comments

Comments

@grekko
Copy link

grekko commented Feb 1, 2021

What Ruby, Rails and RSpec versions are you using?

Ruby version: 2.7.1
Rails version: 6.1.1
RSpec version: 4.0.2

Observed behaviour

One of my specs started to fail while upgrading rails from 6.0 to 6.1.

I noticed that a let(:name)-declaration in my spec was being called from ActiveRecord::TestFixtures#run_in_transaction? which is AFAIU supposed to call RSpec::Rails::FixtureSupport::Fixtures#name instead.

Expected behaviour

I'd want to be warned if I'm redefining a reserved method via RSpec::Core::MemoizedHelpers#let similar to how initialize is a reserved keyword.

Can you provide an example app?

Start with a blank rails app (Version => 6.1):

gem install rails -v '6.1.1'
rails new rspec-rails-issue

Install rspec-rails (Version >= 4.0.2):

group :development, :test do
  gem 'rspec-rails', '~> 4.0.2'
end

Install RSpec:

rails generate rspec:install

Create spec/some_spec.rb:

require 'rails_helper'

RSpec.describe "observes a call to :name" do
  subject { true }

  let(:name) { puts caller; puts "I should never have been called" }

  it { is_expected.to be_truthy }
end

Run the spec:

rspec spec/some_spec.rb
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/memoized_helpers.rb:317:in `block (2 levels) in let'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/memoized_helpers.rb:157:in `block (3 levels) in fetch_or_store'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/memoized_helpers.rb:157:in `fetch'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/memoized_helpers.rb:157:in `block (2 levels) in fetch_or_store'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-support-3.10.2/lib/rspec/support/reentrant_mutex.rb:23:in `synchronize'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/memoized_helpers.rb:156:in `block in fetch_or_store'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/memoized_helpers.rb:155:in `fetch'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/memoized_helpers.rb:155:in `fetch_or_store'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/memoized_helpers.rb:317:in `block in let'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/activerecord-6.1.1/lib/active_record/test_fixtures.rb:102:in `run_in_transaction?'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/activerecord-6.1.1/lib/active_record/test_fixtures.rb:116:in `setup_fixtures'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/activerecord-6.1.1/lib/active_record/test_fixtures.rb:10:in `before_setup'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-rails-4.0.2/lib/rspec/rails/adapters.rb:74:in `block (2 levels) in <module:MinitestLifecycleAdapter>'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/example.rb:455:in `instance_exec'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/example.rb:455:in `instance_exec'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/hooks.rb:390:in `execute_with'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/hooks.rb:628:in `block (2 levels) in run_around_example_hooks_for'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/example.rb:350:in `call'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/hooks.rb:629:in `run_around_example_hooks_for'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/hooks.rb:486:in `run'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/example.rb:465:in `with_around_example_hooks'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/example.rb:508:in `with_around_and_singleton_context_hooks'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/example.rb:259:in `run'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/example_group.rb:644:in `block in run_examples'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/example_group.rb:640:in `map'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/example_group.rb:640:in `run_examples'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/example_group.rb:606:in `run'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/runner.rb:121:in `block (3 levels) in run_specs'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/runner.rb:121:in `map'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/runner.rb:121:in `block (2 levels) in run_specs'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/configuration.rb:2067:in `with_suite_hooks'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/runner.rb:116:in `block in run_specs'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/reporter.rb:74:in `report'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/runner.rb:115:in `run_specs'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/runner.rb:89:in `run'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/runner.rb:71:in `run'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/lib/rspec/core/runner.rb:45:in `invoke'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/rspec-core-3.10.1/exe/rspec:4:in `<top (required)>'
[…path omitted]/.asdf/installs/ruby/2.7.1/bin/rspec:23:in `load'
[…path omitted]/.asdf/installs/ruby/2.7.1/bin/rspec:23:in `<main>'
I should never have been called
.

Finished in 0.02096 seconds (files took 1.89 seconds to load)
1 example, 0 failures
@grekko grekko changed the title RSpec::Rails::FixtureSupport::Fixtures#name may cause weird behaviour [wip] RSpec::Rails::FixtureSupport::Fixtures#name may cause weird behaviour Feb 1, 2021
@JonRowe JonRowe closed this as completed Feb 1, 2021
@JonRowe
Copy link
Member

JonRowe commented Feb 1, 2021

Please use the unreleased version in rails-6-1-dev there is a fix for this.
(Closed as duplicate of #2417)

@grekko grekko changed the title [wip] RSpec::Rails::FixtureSupport::Fixtures#name may cause weird behaviour [wip] Unintended call to memoized helper name Feb 1, 2021
@grekko grekko changed the title [wip] Unintended call to memoized helper name [wip] Unexpected call to memoized helper name Feb 1, 2021
@grekko grekko changed the title [wip] Unexpected call to memoized helper name [wip] Unexpected call to let(:name) Feb 1, 2021
@grekko grekko changed the title [wip] Unexpected call to let(:name) Unexpected call to let(:name) Feb 1, 2021
@grekko
Copy link
Author

grekko commented Feb 1, 2021

Thanks @JonRowe :) Yes, I see that #2423 will fix that issue.

@grekko
Copy link
Author

grekko commented Feb 1, 2021

I wanted to verify that #2423 would fix the issue I described, so I adjusted the setup from the PR description:

What I changed

I replaced the reference to rspec 4.0.2 with rails-6-1-dev:

diff --git a/Gemfile b/Gemfile
index 082f631..8b9771b 100644
--- a/Gemfile
+++ b/Gemfile
@@ -55,6 +55,9 @@ end
 # Windows does not include zoneinfo files, so bundle the tzinfo-data gem
 gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby]
 
-group :development, :test do
-  gem 'rspec-rails', '~> 4.0.2'
+%w[rspec rspec-core rspec-expectations rspec-mocks rspec-support].each do |lib|
+  gem lib, git: "https://github.com/rspec/#{lib}.git", branch: "main"
+end
+git "https://github.com/rspec/rspec-rails", branch: "rails-6-1-dev" do
+  gem "rspec-rails"
 end

… and rerunning the spec above displays the same behaviour:

bundle exec rspec spec/some_spec.rb
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/memoized_helpers.rb:339:in `block (2 levels) in let'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/memoized_helpers.rb:179:in `block (3 levels) in fetch_or_store'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/memoized_helpers.rb:179:in `fetch'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/memoized_helpers.rb:179:in `block (2 levels) in fetch_or_store'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-support-d04c8aef72d0/lib/rspec/support/reentrant_mutex.rb:23:in `synchronize'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/memoized_helpers.rb:178:in `block in fetch_or_store'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/memoized_helpers.rb:177:in `fetch'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/memoized_helpers.rb:177:in `fetch_or_store'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/memoized_helpers.rb:339:in `block in let'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/activerecord-6.1.1/lib/active_record/test_fixtures.rb:102:in `run_in_transaction?'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/activerecord-6.1.1/lib/active_record/test_fixtures.rb:116:in `setup_fixtures'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/activerecord-6.1.1/lib/active_record/test_fixtures.rb:10:in `before_setup'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-rails-3ddfb3abe2b0/lib/rspec/rails/adapters.rb:74:in `block (2 levels) in <module:MinitestLifecycleAdapter>'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/example.rb:455:in `instance_exec'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/example.rb:455:in `instance_exec'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/hooks.rb:390:in `execute_with'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/hooks.rb:628:in `block (2 levels) in run_around_example_hooks_for'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/example.rb:350:in `call'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/hooks.rb:629:in `run_around_example_hooks_for'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/hooks.rb:486:in `run'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/example.rb:465:in `with_around_example_hooks'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/example.rb:508:in `with_around_and_singleton_context_hooks'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/example.rb:259:in `run'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/example_group.rb:644:in `block in run_examples'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/example_group.rb:640:in `map'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/example_group.rb:640:in `run_examples'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/example_group.rb:606:in `run'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/runner.rb:121:in `block (3 levels) in run_specs'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/runner.rb:121:in `map'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/runner.rb:121:in `block (2 levels) in run_specs'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/configuration.rb:2067:in `with_suite_hooks'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/runner.rb:116:in `block in run_specs'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/reporter.rb:74:in `report'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/runner.rb:115:in `run_specs'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/runner.rb:89:in `run'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/runner.rb:71:in `run'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/lib/rspec/core/runner.rb:45:in `invoke'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/rspec-core-ecb65d2d4ea7/exe/rspec:4:in `<top (required)>'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bin/rspec:23:in `load'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bin/rspec:23:in `<top (required)>'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/lib/bundler/cli/exec.rb:63:in `load'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/lib/bundler/cli/exec.rb:63:in `kernel_load'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/lib/bundler/cli/exec.rb:28:in `run'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/lib/bundler/cli.rb:476:in `exec'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/lib/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/lib/bundler/vendor/thor/lib/thor.rb:399:in `dispatch'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/lib/bundler/cli.rb:30:in `dispatch'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/lib/bundler/vendor/thor/lib/thor/base.rb:476:in `start'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/lib/bundler/cli.rb:24:in `start'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/exe/bundle:46:in `block in <top (required)>'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/lib/bundler/friendly_errors.rb:123:in `with_friendly_errors'
[…path omitted]/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/exe/bundle:34:in `<top (required)>'
[…path omitted]/.asdf/installs/ruby/2.7.1/bin/bundle:23:in `load'
[…path omitted]/.asdf/installs/ruby/2.7.1/bin/bundle:23:in `<main>'
I should never have been called
.

Finished in 0.03632 seconds (files took 1.38 seconds to load)
1 example, 0 failures

@bcgraham
Copy link

bcgraham commented Feb 1, 2021

This is an excellent representation of what I was seeing in #2417 (comment here). Like @grekko said above, it seems like the fundamental issue is the ActiveSupport method run_in_transaction? (changed between Rails 6.0 and 6.1), which is run in the example namespace, calling name; any let(:name) will override the method in RSpec::Rails::FixtureSupport:

# spec/some_spec.rb
require 'rails_helper'

RSpec.describe "observes a call to :name" do
  subject { true }

  let(:name) do
    method(__method__).owner # => RSpec::ExampleGroups::ObservesACallToName < RSpec::Core::ExampleGroup
    method(__method__).super_method.owner # => RSpec::ExampleGroups::ObservesACallToName::LetDefinitions
    method(__method__).super_method.super_method.owner # => RSpec::Rails::FixtureSupport
    correct_method = method(__method__).super_method.super_method
    puts "calling to #{correct_method.source_location.join(?:)}"
    # => calling to /yadda/yadda/gems/rspec-rails-3ddfb3abe2b0/lib/rspec/rails/fixture_support.rb:67
    correct_method.call
  end

  it do
    is_expected.to be_truthy
  end
end

@pirj
Copy link
Member

pirj commented Feb 1, 2021

What would be a good solution to this name clash?

@grekko
Copy link
Author

grekko commented Feb 1, 2021

What would be a good solution to this name clash?

I am sure that I only understand parts of what is going on, but I'll try to describe what I understand of the situation:


The change to ActiveRecord::TestFixtures#run_in_transaction? (see https://github.com/rails/rails/pull/37770/files#diff-f13ae79247e372ebb6bbcf12cf91ab51e7d3137136b92342fc71bfa6d8a3ac84L101-R101) decoupled ActiveRecord::TestFixtures from ActiveSupport::TestCase (namely the methods file_fixture_path and method_name).

With that change ActiveRecord::TestFixtures#run_in_transaction? is directly calling Minitest::Runnable#name which seems to be the official way to determine a test methods name. Which eventually lead to the adjustments in rspec-rails.


I have no good solutions or proposals at hand yet but some rough graceless monkeypatching ideas:

a) rspec-rails may monkeypatch ActiveRecord::TestFixtures#run_in_transaction? instead of providing a custom #name-Implementation.

diff --git a/lib/rspec/rails/fixture_support.rb b/lib/rspec/rails/fixture_support.rb
index 2161b115..2bad6aa6 100644
--- a/lib/rspec/rails/fixture_support.rb
+++ b/lib/rspec/rails/fixture_support.rb
@@ -52,9 +52,9 @@ module RSpec
           end
 
           if ::Rails.version.to_f >= 6.1
-            # @private return the example name for TestFixtures
-            def name
-              @example
+            def run_in_transaction?
+              use_transactional_tests &&
+                !self.class.uses_transaction?(@example)
             end
           end
         end

or one might propose to change ActiveRecord::TestFixtures to use some indirect call to a method name that is less likely to collide, e.g. _current_test_name:

diff --git a/activerecord/lib/active_record/test_fixtures.rb b/activerecord/lib/active_record/test_fixtures.rb
index b987dc0ca6..acc43e3d8e 100644
--- a/activerecord/lib/active_record/test_fixtures.rb
+++ b/activerecord/lib/active_record/test_fixtures.rb
@@ -99,7 +99,12 @@ def uses_transaction?(method)
 
     def run_in_transaction?
       use_transactional_tests &&
-        !self.class.uses_transaction?(name)
+        !self.class.uses_transaction?(_current_test_name)
+    end
+
+    def _current_test_name
+      # Minitest::Runnable#name
+      name
     end
 
     def setup_fixtures(config = ActiveRecord::Base)

… which rspec-rails could then hook into.

@JonRowe
Copy link
Member

JonRowe commented Feb 2, 2021

I'm not convinced this is directly the issue, but if it is then the monkey patching is an acceptable solution

@grekko
Copy link
Author

grekko commented Feb 2, 2021

I'm not convinced this is directly the issue […]

Do you mean that there is some problem underneath?


I tried to figure out what the purpose of #ActiveRecord::TestFixtures#run_in_transaction? and its counterpart ActiveSupport::TestFixtures#uses_transaction is.

It is public API and I can find two usages of the API in ActiveRecord tests:

This comment is probably misleading. It's being used to verify `after_commit`-behaviour.

I can also find other tests though that also verify after_commit-behaviour without relying on ActiveSupport::TestFixtures#uses_transaction, by creating nested DB transactions, e.g.:

… I wonder if the two usages of ActiveSupport::TestFixtures#uses_transaction might also be replaced with a nested DB transaction, which would at least remove the purpose of ActiveSupport::TestFixtures#uses_transaction for ActiveRecord itself.

ActiveSupport::TestFixtures#uses_transaction is used to prevent ActiveSupport::TestFixtures to start a DB transaction. I do not yet understand for sure why this is necessary for the tests though.

@bcgraham
Copy link

bcgraham commented Feb 2, 2021

FWIW, this behavior can be reproduced on Rails 6.0.3.4 and rspec-rails 4.0.2 if we replace let(:name) with let(:method_name) in @grekko's example.

@JonRowe JonRowe reopened this Feb 2, 2021
@JonRowe JonRowe changed the title Unexpected call to let(:name) let(:name) will conflict with Rails internals for fixtures Feb 2, 2021
@pirj pirj mentioned this issue Feb 3, 2021
3 tasks
@pirj
Copy link
Member

pirj commented Feb 3, 2021

Any idea where @example comes from? In the snippet, it evaluates to nil if I monkey-patch run_in_transaction? and set a breakpoint there.

@bcgraham
Copy link

bcgraham commented Feb 4, 2021

My theory is it's a very, very old relic - entered with f3de931, but irrelevant in rspec-core since d941ba7. I haven't seen what happens to tests if it gets removed/changed, though.

sankichi92 added a commit to sankichi92/LiveLog that referenced this issue Feb 6, 2021
sankichi92 added a commit to sankichi92/LiveLog that referenced this issue Feb 6, 2021
* Bump rails from 6.0.3.4 to 6.1.1

Bumps [rails](https://github.com/rails/rails) from 6.0.3.4 to 6.1.1.
- [Release notes](https://github.com/rails/rails/releases)
- [Commits](rails/rails@v6.0.3.4...v6.1.1)

Signed-off-by: dependabot-preview[bot] <[email protected]>

* bin/rails app:update

* Avoid a bug of rails 6.1

cf. rspec/rspec-rails#2451

* Make active_decorator work for Elasticsearch response

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Takahiro Miyoshi <[email protected]>
@st0012
Copy link
Contributor

st0012 commented Feb 15, 2021

I'm having issues with the run_in_transaction? method as well. I don't know how @example is set and it's always nil in my cases. So I use this small patch to avoid the issue:

module RSpec
  module Rails
    module FixtureSupport
      def name
        method_name
      end
    end
  end
end

@pirj
Copy link
Member

pirj commented Feb 15, 2021

@st0012 Can you try using rspec-rails from the 4.1 branch (rails-6-1-dev)?

gem 'rspec-rails', github: 'rspec/rspec-rails', branch: 'rails-6-1-dev'

@st0012
Copy link
Contributor

st0012 commented Feb 15, 2021

@pirj I do use the branch now. the patch I mentioned needs it to work 🙂 but just using the branch doesn't solve the issue, unfortunately.

@benoittgt
Copy link
Member

Fixed by #2461

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

No branches or pull requests

6 participants