Unit Testing Correctly

Ruby, Testing Posted on

Let's talk about testing. Testing is fun, it's awesome, and if you want to be agile, it's a necessity. But chances are, you're doing it wrong.

Before we dive into Chef, let's look at a small Ruby example. Consider a class writes a downloads an HTML page from a website and writes the contents to a file:

require 'net/http'

class Scraper
  attr_reader :webpage

  def initialize(webpage = 'https://www.sethvargo.com')
    @webpage = URI.parse(webpage)
    write Net::HTTP.get(@webpage)
  end

  def write(contents)
    File.open("#{@webpage.host}.html", 'w') do |file|
      file.write(contents)
    end
  end
end

Scraper.new

This class should download sethvargo.com to a text file in the current working directory (go ahead and test it out if you don't believe me).

The class has 3 primary functions:

  1. Parse out the URI (raises an except if the URI is bad)
  2. Perform an HTTP#get request
  3. Write the results of the request to a file

And here's the test:

describe Scraper do
  it 'parses the uri' do
    subject.webpage.should be_a URI
    subject.webpage.host.should == 'sethvargo.com'
  end

  it 'writes the file' do
    File.read('sethvargo.com.html').should match "Unit Testing Chef Cookbooks"
  end
end

And this seems like a good test. It's covering all your bases, touches every line of code. Nice. But it's fucking wrong! This is not a unit test!

Back Up

According to the authoritative source known as Wikipedia:

In computer programming, unit testing is a method by which individual units of source code, sets of one or more computer program modules together with associated control data, usage procedures, and operating procedures, are tested to determine if they are fit for use...

Let me break that down:

  1. individual units of source code
  2. control data
  3. fit for use

In our former example, we are actually performing this half-functional, half-integration, half-unit test.

In a true unit test, you control the universe. In this example Net::HTTP, URI, and File are outside our "Scraper" universe. This is bad - we are relying on a third-party, another universe, in our tests. In this example, we are testing the end result of our class, which seems like a good idea, but isn't a true unit test.

Additionally, there are more issues with that unit test:

  • It requires a valid Internet connection to work
  • It's slowwwwwwww (network and file operations)
  • It's not idempotent - it may fail future runs because the content of 'sethvargo.com' hash changed

Let's refactor:

describe Scraper do
  let(:webpage) { double('webpage') }
  let(:response) { double('response') }

  before do
    URI.stub(:parse).and_return(webpage)
    Net::HTTP.stub(:get).and_return(response)
    webpage.stub(:host).and_return('sethvargo.com')
  end

  context '#initialize' do
    it 'sends the parse message URI' do
      URI.should_receive(:parse).with('https://www.sethvargo.com')
    end

    it 'sends a #get request to Net::HTTP' do
      Net::HTTP.should_receive(:get).with(webpage)
    end

    it 'sends the #write message with the response' do
      subject.should_receive(:write).with(response)
    end
  end

  context '#write' do
    let(:file) { double('file') }

    before do
      File.stub(:open).with(any_args()).and_yield(file)
    end

    it 'sends the #open message to File' do
      File.should_receive(:open).with('sethvargo.com.html', 'w')
      file.should_receive(:write).with(response)
    end
  end
end

This is a "true" unit test. We have full control over our entire test. We don't have the latency of Net::HTTP actually making a request or File.open writing to the file. Plus we are testing that messages are actually being sent.

The most common argument I receive for this test is:

But how do you know the file was written?

They go on to say: "You only test that the File.open message was sent - you aren't testing that the file was created." My answer: yes. Because that's not a unit test. That file exists outside of your universe.

I follow that up with science: I don't have to. File.open is a core library in Ruby. It's tested because all the Ruby libraries are tested. I know that, if I send the message :open to the File class, yielding a block, it will write to the file. I don't need to test that. It's a double test. You wouldn't write a test to ensure 'foo' creates a String or {} creates a Hash or [] creates an Array, so why would you test File.open?

Please stop acceptance testing in your Unit tests. You're just causing yourself a headache down the future.

About Seth

Seth Vargo is an engineer at Google. Previously he worked at HashiCorp, Chef Software, CustomInk, and some Pittsburgh-based startups. He is the author of Learning Chef and is passionate about reducing inequality in technology. When he is not writing, working on open source, teaching, or speaking at conferences, Seth advises non-profits.