Add configuration to surpress active_record checks #2266

JonRowe · 2020-01-21T08:38:21Z

Alternative to #2038 fixes #1876.

I'm currently missing a reliable test for this, the approach in #2038 is one I do not want to take, we've never configured a database and our specs have worked, so if this switch is required when no database is present there has to be a way to trigger that error without configuring a database?

Backtrace from stackoverflow:

 ActiveRecord::ConnectionNotEstablished:
   No connection pool with id primary found.
 # activerecord-5.0.1/lib/active_record/connection_adapters/abstract/connection_pool.rb:882:in `retrieve_connection'
 # activerecord-5.0.1/lib/active_record/connection_handling.rb:128:in `retrieve_connection'
 # activerecord-5.0.1/lib/active_record/connection_handling.rb:91:in `connection'
 # activerecord-5.0.1/lib/active_record/fixtures.rb:516:in `create_fixtures'
 # activerecord-5.0.1/lib/active_record/fixtures.rb:1015:in `load_fixtures'
 # activerecord-5.0.1/lib/active_record/fixtures.rb:988:in `setup_fixtures'
 # activerecord-5.0.1/lib/active_record/fixtures.rb:852:in `before_setup'
 # rspec-rails-3.6.1/lib/rspec/rails/adapters.rb:126:in `block (2 levels) in <module:MinitestLifecycleAdapter>'
 # rspec-core-3.6.0/lib/rspec/core/example.rb:447:in `instance_exec'

lib/generators/rspec/install/templates/spec/rails_helper.rb

lib/rspec/rails/fixture_support.rb

pirj · 2020-03-04T00:44:33Z

@JonRowe this seems trivial and mostly working, I can take it from here if you like.

JonRowe · 2020-03-04T17:51:58Z

@pirj Happy for you to take it from here, I might have more time to work on it later.

pirj · 2020-03-04T18:10:58Z

Sounds good 👍

pirj

It's so tempting to remove this.

lib/rspec/rails/fixture_support.rb

spec/generators/rspec/install/install_generator_spec.rb

pirj · 2020-03-06T08:01:38Z

What tests did you have in mind to cover this change, @JonRowe ?

JonRowe · 2020-03-06T21:49:41Z

It's so tempting to remove this.

I mean I guess we could, but maybe in rspec-rails 5 is better, and we deprecate it for now...

What tests did you have in mind to cover this change, @JonRowe ?

We need to disable the db and check that it still works

benoittgt · 2020-03-12T14:25:49Z

I mean I guess we could, but maybe in rspec-rails 5 is better, and we deprecate it for now...

+1

JonRowe · 2020-03-12T16:44:32Z

This passes the build now, for tests I think the best step is to add something to the no_active_record smoke app that tests that nothing happens with fixtures when this is turned off

pirj · 2020-03-12T16:51:13Z

That's what I'm on right now. Trying to reproduce the original failure from this app to mimic it properly in the test.

pirj · 2020-03-12T22:01:19Z

example_app_generator/no_active_record/spec/verify_no_fixture_setup_spec.rb

+      end
+    end
+  end
+end


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.

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.

JonRowe · 2020-03-12T22:00:57Z

example_app_generator/generate_stuff.rb

@@ -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'


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

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

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

JonRowe · 2020-03-13T08:28:24Z

Fixes #1876

JonRowe · 2020-03-13T08:29:13Z

@pirj I rewrote the commits for this and re-pushed, once green I'm going to merge if you're happy?

pirj

Looks good!

pirj · 2020-03-13T09:06:38Z

@Jack12816 @mrgordon @JoostVanAverbeke does this work for you?

mrgordon · 2020-03-13T17:08:15Z

Thanks!

Add configuration to surpress active_record checks

This new environment will be used for smoke testing only. E.g. RAILS_ENV=smoke_test TEST_URL=... bundle exec rake spec:features A new environment is required since RSpec requires a DB to be running and connected to in order to run the tests. We could delete the constant ActiveRecord e.g. Object.send(:remove_const, :ActiveRecord), but I decided to use a new env that doesn't load it instead. Alternative approaches: - We could use https://github.com/nulldb/nulldb rather than not requiring ActiveRecord - There might be a way to use rspec/rspec-rails#2266 but I don't think it helps our situations, since we do need ActiveRecord.

pirj reviewed Jan 22, 2020

