TDD Antipatterns: The Free Ride

There is an excellent thread on Stack Overflow TDD Anti-patterns catalogue
which contains short descriptions of some common traps we can fall into when
writing test-driven code. In the upcoming series of short blog posts, I will
illustrate some of them with snippets of Ruby code.

The Free Ride a.k.a Piggyback: rather than write a new test case method
to test another/distinct feature/functionality, a new assertion rides along
in an existing test case.

In the following spec we are verifying that an event contains a feedback
form whenever a user posts some content that indicates that she was present
at the event. It comes from a real code base, just a little bit adapted for
readability.

What's the problem here? Aside from unnecessary repetition of expectations
that an event without any associated data does not include the feedback form,
the description of the test does not match the things it tests.
It is vague ("is displayed when user generates an activity"), which
naturally comes from the fact that the test contains multiple different
expectations. Once we let ourselves go down this path though, it is easy to
end up with further complications. This example is actually rather simple;
sometimes the expectations may not even appear to be connected when
you look at them with fresh eyes, because of a lacking description.

A failure in any of these expectations would then not point to the exact cause
of the test failure, which breaks down one of the main benefits of tests —
a short feedback loop about our programming decisions.
In a moment when more than one expectation would be broken, eg during a
refactoring session, you would not be able to get that information because
the test runner stops at the first failed expectation.

If we consider this OK, then the logical conclusion would be to make
the whole suite a single test with numerous expectations. But the opposite
is rather easy to achieve:

describe Event do
describe "feedback form" do
before do
@user = create(:user)
@event = create(:event)
end
context "before all activity" do
it "is not displayed" do
expect(@event).not_to include_feedback_form(@user)
end
end
context "user posts a comment" do
before { @event.create_comment(:user => @user) }
it "is displayed" do
expect(@event).to include_feedback_form(@user)
end
end
context "user removes a comment" do
before do
comment = @event.create_comment(:user => @user)
comment.destroy
end
it "is not displayed" do
expect(@event).not_to include_feedback_form(@user)
end
end
context "user posts a photo" do
before { @user.post_photo(@event) }
it "is displayed" do
expect(@event).to include_feedback_form(@user)
end
end
context "user likes it" do
before { @user.like(@event) }
it "is displayed" do
expect(@event).to include_feedback_form(@user)
end
end
end
end

Using different test values in a single test case

A related case, perhaps more a matter of readability, is to avoid grouping
different test values in a single test case. For example:

describe City do
it "generates slug based on name" do
city = create(:city, :name => "San Sebastián")
expect(city.slug).to eq("sansebastian")
city = create(:city, :name => "Maceió")
expect(city.slug).to eq("maceio")
end
end

Instead we can follow the approach which would produce the most informative
form of test output and feedback during development:

Update Jun 25: The examples above could be improved by explicitly
stating what they are testing:

describe City do
describe "slug, generated from name" do
context "name is 'San Domingo'" do
subject { create(:city, :name => "San Domingo") }
it "strips the whitespace and converts to lowercase" do
expect(subject.slug).to eq("sandomingo") }
end
end
context "name is 'Maceió'" do
subject { create(:city, :name => "Maceió") }
it "strips replaces accented character" do
expect(subject.slug).to eq("maceio") }
end
end
end
end