Cop idea: restrict defining global factorybot traits

[t3h2mas]

Factory bot allows for traits to be defined outside of a factory definition. This introduces the risk of collisions in as the tests grow in a codebase.

Example:

FactoryBot.define do
  factory :user do
    first_name { "John" }
    last_name  { "Doe" }
    admin { false }
  end

  trait :admin do
    admin { true }
  end
end

Works in a test

let(:user) { create(:user, :admin) }

Could a cop flag global trait definitions?

[pirj]

This says:

Traits allow you to group attributes together and then apply them to any factory.

And the reason for the very existence suggests that global traits have a reason to exist outside of factories.

But I agree with you that if a trait is meant to be used in several factories, it would make sense to at least define it in a separate file.

Are you certain that FactoryBot wouldn't/shouldn't warn about this? It would be quite hard to tell if e.g. an :admin trait is defined in both spec/factories/common_traits.rb and spec/factories/admin.rb with static analysis.

[t3h2mas]

It looks like factory bot does raise a FactoryBot::DuplicateDefinitionError. In our case, traits were added to the global scope unintentionally. I was hoping a cop could make global factories explicit.

it would make sense to at least define it in a separate file

This makes sense to me. Thoughts on a "global traits shouldn't be defined in the same file as factories" type of cop?

If this doesn't seem generally valuable, feel free to close @pirj

[pirj]

Thoughts on a "global traits shouldn't be defined in the same file as factories" type of cop?

Makes sense to me 👍