Mark Needham

I managed to create an interesting problem for myself while playing around with some code whereby I was ending up with validation errors appearing twice every time I called ‘valid?’ on a specific model.

I figured I was probably doing something stupid and in fact a few replies by Aaron Baldwin on a mailing list thread on ‘rubyonrails-talk’ helped explain exactly what I’d done:

Are you calling require ‘employee’ anywhere? If so you are likely
causing the model to load twice which causes duplicate errors because
the validates_presence_of method gets called twice.

I’d put the following code into a controller elsewhere somewhat unnecessarily since it didn’t seem to be picking up the location of my model at the time:

code_submissions_controller.rb

require 'models/code_submission'
 
class CodeSubmissionsController < ApplicationController
  def new
    CodeSubmission.new  
  end
end

require doesn’t load a file if it’s already been included but Aaron points out why it does on this occasion:

You are right that “require” will only load the file once. But if you load the class another way calling “require” will load it again.

As I understand it that controller code would also implicitly require ‘code_submission’ by the convention of inserting an underscore between the ‘CodeSubmission’ constant’s names.

We therefore effectively have the following two requires:

require 'code_submission'
require 'models/code_submission'

Which explain how the file gets loaded twice and therefore why the validation method fires twice and therefore creates two errors!

We’re using Web Mock on my current project to stub out some of the external web requests in some of our integration tests and I managed to get myself very confused while trying to use a variable inside a regular expression that I was trying to pass to the ‘stub_request’ method.

The code was roughly like this:

some_url = "http://service.com/method"
 
stub_request(:any, /some_url/).
        to_return(:body => File.new('/path/to/some.xml'),
                                      :headers => {'Content-Length' => 666, 'Content-Type' => 'text/xml'},
                  :status => 200,
                  :headers => {'Content-Type' => 'text/xml'})

The request was being stubbed when I hard coded the url inside the regular expression but not being stubbed when I used the variable like in the example above.

I somehow missed the blindingly obvious in hindsight solution of treating variables inside a regular expression as if they were being used in a string.

If we wrap the variable inside ‘#{}’ then our problem is solved:

some_url = "http://service.com/method"
 
