Inconsistent and brittle trait inheritance behavior

[lordofthelake]

Given this this class and this factory definition:

DataClass = Struct.new(:x, :y, :z)

FactoryGirl.define do
  factory :parent_factory, class: DataClass do
    x 1

    trait :y do
      y 1
    end

    trait :z do
      y
      z 1
    end
  end

  factory :child_factory, class: DataClass, parent: :parent_factory do
    x 2

    trait :y do
      y 2
    end
  end
end

You observe this behaviour:

require 'factory_girl'
FactoryGirl::VERSION
# => '4.7.0'

FactoryGirl.find_definitions
FactoryGirl.build(:parent_factory, :z)
# => <struct DataClass x=1, y=1, z=1>
FactoryGirl.build(:child_factory, :z)
# => <struct DataClass x=2, y=1, z=1>

FactoryGirl.reload
FactoryGirl.build(:child_factory, :z)
# => <struct DataClass x=2, y=2, z=1>
FactoryGirl.build(:parent_factory, :z)
# => <struct DataClass x=1, y=2, z=1>

[composerinteralia]

This has to do with the order in which things get "compiled" by factory_bot. Part of the compilation process involves copying over trait definitions so we can refer to one trait within another.

So given this factory (a simplified version of the one above):

class Demo
  attr_accessor :value
end

FactoryBot.define do
  factory :parent, class: :demo do
    trait :y do
      value { :parent_value }
    end

    trait :z do
      y
    end

    factory :child do
      trait :y do
        value { :child_value }
      end
    end
  end
end

In this first example:

FactoryBot.build(:parent, :z).value
# :parent_value
FactoryBot.build(:child, :z).value
# :parent_value

we compile parent first, which involves copying the y trait over from parent to z. We later compile child and copy thatchild factory's y trait over to z as well, but because the parent factory's y was copied over first https://github.com/thoughtbot/factory_bot/blob/3a4d6f489ccd3bb58674e470c3bfaad642888975/lib/factory_bot/definition.rb#L118 always finds that one.

In the second example:

FactoryBot.build(:child, :z).value
# :child_value
FactoryBot.build(:parent, :z).value
# :child_value

we copy over child factory's y trait first, and so that is the one we will always find.

As mentioned in https://github.com/thoughtbot/factory_bot/pull/1064#issuecomment-448075022, which I think is rather closely related, this is one of the more complex parts of factory_bot, and straightening it out will be no simple task!

[composerinteralia]

This and #974 may require doing some shifting around of the guts of factory_bot along the lines of what was started in https://github.com/thoughtbot/factory_bot/compare/jc-gbw-modules-round-three

[NoahTheDuke]

What is the desired behavior for this situation? Given the Demo factory, what should FactoryBot.build(:parent, :z).value and FactoryBot.build(:child, :z).value return?

[composerinteralia]

I think it should probably be:

FactoryBot.build(:parent, :z).value
# :parent_value

FactoryBot.build(:child, :z).value
# :child_value