RSpec 3 added the concept of verifying doubles (As an aside the rspec 3 upgrade process was amazing because it treated deprecation as a feature rather than just vomiting untraceable warnings all over the place). Others have written at length about this but in a nutshell it means that things like

1
2

class_double(User, :name => 'Bob')
allow(some_user).to receive(:name).and_return('Bob')

will raise an exception if the User class does not have a name method. When you make a call to the name method on the (partial) double it will also check whether the arguments you pass are compatible with the method signature of the original object/class (including validating mandatory keywords).

This remedies my main niggle with mocks - it’s all to easy to remove or change code and miss a use of that code because its tests mock out the method question and so do not fail.

Over Easter I turned on this feature (it’s on by default in newly generated rspec configs) in our main application to see how much breakage it would cause and whether remedying that was a weekend project or something bigger. This app is over 4 years old, across 3 major rspec versions, many major rails versions (starting with 2.3.x) and a number of contributors so it’s definitely got some warts on it.

Some of the solutions I came up with might not be needed if some aspect of the app was rearchitected, but that would almost certainly put it out of scope for a weekend project (even if it was a 4 day weekend). I was also quite happy with the idea that some specs would need a workaround that was no better than an unverified double - I’ll happily have a few specs like that if the other 8000 have gained the safety net double verification affords.

TLDR;

Just do it - it didn’t take too long and I did find some genuine mistakes while doing this. You’ll also sleep safer at night.

The long version

There were several hundred failing specs, which made for some pretty verbose output (especially as many of the failure ended up calling inspect on a controller instance) but the failing specs fell broadly into these categories

• View specs that abused stubs to simulate local variables that would have been passed to the view.
• Views that relied on helpers other than what rspec-rails sets up by default.
• Helper specs that stubbed helper methods that are created on the controller and made available via helper_method.
• View & helper specs that stubbed methods on the controller.
• Specs for abstract classes that used stubs to implement the methods that a concrete subclass must implement.
• Cruft: specs stubbing methods that no longer exist, had typos in their name etc.

Better Helper detection

First up were the view specs. Out of the box rspec-rails will include ApplicationHelper and the controller’s helper in the view used for specs (it just gets the controller name from the example description and adds ‘Helper’ to the end). This sensible default covers most cases but we have some extra helper modules used by multiple controllers (eg a GarmentsHelper used by just about anyone that renders garments) as well as some helpers defined by helper_method. We’ve long supplemented rspec-rails defaults with this

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24

module ViewHelpersSpecHelper
  extend ActiveSupport::Concern

  included do
    base = metadata[:description].split('/')[0..-2].join('/')
    controller_name = base.camelize
    unless controller_name =~ /\wMailer/
    #for a mailer spec, the 'controller' class is just FooMailer
      controller_name += 'Controller'
    end

    controller_class = begin
      controller_name.constantize
    rescue NameError
      ApplicationController
    end
    helpers = controller_class._helpers
    before do
      view.singleton_class.class_eval do
        include helpers unless included_modules.include?(helpers)
      end
    end
  end
end

This starts off the same as rspec-rails but instead of finding the helper module we find the controller instead (the fallback to ApplicationController is for things like layouts). We then pick out the controller’s _helpers module and use that. This is more reliant on some actionpack internals but has the advantage of replicating the precise set of helpers available when the view is rendered in real usage.

Stealing controller methods

Next up were a bunch of view specs and helper specs that were mocking out bits of the controller. In the real world the controller is a subclass of ApplicationController and so has a bunch of functionality useful to us, but in these specs the controller is an instance of ActionView::TestCase::TestController which inherits straight from ActionController::Base. I didn’t want to just define stub methods on this class because that be effectively the same as the unverified doubles I’m trying to getting rid of. Rspec would check that my stubs matched methods on ActionView::TestCase::TestController but they might not actually exist (or exist with different signature) on ApplicationController.

I came up with this little helper

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15

def add_dummy_method(klass, method, should_exist_in:, &implementation)
  klass.send(:define_method, method, &implementation)
  if should_exist_in.instance_method(method).parameters != \
               klass.instance_method(method).parameters
    raise "Dummy method #{method} has different signature than #{should_exist_in.inspect}"
  end
end

class ActionView::TestCase::TestController
  add_dummy_method(self,
                   :display_navigation?,
                   should_exist_in: ApplicationController) {|foo|}
  #will raise unless ApplicationController has def display_navigation(foo); 
  ...
end

What this does is define an empty method of the same name, but then checks that the method exists in the named class (usually ApplicationController) and that it has the same method signature. This checks for number of arguments, default arguments (and their value) and required arguments. It’s a tad overzealous in that it also checks method names too. I used a similar method to lift methods from individual controllers where appropriate

Abstract classes

We had a few classes that looked like this:

1
2
3
4
5
6
7
8

class SomethingAbstract
# subclasses must implement foo
...
end

# something_abstract_spec.rb
instance = SomethingAbstract.new
allow(instance).to receive(:foo).and_return(123)

Of course this only works at all because ruby doesn’t have a concept of abstract classes as other languages do and was largely down to laziness. Either the base class should have a stub method definition (raise UnimplementedError if you’re concerned about people forgetting to override), which has the advantage of making it crystal clear what subclassers should do or create a (possbily anonymous) subclass that the specs can use

View locals

Quite a few of our view specs abused stubs to simulate passing local variables when rendering the view. You can of course add those options when you call render from the spec but then you have to also repeat the default arguments rspec passes by defeult. It also feels quite natural for that sort of setup to be in a before block. I found an issue from a few months ago where a user describes this problem but there hasn’t yet been any resolution. For now I stuck this in a module included in all view specs:

1
2
3
4
5
6
7
8
9
10
11

def _default_render_options
  super.merge(:locals => locals)
end

def locals
  @_locals || {}
end

def template_locals value
  @_locals = value
end

This means I can write

1
2
3
4
5
6
7
8
9
10
11

describe "foo/index.html.haml" do

  before(:each) do
    template_locals(bar: 'baz')
  end

  it "display bar" do
    render
    ...
  end
end

The Nuclear Option

If all else fails you could always use define_singleton_method to create the missing method before setting expectations on it, but if you do that you’re just writing unverified partial doubles using a slightly different syntax. Luckily I only had to resort to this twice (some gnarly code that assumes that extra attributes have been created on an AR object because it was loaded with a custom SELECT cause).

The Rest

The rest were basically genuine errors that would have gone undetected were it not for this exercise - things like typos in calls to receive stubs set for methods that have long since been removed. Hopefully we’ll be kept safer in the future too - a big round of thanks to the rspec core team!