Jay Fields' Thoughts

In a previous entry I discussed why I prefer One Assertion Per Test. In that entry I give a few state based examples, but didn't address the topic of mocks. This entry is going to focus on how you can use mocks while maintaining focus on maintainability.

Let's start with an example.

require 'test/unit'
require 'rubygems'
require 'mocha'

module Expectations
  class SuiteRunner
    attr_accessor :suite
  
    def initialize
      self.suite = Expectations::Suite.new
    end
  
    def self.suite_eval(&block)
      self.new.suite.instance_eval &block
    end
  end
end

class ObjectTests < Test::Unit::TestCase
  def test_suite_eval_evals_the_block_in_the_context_of_the_suite
    suite = mock
    suite.expects(:instance_eval)
    runner = mock
    runner.expects(:suite).returns(suite)
    Expectations::SuiteRunner.expects(:new).returns runner
    Expectations::SuiteRunner.suite_eval {}
  end
end

The SuiteRunner class is fairly straightforward, it's simply delegating the block on to the instance_eval method on the suite attribute, which is an instance of the Suite class. Even though the class isn't doing anything very interesting the test isn't completely simple. Due to the chain of method calls you need to set up 2 mocks and a total of 3 expectations. While the test isn't unmaintainable, if the description (the method name) isn't kept up to date you could easily lose the intent of the test.

A step in the right direction toward making this test more readable is to introduce a few stubs. However, before we decide what to stub we need to decide what this test is trying to verify. Given the description it seems that eval'ing in the context of the suite is what is being tested, thus the suite mock should probably remain. Since the suite mock is the focus of the test, the rest of the mocks are probably better expressed as stubs. The test can be written more concisely now that we've chosen to use stubs.

class ObjectTests < Test::Unit::TestCase
  def test_suite_eval_evals_the_block_in_the_context_of_the_suite
    suite = mock
    suite.expects(:instance_eval)
    Expectations::SuiteRunner.stubs(:new).returns stub(:suite => suite)
    Expectations::SuiteRunner.suite_eval {}
  end
end

With one mock and one expectation the test expresses it's purpose. Code should not only express how it works, but also why it's been written in a particular way. The new test not only executes, but it conveys the intent of the test (even if the description becomes stale). Converting that test was easy, but tests that have behavior expectations and state based assertions can be a bit more complicated to clean up.

require 'test/unit'
require 'rubygems'
require 'mocha'

class ReservationService
  # implementation
end
class MaidService
  # implementation
end
class VipService
  # implementation
end

class HotelRoom
  attr_accessor :booked
  def book_for(customer)
    reservation = ReservationService.reserve_for(customer)
    self.booked = true
    MaidService.notify(reservation)
    VipService.notify(reservation) if reservation.for_vip?
    reservation.confirmation_number
  end
end

class HotelRoomTests < Test::Unit::TestCase
  def test_book_for_returns_confirmation_number
    customer = mock
    room = HotelRoom.new
    reservation = mock
    reservation.expects(:for_vip?).returns true
    reservation.expects(:confirmation_number).returns 1979
    ReservationService.expects(:reserve_for).with(customer).returns(reservation)
    MaidService.expects(:notify).with(reservation)
    VipService.expects(:notify).with(reservation)
    assert_equal 1979, room.book_for(customer)
    assert_equal true, room.booked
  end
end
# >> Loaded suite -
# >> Started
# >> .
# >> Finished in 0.001121 seconds.
# >> 
# >> 1 tests, 7 assertions, 0 failures, 0 errors

There's a lot going on in this test. Test::Unit even reports that 7 assertions have been met. Unfortunately, with that much going on in a test, it's hard to tell what the original intent of the test was. Even worse, it would be hard to change anything in the book_for method without breaking this test. The test is completely tied to the implementation and if that implementation changes you'll break one or more of the assertions within the test. What really bothers me about this test is the ability to break "one or more" assertions. If you change the reserve_for method to also take the hotel as an argument the test will immediately stop executing with the following error.

