Ability to overwrite a property of association

[bogdan]

Let me show the example:

factory :category do
  name 'hello'
end
factory :post do
  title 'hello'
  body 'world'
  association :category
end

factroy :comment do
   body 'hello world'
   association :post
end

If I want to create a comment within given category:

create(:comment, :post => create(:post, :category => @category))

This doesn't look funny. My current approach is to use ignore in combination with after_build:

factory :comment do
  ignore do
    post_category nil
  end
  after(:build) do |comment, proxy|
    if proxy.post_category
      comment.post.category = proxy.post_category
      comment.save!
  end
end

This is funny to use create(:comment, :post_category => @category)

but not funny to implement for every such use case across all factories.

What do you think about an api like this to encapsulate this pattern:

association :post, :accessors => {:post_category => :category}

Would you like to see the API of this kind in factory girl?

[joshuaclayton]

@bogdan as far as I know, this hasn't really come up and I'm not sure it'd be a good addition. In this example, the emphasis on the object seems wrong - you're creating a comment, but if it matters that much that the comment is on a post with a category, I'd think that you'd do something like this:

post_with_category = create(:post, category: create(:category, name: 'Ruby'))
comment = create(:comment, post: post_with_category)

Again, maybe I'm not understanding the desired use case, but it seems like you're flipping the subject a bit by hiding the importance of the parent's properties (e.g. category) within a child (comment). Do you have any more (preferably real-world) examples of where you'd want to do this so I can have some more clarity on use cases?

[bogdan]

In a large project context setup is the most complicated part and it is always flipped no matter how you define it. In general, I feel like this feature gives additional flexibility to generate data with FactoryGirl, which is its main goal. I don't want FactoryGirl to decide how should I generate my context, it's my job. Just want a flexible tool to do it.

Live examples:

https://gist.github.com/5795005 https://gist.github.com/5795040

Simplified version of live examples, that is too complex to show real code

I am doing things from your code example about 3 times a day. In a team of 5 people it is 15 times a day average.

Let's take the following association chain:

Site.has_many :campaigns
Campaign.has_many :offers
Offer.has_many :visitors

At first data from different Site's never mixed up. So most tests should run within one Site.

And now when you want to test in interaction between Visitor's. They should be on same site. That makes a problem to define:

