Add a global switch for ActiveRecord

[Jack12816]

Hey guys, first thank you for this great project! :)

I encountered lately an issue like this, where the ActiveRecord constant was loaded,
but on purpose not used on an app. The rails_helper.rb was configured
not to use transactions, etc.

Unfortunately, the FixtureSupport module was configured for ActiveRecord
and errored like this:

   ActiveRecord::ConnectionNotEstablished:
     No connection pool with 'primary' found.     

I figured out that ActiveStorage had a dependency on ActiveRecord, which I do not use.
The app just makes use of the ActiveStorage service, without any database records.
So I had to unload the ActiveRecord constant on my rails_helper.rb like this:

# This file is copied to spec/ when you run 'rails generate rspec:install'
require 'spec_helper'
ENV['RAILS_ENV'] ||= 'test'
require File.expand_path('../../config/environment', __FILE__)

# Remove the ActiveRecord constant, because it is autloaded by
# ActiveStorage and not needed for our application. The presence
# of the ActiveRecord constant causes rspec-rails to include
# extra fixture support (see: https://goo.gl/6Ed17Y), which
# results in:
#
#   ActiveRecord::ConnectionNotEstablished:
#     No connection pool with 'primary' found.
#
Object.send(:remove_const, :ActiveRecord)   

It would be better to have a configuration switch on rspec-rails to turn off any
handling of ActiveRecord. The guessing based on the availability of the constant
results in false-positive cases like mine.

Something like that would be helpful (guessing by default, but configurable):

RSpec.configure do |config| 
  # Disable all ActiveRecord handling
  config.use_active_record = false
end

I'm also willing to send in a PR, if this is proposal is accepted. :)

[JonRowe]

Hmm this feels like a bug in active storage, if it's not using Active Record it shouldn't load ActiveRecord, can you not only load the parts you need? I'm not overall against the idea of a flag for it but we'd need to keep the auto detection as well and that feels like extra maintenance burden.

[Jack12816]

Yeah, it's probably an issue with ActiveStorage. The implementation makes it possible to go without a database by just using the configuration and the storage service. But hey, I could imagine there are more gems like that out there and it would be handy to have a flag for that.

I thought of something like this:

require 'ostruct'
require 'rspec'

# The logic under test. This should accomplish the auto configuration
# when no explicit user configuration was set. And it should honor a
# user configuration, when it is safe.
def process_config(config)
  if config.use_active_record != false && defined?(ActiveRecord)
    config.use_active_record = true
  else
    config.use_active_record = false
  end
end

def add_active_record
  Kernel.const_set('ActiveRecord', nil) unless defined?(ActiveRecord)
end

def remove_active_record
  Kernel.send(:remove_const, :ActiveRecord) if defined?(ActiveRecord)
end

RSpec.shared_examples 'config value' do |prop, actual, expected|
  it ['sets config to', expected.inspect, 'when',
      actual.inspect, 'was given'].join(' ') do
    subject[prop] = actual
    process_config(subject)
    expect(subject[prop]).to be(expected)
  end
end

RSpec.describe 'use_active_record configuration' do
  subject { OpenStruct.new(use_active_record: nil) }

  context 'ActiveRecord constant is defined' do
    before { add_active_record }

    [
      # given config value , expected config value
      [nil                 , true ],
      [true                , true ],
      [false               , false],
    ].each do |actual, expected|
      include_examples 'config value', 'use_active_record', actual, expected
    end
  end

  context 'ActiveRecord constant is undefined' do
    before { remove_active_record }

    [
      # given config value , expected config value
      [nil                 , false],
      [true                , false],
      [false               , false],
    ].each do |actual, expected|
      include_examples 'config value', 'use_active_record', actual, expected
    end
  end
end

use_active_record configuration
  ActiveRecord constant is defined
    sets config to true when nil was given
    sets config to true when true was given
    sets config to false when false was given
  ActiveRecord constant is undefined
    sets config to false when nil was given
    sets config to false when true was given
    sets config to false when false was given

Finished in 0.00162 seconds (files took 0.06667 seconds to load)
6 examples, 0 failures   

Should I provide a PR for this?

[JonRowe]

You don't want to set the value based on autodetect, (because of load ordering issues) but suppress the auto check when it's explicitly disabled, I think that'd work for me

[Jack12816]

I'm a bit confused about your comment, do you propose a logic change? :)

[JonRowe]

Take a look at how our config is built, what you want is it to fall back to the defined? check if it's not been set, and don't memomize that value

[ypicard]

Any news on this feature implementation ?

[JonRowe]

None of the core team have any time to work on this, and its unlikely to get anywhere without an outside volunteer.

[Jack12816]

I worked a bit on this topic and there is a solution shipped with this PR: #2038

[mrgordon]

Sad to see that its impossible to run a Rails app by removing ActiveRecord without Jack's unmerged patch or removing a bunch of unrelated ActiveStorage stuff...

Wasted an hour fixing this today and I know I'm not the only one. Makes me hesitant to use ActiveStorage in new projects. Seems like fixing this should be a higher priority than new features...

[JonRowe]

Its not impossible, you just need to ensure ActiveRecord is configured if it is present.