# >> Loaded suite -
# >> Started
# >> F
# >> Finished in 0.001728 seconds.
# >> 
# >>   1) Failure:
# >> test_book_for_returns_confirmation_number(HotelRoomTests)
# >>     [(eval):1:in `reserve_for'
# >>      -:18:in `book_for'
# >>      -:36:in `test_book_for_returns_confirmation_number']:
# >> #<Mock:0x50f2a8>.reserve_for(#<Mock:0x50fa50>, #<HotelRoom:0x50fadc>) - expected calls: 0, actual calls: 1
# >> Similar expectations:
# >> #<Mock:0x50f2a8>.reserve_for(#<Mock:0x50fa50>)
# >> 
# >> 1 tests, 0 assertions, 1 failures, 0 errors

Knowing that an assertion failed is good, knowing that other assertions might also fail when you fix the first problem is not so good. The problem with this test is that it's verifying 7 different things. This can most likely be resolved by writing several different tests that all focus on one thing. (There are several tests that can be written, here are 5 examples)

require 'test/unit'
require 'rubygems'
require 'mocha'

class ReservationService
  # implementation
end
class MaidService
  # implementation
end
class VipService
  # implementation
end

class HotelRoom
  attr_accessor :booked
  def book_for(customer)
    reservation = ReservationService.reserve_for(customer, self)
    self.booked = true
    MaidService.notify(reservation)
    VipService.notify(reservation) if reservation.for_vip?
    reservation.confirmation_number
  end
end

class HotelRoomTests < Test::Unit::TestCase
  def test_book_for_reserves_via_ReservationService
    room = HotelRoom.new
    ReservationService.expects(:reserve_for).with(:customer, room).returns(stub_everything)
    MaidService.stubs(:notify)
    room.book_for(:customer)
  end
  
  def test_book_for_notifys_MaidService
    reservation = stub_everything
    MaidService.expects(:notify).with(reservation)
    ReservationService.stubs(:reserve_for).returns(reservation)
    HotelRoom.new.book_for(:customer)
  end
  
  def test_book_for_notifys_VipService_if_reservation_if_for_vip
    reservation = stub_everything(:for_vip? => true)
    VipService.expects(:notify).with(reservation)
    MaidService.stubs(:notify)
    ReservationService.stubs(:reserve_for).returns(reservation)
    HotelRoom.new.book_for(:customer)
  end
  
  def test_book_for_sets_booked_to_true
    room = HotelRoom.new
    MaidService.stubs(:notify)
    ReservationService.stubs(:reserve_for).returns(stub_everything)
    room.book_for(:customer)
    assert_equal true, room.booked
  end
  
  def test_book_for_returns_confirmation_number
    reservation = stub_everything
    reservation.stubs(:confirmation_number).returns 1979
    ReservationService.stubs(:reserve_for).returns(reservation)
    MaidService.stubs(:notify)
    assert_equal 1979, HotelRoom.new.book_for(:customer)
  end
end
# >> Loaded suite -
# >> Started
# >> .....
# >> Finished in 0.002766 seconds.
# >> 
# >> 5 tests, 5 assertions, 0 failures, 0 errors

The above tests take more lines of code than the original, but they are far more maintainable. You can change the implementation of the book_for method in various ways and only break the tests that are relevant to the change. The tests have become more robust. They've also become more readable because they express what their focus is. The tests that have a mock with an expectation are written to test a behavior that is expected. The tests that have an assertion are written to statefully verify that the object has been set as expected. When the implementation does inevitably change the tests can communicate on their own what the original author intended.

Writing tests in this way is actually quite easy if you follow a few simple suggestions.

• If your test has an assertion, do not add any mock expectations (instead use stubs for any methods that need to be changed for the test).
• If you add a mock expectation you probably don't need it to return a meaningful value (since you won't be verifying that value).
• If you already have a mock expectation you should use the stubs method for any other method where you don't want to use the implementation.
• Do not use .with when using a stub unless you need to. If you are stubbing, you don't need to verify the arguments.
• When returning stubs, prefer stub_everything so that additional calls to the stub will not cause unnecessary exceptions to be raised.

The above suggestions focus on creating tests that assert (or expect) one thing at a time and specify as little other implementation as possible. Every expectation, .with, and .returns statement is specifying implementation. The less implementation you express in a test, the more that test becomes.

I consider each call to stub (or stub_everything), stubs, mock, expects, with, and returns to be 1 implementation specification point. Any test that has an implementation specification score of 5 or more has the high implementation specification smell. As with all smells, everything might be okay, but I'm going to take the extra time to see if anything can be broken up by moving responsibilities or creating smaller methods.

