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:
- Surely there's a better way to do this. Can anyone suggest a refactoring?
- 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.
精彩评论