Factory_bot: Feature request: trait inheritance

[henrik]

We sometimes have traits that are subsets of other traits, something like:

trait :complete do
  name "Foo"
  address "Bar"
end

trait :complete_and_ready_to_go do
  name "Foo"
  address "Bar"
  go "Okay"
end

We could duplicate like this, or we could nest fabricators like item > complete_item > complete_and_ready_item, but I like the idea of always fabricating :items, only passing in the relevant traits.

Is this too far outside your idea of how Factory Girl should be used, or would it make sense to add nested/inherited traits?

[joshuaclayton]

We actually support this already!

You could do something like this:

FactoryGirl.define do
  factory :deal do
    title "Buy 1, get 2 free!"

    trait(:published)          { published true }
    trait(:unpublished)        { published false }
    trait(:expires_in_1_day)   { expires_at 1.day.from_now }
    trait(:expires_in_1_week)  { expires_at 1.week.from_now }
    trait(:expires_in_1_month) { expires_at 1.month.from_now }
  end
end

FactoryGirl.create(:deal, :unpublished, :expires_in_1_day, name: "Really great deal!")
FactoryGirl.create(:deal, :published, :expires_in_1_week, name: "One week left!")

This allows you to mix and match and apply traits to factories on the fly. It sounds like this is what you're looking for. Good luck!

[henrik]

@joshuaclayton Thanks! I think I didn't explain it well enough, though:

In your example, say that only published deals can expire. If a deal expires, I also want it to have all the :published attributes. So instead of having to do :published, :expires_in_1_day all the time, or having to duplicate published true in both traits, I'd like to be able to inherit the trait :expires_in_1_day from the trait :published so that whenever I use the former trait, I also get the attributes from the latter.

Much like I could do when inheriting factories, if I nested them deal > published_deal > deal_that_expires_in_1_day.

[joshuaclayton]

Hmm... seems like an interesting idea. Something like this?

trait(:published) { published true }
trait(:unpublished) { published false }
trait(:expires_in_1_day) do
  published
  expires_at { 1.day.from_now }
end

[henrik]

@joshuaclayton Exactly. That syntax would be great. Principle of least surprise might be to support all the same kinds of inheritance as factories, e.g. nesting and parent: as well.

[joshuaclayton]

@henrik This doesn't cover parent/children attributes getting copied, but implicit traits (the code you demoed above) are covered in this commit: https://github.com/thoughtbot/factory_girl/commit/779eafccbdb2a404ceabf2a4a2ad3d7b04eaf498

Let me know how it goes!!!

[henrik]

Just looking at the commit, it looks great; thank you! Will give it a shot in the coming work week.

[jmuheim]

This is nice, but like with factories, it would be cool to effectively nest traits in the declaration itself:

FactoryGirl.define do
  factory :activity_type do
    name 'Some type'

    trait :email do
      technology      ActivityType::TECHNOLOGY_EMAIL

      trait :email_in do
        email
        name 'Email In'
        direction       ActivityType::DIRECTION_IN
      end

      trait :email_out do
        email
        name 'Email Out'
        direction       ActivityType::DIRECTION_OUT
      end
    end

    factory :activity_type_email_in, traits: [:email_in]
    factory :activity_type_email_out, traits: [:email_out]
  end
end

What do you think about this?

[jmuheim]

By the way, I tried it the way you explain above, and it doesn't seem to work for me:

FactoryGirl.define do
  factory :activity_type do
    name 'Some Type'

    trait :email do
      technology      ActivityType::TECHNOLOGY_EMAIL
    end

    trait :email_in do
      email
      name 'Email In'
      direction       ActivityType::DIRECTION_IN
    end

    trait :email_out do
      email
      name 'Email Out'
      direction       ActivityType::DIRECTION_OUT
    end

    factory :activity_type_email_in, traits: [:email_in]
    factory :activity_type_email_out, traits: [:email_out]
  end
end

I'm getting:

NoMethodError:
undefined method `email=' for #ActivityType:0x007fff2c04aa50

[joshuaclayton]

@sientia-jmu Does your ActivityType instance have the email= method available? Have you run rake db:test:prepare? Can you assign email directly on an instance without raising?

Regarding nesting traits within traits, it's an interesting concept but I haven't seen a huge benefit over accessing traits from other traits. If there's a particularly good argument for it (or a pull request with tests!), I may reconsider - even after seeing a pull req. though, I'm not sure if it'd make it in; it depends on the impact to the codebase.

[jmuheim]

I will take a look at it, Joshua (nice name, it's the same I have :) ), as soon as I'm at work again, maybe tomorrow. Thanks!

[jmuheim]

Just wanted to say it's working for me now. I forgot about this discussion and accidently stumbled over it right now. :+1:

[joshuaclayton]

@sientia-jmu awesome!

[jmuheim]

@joshuaclayton supercalafragalisticexpialadoshus!