By limiting your tests to one assertion or one expectation you create tests that express their intent. By creating tests that specify as little implementation as possible you reduce the amount of noise taking away from the intent of the test. An additional consequence of creating tests that focus on intent is that they specify less and are therefore more robust.

Labels: mock, one assertion per test, Testing Refactorings

At one of last years Rails Edge conferences I was in a game of Werewolf with Chad Fowler and Dave Thomas. Given the current information I decided that Chad and Dave were in fact the werewolves. Of course, I was not even close to certain, but of the options, it was the one that made the most sense. I started explaining my thought process and suddenly there was a vote to see if I should be killed. This did not surprise me. In life you will generally be better off taking the side of Chad Fowler and Dave Thomas if you find yourself in a situation where they both disagree with me. But, this time, I was right, they killed me and the game ended all villagers dead.

Interestingly, this exact pattern often plays out in software development also. People are afraid to challenge leaders in general. Unfortunately, some of the best solutions are found because leaders are challenged. There's no question that I owe quite a bit of credit to the various team members I have had that wouldn't take my first solution as 'good enough'. I'm a better developer because of them, and the resulting code (some of which is available as Open Source) is higher quality.

Some leaders do not take criticism well. Please do not let that stop you from challenging them. If they are truly good leaders they will recognize that by challenging them, you are doing them a favor. If they do not recognize that, it's important for everyone else to be exposed to your ideas, so that a crowd can overturn an unfit leader.

So don't be afraid to challenge leaders, everyone will benefit from it.

Labels: leadership

For some reason, DRY and "type less" seem to have become equivalent to many in the the Ruby community. I believe that the essence of DRY is when an application change is localized to one area of the codebase. While DRY is important, it's no more important than maintainability and readability. Therefore, if logic is stored in one and only one location, but that location cannot be found or understood, you've traded maintainability and as a result have gained nothing.

But, I've grown tired of this debate. No one ever wins, instead people just get tired of listening so they stop interacting. If you want to define methods of a module in a loop that later gets included in 3 classes that have nothing else in common, you have the right to do that. In fact, I'm excited about helping you.

Here's how DRY your code could be.

# what's the sense in writing the class name? 
# you already wrote it once when you named the file
C do
  # attr_accessor clearly has 12 too many characters
  a :first_name, :last_name, :favorite_color
  # include is short, but i is good enough
  i Enumerable
  # ctor defines a constructor
  ctor { |*args| s.first_name, s.last_name = *args }
  # d allows you to define any method by calling it from d. 
  # you can also chain the calls together to if you have several methods that are similar.
  d.complete_info? { first_name && last_name && true }
  d.white?.red?.blue?.black? { |color| self.favorite_color.to_s == color.to_s.chomp("?") }
end

That's some DRY code, perhaps even Extremely DRY (EDRY). Code isn't any good unless it runs.

Foo < Enumerable                   # => true
f = Foo.new("Mike", "Ward")        # => #<Foo:0x1e6f4 @first_name="Mike", @last_name="Ward">
f.first_name                       # => "Mike"
f.last_name                        # => "Ward"
f.complete_info?                   # => true
f.red?                             # => false
f.favorite_color = :red            # => :red
f.red?                             # => true

It runs without problem. Dying for the implementation? No worries, here it is.

class Object
  def C(base_class=Object, &block)
    name = File.basename(eval("__FILE__", block.binding),".rb")
    klass = eval "class #{name.capitalize}; end; #{name.capitalize}", binding, __FILE__, __LINE__
    klass.class_eval(&block)
  end
  
  def s
    self
  end
end

class Class
  def ctor(&block)
    define_method :initialize, &block
  end

  def i(mod)
    include mod
  end
  
  def d
    DefineHelper.new(self)
  end
  
  def a(*args)
    attr_accessor(*args)
  end
end

class DefineHelper
  def initialize(klass)
    @klass = klass 
  end
  
  def method_stack
    @method_stack ||= []
  end
  
  def method_missing(sym, *args, &block)
    method_stack << sym
    if block_given?
      method_stack.each do |meth|
        @klass.class_eval do
          define_method meth do
            instance_exec meth, &block
          end
        end
      end
    end
    self
  end
end

