Small deprecation with "base" factories wanting to find a Base class in 6.0

[PragTob]

Hi there,

and first thanks for all your wonderful OSS projects! :green_heart:

Description

After upgrading to factory_bot 6.0 (from 5.2) FactoryBot.lint on our factories gives us the following:

FactoryBot::InvalidFactoryError:
  The following factories are invalid:

  * account - uninitialized constant Base
  Did you mean?  Base64 (NameError)
  * action - uninitialized constant Base
  Did you mean?  Base64 (NameError)

Which I believe can be traced back to our factories relying on a common base factory:

# factories/account.rb
FactoryBot.define do
  factory :account, class: Models::Account, parent: :base do
    # stuff
  end
end

# factories/base.rb
FactoryBot.define do
  factory :base do
    to_create(&:save)
  end
end

Aka it tries to infer the Base class name, but that isn't there.

The fix is rather simple, to define an explicit class in factories/base.rb:

FactoryBot.define do
  factory :base, class: Object do
    to_create(&:save)
  end
end

I'm sure the failure is directly related to the factory bot upgrade as I isolated this gem update.

Reproduction Steps

The steps above should show that, the factory setup isn't complicated if it doesn't work/click for you straight away happy to do a production repo.

Expected behavior

Not sure. What was done in the project might have always been broken, the latest release just exposed that (not sure how) - I didn't expect it because nothing in the NEWS.md pointed towards this breakage to my eyes.

Actual behavior

Failure/Error: FactoryBot.lint factories_to_lint, traits: true

FactoryBot::InvalidFactoryError:
  The following factories are invalid:

  * account - uninitialized constant Base
  Did you mean?  Base64 (NameError)
  * action - uninitialized constant Base
  Did you mean?  Base64 (NameError)

System configuration

factory_bot version: 6.0.0 rails version: none (ActiveSupport/ActiveModel 6.0.3.2 ruby version: 2.6.6p146

Thanks!

[composerinteralia]

I'm guessing this is related to https://github.com/thoughtbot/factory_bot/issues/1410. I will push out a fix later today and you can see if that works for you.

[composerinteralia]

factory_bot 6.0.1 is out. Could you check if this release fixes the problem you were having?

[PragTob]

Hi there :grin:

I was just upgrading another service and saw that it installed 6.0.1 so I came back here :) Thanks for the quick response :+1:

Sorry but the new version doesn't resolve the problem:

An error occurred in a `before(:suite)` hook.
Failure/Error: FactoryBot.lint factories_to_lint, traits: true

FactoryBot::InvalidFactoryError:
  The following factories are invalid:

  * account - uninitialized constant Base
  Did you mean?  Base64 (NameError)
  * action - uninitialized constant Base
  Did you mean?  Base64 (NameError)
  * action+project - uninitialized constan

With my git diff at:

git diff
diff --git a/Gemfile.lock b/sGemfile.lock
index 31379039b..5a2b31b61 100644
--- a/\Gemfile.lock
+++ b\/Gemfile.lock
@@ -95,7 +95,7 @@ GEM
     event_emitter (0.2.6)
-    factory_bot (6.0.0)
+    factory_bot (6.0.1)
       activesupport (>= 5.0.0)
diff --git a/services/actions/spec/factories/base.rb b/services/actions/spec/factories/base.rb
index 806742f8a..31a4a90ee 100644
--- a\/spec/factories/base.rb
+++ b\/spec/factories/base.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true

 FactoryBot.define do
-  factory :base, class: Object do
+  factory :base do
     to_create(&:save)
   end

Should I make a small repro repo to help you better diagnose the problem?

[composerinteralia]

Should I make a small repro repo to help you better diagnose the problem?

That would be super helpful indeed!

[composerinteralia]

Ah, I just had another hunch. I bet it is this line: https://github.com/thoughtbot/factory_bot/pull/1380/files#diff-6e48425c938e2d7bf4c0f3fd20dca3e5R87. We refer to the factory's build class a little earlier than we used to. We now refer to it at "compile" time, rather than at "run" time. Child factories do "compile" their parent factories, but they don't "run" them. We needed to make this change to support automatic enum trait definitions.

Since you have found a suitable workaround, let's leave this issue open for a bit and see if it comes up again. If it is common for folks to use parent factories for inheritance, not meant to be run, perhaps we could introduce an official syntax for that, something like factory :base, abstract: true. If this is not common, then I may close the issue without fixing.

Another option would be for you to introduce a helper class:

# abstract_factory.rb

class AbstractFactory
  def initialize
    raise "abstract factories cannot be instantiated"
  end
end

# factories/base.rb

require 'abstract_factory'

FactoryBot.define do
  factory :base, class: AbstractFactory do
    to_create(&:save)
  end
end

[PragTob]

@composerinteralia thanks! :green_heart:

I agree it's somewhat of a special use case and an easy work around.

I believe at its heart (I'm not quite sure, relatively new to the project) it has to do with us using Sequel instead of ActiveRecord. I at least recall running into an issue where I tried to use my old favorite build_stubbed but it didn't work since FactoryBot tried setting id while we only have uuid but that'd be the topic for another issue.

Thanks heaps! :rocket:

[tindron]

We also ran into this with an abstract base factory.

[adamransom]

We also ran into this with an abstract base factory (in a project using ActiveRecord).

[composerinteralia]

Thanks all for letting me know.

As I think about this more, I realize we already have a way of sharing behavior across multiple factories by using traits.

With the original example:

# factories/account.rb
FactoryBot.define do
-  factory :account, class: Models::Account, parent: :base do
+  factory :account, class: Models::Account, traits: [:base] do
    # stuff
  end
end

# factories/base.rb
FactoryBot.define do
-  factory :base do
+  trait :base do
    to_create(&:save)
  end
end

Let me know if that works for you. If it does, I will plan to close this issue.

[PragTob]

@composerinteralia thank you very much - works for us!

I guess adding a small note to News might be nice so that other people would be informed of this earlier in their upgrade process.

[adamransom]

@composerinteralia Thank you for the suggestion! Unfortunately, this doesn't work in our case:

FactoryBot.define do
  factory :interview do
    trait :rejected do
      ...
    end
  end
end

FactoryBot.define do
  factory :phone_interview, parent: :interview, class: 'PhoneInterview' do
    ...
  end
end

where Interview is not an actual class. We use composition rather than inheritance in this case, so it doesn't quite fit the idea behind parent factories, but with the workaround mentioned above it does suite our needs.

Another workaround would be to use the BaseInterview module (used for composition) as the class, which gets around the uninitialized constant exception, but doesn't feel right either.

[composerinteralia]

@adamransom thanks for that example. I can see why that doesn't work: applying :inverview as a base trait would not make the :rejected trait available to the :phone_interview factory. Interesting!

[composerinteralia]

For now I am going to say that abstract factories are not supported in factory_bot 6 (they weren't explicitly supported before either, but they did happen to work).

My reasoning is that so far it seems like most cases can use traits instead, which were built to share behavior across factories.

The use case of an abstract parent class providing its children with shared traits seems less common, and we have a workaround for it (explicitly passing in a class like AbstractFactory or even Object). If this does turn out to be a common use case, it might be worth considering that as we rework traits as part of https://github.com/thoughtbot/factory_bot/issues/968. Perhaps there is a way to apply a trait and have it make other traits available.

Thank you everyone for your thoughts and examples.