How Not To Write Feature Tests
Without tests, code rots
As your codebase grows, fed by a stream of idiosyncratic contributors racing to meet the varied demands of your audience, it quickly starts to sprawl. The intent behind older design choices is lost amid endless patches, while division of responsibility crumbles and duplicate functions loiter in the cracks between systems.
You need to refactor, but you’re scared. The codebase is too large to understand in its entirety, and there’s no knowing whether refactoring a function in one place will cause the sky to fall in another.
A robust test suite removes the fear. With one console command, you can tell if you’ve safely cleaned up a class or unleashed the four horsemen. Tests take the danger out of essential maintenance.
However, poorly written tests are easily as dangerous than no tests at all. I learned this firsthand in the process of building 2005book, a retro Facebook clone.
If you don’t write tests to the standard of production code, you effectively double your maintenance load.
Tests must be built to change as production code evolves. If they can’t be understood by anyone but the original author (or the original author has created such a spectacular mess they can’t retrace their own steps), your squad will do anything it can to avoid touching the test suite. Not only is your production code rotting, but you’ve added useless test code to the garbage heap.
Tests demand the same adherence to the principles of DRY, separation of concerns, and the careful, expressive naming of variables that you give to your production code.
To put this into focus, here’s everything I did wrong in writing the feature tests for 2005book’s edit profile form — and everything I did to fix it.
Ruby on Rails
One of the most popular test platforms for the Ruby ecosystem.
A Ruby test framework that simulates how users interact with your app in the browser.
The First Attempt
There’s very little separating old-school social networks from giant forms. My take on Facebook’s original edit profile form was particularly vast:
The test requirements were formidable: Ensure that every field correctly saves its input to the database and that those values are correctly retrieved and populated on subsequent page loads.
This is just a sample of the 360-line behemoth that resulted from my first pass:
The three laws of test-driven development (TDD)
Before we even touch on the state of the code, it should be apparent that I’ve broken Robert C. Martin’s three laws of TDD:
- You may not write production code until you have written a failing unit test.
- You may not write more of a unit test than is sufficient to fail.
- You may not write more production code than is sufficient to pass the currently failing test.
I had skipped straight past unit tests to feature tests, taking my models’ functionality for granted. Strike one.
Worse, I was retrofitting these tests to an already-written application I was was reasonably confident worked. That might prevent future bugs from developing, but there could be something nasty already lurking that I knew nothing about.
Don’t do this. Write one test, write the code that passes that tests, then write the next one. Testing is part of programming, not a secondary activity that follows as and when you find the time.
The single responsibility principle (SRP)
The next thing you should notice is that there’s an awful lot going on before we reach the test code. Inside the test suite, there are multiple, generic setup methods preparing the ground for the tests to run.
let helper methods are required to live inside a test suite, but the plain old Ruby methods have no business cluttering this file, especially not if they could be reused by other test suites. It is the job of the test suite to run tests — it shouldn’t need to know how to configure the database to make this happen.
There’s a second violation of the SRP here: The sole test suite,
New Profile, should test whether a new user has a correctly configured profile, but it’s also testing whether profile edits are correctly saved to and retrieved from the database. This is dangerously misleading.
Write high-performance tests
Tests that run slowly don’t get used as often as tests that give immediate feedback. Once your test works, optimise for speed.
On my first attempt, this
before block was set up to run before every single test:
Before every test, the same records were created in the database, then destroyed once more at the end. As each test relied on the exact same records, this amounted to a pointless waste of processing power. Tests that should have run in the blink of an eye were creeping into whole seconds.
Don’t repeat yourself (DRY)
Repetition is the hallmark of unmaintainable code. Here, almost every test shares code with another. Some lines appear in every test. Here’s a particularly nasty example:
In the example above, note that the
expect call specifies not only the text that should be visible on screen but the CSS that goes with it.
If the HTML structure of the webpage ever changes, the test will break even though the underlying systems still work. This is obviously insane.
In fact, it was being forced to carry out a complete front-end restructure of my form that convinced me enough was enough. The tests were unmaintainable — they had to change or rot with the project.
Over the course of a day, this lumpen, ungainly, unmaintainable giant was reduced to 140 lines.
All those generic methods cluttering the top of the previous test suite? Swept into a
If these same test objects were required by different specs in future, it would be trivial to genericize this module further.
A number of helper methods remain in the primary test file:
test_dropdown. These are specific to the test in question, so I opted to keep them in place. However, should a new spec file require a generic field test, the first port of call would be to extract these into a module of their own.
The original test suite has also been split in two:
New Profile handles functionality related to the creation of new profiles, while
Edit Profile takes responsibility for ensuring the profile can be updated.
Test speed improved
Where once database records were generated before every single test, each suite now has distinct
before(:all) creates the shared test objects once for each test suite. All that’s left for
before(:each) to do is call the highly abstracted
login action from
The new tests run about 80% faster than the first pass.
Testing a form is an exercise in repetition. One text field is much the same as another. The method to test a drop-down doesn’t change between one drop-down and the next.
To abstract this repetition away from the tests themselves, I wrote two methods:
Running a test for a specific field is as simple as calling the corresponding method with the relevant field ID and the value to fill and expect:
For the time being, the tests for whether the form is correctly populated with the new values on subsequent edits have been removed on the basis that Rails’ form helpers will necessarily populate forms correctly if the controller passes them the correct objects. A future controller spec will restore this check.
No front-end dependencies
All those hard-coded checks for specific CSS have been removed. Instead, I test only for the presence of the correct content. This allows much greater flexibility for design changes.
To keep your tests as clean as your production code — ideally cleaner — remember:
- Don’t write production code until you have a failing test
- Each test suite, and each test within it, should do one thing only. Abstract helper methods to modules where possible.
- Optimise your tests for speed: Consider which database transactions are strictly necessary and which can be run on a per-suite basis
- Do not repeat yourself: Check for tests that are doing essentially the same thing in a different context, and build generic functions to handle the duplication
- Minimise dependencies: Don’t write tests that depend upon the style and structure of the front end to run