thoughtbot / factory_bot_rails

Factory Bot ♥ Rails
https://thoughtbot.com/services/ruby-on-rails
MIT License
3.07k stars 369 forks source link

Only reload if cache_classes is disabled, reload_classes_only_on_change enabled #418

Open stanhu opened 1 year ago

stanhu commented 1 year ago

https://guides.rubyonrails.org/configuring.html#config-reload-classes-only-on-change states:

3.2.30 config.reload_classes_only_on_change

Enables or disables reloading of classes only when tracked files change. By default tracks everything on autoload paths and is set to true. If config.cache_classes is true, this option is ignored.

It was surprising to us that even though cache_classes were set to true that factory_bot_rails still instantiated file watchers.

mike-burns commented 1 year ago

The test suite passes without this patch on Rails 7.1.0-beta2. If this does fix an issue you're seeing on Rails 7.1, @mathieujobin , I'm interested in hearing more about that so we can write a test to catch it.

@stanhu this patch needs a test. I can try to write one later, but maybe you can help write one first.

Here's my sketch but I think it needs more finesse:

    context "when cache_classes is true and reload_classes_only_on_change is true" do
      it "does not reload factory definitions" do
        Rails.application.config.cache_classes = true
        Rails.application.config.reload_classes_only_on_change = true
        allow(FactoryBot).to receive(:reload)

        reload_rails!

        expect(FactoryBot).not_to have_received(:reload)
      end
    end

    context "when cache_classes is true and reload_classes_only_on_change is false" do
      it "does not reload factory definitions" do
        Rails.application.config.cache_classes = true
        Rails.application.config.reload_classes_only_on_change = false
        allow(FactoryBot).to receive(:reload)

        reload_rails!

        expect(FactoryBot).not_to have_received(:reload)
      end
    end

    context "when cache_classes is false and reload_classes_only_on_change is true" do
      it "reloads the factory definitions" do
        Rails.application.config.cache_classes = false
        Rails.application.config.reload_classes_only_on_change = true
        allow(FactoryBot).to receive(:reload)

        reload_rails!

        expect(FactoryBot).to have_received(:reload).at_least(1).times
      end
    end

    context "when cache_classes is false and reload_classes_only_on_change is false" do
      it "does not reload factory definitions" do
        Rails.application.config.cache_classes = false
        Rails.application.config.reload_classes_only_on_change = false
        allow(FactoryBot).to receive(:reload)

        reload_rails!

        expect(FactoryBot).not_to have_received(:reload)
      end
    end