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

Add configuration to surpress active_record checks #2266

Merged
merged 4 commits into from Mar 13, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Changelog.md
Expand Up @@ -6,6 +6,7 @@ Enhancements:
* Allow `ActiveJob` matchers `#on_queue` modifier to take symbolic queue names. (Nils Sommer, #2283)
* The scaffold generator now generates request specs in preference to controller specs.
(Luka Lüdicke, #2288)
* Add configuration option to disable ActiveRecord. (Jon Rowe, Phil Pirozhkov, Hermann Mayer, #2266)

Bug Fixes:

Expand Down
1 change: 1 addition & 0 deletions example_app_generator/generate_stuff.rb
Expand Up @@ -44,6 +44,7 @@ def setup_tasks

def final_tasks
copy_file 'spec/verify_no_active_record_spec.rb'
copy_file 'spec/verify_no_fixture_setup_spec.rb'
Copy link
Member Author

Choose a reason for hiding this comment

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

?

Copy link
Member

Choose a reason for hiding this comment

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

Added another spec to the smoke app. Should it be done differently?

Copy link
Member Author

@JonRowe JonRowe Mar 12, 2020

Choose a reason for hiding this comment

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

Sorry no, I read this as being identical several times, must be getting late 😂

Copy link
Member

Choose a reason for hiding this comment

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

I was confused as well when I've finished typing its name 😆

end

def skip_active_record?
Expand Down
@@ -0,0 +1,22 @@
# Pretend that ActiveRecord::Rails is defined and this doesn't blow up
# with `config.use_active_record = false`.
# Trick the other spec that checks that ActiveRecord is
# *not* defined by wrapping it in RSpec::Rails namespace
# so that it's reachable from RSpec::Rails::FixtureSupport.
# NOTE: this has to be defined before requiring `rails_helper`.
module RSpec
module Rails
module ActiveRecord
module TestFixtures
end
end
end
end
Copy link
Member

Choose a reason for hiding this comment

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

It's like that since the original problem was that active_record was defined, just wasn't used and DB wasn't set up.

I've double-checked that this test does blow up if config.use_active_record = true inside no_active_record app.

Copy link
Member

Choose a reason for hiding this comment

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

I tried hard, but couldn't make this example repo work to reproduce the original issue. Mostly due to bundler 1/2 problem, and then dependencies hell with Rails version which I had to update to 5.2, and in the end with missing rake-test's methods, probably due to version mismatch.


require 'rails_helper'

RSpec.describe 'Example App', :use_fixtures, type: :model do
it "does not set up fixtures" do
expect(defined?(fixtures)).not_to be
end
end
14 changes: 14 additions & 0 deletions lib/generators/rspec/install/templates/spec/rails_helper.rb
Expand Up @@ -42,6 +42,20 @@
# instead of true.
config.use_transactional_fixtures = true

# You can uncomment this line to turn off ActiveRecord support entirely.
# config.use_active_record = false

<% else -%>
# Remove this line to enable support for ActiveRecord
config.use_active_record = false

# If you enable ActiveRecord support you should unncomment these lines,
# note if you'd prefer not to run each example within a transaction, you
# should set use_transactional_fixtures to false.
#
# config.fixture_path = "#{::Rails.root}/spec/fixtures"
# config.use_transactional_fixtures = true

<% end -%>
# RSpec Rails can automatically mix in different behaviours to your tests
# based on their file location, for example enabling you to call `get` and
Expand Down
1 change: 1 addition & 0 deletions lib/rspec/rails/configuration.rb
Expand Up @@ -63,6 +63,7 @@ def self.initialize_configuration(config) # rubocop:disable Metrics/MethodLength
config.add_setting :infer_base_class_for_anonymous_controllers, default: true

# fixture support
config.add_setting :use_active_record, default: true
config.add_setting :use_transactional_fixtures, alias_with: :use_transactional_examples
config.add_setting :use_instantiated_fixtures
config.add_setting :global_fixtures
Expand Down
55 changes: 33 additions & 22 deletions lib/rspec/rails/fixture_support.rb
Expand Up @@ -10,42 +10,53 @@ module FixtureSupport
include ActiveRecord::TestFixtures

included do
self.fixture_path = RSpec.configuration.fixture_path
if ::Rails::VERSION::STRING > '5'
self.use_transactional_tests = RSpec.configuration.use_transactional_fixtures
else
self.use_transactional_fixtures = RSpec.configuration.use_transactional_fixtures
if RSpec.configuration.use_active_record?
include Fixtures

self.fixture_path = RSpec.configuration.fixture_path
if ::Rails::VERSION::STRING > '5'
self.use_transactional_tests = RSpec.configuration.use_transactional_fixtures
else
self.use_transactional_fixtures = RSpec.configuration.use_transactional_fixtures
end
self.use_instantiated_fixtures = RSpec.configuration.use_instantiated_fixtures

fixtures RSpec.configuration.global_fixtures if RSpec.configuration.global_fixtures
end
self.use_instantiated_fixtures = RSpec.configuration.use_instantiated_fixtures
end

def self.fixtures(*args)
orig_methods = private_instance_methods
super.tap do
new_methods = private_instance_methods - orig_methods
new_methods.each do |method_name|
proxy_method_warning_if_called_in_before_context_scope(method_name)
module Fixtures
extend ActiveSupport::Concern

class_methods do
def fixtures(*args)
orig_methods = private_instance_methods
super.tap do
new_methods = private_instance_methods - orig_methods
new_methods.each do |method_name|
proxy_method_warning_if_called_in_before_context_scope(method_name)
end
end
end
end

def self.proxy_method_warning_if_called_in_before_context_scope(method_name)
orig_implementation = instance_method(method_name)
define_method(method_name) do |*args, &blk|
if inspect.include?("before(:context)")
RSpec.warn_with("Calling fixture method in before :context ")
else
orig_implementation.bind(self).call(*args, &blk)
def proxy_method_warning_if_called_in_before_context_scope(method_name)
orig_implementation = instance_method(method_name)
define_method(method_name) do |*args, &blk|
if inspect.include?("before(:context)")
RSpec.warn_with("Calling fixture method in before :context ")
else
orig_implementation.bind(self).call(*args, &blk)
end
end
end
end

if ::Rails.version.to_f >= 6.1
# @private return the example name for TestFixtures
def name
@example
pirj marked this conversation as resolved.
Show resolved Hide resolved
end
end

fixtures RSpec.configuration.global_fixtures if RSpec.configuration.global_fixtures
end
end
end
Expand Down
44 changes: 31 additions & 13 deletions spec/generators/rspec/install/install_generator_spec.rb
Expand Up @@ -12,7 +12,7 @@ def content_for(file_name)
end

def have_a_fixture_path
match(/config\.fixture_path/m)
match(/^ config\.fixture_path = /m)
end

def maintain_test_schema
Expand All @@ -23,8 +23,16 @@ def require_rspec_rails
match(/^require 'rspec\/rails'$/m)
end

def have_transactional_fixtures_enabled?
match(/config\.use_transactional_fixtures/m)
def have_active_record_enabled
match(/^\ # config\.use_active_record = false/m)
end

def have_active_record_disabled
match(/^\ config\.use_active_record = false/m)
end

def have_transactional_fixtures_enabled
match(/^ config\.use_transactional_fixtures = true/m)
end

def filter_rails_from_backtrace
Expand Down Expand Up @@ -59,19 +67,24 @@ def filter_rails_from_backtrace
expect(rails_helper).to require_rspec_rails
end

specify "with transactional fixtures" do
specify "with ActiveRecord" do
run_generator
expect(rails_helper).to have_a_fixture_path
expect(rails_helper).to have_active_record_enabled
expect(rails_helper).not_to have_active_record_disabled
end

specify "with default fixture path" do
run_generator
expect(rails_helper).to have_transactional_fixtures_enabled?
pirj marked this conversation as resolved.
Show resolved Hide resolved
expect(rails_helper).to have_a_fixture_path
end

specify "excluding rails gems from the backtrace" do
specify "with transactional fixtures" do
run_generator
expect(rails_helper).to have_transactional_fixtures_enabled
end

specify "excluding rails gems from the backtrace" do
run_generator
expect(rails_helper).to filter_rails_from_backtrace
end

Expand All @@ -86,26 +99,31 @@ def filter_rails_from_backtrace
hide_const("ActiveRecord")
end

it "requires rspec/rails" do
specify "requiring rspec/rails" do
run_generator
expect(rails_helper).to require_rspec_rails
end

it "does not include config.fixture_path" do
specify "without ActiveRecord" do
run_generator
expect(rails_helper).not_to have_active_record_enabled
expect(rails_helper).to have_active_record_disabled
end

specify "without fixture path" do
run_generator
expect(rails_helper).not_to have_a_fixture_path
end

it "does not include config.use_transactional_fixtures" do
specify "without transactional fixtures" do
run_generator
expect(rails_helper).not_to have_transactional_fixtures_enabled?
expect(rails_helper).not_to have_transactional_fixtures_enabled
end

it "does not check use active record migration options" do
specify "without schema maintenance checks" do
run_generator
expect(rails_helper).not_to use_active_record_migration
expect(rails_helper).not_to maintain_test_schema
end
end

end