开发者

RSpec Refactoring Help

I'm a big noob when it comes to testing or test-driven development, so I'm really struggling with testing my code. The biggest reason why is situations like this...

I have a model, Photo, which has the fields "place", "city", "state", and "country".

I created a method, "location", which can accept one argument, and will return either the minimum form, short form, or full form location. Here's the model code:

def location( form=:full )
  usa = country.eql?('US') || country.eql?('United States')
  country_name = Carmen::country_name( self.country )

  if self.state.present?
    state_name = Carmen::state_name( self.state )
  end

  case [form, usa]
  when [:min, true] then ( self.city.blank? ? self.place : self.city )
  when [:min, false] then ( self.city.blank? ? self.place : self.city )
  when [:short, true] then [ ( self.city.blank? ? self.place : self.city ), state_name ].join(', ')
  when [:short, false] then [ ( self.city.blank? ? self.place : self.city ), country_name ].join(', ')
  when [:full, true] then [ self.place, self.city, state_name ].delete_if{|a| a.blank? }.join(', ')
  when [:full, false] then [ self.place, self.city, country_name ].delete_if{|a| a.blank? }.join(', ')
  else raise "Invalid location format"
  end
end

As you can see that's fairly clean and concise. Now, here's my spec:

describe "location" do

  before do
    @us = Photo.new(:place => "Main St.", :city => "Barville", :state => "TX", :country => "US")
    @uk = Photo.new(:place => "High St.", :city => "Barchester", :country => "GB")
    @it = Photo.new( :place => "il Commune", :city => "Baria", :state => "AR", :country => "IT" )
  end

  context "minimum form" do
    it "should return city or place for US Photos" do
      @us.location(:min).should == "Barville"
      @us.city = ""
      @us.location(:min).should == "Main St."
    end

    it "should return city or place for Internationals without states" do
      @uk.location(:min).should == "Barchester"
      @uk.city = ""
      @uk.location(:min).should == "High St."
    end

    it "should return city or place for Internationals with states" do
      @it.location(:min).should == "Baria"
      @it.city = ""
      @it.location(:min).should == "il Commune"
    end
  end

  context "short form" do
    it "should return city,state or place,state for US photos" do
      @us.location(:short).should == "Barville, Texas"
      @us.city = ""
      @us.location(:short).should == "Main St., Texas"
    end

    it "should return city,country or place,country for Internationals without states" do
      @uk.location(:short).should == "Barchester, United Kingdom"
      @uk.city = ""
      @开发者_C百科uk.location(:short).should == "High St., United Kingdom"
    end

    it "should return city,country or place,country for Internationals with states" do
      @it.location(:short).should == "Baria, Italy"
      @it.city = ""
      @it.location(:short).should == "il Commune, Italy"
    end
  end

  context "full form" do
    context "US Photos" do
      it "should return place, city, state" do
        @us.location(:full).should == "Main St., Barville, Texas"
      end

      it "should return place, state if city is blank" do
        @us.city = ""
        @us.location(:full).should == "Main St., Texas"
      end

      it "should return city, state if place is blank" do
        @us.place = ""
        @us.location(:full).should == "Barville, Texas"
      end
    end

    context "Internationals without states" do
      it "should return place, city, state" do
        @uk.location(:full).should == "High St., Barchester, United Kingdom"
      end

      it "should return place, state if city is blank" do
        @uk.city = ""
        @uk.location(:full).should == "High St., United Kingdom"
      end

      it "should return city, state if place is blank" do
        @uk.place = ""
        @uk.location(:full).should == "Barchester, United Kingdom"
      end
    end
  end

  context "Internationals with states" do
    it "should return place, city, state" do
      @it.location(:full).should == "il Commune, Baria, Italy"
    end

    it "should return place, state if city is blank" do
      @it.city = ""
      @it.location(:full).should == "il Commune, Italy"
    end

    it "should return city, state if place is blank" do
      @it.place = ""
      @it.location(:full).should == "Baria, Italy"
    end
  end
end

So, 98 lines of test code to test 17 lines of model code. That just seems insane to me. Plus the time it takes to test this out in RSpec is frankly a lot more than the time it takes to test it out in the console.

So, I have two questions:

  1. Surely there's a better way to do this. Can anyone suggest a refactoring?
  2. Is this ratio of test code to model code normal, and if so, why is it worth the time?

Thanks!!

-- EDIT --

Just to be clear, I'm mostly interested in a refactoring of the test code, not the location method.


I've found that a high test/code ratio is perfectly normal and even desired. Generally test code should be more "unravelled" than normal code. They're called examples for a reason: tests should show people how to use your code, and the best way to do that is to be stupidly simple with each example.

Writing relatively long-winded tests also helps you keep from having to debug them. Tests should be straightforward and readable rather than compact and efficient. It's much easier to understand what each test is doing when you don't have to first figure out any complicated loops, iterations, recursion or metaprogramming.

That said, you should always be vigilant about looking for segments of code that can be placed in a before(:each) block. It looks like you're all good on that front.


A great observation. My jury is similarly out on the sanity of TDD. Just checking here, but are you running something like autotest + growler (Mac OS) to speed your testing? You may want to look into the gem spork and perhaps mull through this thread Why is RSpec so slow under Rails? for lots of good advice on speeding rspec execution.

None of this really addresses your comments on the (rspec code)/(program code) ratio being so high. The irony of having to debug my testing code is not lost on me either. As a happy and enthusiastic newcomer to ruby and rails, I am keeping an open mind on testing driven development, but I remain suspicious that it is more a hidden cost rather than good feature of the development paradigm.


I refactored your location method, and it should be noted that the only reason I could do this and be sure I wasn't breaking anything was because... wait for it... you had tests!

This isn't necessarily better, or more performant, but in my opinion it's much more readable. And I'm sure that could be further improved upon.

  def location( form=:full )
    if respond_to? "location_#{form}"
      send("location_#{form}")
    else
      raise ArgumentError, "Invalid location format"
    end
  end

  protected

  def location_min
    city.blank? ? place : city
  end

  def location_short
    [(city.blank? ? place : city), (usa? ? state_name : country_name)].join(', ')
  end

  def location_full
    [place, city, (usa? ? state_name : country_name)].delete_if { |v| v.blank? }.join(', ')
  end

  def usa?
    country.eql?('US') || country.eql?('United States')
  end

  def state_name
    Carmen::state_name(self.state) if self.state.present?
  end

  def country_name
    Carmen::country_name(self.country)
  end

There are ways to reduce the line count of your specs if you really care about that number. Custom matchers are a great one.

The point is, the tests prevent other people, and even yourself later down the line, from breaking code you wrote. Sure, you could just test things in the console, but that can get complicated very quickly, and tedious, and is way more prone to human error.

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