As developers, we agree that we want to spend our time developing features that deliver value to our projects. Testing should be a tool that helps us to focus on the features that matter, while giving us the confidence to do important things like refactoring existing code. They should help us to focus on essential complexity in the problems that we're trying to solve rather than being another obstacle that we have to overcome in order to complete our task.
I've seen a number of cases, however, where novices in particular get caught up in sporadically failing tests or teams get dragged down by slow test suites. Below I've tried to identify some of the most common mistakes I've seen in Rails test suites, along with best practices and solutions to common errors.
Creating ActiveRecord objects when building would suffice
In most Rails applications, much of our time testing is spent writing and running model tests. Following the traditional model of "skinny controller, fat model," this makes sense. What many people don't realize until their test suites start taking more than 10 minutes to run is the cost of persisting these records.
Writing to ACID-compliant databases is an expensive thing to do. This isn't the fault of the database implementations, it's because they care about not losing your data. The down-side of this is that your tests will start to drag if you always create persisted instances of your objects whenever you have to test a method. Instead, just instantiate a model with the attributes you need, and save the record only when absolutely necessary. If you're using FactoryGirl, this means using Factory.build instead of Factory.create whenever possible.
If you're reading a code base or a test, and see a place where a record is being persisted, do yourself and your team a favor and replace it with a test on a non-persisted object.
Creating Persisted ActiveRecord objects in a loop
Following on the above point about minimizing writes to the database in your tests, if you find yourself writing a loop to create 30 records in the database, or even a few, stop. Think about how you can test your function with a couple of mock or stub objects instead.
Failing to stub time, or not recognizing time-related code dependencies
In the past two years, I've spent some amount of time on the first day after January 1st fixing tests that depended on the year. In many large applications where a mixture of experienced and senior developers are working on code, chances are, someone will forget that time has an effect on the tests that you write. It's your responsibility when writing tests to make sure that they're not going to break due to something external changing. If you're not thinking about how time may affect your tests, start now. TimeCop is a useful gem for stubbing out time in your application where necessary.
A more insidious manifestation of this problem is the following:
user_a = Factory.create(:user)
user_b = Factory.create(:user)
User.all(order: 'created_at').should == [user_a, user_b]
Although this seems like it should work, the created_at
timestamp is recorded with precision only to the
second. Since you can probably persist many records per second on your machine, the two users likely have
the same created_at
timestamp, and you've just written code that contains a strange sort of time dependency
which will cause sporadic failure.
See the section below, "Depending on an order that isn't guaranteed," for more common order-related issues
Forgetting to prepare or migrate the database
This is a common beginner mistake in testing, and it even occasionally throws off people who have been
running tests for a while. You'll waste less time if you understand the way that tests interact with the database
schema.First, when you run rake,
rspec
will run the db:test:prepare
task. This loads the current database
from the schema.rb
file. If you run a single spec
example without running the full test suite with rake,
your
test schema won't necessarily be in sync with what you expect, and you'll get weird failures. Make sure you
run rake db:test:prepare
before you run rspec
on a single file. Also, if you just created a migration, but haven't
run the migrations yet, don't expect those changes to be visible to your test. You have to migrate first, then
rake db:test:prepare
before you see those schema changes in your tests.
Not understanding that javascript-enabled tests are running in another thread
Javascript-enabled tests in Capybara run in another thread.This is stated clearly in the README,but I've seen a lot of tests showing that developers haven't yet internalized this concept. This has a few implications:
- Your stubs and mocks won't work, since the objects you're modifying in the test suite will have been re-loaded in the server thread when you're running tests
- Database elements (from factories, etc) you create in the test example won't be visible in the server thread (this is in the README, and related to 'Not understanding transactions' below)
Not understanding transactions and how they affect tests
If you're using transactional fixtures (this is an rspec default, and usually the right thing to do if you're not using js tests in your suite), you need to understand what this means. A transaction is a set of database statements that are executed as an atomic action. The classic example is moving money from one bank account to another – if anything happens in between, you should completely roll back the transaction to its initial state, rather than only completing part of the transaction.
Rspec leverages transactions to clean out your database tables between examples. A transaction block is started at the beginning of the test, and rolled back at the end, thus clearing out the records that you persisted during the scope of the test. There are a couple of ways that I've seen developers waste time troubleshooting tests when they don't understand transactions well:
- They try to access data in another thread, e.g. trying to access data in the server thread in a Selenium test, when the test code is in a transaction block creating factories. You won't be able to see the data in the server thread, since the effects of a thread are only visible to the database connection that has the transaction transaction open.
- If your database state is not getting reset, make sure if you're using transactional fixtures that your database engine actually supports transactions. I've seen this catch developers off-guard when they added a connection to a MySQL database that used MyISAM (which doesn’t support transactions) instead of INNODB (which supports transactions).
Persisting an ActiveRecord object in a place where it won't get rolled back or otherwise removed
Rspec
and DatabaseCleaner
are generally configured to roll things back that are created in the context of a
test example. This also applies to code that is run in a before(:each) block. Developers expect the database
state to be clean
between examples. I have seen a number of cases where stray data at the start of the test
breaks things, and causes sporadic test failures because of inter-spec dependency issues. Here are some
places to look if you think you have this problem:
- before(:all) blocks – if you persist things to the database here, it's your responsibility to clear it out after your test.
- Insidea factory, but in code that isn't in a proc or lambda. Maybe you set up an association, and forgot to wrap it in a lambda {}. This will be created in the database when the test file is first loaded, and will never be cleaned out until the next time you drop and re-create the database.
Changing global state in a test without rolling it back
While rspec
and mocha do a good job of limiting the scope of stubs and mocks to a particular example, there
are certain things that won't be reset between test examples. For example, I found one really awful spec that
did this:
ActionController::Base.asset_host = nil
The result is that after this particular line was executed, every subsequent test that was run would have a
slightly different state than those that ran before the asset_host
was set. In this case, links to assets like the
application javascript were broken for every example after the above line, and depending on the order that
rspec evaluated things, sometimes the integration tests would be broken, and sometimes they wouldn't.
Be particularly careful about setting class-level variables or other configuration options during a test, as these will affect every subsequent test that you run. Often, this is a sign that your code should be refactored so that you don't have to change this kind of global state. In the few cases where this is the most pragmatic decision, make sure that you use an 'ensure' block to roll back the state after your test.
Depending on an order that isn't guaranteed
This is one of the most frequent causes of failure that I see when developers break the CI build and then say,
"it passes on my workstation!" Most database engines don't guarantee the order in which results are returned
when an explicit ORDER clause is absent. Just because you asked for Person,where(hair_color: 'blue'
) and
got back [person1, person2, person3]
doesn't mean that the CI server will return results in the same order – it
may not randomize the results locally, so it may seem it's consistent, but the CI server could just as well have
a database that decides to return [person2, person1, person3]
. Your test fails, your team gets mad, and
everyone becomes less productive for twenty minutes while you push a change to fix the test.
There are two fixes for this scenario:
- Provide an explicit ORDER. Remember that if you order by something where two records may have equal values, the database will never guarantee the results of those records unless you give them distinct values, or give it something it can order by without ambiguous results.
- Compare
Sets
instead ofArrays
. Two ruby Sets are equal regardless of the order of contents, so if your test example doesn't care about order, just compare your expected to actual results as two sets.Rspec
provides syntactic sugar for this case with the=~
operator. For example,Person.where(hair_color: 'blue') =~ [person1, person2, person3]
will pass even if the returned results are[person3, person2, person1].
General Fixes and Best Practices
Prevention is the best cure for most of the cases above. Learning Ruby, and frequently reading the source
code for rspec
and Capybara
when you don't understand how something works will do wonders for your
testing and application development skills in general. There are some things that you can do to detect where
bad things are happening in code, though:
- If you're not running your rspec test suite with –order random, add that line now and fix any inter-spec dependencies. You'll be glad that bad tests become visible sooner rather than later.
- Most projects should use continuous integration. Jenkins is a good option for this. If you have a test suite with sporadic failures that are tough to reproduce, schedule a build that runs every hour. Figure out which seeds (visible at end of test run when –order random is specified) break the build, and fix those tests.
- Turn on profiling output with –format profile. This will show you your slowest-running specs. Chances are, someone committed code that writes records in a loop, and this will help you to find out where it happened.
- Make sure that the last build and build history is visible in Jenkins, and don't deploy the bad builds to staging. It's much harder to figure out where in recent commits bad code was written if you don't have the build history available.Continuous integration is nothing new, and if more than one developer is on a project there is no reason to ignore this best practice.
Tests are supposed to provide a quick and reliable way to see if you've changed unintended application behavior by adding features or refactoring. It's annoying, and it makes you feel like you're wasting time if tests don't fulfill this basic purpose. I hope that these tips help you to waste less time in testing so that you can create tests that add value to the product that you're creating, rather than senselessly reduce your development velocity.