I left the following feedback on a recent pull request:
factory :deliverable do
title { "A cool title" }
tags { { assigned: true, status: "draft" } }
Is this required for the factory to be valid? How would you feel about having the factory only add the bare minimum required to create this model and then add the other attributes in the test or as traits?
This is beneficial as tests that don’t require this additional setup can avoid that work. Additionally, our test suite could then catch if there is a problem with nil value here. By having our factories only give us the minimum to create a valid record, we are testing creation of models with the minimal set of data.
If a property or relationship is really required for a factory to make sense then we should add a validation before adding it to the base factory of that model. Let’s take the checklist example: does a checklist make sense without any list items? This is the first state your users may see. By adding list items to the default factory you’re opening your test suite up to surprising behavior. Not only that but you are introducing a mystery guest!
Imagine that you are new on a project and for your first feature you write a presenter for some data. You write the following test:
require "rails_helper"
describe CodeListPresenter do
context "#data" do
context "with code_list_items" do
it "returns those code_list_items for a standard" do
standard = create(:standard)
code_list = create(:code_list, standard: standard)
code_list_item = create(:code_list_item, code_list: code_list)
code_list = CodeListPresenter.new(standard: standard)
expect(code_list.data[:code_list_items].count).to eq(1)
end
end
end
end
And you get the following failure:
F
Failures:
1) CodeListPresenter#data with code_list_items returns those code_list_items for a standard
Failure/Error: expect(parameter_design.data[:values].count).to eq(2)
expected: 1
got: 3
(compared using ==)
# ./spec/models/parameter_design_spec.rb:44:in `block (5 levels) in <top (required)>'
Finished in 0.35508 seconds (files took 3.86 seconds to load)
1 example, 1 failure
Failed examples:
rspec ./spec/models/parameter_design_spec.rb:33 # CodeListPresenter#data with code_list_items return those code_list_items for a standard
Randomized with seed 22400
What? Does this mean that the factory creates the extra two items or does
code_list
create two code_list_item
s in a callback? It’s not clear from
this test what is happening. After some investigation you find out that it’s in
the factory and you remove it. Removing it causes 15 or so specs to start
failing. You fix the ones that obviously depend on a code_list_item
being
present but you find a couple that fail because the case of no code_list_items
had not been considered. You fix the bugs and submit a PR. Not bad for your
first contribution!
Of course, always consider if you really need a factory in the first place.
We talk about this more in the upcoming testing-rails book. You can see the announcement post here.
If you’d like to find out more about factory bot, checkout this free episode of the Weekly Iteration.