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.

Sign up for GitHub

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

Jump to bottom

Anonymous subclass of ActiveRecord::Base causes exception #90

Closed
owst opened this issue Mar 11, 2022 · 6 comments · May be fixed by #93
Closed

Anonymous subclass of ActiveRecord::Base causes exception #90

owst opened this issue Mar 11, 2022 · 6 comments · May be fixed by #93

Comments

@owst
Copy link

owst commented Mar 11, 2022

As part of our test suite we run ActiveRecordDoctor and also create anonymous subclasses of ActiveRecord::Base - the latter causes an exception in the former

Traceback (most recent call last):
	9: from repro.rb:34:in `<main>'
	8: from /Users/owenstephens/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/active_record_doctor-1.9.0/lib/active_record_doctor/runner.rb:14:in `run_one'
	7: from /Users/owenstephens/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/active_record_doctor-1.9.0/lib/active_record_doctor/errors.rb:5:in `handle_exception'
	6: from /Users/owenstephens/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/active_record_doctor-1.9.0/lib/active_record_doctor/runner.rb:15:in `block in run_one'
	5: from /Users/owenstephens/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/active_record_doctor-1.9.0/lib/active_record_doctor/detectors/base.rb:11:in `run'
	4: from /Users/owenstephens/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/active_record_doctor-1.9.0/lib/active_record_doctor/detectors/base.rb:40:in `run'
	3: from /Users/owenstephens/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/active_record_doctor-1.9.0/lib/active_record_doctor/detectors/missing_unique_indexes.rb:32:in `detect'
	2: from /Users/owenstephens/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/active_record_doctor-1.9.0/lib/active_record_doctor/detectors/base.rb:145:in `models'
	1: from /Users/owenstephens/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/active_record_doctor-1.9.0/lib/active_record_doctor/detectors/base.rb:145:in `reject'
/Users/owenstephens/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/active_record_doctor-1.9.0/lib/active_record_doctor/detectors/base.rb:146:in `block in models': undefined method `start_with?' for nil:NilClass (NoMethodError)

here's a simple reproduction script:

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem "rails", "= 5.2.6.2"
  gem "active_record_doctor", "= 1.9.0"
end

require "active_record"
require "active_record_doctor"

some_anonymous_model = Class.new(ActiveRecord::Base)

runner = ActiveRecordDoctor::Runner.new(ActiveRecordDoctor.load_config_with_defaults(nil))
runner.run_one(:missing_unique_indexes)
@fatkodima
Copy link
Contributor

fatkodima commented Mar 11, 2022 •
edited

Why not to use

SomeAnonymousModel = Class.new(ActiveRecord::Base)

?

This is not a trivial error to fix in active_record_doctor, since other methods used internally like .table_exists? aren't also working for anonymous models.

@owst
Copy link
Author

owst commented Mar 14, 2022

Hi @fatkodima,

The classes are defined in a test, and it isn't otherwise necessary to explicitly name the class (we are using RSpec and generating the subclass in a let call, so can refer to the anonymous class directly). As it happens, in the relevant test we explicitly assign self.table_name for the generated subclasses and I checked that model.table_exists? does work.

Is there any other reason to explicitly require model.name be non-nil? Perhaps active_record_doctor could just skip any models where name.nil?

@fatkodima fatkodima mentioned this issue Mar 14, 2022
Open
@fatkodima
Copy link
Contributor

Ah, ok, you're right 👍

@gregnavis
Copy link
Owner

Before merging the corresponding PR, I'd like to understand your situation better, @owst. My initial reaction is we shouldn't make active_record_doctor compatible with implementation details of a particular test suite. I understand you're calling active_record_doctor methods directly from your specs which isn't an "officially" recognized way to use active_record_doctor (though, maybe we should make it official).

I'd like to understand your use case better: why do you call active_record_doctor directly instead of using rails active_record_doctor (+ per-detector configuration if customization is needed). Is that to overcome certain limitations imposed by calling directly from the command line? If so, what kind?

@gregnavis
Copy link
Owner

@owst, let me know what you think about the above.

@owst
Copy link
Author

owst commented Aug 25, 2023

Hi @gregnavis - Apologies for the massive amount of time that's passed on this issue without reply.

Our reason for calling active_record_doctor directly within our test suite is that our app has a somewhat slow startup and therefore rather than pay the startup cost twice (once for test suite loading and then again for active_record_doctor loading) we thought we could get away with only once.

However, upon reflection, since it's not a supported setup and somewhat unusual, I'm happy just close this issue and pay the cost an additional time in our CI pipeline.

Thanks

@owst owst closed this as completed Aug 25, 2023
@owst owst closed this as not planned Won't fix, can't repro, duplicate, stale Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make it work with anonymous models fatkodima/active_record_doctor
3 participants
@gregnavis @owst @fatkodima