# http://eigenclass.org/hiki.rb?instance_exec

module Kernel
  
  def instance_exec(*args, &block)
    mname = "__instance_exec_#{Thread.current.object_id.abs}_#{object_id.abs}"
    Object.class_eval{ define_method(mname, &block) }
    begin
      ret = send(mname, *args)
    ensure
      Object.class_eval{ undef_method(mname) } rescue nil
    end
    ret
  end

end

Of course, any EDRY code will be write only. It would be painful to try to actually read code written in this way, but that's already true of the 150 modules that each contain only one method definition. The difference is generally it's not immediately obvious when you first look at a codebase that has been DRYed up for the sake of keystroke savings. If the implementer instead chose to utilize EDRY, I'd know what I was getting into from the first file opened.

Labels: DRY, ruby

A friend, who has been a long time Java developer, asked me if there's any truth to the idea that you need to be a better programmer to utilize Ruby. While I wish it were true, I don't believe that to be the case.

The truth is, weak programmers probably wont get themselves in trouble with Ruby because they don't know enough to do anything interesting or dangerous. I've seen plenty of Ruby code that can be written in exactly the same way in Java or C#.

Average programmers are the most likely to cause problems because they can become drunk with their own power and do things that they shouldn't, but can. But, I'm not sure that the situation is any different in a statically typed language. I've seen interesting things like an application that utilized a dependency injection framework to create every object in the application. Clearly someone was overextending a concept and the project suffered because of it.

Good programmers are generally good regardless of the language. They tend to care about things like performance, maintainability, readability, and extendability. If you keep those things in mind, you generally don't get yourself in trouble no matter how much power a language bestows upon you.

You can write good or bad code in any language, and more junior team members slow productivity no matter what tools you give them. Utilizing a less powerful language just increases the chances that they will cause trouble in some other way. The only real way to ensure that a junior team member wont get you in to trouble is to work with them and raise their skill level.

Labels: ruby

Today I was browsing the web for Ruby/Rails conferences to submit speaking proposals to. Unfortunately there doesn't seem to be a list of Ruby/Rails conferences anywhere (that I could find). Hopefully this list will save some time for anyone else looking for a similar list.

• Scotland On Rails
• Ruby En Rails
• Rails To Italy
• Ostrava On Rails
• Confrencia Rails
• Ruby Conf
• RailsConf
• RailsConf Europe
• Ruby Fools - Copenhagen
• Ruby Fools - Oslo
• Acts As Conference
• Ruby & Python Conference
• Paris On Rails
• The Rails Edge
• RubyKaigi
• eRubyCon

U. S. Regional Conferences

• Mountain West Ruby Conf
• Gotham Ruby Conference
• Ruby East

What other Ruby/Rails conferences have I missed?

Labels: conference, rails, ruby

Several months ago Charles asked Is Werewolf Killing the Hackfest. Obie responded, noting that the open communication and friendliness in the community is a good thing. Obie was on the right track, but I don't think he hit it on the head.

It's very easy to forget that People Matter Most. A Hackfest might bring about a JRuby version of Mongrel, but a game of Werewolf with Charles ensures that I will feel comfortable contacting him with JRuby questions in the future. Those relationships are much more likely to ensure that my project succeeds than any framework would.

I love attending conferences, but I usually dislike conference sessions. There's generally 2 kinds of sessions: beginner and very specific. The beginner talks always put me to sleep because they cover what I could read in a blog entry in half the time. Conversely, the specific sessions rarely cover things I care about. It's very rare that I need to include a framework in my application and at the same time I happen to be at a conference where the framework is being shown. The specific sessions are good at showing what's available, but they are too detailed. Most framework sessions are better off as lightning (5 minute) talks.

So why do I love conferences? For the informal conversations that always occur. Happy hour used to be the place that I actually heard what the industry leaders were looking into. Happy hour is good, but industry leaders aren't always approachable. However, Werewolf puts you in a game where you are forced to interact with someone you may otherwise be apprehensive about talking to. Building relationships with industry leaders is the number one reason I attend conferences and Werewolf is the easiest way to expand your network.

While Hackfests are about building software for our community, Werewolf is about building the community itself. Because I believe that people matter most I also believe that Werewolf is actually the best way to spend your conference time.

Labels: conference

