rails

Mocha expectations are incredibly useful for ruby unit testing. You can stub out all kinds of functionality you depend on, specify exactly what values those dependencies return, and validate that the object under test behaves exactly as you want it to, right down to the methods it does or doesn’t call.

Unfortunately, I’ve bumped up against one glaring case where this can get you into trouble. To make matters worse, the symptom of this situation is that your tests just always pass.

Expects never

Take a simple class like this:

class Foo
  def a_method
    another_method

  rescue => ex
    logger.error("#{ex}")
  end

  # ...

end

Say we want to update it so that another_method is only called if some condition is met.

Let’s play hardcore TDD here; we’ll write and run the test first:

class FooTest < Test::Unit::TestCase
  def test_a_method
    foo = Foo.new

    # condition met
    foo.stubs(:some_condition?).returns(true)

    # should call it
    foo.expects(:another_method).once
    foo.a_method

    # condition not met
    foo.stubs(:some_condition?).returns(false)

    # should not call it
    foo.expects(:another_method).never
    foo.a_method
  end
end

Simple, easy to follow – you should absolutely fail with “Unexpected invocation” on that second call due to the never expectation you’ve set. There’s no reason to run this test now right? You know it’s going to fail, right?

You run the test anyway, and it… Passes. Um, wat?

I know what you might be saying here, it’s as simple as a single postfix unless some_condition?. So why am I insisting on figuring this out, wasting time just to see this test fail before I implement?

Well, in actuality I didn’t do things in this order. I wrote the implementation, then ran the test, saw it pass and moved on. It was only later that I regressed, broke the implementation and didn’t find out for a good while because the test never started failing.

Luckily, it hadn’t gone to production, but this scenario makes a strong case for writing and running your tests before your implementations – it’s the only chance you have to ensure your test actually covers what it should.

Anti-rescue

Let me save you the frustration of debugging this. What’s happening here is that when the method gets (incorrectly) called, Mocha raises an ExpectationError which is (by design) promptly rescued and logged.

There are a couple of ways we can work around this limitation though. One approach could be to re-raise the error when testing:

  # ...

rescue => ex
  logger.error(...)

  raise ex if Rails.env.test?
end

That’s only moderately smelly and might suit you in most cases. In my case, I couldn’t do this because swallowing all errors was by-design and (of course) backed up with test coverage, so those would start failing if I re-raise in that environment.

That and I hate modifying implementation code specifically to support some testing-related concern.

Another option might be to specifically handle the Mocha exception:

  # ...

rescue Mocha::ExpectationException => ex
  raise ex
rescue => ex
  logger.error(...)
end

That exception class is not in scope when you’re running in production, so that wouldn’t be fun. And I’d be very against requiring the Mocha gem in non-test environments.

Rewrite never

Anyway, here’s the solution we ended up with: redefine the method to increment a class-level counter, then assert that it was never called by checking that counter afterwards.

class FooTest < Test::Unit::TestCase
  def test_a_method
    foo = Foo.new
    foo.stubs(:some_condition?).returns(false)

    assert_not_called(foo, :another_method) do
      foo.a_method
    end
  end

  private

  # Note: not thread-safe
  def assert_not_called(obj, method, &block)
    # set a class level counter
    @@counter = 0

    # redefine the method so, if called, it increments that counter
    obj.instance_eval %{
      def #{method}(*args)
        #{self.class}.instance_eval "@@counter += 1"
      end
    }

    # run your code
    yield

    # see if it was ever called
    assert_equal 0, @@counter, "#{obj}.#{method}: unexpected invocation."
  end
end

Now, do yourself a favor and run this test before you write the implementation. It’s the only way to be sure the test works and regressions will be caught down the line.

No one can deny that rails likes to do things for you. The term “auto-magically” comes to mind. This can be a blessing and a curse.

For the most part, rails tries to give you “outs” – a few hoops here and there that, if jumped though, will let you do things in different or more manual ways. Sometimes though, it doesn’t.

Find In Batches

One of the many ORM helpers provided by rails is find_in_batches. It will repeatedly query the database with a limit and offset, handing you chunks of records to work through in sequence. Perfect for processing a very large result set in constant memory.

Order.find_in_batches(:batch_size => 10) do |orders|
  orders.length # => 10

  orders.each do |order|

    # yay order!

  end
end

The problem is that any conditions you add to find_in_batches are inherited by any and all sql performed within its block. This is called “implicit scope” and there’s no way around it.

Why is this an issue? I’m glad you asked, here’s a real life example:

#
# SELECT * from orders
# WHERE orders.status = 'pending'
# LIMIT 0, 10;
#
# adjusting LIMIT each time round
#
Order.find_in_batches(:batch_size => 10,
                      :conditions => 'orders.status' = 'pending') do |orders|

  orders.each do |order|
    #
    # UPDATE orders SET orders.status = 'timing_out'
    # WHERE orders.id     = ?
    #   AND orders.status = 'pending'; <-- oh-hey implicit scope
    #
    order.update_attribute(:status, 'timing_out')

    #
    # some long-running logic to actually "time out" the order...
    #

    #
    # UPDATE orders SET orders.status = 'timed_out'
    # WHERE orders.id     = ?
    #   AND orders.status = 'pending';
    #
    order.update_attribute(:status, 'timed_out')
  end
end

Do you see the problem? The second update fails because it can’t find the order due to the implicit scope. The first update was only successful due to coincidence.

Workaround

I would love to find a simple remove_implicit_scope macro that can get around this issue, but it’s just not there.

I even went so far as to put the update logic in a Proc or lambda hoping to bring in a binding without the implicit scope – no joy.

I had to resort to simply not using find_in_batches.

At the time, I just rewrote that piece of the code to use a while true loop. Thinking about it now, I realize I could’ve factored it out into my own find_in_batches; also, I could put it in a module so you can extend it in your model to have the better (IMO) behavior…

module FindNoScope

  def find_in_batches(options)
    limit = options.delete(:batch_size)
    options.merge!(:limit => limit)

    offset = 0

    while true
      chunk = all(options.merge(:offset => offset))

      break if chunk.empty?

      yield chunk
    end

    offset += limit
  end

end

class Order < ActiveRecord::Base
  extend FindNoScope

  # ...

end

Note that the above was written blind, is completely untested, and will likely not work