Skip to content

Prevent collisions on let(:name) and let(:method_name) #2467

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

Merged
merged 2 commits into from
Feb 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions BUILD_DETAIL.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,21 @@ $ bundle exec cucumber
$ bin/cucumber
```

## Snippets

RSpec Rails uses snippets, self-contained examples that are used to cover
cases and regressions that don't need a full-blown example application to
reproduce.

Snippets reuse the already installed gems, and don't attempt to install gem
versions that are not on the system already to prevent version mismatches.

Run with:

```
$ script/run_snippets.sh
```

## YARD documentation

RSpec uses [YARD](https://yardoc.org/) for API documentation on the [rspec.info site](https://rspec.info/).
Expand Down
3 changes: 3 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ Enhancements:
(Andrew W. Lee, #2372)
* Improve request spec "controller" scafold when no action is specified.
(Thomas Hareau, #2399)
* Introduce testing snippets concept (Phil Pirozhkov, Benoit Tigeot, #2423)
* Prevent collisions with `let(:name)` for Rails 6.1 and `let(:method_name)` on older
Rails. (Benoit Tigeot, #2461)

Bug Fixes:

Expand Down
21 changes: 1 addition & 20 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
source "https://rubygems.org"
version_file = File.expand_path('.rails-version', __dir__)
RAILS_VERSION = ENV['RAILS_VERSION'] || (File.exist?(version_file) && File.read(version_file).chomp) || ""

gemspec

Expand All @@ -22,28 +20,11 @@ end

gem 'capybara'

MAJOR =
case RAILS_VERSION
when /5-2-stable/
5
when /stable/, nil, false, ''
6
else
/(\d+)[\.|-]\d+/.match(RAILS_VERSION).captures.first.to_i
end

if MAJOR >= 6
# sqlite3 is an optional, unspecified, dependency and Rails 6.0 only supports `~> 1.4`
gem 'sqlite3', '~> 1.4', platforms: [:ruby]
else
# Similarly, Rails 5.0 only supports '~> 1.3.6'. Rails 5.1-5.2 support '~> 1.3', '>= 1.3.6'
gem 'sqlite3', '~> 1.3.6', platforms: [:ruby]
end

# Until 1.13.2 is released due to Rubygems usage
gem 'ffi', '~> 1.12.0'

custom_gemfile = File.expand_path('Gemfile-custom', __dir__)
eval_gemfile custom_gemfile if File.exist?(custom_gemfile)

eval_gemfile 'Gemfile-sqlite-dependencies'
eval_gemfile 'Gemfile-rails-dependencies'
20 changes: 20 additions & 0 deletions Gemfile-sqlite-dependencies
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
version_file = File.expand_path('.rails-version', __dir__)
RAILS_VERSION = ENV['RAILS_VERSION'] || (File.exist?(version_file) && File.read(version_file).chomp) || ""

MAJOR =
case RAILS_VERSION
when /5-2-stable/
5
when /master/, /stable/, nil, false, ''
6
else
/(\d+)[\.|-]\d+/.match(RAILS_VERSION).captures.first.to_i
end

if MAJOR >= 6
# sqlite3 is an optional, unspecified, dependency and Rails 6.0 only supports `~> 1.4`
gem 'sqlite3', '~> 1.4', platforms: [:ruby]
else
# Similarly, Rails 5.0 only supports '~> 1.3.6'. Rails 5.1-5.2 support '~> 1.3', '>= 1.3.6'
gem 'sqlite3', '~> 1.3.6', platforms: [:ruby]
end
14 changes: 7 additions & 7 deletions lib/rspec/rails/fixture_support.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ module FixtureSupport
include RSpec::Rails::MinitestAssertionAdapter
include ActiveRecord::TestFixtures

# @private prevent ActiveSupport::TestFixtures to start a DB transaction.
# Monkey patched to avoid collisions with 'let(:name)' in Rails 6.1 and after
# and let(:method_name) before Rails 6.1.
def run_in_transaction?
use_transactional_tests && !self.class.uses_transaction?(self)
end

included do
if RSpec.configuration.use_active_record?
include Fixtures
Expand Down Expand Up @@ -50,13 +57,6 @@ def proxy_method_warning_if_called_in_before_context_scope(method_name)
end
end
end

if ::Rails.version.to_f >= 6.1
# @private return the example name for TestFixtures
def name
@example
end
end
end
end
end
Expand Down
2 changes: 2 additions & 0 deletions script/run_build
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ fi

fold "cukes" run_cukes

fold "snippets" script/run_snippets.sh

if documentation_enforced; then
fold "doc check" check_documentation_coverage
fi
Expand Down
13 changes: 13 additions & 0 deletions script/run_snippets.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/bin/bash
set -e

(
cd snippets
# This is required to load `bundle/inline`
unset RUBYOPT
for snippet in *.rb;
do
echo Running $snippet
ruby $snippet
done
)
57 changes: 57 additions & 0 deletions snippets/avoid_fixture_name_collision.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
if __FILE__ =~ /^snippets/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note that we have no support for snippets in main, the code to run them is in rails-6-1-dev only.
But if CI passes on rails-6-1-dev, the patch should most possibly work for main, too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it easy-ish to backport that support?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be easy. Looks like cherry-picking 040fecc and accepting the removed lines from Gemfile should do it.

fail "Snippets are supposed to be run from their own directory to avoid side " \
"effects as e.g. the root `Gemfile`, or `spec/spec_helpers.rb` to be " \
"loaded by the root `.rspec`."
end

# We opt-out from using RubyGems, but `bundler/inline` requires it
require 'rubygems'

require "bundler/inline"

# We pass `false` to `gemfile` to skip the installation of gems,
# because it may install versions that would conflict with versions
# from the main `Gemfile.lock`.
gemfile(false) do
source "https://rubygems.org"

git_source(:github) { |repo| "https://github.com/#{repo}.git" }

# Those Gemfiles carefully pick the right versions depending on
# settings in the ENV, `.rails-version` and `maintenance-branch`.
Dir.chdir('..') do
eval_gemfile 'Gemfile-sqlite-dependencies'
# This Gemfile expects `maintenance-branch` file to be present
# in the current directory.
eval_gemfile 'Gemfile-rspec-dependencies'
# This Gemfile expects `.rails-version` file
eval_gemfile 'Gemfile-rails-dependencies'
end

gem "rspec-rails", path: "../"
end

# Run specs at exit
require "rspec/autorun"

require "rails"
require "active_record/railtie"
require "rspec/rails"

# This connection will do for database-independent bug reports
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")

RSpec.configure do |config|
config.use_transactional_fixtures = true
end

RSpec.describe 'Foo' do
subject { true }

# Rails 6.1 and after
let(:name) { raise "Should never raise" }
# Before Rails 6.1
let(:method_name) { raise "Should never raise" }

it { is_expected.to be_truthy }
end
61 changes: 61 additions & 0 deletions snippets/use_active_record_false.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
if __FILE__ =~ /^snippets/
fail "Snippets are supposed to be run from their own directory to avoid side " \
"effects as e.g. the root `Gemfile`, or `spec/spec_helpers.rb` to be " \
"loaded by the root `.rspec`."
end

# We opt-out from using RubyGems, but `bundler/inline` requires it
require 'rubygems'

require "bundler/inline"

# We pass `false` to `gemfile` to skip the installation of gems,
# because it may install versions that would conflict with versions
# from the main `Gemfile.lock`.
gemfile(false) do
source "https://rubygems.org"

git_source(:github) { |repo| "https://github.com/#{repo}.git" }

# Those Gemfiles carefully pick the right versions depending on
# settings in the ENV, `.rails-version` and `maintenance-branch`.
Dir.chdir('..') do
eval_gemfile 'Gemfile-sqlite-dependencies'
# This Gemfile expects `maintenance-branch` file to be present
# in the current directory.
eval_gemfile 'Gemfile-rspec-dependencies'
# This Gemfile expects `.rails-version` file
eval_gemfile 'Gemfile-rails-dependencies'
end

gem "rspec-rails", path: "../"
end

# Run specs at exit
require "rspec/autorun"

# This snippet describes the case when ActiveRecord is loaded, but
# `use_active_record` is set to `false` in RSpec configuration.

# Initialization
require "active_record/railtie"
require "rspec/rails"

# This connection will do for database-independent bug reports
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")

# RSpec configuration
RSpec.configure do |config|
config.use_active_record = false
end

# Rails project code
class Foo
end

# Rails project specs
RSpec.describe Foo do
it 'does not not break' do
Foo
end
end