Tests all the way down

August 28th, 2008 by ymendel 0 comments »

UPDATE: An updated gist of the code in this article can be found here.

A recent project had us using ThinkingSphinx for the first time. We've done some with UltraSphinx, but we were open to new ideas. And ReinH was (at least tangentionally) involved, so we bowed to the heavy weight of authority that comes from writing a blog post.

I admit that defining indexes in ThinkingSphinx is nicer than UltraSphinx, but there are method_missing and state-keeping tricks that give me pause. For me, though, it all comes down to writing the spec. Being BDD-infected as we are, writing code without specs hurts. It's simple to say that a column is indexed

class User < ActiveRecord::Base
  define_index do
    indexes email
  end
end

But how to write the failing test that passes when that code is put in?

(As an aside, we know that this sort of test is punting on the real problem. I'd like to see the behavior in action rather than just query the class to see if it has indexes. That's what I do with validations, but with searching it becomes a full-scale integration test involving a search server and too much pain to manage, so we punt. For the record, we asked around and apparently Evan Weaver is the only one who does that kind of integration testing. I figure it's kind of telling that only one person tests it that way, and it's the author of UltraSphinx.)

A spike showed us the way. It was a bit of a pain to traverse the maze of index information, and those method_missing tricks pained me. I don't have that console session to recreate here, but this is the final result:

User.indexes.collect(&:fields).flatten.collect(&:columns).flatten.collect(&:__name).include?(:email)

Now, one could say this convoluted access to indexing information means ThinkingSphinx wasn't written spec-first. I'm here to point out that BDD isn't a panacea. All BDD does is prove that the code matches the spec. As such there's always the looming specter of unspecified behavior, and of course nothing can save you when you've written the wrong specs.

Some time ago, we found a blog post called "If you aren't writing Matchers, you aren't using RSpec", and at the time I discounted it. Mostly I figure if the test looks like butt, I change the code to be easier to test. This situation was different for two reasons: 1) the code wasn't mine to change, and A) it was a severe time-crunch situation. So Rick and I paired on writing a matcher for this thing. And it worked. We referred to the article a few times since it was so informative. There was one glaring omission, though. There was a complete lack of tests.

Remember what I said about being BDD-infected? It's no joke. And I think the things you depend on in your tests need to be tested the best. They're a foundation of your entire testing structure. Would you want to build a house on quicksand? Now, this was test-after because we followed the article to write the matcher, but I still think the tests are nothing to sneeze at.

The matcher code (which went into spec/custom_matchers.rb):

module ThinkingSphinxIndexMatcher
  class HaveIndex
    def initialize(expected)
      @expected = expected
    end

    def matches?(target)
      @target = target
      target.indexes.collect(&:fields).flatten.collect(&:columns).flatten.collect(&:__name).include?(@expected)
    end

    def failure_message
      "expected #{@target} to index #{@expected}"
    end

    def negative_failure_message
      "expected #{@target} not to index #{@expected}, but did"
    end
  end

  def have_index(expected)
    HaveIndex.new(expected)
  end
end

The spec (which was in sanity/have_index_matcher_spec.rb):

require File.dirname(__FILE__) + '/../spec_helper'

