I find that although using let
and before
blocks often leads to less code, it can result in scenarios where it’s difficult to follow the flow of the test.
My preference is to keep the setup phase short and focused on the specific behaviour being tested, and have the incidental aspects handled by a helper method.
In your situation, I would probably extract a build_player
method:
def build_player(goals_scored:)
return @player if @player # memoize the result
user = create :user
game = create :game, score: 0
@player = create :player, user: user, game: game, goals_scored: goals_scored
end
so the test then becomes:
it "increases goals_scored by one" do
player = build_player(goals_scored: 0)
player.score!
expect(player.goals_scored).to eq 1
end
Notice that I’m passing goals_scored
to the helper method. This means the test can be read on its and should be broadly understood without having to refer to the ‘implementation details’ in the helper method.
You can then reuse that helper method in other specs by adding additional keyword arguments with reasonable defaults. That means you only need to override the attributes that you care about for a given spec, e.g.
def build_player(user: create(:user), game: create(:game, score: 0), goals_scored: 7)
@player ||= create(:player, user: user, game: game, goals_scored: goals_scored)
end
I’m always hesitant about creating associated records within the FactoryGirl definition, as it can lead to large graphs of objects being created unnecessarily, which slows down the tests and can be difficult to debug.
A couple of other things to note:
- You can probably use FactoryGirls’
build
method instead of create
to avoid unnecessary database calls.
- You might want to consider setting the
score
attribute of Game
in the factory, since it’s not really relevant to the test itself.