site = create(:site)
s1 = create(:visitor, :offer => create(:offer, :campaign =>
create(:campaign , :site => site)
s2 = create(:visitor, :offer => create(:offer, :campaign =>
create(:campaign , :site => site)

I don't think your fingers will enjoy typing this over and over again.

Suppose that Campaign is a huge configuration object. It's properties has a great impact on all related objects. You want to test the behavior of Visitor based on different Campaign setup. So you have around 5 different variations of value1 and value2:

campaign = create(:campaign, :option1 => VALUE1, :option2 => VALUE2)
visitor = create(:visitor, :offer => create(:offer, :campaign => campaign))
# test some behavior of visitor

[bogdan]

any news on this?

[joshuaclayton]

@bogdan Thanks for your patience while I dug through your example and came up with a demo.

At the end of the day, your tests are still Ruby. You can extract to methods, extract to classes, and use Ruby to simplify your code. Here's some quick refactoring that I'd have done to simplify the code. I'll go through it step-by-step so it's clear why I'm doing what I am.

Round 1

describe ".search_facets" do

  let(:first_category) { category_titled("first cat") }
  let(:second_category) { category_titled("second cat") }
  let(:third_category) { category_titled("third cat") }

  let(:first_product) { FactoryGirl.create(:product, set: FactoryGirl.create(:product_set, category: first_category)) }
  let(:second_product) { FactoryGirl.create(:product, set: FactoryGirl.create(:product_set, category: first_category)) }
  let(:third_product) { FactoryGirl.create(:product, set: FactoryGirl.create(:product_set, category: third_category)) }

  subject  { Product.search_facets([first_product.id, second_product.id, third_product.id]) }

  it { should have(2).size }

  its("first.product_count") {should == "2"}
  its("first.title") {should == "first cat"}
  its("second.product_count") {should == "1"}
  its("second.title") {should == "third cat"}

  def category_titled(title)
    FactoryGirl.create(:product_category, title: title)
  end
end

First step - move the category creation out into a named method which accepts just a string. Still a lot of duplication, but categories look better.

Round 2

describe ".search_facets" do

  let(:first_category) { category_titled("first cat") }
  let(:second_category) { category_titled("second cat") }
  let(:third_category) { category_titled("third cat") }

  let(:first_product) { FactoryGirl.create(:product, set: set_for_category(first_category)) }
  let(:second_product) { FactoryGirl.create(:product, set: set_for_category(first_category)) }
  let(:third_product) { FactoryGirl.create(:product, set: set_for_category(third_category)) }

  subject  { Product.search_facets([first_product.id, second_product.id, third_product.id]) }

  it { should have(2).size }

  its("first.product_count") {should == "2"}
  its("first.title") {should == "first cat"}
  its("second.product_count") {should == "1"}
  its("second.title") {should == "third cat"}

  def category_titled(title)
    FactoryGirl.create(:product_category, title: title)
  end

  def set_for_category(category)
    FactoryGirl.create(:product_set, category: category)
  end
end

Next, we tackle the nested factory per product. Things are looking good, but there's two layers to map 'first cat' and 'third cat' to the products themselves.

Round 3

describe ".search_facets" do

  let(:first_product) { FactoryGirl.create(:product, set: set_for_category_titled('first cat')) }
  let(:second_product) { FactoryGirl.create(:product, set: set_for_category_titled('first cat')) }
  let(:third_product) { FactoryGirl.create(:product, set: set_for_category_titled('third cat')) }

  subject  { Product.search_facets([first_product.id, second_product.id, third_product.id]) }

  it { should have(2).size }

  its("first.product_count") {should == "2"}
  its("first.title") {should == "first cat"}
  its("second.product_count") {should == "1"}
  its("second.title") {should == "third cat"}

  def set_for_category_titled(title)
    set_for_category(category_titled(title))
  end

  def category_titled(title)
    FactoryGirl.create(:product_category, title: title)
  end

  def set_for_category(category)
    FactoryGirl.create(:product_set, category: category)
  end
end

Now we're getting somewhere. The category and product set factories are completely hidden behind methods and it's easy to see a mapping from the product to the category title.

Round 4

FactoryGirl.define do
  factory :product do
    ignored do
      set_category_title nil
    end

    after(:create) do |product, evaluator|
      if set_category_title
        category = FactoryGirl.create(:product_category, title: set_category_title)
        product.set = FactoryGirl.create(:product_set, category: category)
        product.save
      end
    end
  end
end

describe ".search_facets" do

  let(:first_product) { FactoryGirl.create(:product, set_category_title: 'first cat') }
  let(:second_product) { FactoryGirl.create(:product, set_category_title: 'first cat') }
  let(:third_product) { FactoryGirl.create(:product, set_category_title: 'third cat') }

  subject  { Product.search_facets([first_product.id, second_product.id, third_product.id]) }

  it { should have(2).size }

  its("first.product_count") {should == "2"}
  its("first.title") {should == "first cat"}
  its("second.product_count") {should == "1"}
  its("second.title") {should == "third cat"}
end

At this point, we can piggyback off of FactoryGirl's ignored attributes to pass in set_category_title. Whenever set_category_title is provided, it will generate a category, a set for that category, and save it. The test now calls FactoryGirl.create once per product, hiding the additional interactions of the factory for what's necessary when a category is assigned in the factories file itself.

This is not the end-all-be-all solution. I've created classes which generate users, place orders, have refunds issued, track credits, and do it with a clean DSL, all because the object structure is tough. FactoryGirl can't (and won't ever be able to) handle every case under the sun; however, as developers we should leverage Ruby in conjunction with FactoryGirl to simplify tests (because no one wants to type all that code all over the place) and new devs on a project should be able to (at a glance) grasp simple concepts about how the application behaves based on the tests.

Hopefully this clarifies things and gives you some ideas for simplifying your test suite.

[bogdan]

Thanks for response. Don't understand why did you close the issue before hearing my thoughts about it.

Custom helper methods is something we use before in order to make advanced data generation. And problem is: they are not flexible enough. You can define helpers on per test basis, but this is still inefficient - lead to a lot of copy-paste across tests. Their behavior is specific and people use to forget how they works. And we spent time over and over again explaining people how to create data with custom helpers.

If you would join a project you would prefer to write: create(:product_set, category: category) instead of create_set_for_category(category). Because there is no sense to invent new API that does same thing in less flexible way.

This is the whole idea behind FactoryGirl: people recognized the pattern and they don't need helper methods anymore.

So, ignore and after(:build) trick is 100% better than helper methods because it provides

• better code reuse.
• better data generation API
• less lines of code

We just know that from our practice.

In round 4 you still used that trick. But it will not work because most belongs_to associations are required and object can not be saved without them. So you need to include those associations in factory by default. 90% of tests will not use set_category_title. Minimum working factory for product is:

FactoryGirl.define do
  factory :product do
    association :set
    ignored do
      set_category_title nil
    end
    # Note that after build is better than after create
    after(:build) { .... }
  end
end

So at the end you have reinvented the pattern we use right now.

But the drawback of this is that during FactoryGirl.create(:product, set_category_title: 'third cat'). you insert 2 ProductSet into database instead of one. That could be also a big issue.

In a perfect world I don't want to do more than one Factory.create call to build a dataset. Because for me it is just belongs_to association chain and I don't need 2-3 helper methods or ignore after(:build) tricks to express that. We have recognized the pattern and we don't want custom solutions anymore

Same thing happened when traits were added: no body wants custom solutions.

Also I don't understand why is it a big deal to implement an API I propose. Can you explain any hidden cost you see or future problems? For me it seems 10-15 minutes feature.

[joshuaclayton]

@bogdan I apologize, I didn't mean to comment and close, just comment; I've reopened the ticket to keep the discussion flowing.

My concern is that, while it may be an easy solution in the sense that it is easy to get FG to do that, FG also exists to make tests more readable, decoupled from the application, and maintainable. By allowing attributes to be overridden (at any number of levels) in factories, it effectively encourages Law of Demeter violations and tight coupling of attributes and associations throughout the entire test suite. FactoryGirl is built to reduce coupling to object structures in tests by moving that logic to the factories file, which this bypasses completely and encourages tight coupling to the database. I will not introduce that as a "feature" to FG because it encourages the exact opposite of what we're trying to achieve.

Does that explanation help?

[bogdan]

Law of Demeter is very theoretical thing. Any rails projects breaks it in 99% of files (especially in views). For me it is just something impossible to follow. I think that good API and problems with making too many objects in database is more important than Demeter.

But anyway I'll try to explain my treatment of Demeter:

In order to follow this law, we use delegates in ruby:

class User
  delegate :first_name, :last_name, :to => :profile
end

I interpret association property overwrite exactly in the same way. Instead of doing:

product = create(:product)
product.category.title = "my title"
product.save!
# or same variant in callback:
after(:build) do |o, f|
  if f.category_title
    o.category.title = f.category_title
    o.save!
  end
end

Both break Demeter law.

So, we are making a delegation:

factory :product do
  association :category
  delegate :category_title, :to => :category
end
create(:product, :category_title => "my title")

Your vision is different from mine because you are trying to make FG to follow Law of Demeter internally. While I am trying to make my code to not break Law of Demeter when I am using FG.

Unfortunately, Factory is not a class, I am not very flexible to use delegation.

[gamov]

I came here to write the same feature request as this one. I also have a large project and very often I need to configure associations when creating records with FG. I usually prepare objects before then I pass them or assign them. However, I was thinking of something simpler, mimicking the way we do joins in AR:

FactoryGirl.create(:comment, body: 'my body', post: {title: 'my title', category: {name; 'my cat name'}})

#currently, I usually do something along those lines:

cat = FactoryGirl.create :category, name: 'my cat name'
@comment = FactoryGirl.create :comment, body: 'my body', post: FactoryGirl.create(:post, category: cat)

In the beginning, I was writing more factories to settle cases, but it turned out there is too many variations and factories were becoming messy.That's my 2 cents.

[joshuaclayton]

Law of Demeter goes from theoretical to practical very quickly when you have to update half a test suite because LoD seems fine to violate by assigning values on associations. We're working on some ideas of how to make this easier but it definitely won't be done where you pass a deeply-nested hash to override associated data.