describe ThinkingSphinxIndexMatcher::HaveIndex do
  before :each do
    @expected = 'some val'
    @target   = 'some tgt'
    @matcher  = ThinkingSphinxIndexMatcher::HaveIndex.new(@expected)
  end

  describe 'when initialized' do
    it 'should accept an expected value' do
      lambda { ThinkingSphinxIndexMatcher::HaveIndex.new(@expected) }.should_not raise_error(ArgumentError)
    end

    it 'should require an expected value' do
      lambda { ThinkingSphinxIndexMatcher::HaveIndex.new }.should raise_error(ArgumentError)
    end

    it 'should store the expected value' do
      ThinkingSphinxIndexMatcher::HaveIndex.new(@expected).instance_variable_get('@expected').should == @expected
    end
  end

  it 'should have a failure message' do
    @matcher.should respond_to(:failure_message)
  end

  describe 'failure message' do
    before :each do
      @matcher.instance_variable_set('@target', @target)
    end

    it 'should explain the problem, containing the target and expected value' do
      @matcher.failure_message.should == 'expected some tgt to index some val'
    end
  end

  it 'should have a negative failure message' do
    @matcher.should respond_to(:negative_failure_message)
  end

  describe 'negative failure message' do
    before :each do
      @matcher.instance_variable_set('@target', @target)
    end

    it 'should explain the problem, containing the target and expected value' do
      @matcher.negative_failure_message.should == 'expected some tgt not to index some val, but did'
    end
  end

  it 'should tell if a target matches the expectation' do
    @matcher.should respond_to(:matches?)
  end

  describe 'telling if a target matches the expectation' do
    before :each do
      @target.stubs(:indexes).returns([])
    end

    it 'should accept a target' do
      lambda { @matcher.matches?(@target) }.should_not raise_error(ArgumentError)
    end

    it 'should require a target' do
      lambda { @matcher.matches? }.should raise_error(ArgumentError)
    end

    it 'should set the target' do
      @matcher.matches?(@target)
      @matcher.instance_variable_get('@target').should == @target
    end

    it "should go through the target's indexes" do
      @target.expects(:indexes).returns([])
      @matcher.matches?(@target)
    end

    describe 'when the target has no indexes' do
      before :each do
        @target.stubs(:indexes).returns([])
      end

      it 'should return a false value' do
        result = @matcher.matches?(@target)
        (!!result).should == false
      end
    end

    describe 'when the target has indexes' do
      describe 'and an index includes the expected value' do
        before :each do
          indexes = [
            stub('index', :fields => []),
            stub('index', :fields => [stub('field', :columns => [])]),
            stub('index', :fields => [stub('field', :columns => [stub('column', :__name => 'blah')])]),
            stub('index', :fields => [stub('field', :columns => [stub('column', :__name => @expected)])]),
            stub('index', :fields => [stub('field', :columns => [stub('column', :__name => 'other'), stub('column', :__name => 'feh')])])
          ]
          @target.stubs(:indexes).returns(indexes)
        end

        it 'should return a true value' do
          result = @matcher.matches?(@target)
          (!!result).should == true
        end
      end

      describe 'and no index includes the expected value' do
        before :each do
          indexes = [
            stub('index', :fields => []),
            stub('index', :fields => [stub('field', :columns => [])]),
            stub('index', :fields => [stub('field', :columns => [stub('column', :__name => 'blah')])]),
            stub('index', :fields => [stub('field', :columns => [stub('column', :__name => 'other'), stub('column', :__name => 'feh')])])
          ]
          @target.stubs(:indexes).returns(indexes)
        end

        it 'should return a false value' do
          result = @matcher.matches?(@target)
          (!!result).should == false
        end
      end
    end
  end
end

describe ThinkingSphinxIndexMatcher do
  before :each do
    @expected = 'some val'
    @helper = Object.new
    @helper.extend(ThinkingSphinxIndexMatcher)
    @matcher = stub('matcher')
    ThinkingSphinxIndexMatcher::HaveIndex.stubs(:new).returns(@matcher)
  end

  it "should provide a 'have_index' method" do
    @helper.should respond_to(:have_index)
  end

  describe "'have_index' method" do
    it 'should accept an expected value' do
      lambda { @helper.have_index(@expected) }.should_not raise_error(ArgumentError)
    end

    it 'should require an expected value' do
      lambda { @helper.have_index }.should raise_error(ArgumentError)
    end

    it 'should create a matcher for the expected value' do
      ThinkingSphinxIndexMatcher::HaveIndex.expects(:new).with(@expected)
      @helper.have_index(@expected)
    end

    it 'should return the matcher' do
      @helper.have_index(@expected).should == @matcher
    end
  end
end

The addition to spec_helper:

require File.expand_path(File.dirname(__FILE__) + '/custom_matchers')

Spec::Runner.configure do |config|
  config.include ThinkingSphinxIndexMatcher
end

(These are all available at gist #7765)

Note that I never made a spec to test that have_index is available in the specs. Mea culpa, folks. I wasn't sure where those methods go and did I mention the time crunch? I think it would be easier if should returned an object that had methods directly called on it so things like respond_to and include weren't shoved into the wrong place. Remember the good ol' days when RSpec looked like object.should.be_nil? Apparently that was too ugly to keep.

All in all, this was something of an adventure. As I told Rein, when it comes to learning how something works, there's nothing quite like trying to figure out how in the hell to test it.

Oh, and in the end the spec looks like

describe User do
  describe 'as a class' do
    it 'should be searchable' do
      User.should respond_to(:search)
    end

    it 'should allow searching by email address' do
      User.should have_index(:email)
    end
  end
end

Leave a Reply