View reviewed changes

lib/generators/rspec/install/templates/spec/rails_helper.rb Outdated Show resolved Hide resolved

pirj reviewed Feb 4, 2020

View reviewed changes

lib/rspec/rails/fixture_support.rb Outdated Show resolved Hide resolved

JonRowe force-pushed the configure-active-record branch from 02dfffb to 71934df Compare March 4, 2020 10:19

pirj approved these changes Mar 6, 2020

View reviewed changes

lib/rspec/rails/fixture_support.rb Show resolved Hide resolved

spec/generators/rspec/install/install_generator_spec.rb Show resolved Hide resolved

pirj marked this pull request as ready for review March 6, 2020 08:01

pirj force-pushed the configure-active-record branch from bf45618 to 5bab9ea Compare March 6, 2020 08:04

pirj self-assigned this Mar 9, 2020

pirj force-pushed the configure-active-record branch from 87a03e4 to 1a6b528 Compare March 12, 2020 21:58

pirj reviewed Mar 12, 2020

View reviewed changes

JonRowe commented Mar 12, 2020

View reviewed changes

JonRowe and others added 3 commits March 13, 2020 08:17

Add configuration to supress active_record checks

8ebdb6f

Add a smoke test to make sure flag suppresses fixture support

01adcb0

Add changelog entry for #2266

beda644

JonRowe force-pushed the configure-active-record branch from 1a6b528 to beda644 Compare March 13, 2020 08:23

JonRowe mentioned this pull request Mar 13, 2020

Optional ActiveRecord config #2038

Closed

pirj approved these changes Mar 13, 2020

View reviewed changes

Merge branch 'master' into configure-active-record

4e18dc3

JonRowe merged commit 148f7ce into master Mar 13, 2020

JonRowe deleted the configure-active-record branch March 13, 2020 10:21

JonRowe added a commit that referenced this pull request Mar 13, 2020

Merge pull request #2266 from rspec/configure-active-record

40e9fef

Add configuration to surpress active_record checks

bilbof mentioned this pull request Apr 14, 2020

Add smoke_test environment alphagov/govuk-coronavirus-business-volunteer-form#225

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add configuration to surpress active_record checks #2266

Add configuration to surpress active_record checks #2266

JonRowe commented Jan 21, 2020 •

edited by pirj

pirj commented Mar 4, 2020

JonRowe commented Mar 4, 2020

pirj commented Mar 4, 2020

pirj left a comment

pirj commented Mar 6, 2020

JonRowe commented Mar 6, 2020

benoittgt commented Mar 12, 2020

JonRowe commented Mar 12, 2020

pirj commented Mar 12, 2020

pirj Mar 12, 2020

pirj Mar 12, 2020

JonRowe Mar 12, 2020

pirj Mar 12, 2020

JonRowe Mar 12, 2020 •

edited

pirj Mar 12, 2020

JonRowe commented Mar 13, 2020

JonRowe commented Mar 13, 2020

pirj left a comment

pirj commented Mar 13, 2020

mrgordon commented Mar 13, 2020

Add configuration to surpress active_record checks #2266

Add configuration to surpress active_record checks #2266

Conversation

JonRowe commented Jan 21, 2020 • edited by pirj

pirj commented Mar 4, 2020

JonRowe commented Mar 4, 2020

pirj commented Mar 4, 2020

pirj left a comment

Choose a reason for hiding this comment

pirj commented Mar 6, 2020

JonRowe commented Mar 6, 2020

benoittgt commented Mar 12, 2020

JonRowe commented Mar 12, 2020

pirj commented Mar 12, 2020

pirj Mar 12, 2020

Choose a reason for hiding this comment

pirj Mar 12, 2020

Choose a reason for hiding this comment

JonRowe Mar 12, 2020

Choose a reason for hiding this comment

pirj Mar 12, 2020

Choose a reason for hiding this comment

JonRowe Mar 12, 2020 • edited

Choose a reason for hiding this comment

pirj Mar 12, 2020

Choose a reason for hiding this comment

JonRowe commented Mar 13, 2020

JonRowe commented Mar 13, 2020

pirj left a comment

Choose a reason for hiding this comment

pirj commented Mar 13, 2020

mrgordon commented Mar 13, 2020

JonRowe commented Jan 21, 2020 •

edited by pirj

JonRowe Mar 12, 2020 •

edited