stub_request(:any, /#{some_url}/).
        to_return(:body => File.new('/path/to/some.xml'),
                                      :headers => {'Content-Length' => 666, 'Content-Type' => 'text/xml'},
                  :status => 200,
                  :headers => {'Content-Type' => 'text/xml'})

I still can’t quite believe I didn’t spot it straight away.

An approach to testing which I haven’t seen before and am therefore assuming is more specific to Ruby is the idea of stubbing or mocking out functions on the system under test.

I’ve come across a couple of situations where this seems to be done:

1. When stubbing out calls to methods which are being mixed into the class via a module
2. When stubbing out calls to private methods within the class

I think the first approach is fine because typically we’ll have a direct test against that module’s methods elsewhere and it doesn’t make much sense to test the same thing again.

I initially liked the second approach as well because it simplifies the spec we’re writing.

For example if we have this code:

class FoosController : ApplicationController
   def index
      populate_something
   end
 
   private
 
   def populate_something
      @something = Something.find(...)    
   end
end

Then by stubbing out ‘populate_something’ we can write a spec like this:

describe FoosController do
   it "should populate something" do
      controller.stub(:populate_something) # and so on
 
      get :index
   end
end

Rather than:

describe FoosController do
   it "should populate something" do
      Something.stub(:find) # and so on
 
      get :index
   end
end

This is a fairly contrived example of course and the temptation to stub out a method on the system under test increases as the amount of dependencies and the complexity of them increases.

The problem we create for ourselves is that if we want to change the internal structure of our class, perhaps by inlining those private methods, then we’ll end up with specs failing even if the external behaviour remains exactly the same.

My current thinking is that if we get in a situation where we want to stub out something on the system under test then it’d probably be better to listen to that pain and try and work out a cleaner way of solving the problem.

I’ve been browsing through some of the Rails routing code while following Jamis’ Buck’s blog post and I came across something I hadn’t seen before while inside the ‘NamedRouteCollection’ class.

The bit of code which initially confused me is in RouteSet.add_named_route:

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

module ActionController
  module Routing
    class RouteSet
      def initialize
        ...
        self.named_routes = NamedRouteCollection.new
      end
 
      def add_named_route(name, path, options = {})
        # TODO - is options EVER used?
        name = options[:name_prefix] + name.to_s if options[:name_prefix]
        named_routes[name.to_sym] = add_route(path, options)
      end	
    end
  end
end

Reading the code on line 12 I was convinced that this code was being used to set a value into an array or hash so I was confused as to how the url/path helper methods which get added for named routes were being created since there didn’t seem to be any code which was calling the method in ‘NamedRouteCollection’ which would create them.

I eventually stumbled into the initializer code above which made me realise that ‘named_routes’ wasn’t actually a hash or array but an instance of ‘NamedRouteCollection’.

The methods ‘[]=’ and ‘[]‘ are aliases which call the ‘add’ and ‘get’ methods on ‘NamedRouteCollection’

module ActionController
  module Routing
    class RouteSet
      class NamedRouteCollection
        def add(name, route)
          routes[name.to_sym] = route
          define_named_route_methods(name, route) # creates the helper methods
        end
 
        def get(name)
          routes[name.to_sym]
        end
 
        alias []=   add
        alias []    get
      end
    end
  end
end

For me it seems like perhaps the creation of the helper methods could be the responsibility of another object although I can see why it’s been put in NamedRouteCollection since those helper methods are only created if you have a named route.

Either way it was pretty confusing for me initially that you could create this type of side effect from a method call that looked like it was just adding a key, value pair to an array.

I’ve been pairing a fair bit with Ashwin this week and one thing he showed me which I hadn’t previously seen is the ability to set a default value for a hash which gets returned if we search for a key that doesn’t exist.

This is an idea that I originally came across while playing around with Clojure but with Clojure the default value was defined in the calling code rather than in the hash definition.

For example:

(def scores {:mark 10 :dave 20})
 
(get scores :tony :0)

For a similar piece of code in Ruby we could write the following:

scores = { :mark => 10, :dave => 20 }
scores.default = 0

Now if we search for a key that doesn’t exist in the hash then we’ll get the value 0:

scores[:some_other_guy]
0

We’re using this in a couple of places in our code base already but it’ll be particularly useful for an upcoming piece of functionality where we want to vary the look and feel of some pages depending on certain criteria.

For the majority of users the look and feel will remain the same but for some of them it will vary slightly. Controlling that behaviour by using a hash with some default settings therefore seems like a reasonably good fit.

Something which I keep forgetting is how to write controller tests where I want to check whether an action correctly redirected to another action.

With most of the routes in our application we’ve created a ‘resourceful route’ where each action maps to a CRUD operation in the database.

We can do that with this type of code in routes.rb:

ActionController::Routing::Routes.draw do |map|
	map.resources :foos
end

Several helper methods based on named rotes get created and included in our controllers when we do this and we have access to those inside our specs.

We can see the named routes by executing the following command from the terminal:

mneedham@markneedham.local ~/SandBox/rails_test$ rake routes CONTROLLER=foos
(in /Users/mneedham/SandBox)
    foos GET    /foos(.:format)          {:action=>"index", :controller=>"foos"}
         POST   /foos(.:format)          {:action=>"create", :controller=>"foos"}
 new_foo GET    /foos/new(.:format)      {:action=>"new", :controller=>"foos"}
edit_foo GET    /foos/:id/edit(.:format) {:action=>"edit", :controller=>"foos"}
     foo GET    /foos/:id(.:format)      {:action=>"show", :controller=>"foos"}
         PUT    /foos/:id(.:format)      {:action=>"update", :controller=>"foos"}
         DELETE /foos/:id(.:format)      {:action=>"destroy", :controller=>"foos"}

The following helper methods would be created for this resource as per the documentation in resources.rb:

# Named Route Helpers
# ============ =====================================================
# foos foos_url, hash_for_foos_url,
# foos_path, hash_for_foos_path
#
# foo foo_url(id), hash_for_foo_url(id),
# foo_path(id), hash_for_foo_path(id)
#
# new_foo new_foo_url, hash_for_new_foo_url,
# new_foo_path, hash_for_new_foo_path
#
# edit_foo edit_foo_url(id), hash_for_edit_foo_url(id),
# edit_foo_path(id), hash_for_edit_foo_path(id)

Keeping this in mind means that if we do something like this inside our ‘create’ action:

class FoosController < ApplicationController
   def create
      ...
      redirect_to :action => :index
   end
end

We’d be able to test that redirection with a spec along the following lines:

describe "POST create" do
   it "should redirect back to the index page" do
      post :create, :foo => { :bar => "value" }
 
      response.should redirect_to foos_path
   end
end

We can also use any of those other helper methods inside our tests which seems obvious looking at it now but wasn’t for me until a colleague pointed it out.

It is pretty cool to be able to write specs like this and they seem much more readable than the equivalents you’d be able to test around ASP.NET MVC code in C#.

Ashwin and I have been working recently on a bit of code to make it possible to ‘soft delete’ some objects in our system.

We’re doing this by creating an additional column in that table called ‘deleted_at_date’ which we populate if a record is ‘deleted’.

As we wanted the rest of the application to ignore ‘deleted’ records we added a default scope to it:

class Foo < ActiveRecord::Base
   default_scope :conditions => "deleted_at_date is null"   
end

This works fine but we wanted to be able to see the status of all the records in IRB and with the default scope ‘Foo.all’ no longer returns ‘soft deleted’ records.

Luckily Active Record provides a protected method called ‘with_exclusive_scope‘ which we can use to get around this:

Foo.send(:with_exclusive_scope) { Foo.find(:all) }

Since it’s a protected method we can only access it in IRB by using ‘send’ which is a bit hacky, something David Heinemeier Hansson would refer to as syntactic vinegar.

Interestingly our first attempt to use ‘with_exclusive_scope’ involved writing the above code like this…

Foo.send(:with_exclusive_scope) { find(:all) }

..which results in the following error because when the closure was created ‘self’ referred to the main object rather than to ‘Foo’:

NoMethodError: undefined method `find' for #<Object:0xb77cd94c>
    from (irb):62
    from /var/lib/gems/1.8/gems/activerecord-2.3.5/lib/active_record/base.rb:2143:in `with_scope'
    from /var/lib/gems/1.8/gems/activerecord-2.3.5/lib/active_record/base.rb:2151:in `with_exclusive_scope'
    from (irb):62:in `send'
    from (irb):62
    from :0

Since the main object has no method called ‘find’ we get an exception thrown.

One of the things I’ve been finding when debugging Cucumber specs is that due to the number of levels of indirection present in those examples it becomes quite difficult to work out exactly how certain pieces of code got called.

In one cuke we were trying to work out how 4 objects of the same type were ending up in the database when it seemed like there should only be two.

Having failed to figure it out just by reading the code we resorted to putting calls to Kernel.caller inside the Factory Girl setup code so we could see how we’d ended up at that code:

p caller.inspect

It’s not a perfect solution – since the scenarios we write get parsed by Cucumber it’s hard to tell exactly which line the call stack started on – but it provided enough information for us to track down the steps which were calling the Factory Girl code in question.

One thing I’ve noticed while browsing our Rails code base is that the first entry point inside a controller is much less frequently the method corresponding to the action than it would be with a C# ASP.NET MVC application.

The concept of filters exists in ASP.NET MVC but on the projects I’ve worked on they’ve been used significantly less than before filters would be in a Rails application.

As a result I’m getting much more in the habit of checking for the before filters in the ApplicationController when an action isn’t working as expected to try and figure out what’s going on.

One which caught us out the other day is the ‘rescue_from’ method which gets mixed in from Rescuable.

Every single time we tried to login we were just getting redirected back to the login page again with no exception being reported anywhere.

Eventually we realised that the InvalidAuthenticityToken exception was being thrown because we somehow had a different token stored in our browser’s cookie than was in the form.

Since our LoginController extends the ApplicationController, the following logic was being used to handle the exception:

class ApplicationController < ActionController::Base
   rescue_from ActionController::InvalidAuthenticityToken, :with => :redirect_to_referer
 
  def redirect_to_referer_or_path
    redirect_to request.referer
  end

We initially didn’t spot it because it was hidden away between a whole load of other before filters and includes of modules.

The convention as far as I can tell is to try and keep as little code as possible inside the action method and handle common logic – such as looking up a user based on an id or handling exceptions – in filters.

It helps to keep the code inside actions pretty clean and remove duplication but I just need to remember to check both the action and the top of the class for any filters when investigating problems.

We recently had a spec which was checking that we didn’t receive a call to a specific method on an object…

describe "Our Object" do
	it "should not update property if user is not an admin" do
		our_user = Factory("user_with_role_x)
		User.stub!(:find).and_return(our_user)
 
		user.stub!(:is_admin?).and_return(false)
		user.should_not_receive(:property)
	end
end

…where ‘property’ refers to a field in the users table. In the code ‘property’ would get set like this:

class ObjectUnderTest
 
	def method_under_test
		user = User.find 4
 
		if user.is_admin?
			user.property = "random value"			
		end
	end

That test always passed but when we wrote the test to check that ‘property’ would be set if the user was an admin we found that it always failed.

The mistake is quite simple – the method that we wanted to check was received is actually called ‘property=’ rather than ‘property’!

Changing the spec to read like this solves the problem:

describe "Our Object" do
	it "should describe something" do
		our_user = Factory("user_with_role_x)
		User.stub!(:find).and_return(our_user)
 
		user.stub!(:is_admin?).and_return(false)
		user.should_not_receive(:property=)
	end
end

It didn’t take that long to figure it out but this is certainly a mistake you wouldn’t be able to make in a statically typed language.