I'm asked fairly often what I do to stay current in the Ruby/Rails/Software Development communities. Previously, I published a list of the blogs I read, but manually posting that list now and then isn't really a long term solution.

So, to solve the issue I created whatdoiread.com. You can subscribe to the Ruby feed or the Software Development feed (or both) and you'll be reading the same blog posts that I read.

Other than blogs I also really like the audio and video content that's becoming more popular. In particular I try to stay up to date on the Rails Envy Podcast and Peepcode Screencasts.

Of course, I read books also, mostly Pragmatic Programmer titles these days, but I've also enjoyed the AW Professional Ruby Series. Generally I stick to titles that have been recommended by a few people I trust.

I also find conferences to be great places to meet the people who are shaping the industry. Some of the most enlightening conversations of my career have been over drinks when the sessions have ended.

Lastly, working for ThoughtWorks is the best possible career decision I could have made. The internal mailing lists and the people I work with on a daily basis constantly challenge and educate me.

Labels: learning

I was recently working with some code where I wanted to initialize an object with a hash. The following example should illustrate what I'm trying to do.

Movie.new(:title => 'Godfather', :subtitles => 'English', 
          :audio_track_1 => "English", :audio_track_2 => "French")

I'm sure that doesn't look very surprising if you are familiar with creating ActiveRecord::Base (AR::B) subclasses; however, my current project has no database and the Movie class was not a subclass of AR::B. Additionally, the Movie instance needed readers for title, subtitles, audio_track_1, and audio_track_2, but the instance is immutable so writers are not necessary. Lastly, I wanted my Movie instances to be able to have any attribute that the hash passes in as a key.

Since I needed the attributes to be dynamic based on the hash I started with a loop that defined methods on the instance.

class Object
  def metaclass; class << self; self; end end
end

class Movie
  def initialize(options)
    metaclass.class_eval do
      options.each_pair do |key, value|
        define_method key do
          value
        end
      end
    end
  end
end

movie = Movie.new(:title => 'Godfather', :subtitles => 'English')
movie.title # => "Godfather"
movie.subtitles # => "English"

The above code works fine, and the constructor logic could be abstracted and defined in Class instead to make the Movie definition cleaner; however, I thought it would be nice to create a solution that didn't need to class_eval on the metaclass. Extending a module is another easy way to define additional behavior on an object, so I put together the following code that converts a hash into a module that has a method defined for each key and the return value of the method is the associated value.

class Hash
  def to_mod
    hash = self
    Module.new do
      hash.each_pair do |key, value|
        define_method key do
          value
        end
      end
    end
  end
end

With the Hash#to_mod method available the constructor of Movie simply extends the new instance with the module created by the hash.

class Movie
  def initialize(options)
    self.extend options.to_mod
  end
end

movie = Movie.new(:title => 'Godfather', :subtitles => 'English')
movie.title # => "Godfather"
movie.subtitles # => "English"

Pat Farley pointed out that this solution is also nice because it allows me to avoid costly typos by converting my Hash instances into an object with methods. For example, If I have a params hash in a controller I can easily create an object and access it's methods instead of using keys with the hash.

class PersonService
  def self.process(name, blog)
    # ...
  end
end

class PersonController < ApplicationController
  def create
    params # => {:person=>{:name=>"Pat Farley", :blog=>"http://www.klankboomklang.com/"}}
    person = Object.new.extend(params[:person].to_mod)
    PersonService.process(person.name, person.blog)
  end
end

Of course, Pat also pointed out that Hash#to_obj could be defined to make things look even a bit nicer.

class Hash
  def to_mod
    hash = self
    Module.new do
      hash.each_pair do |key, value|
        define_method key do
          value
        end
      end
    end
  end
  
  def to_obj
    Object.new.extend self.to_mod
  end
end

class PersonService
  def self.process(name, blog)
    # ...
  end
end

class PersonController < ApplicationController
  def create
    params # => {:person=>{:blog=>"http://www.klankboomklang.com/", :name=>"Pat Farley"}}
    person = params[:person].to_obj
    PersonService.process(person.name, person.blog)
  end
end

Lastly, Pat recommended a recursive version that would allow you to access the entire hash by way of method calls.

class Object
  def to_obj
    self
  end
end

