14 Dec 2021 · Semaphore News

    Rails Testing Antipatterns: Models

    7 min read
    Contents

    This is the second post in the series about antipatterns in testing Rails applications. See part 1 for thoughts related to fixtures and factories.

    Creating records when building would also work

    It is common sense to say that with plain ActiveRecord or a factory library you can both create and only instantiate new records. However, probably because testing models so often needs records to be present in the database, I have seen myself and others sometimes forget the build-only option. It is, however, one of the key ways to making your test suite faster.

    Consider this spec for full_name:

    RSpec.describe User do
      describe "#full_name" do
    
        before do
          @user = User.create!(first_name: "Johnny", last_name: "Bravo")
        end
    
        it "is composed of first and last name" do
          expect(@user.full_name).to eql("Johnny Bravo")
        end
      end
    end
    

    It triggers an interaction with database and disk drive to test logic which does not require it at all. Using User.new would suffice. Multiply that by the number of examples reusing that before block and all similar occurrences and you get a lot of overhead.

    All model, no unit tests

    MVC plus services is good enough for typical Rails apps when they’re starting out. Over time, however, the application can benefit from some domain specific classes and modules. Having all logic in models ties it to the database, eventually leading to tests so slow that you avoid running them.

    If your experience with testing started with Rails, you may be associating unit tests with Rails models. However, in “classical” testing terminology, unit tests are considered to be mostly tests of simple, “plain-old” objects. As described by the test pyramid metaphor, unit tests should be by far the most frequent and the fastest to run.

    Not having any unit tests in a large application is not technically an antipattern in testing, but an indicator of a possible architectural problem.

    Take for example a simple shopping cart model:

    class ShoppingCart < ApplicationRecord
      has_many :cart_items
    
      def total_discount
        cart_items.collect(&:discount).sum
      end
    end
    

    The test would go something like this:

    require "rails_helper"
    
    RSpec.describe ShoppingCart do
    
      describe "total_discount" do
    
        before do
          @shopping_cart = ShoppingCart.create!
    
          @shopping_cart.cart_items.create!(discount: 10)
          @shopping_cart.cart_items.create!(discount: 20)
        end
    
        it "returns the sum of all items' discounts" do
          expect(@shopping_cart.total_discount).to eql(30)
        end
      end
    end
    

    Observe how the logic of ShoppingCart#total_discount actually doesn’t care where cart_items came from. They just need to be present and have a discount attribute.

    We can refactor it to a module and put it in lib/. That would be a good first pass — merely moving a piece of code to a new file has benefits. I personally prefer to use modules only when I see behavior which will be shared by more than one class. So let’s try composition. Note that in real life you usually have more methods that can be grouped together and placed in a more sophisticated directory structure.

    # lib/shopping_cart_calculator.rb
    class ShoppingCartCalculator
    
      def initialize(cart)
        @cart = cart
      end
    
      def total_discount
        @cart.cart_items.collect(&:discount).inject(:+)
      end
    end
    

    To test the logic defined in ShoppingCartCalculator, we now no longer need to work with the database, or even Rails at all:

    require 'shopping_cart_calculator'
    
    RSpec.describe ShoppingCartCalculator do
    
      describe "#total_discount" do
    
        before do
          cart = double
          cart_items = [double(discount: 10),
                        double(discount: 20)]
          allow(cart).to receive(:cart_items).and_return(cart_items)
    
          @calculator = ShoppingCartCalculator.new(cart)
        end
    
        it "returns the sum of all cart items' discounts" do
          @calculator.total_discount.should eql(30)
        end
      end
    end
    

    Notice how the spec for the calculator is requiring spec_helper instead of rails_helper. Requiring rails_helper in a spec file generally means that your whole Rails application needs to load before the first example runs. Depending on your machine and application size, running a single spec may then take from a few to 30+ seconds. This is avoided with use of application preloading tools such as Spring. It is nice, however, if you can achieve indepedence of them.

    To get more in-depth with the idea I recommend watching the Fast Rails Tests talk by Corey Haines. Code Climate has a good post on practical ways to decompose fat ActiveRecord models.

    Note that new Rails 5 apps are encouraged to use concerns, which are a handy way of extracting model code that DHH recommended a while ago.

    The elegance of the entire suite being extremely fast, not requiring rails_helper, etc, is secondary in my opinion. There are many projects that can benefit from this technique even partially applied, because they have all logic in models, as in thousands of lines of code that can be extracted into separate modules or classes. Also, do not interpret the isolated example above as a call to remove all methods from models in projects of all size. Develop and use your own sense when it is a good moment to perform such architecture changes.

    Using let to set context

    Using let(:model) (from RSpec) in model tests may lead to unexpected results. let is lazy, so it doesn’t execute the provided block until you use the referenced variable further in your test. This can lead to errors which can make you think that there is something wrong with the database, but of course it is not; the data is not there because the let block has not been executed yet. Non-lazy let! is an alternative, but it’s not worth the trouble. Simply use before blocks to initialize data.

    RSpec.describe ShoppingCart do
    
      describe ".empty_carts" do
        let(:shopping_cart) { ShoppingCart.create }
    
        it "returns all empty carts" do
          # fails because let is never executed
          expect(ShoppingCart.empty_carts).to have(1).item
        end
      end
    end
    

    On a sidenote, some people prefer not to use let at all, anywhere. The idea is that let is often used as a way to “DRY up” specs, but what is actually going on is that we’re trying to hide the complexity of the test setup, which is a code smell.

    Incidental database state

    Magic numbers and incidental state can sneak in specs that deal with database records.

    RSpec.describe Task do
      describe "#complete" do
    
       before do
         @task = create(:task)
       end
    
        it "completes the task" do
          @task.complete
    
          # Creating another completed task above while extending the test suite
          # would make this test fail.
          expect(Task.completed.count).to eq(1)
        end
    
        # Take 2: aim to measure in relative terms:
        it "completes the task" do
          expect { @task.complete }.to change(Task.completed, :count).by(1)
        end
      end
    end
    

    In this article, we’ve covered most common antipatterns in writing tests for Rails applications when testing models. Have some questions or examples of your own? Share them in the comments below.

    Want to focus on writing code and not worry about how your tests run?
    Try Semaphore, the simplest and fastest CI/CD for free. Among other helpful features, it has Test Reports that allow you to get an overview of how your test suite is doing. See skipped and failed tests and find the slowest tests – all from the Tests Dashboard.

    Post updated on 9 May 2018 by Thiago Araújo Silva.

    Leave a Reply

    Your email address will not be published. Required fields are marked *

    Avatar
    Writen by:
    Marko Anastasov is a software engineer, author, and co-founder of Semaphore. He worked on building and scaling Semaphore from an idea to a cloud-based platform used by some of the world’s engineering teams.