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

Avoid loading ActiveRecord::Base early #91

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

casperisfine
Copy link
Contributor

Doing this cause a slew of problems with Rails loading process, causing some configurations not to be applied, etc.

Fix: #89
Fix: #90

Doing this cause a slew of problems with Rails loading process, causing some configurations not to be applied, etc.

Fix: DatabaseCleaner#89
Fix: DatabaseCleaner#90
@benoittgt
Copy link

I confirm that is a good proposal that fix some issue on other gem.

benoittgt added a commit to benoittgt/factory_bot_rails that referenced this pull request Nov 23, 2023
benoittgt added a commit to benoittgt/factory_bot_rails that referenced this pull request Nov 23, 2023
Copy link
Member

@etagwerker etagwerker left a comment

Choose a reason for hiding this comment

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

@casperisfine Thanks for submitting this patch!

Just waiting for CI to finish before I merge.

@etagwerker etagwerker merged commit 4afa131 into DatabaseCleaner:main Nov 23, 2023
24 checks passed
neilvcarvalho pushed a commit to thoughtbot/factory_bot_rails that referenced this pull request Nov 23, 2023
* CI against Rails 7.1

* Ignore the directory that keeps factory files under the `lib`

Some tests put factory files under the `lib`. But since Rails 7.1,
Rails loads `lib` directory by default in a new application.
https://guides.rubyonrails.org/7_1_release_notes.html#introducing-config-autoload-lib-and-config-autoload-lib-once-for-enhanced-autoloading

But factory files don't follow the naming rule of Zeitwerk. So
Zeitwerk raises `Zeitwerk::NameError`.

To avoid the error, this changed to ignore the directory that puts
factory files.

* Remove the route that is defined by default

Some test code loads the route file twice. This wasn't a problem
before Rails 7.0 because we didn't have a route.

But, Rails 7.1 has one route by default. So the test application
raises the following error during the load.

```
Invalid route name, already in use: 'rails_health_check'  (ArgumentError)
You may have defined two routes with the same name using the `:as` option, or you may be overriding a route already defined by a resource with the same naming. For the latter, you can restrict the routes created with `resources` as explained here:
https://guides.rubyonrails.org/routing.html#restricting-the-routes-created
```

The routing isn't a matter of this gem, so just ignore that for
running the test.

* Prevent warning about deprecated cache_format_version

* Wrap ActiveRecord configuration into initializer

`Rails.application` is not available when the railties are collected,
so we move the ActiveRecord configuration into the initializer phase.

```
NoMethodError: undefined method `config' for nil:NilClass (NoMethodError)
      application.config
                 ^^^^^^^
/project/app/vendor/bundle/ruby/3.2.0/gems/railties-7.0.8/lib/rails.rb:47:in `configuration'
/project/app/vendor/bundle/ruby/3.2.0/gems/factory_bot_rails-6.4.0/lib/factory_bot_rails/railtie.rb:25:in `block in <class:Railtie>'
```

* Provide non regression feature test when ActiveRecord::Base is loaded

It happens if the AR::Base as already been loaded.

For example:
DatabaseCleaner/database_cleaner-active_record#91

---------

Co-authored-by: Yuji Yaginuma <yuuji.yaginuma@gmail.com>
Co-authored-by: Leo Arnold <github@leoarnold.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue with the upcoming Factory Bot Rails due to lacking of lazy load hooks
3 participants