class Hash
  def to_mod
    hash = self
    Module.new do
      hash.each_pair do |key, value|
        define_method key do
          value.to_obj
        end
      end
    end
  end
  
  def to_obj
    Object.new.extend self.to_mod
  end
end

class PersonService
  def self.process(name, blog)
    name # => "Pat Farley"
    blog # => "http://www.klankboomklang.com/"
  end
end

class PersonController < ApplicationController
  def create
    params # => {:person=>{:blog=>"http://www.klankboomklang.com/", :name=>"Pat Farley"}}
    PersonService.process(params.to_obj.person.name, params.to_obj.person.blog)
  end
end

Labels: hash, ruby

Update: Koz killed the ticket for now so don't bother +1ing it. I've posted the idea to the Rails Core list already here so I guess we'll all have to live with the flaw for now.

Update #2, Koz et al are on board with making the change assuming the community doesn't come up with any upgrade blockers. Cross your fingers, we might see the change in the near future.

I've always been slightly annoyed that ApplicationController does not follow convention and is defined in application.rb, but I never bothered to do anything about it, until now.

I was recently asked to look over a fairly new codebase that was being actively worked on by someone new to Rails. In one of the controllers I found code similar to the following snippet.

if RAILS_ENV == 'development'
  caches_page :index
end

The code functionally works, but I prefer to see environment specific logic in the environment specific files (development.rb, test.rb, and production.rb). I suggested that the developer set the caching logic in the development.rb file. Unfortunately, referencing a controller in development.rb will raise an error because ApplicationController is not yet available. This stems from the fact that require 'application' has not yet been executed and due to the naming inconsistency the class can not be auto-loaded. The fix is easy enough, you can require 'application' before you reference a controller, but I wouldn't expect any Rails novice to know that.

Is the inconsistency a blocker for Rails adoption? Absolutely not, but is it necessary? Again, absolutely not.

I wanted to patch* Rails and remove the inconsistency, so I started by creating a sample app to see what the change will break. Using Rails 2.0.2 I created a sample application and froze the 2.0.2 gems. Next I created a simple controller that would allow me to see what breaks when I change application.rb to application_controller.rb.

class HelloController < ApplicationController
  def world
    render :text => "hello world"
  end
end

At this point it's time to make the file change and see what breaks. After making the filename change I restart my server (using script/server) and to my surprise, nothing breaks. I refresh my browser and everything just works. Well, that was much better than expected, but I must have broken something or I expect this change would have already been made.

So after a bit of looking around I found that application.rb was explicitly being referenced in the following places:

• activesupport/lib/active_support/dependencies.rb
• actionpack/lib/action_controller/dispatcher.rb
• railties/lib/console_with_helpers.rb
• railties/lib/test_help.rb
• railties/lib/commands/performance/request.rb

activesupport/lib/active_support/dependencies.rb
and actionpack/lib/action_controller/dispatcher.rb
The special cases defined in these files are actually bypassed (invisibly to the user) when ApplicationController follows naming conventions.

railties/lib/console_with_helpers.rb
Making the change does break script/console. On line 19 'application' is explicitly required and since this file no longer exists an error is raised. The fix here is simple, remove the explicit require and let auto-loading take care of things.

railties/lib/test_help.rb
Line 1 explicitly requires application, again, the simple fix is to remove this line completely.

railties/lib/commands/performance/request.rb
Same as above, remove the explicit require on line 3 and everything just works.

Should you follow the instructions above? Maybe. If you never plan on changing your frozen version of rails then it might be a good idea. If you plan on changing your frozen version at some point but you think it's worth doing this work again at that point then it might also make sense. If you plan on changing your frozen version and you are worried that the change might cause issues in the future then you are probably better off living with the inconsistency for now.

I'm going to go ahead and make the change because I don't enjoy remembering the special case every time I want to Command+t and jump to the file.

Like I said before, I did the research so I could contribute back to the community by way of a Rails patch. I've put together a patch and submitted it, and it's gotten plenty of support, but it needs more, much more I suspect since Koz appears to have put it on hold. So, if you would also like to see the inconsistency removed please add a +1 to the ticket. (at http://dev.rubyonrails.org/ticket/10570)

* If you've ever wanted to submit a patch to Rails but not known where to start, I highly suggest starting at http://dev.rubyonrails.com.

Labels: rails