3
\$\begingroup\$

I have written a class and spec for testing my class. Can anyone determine whether or not this is a correct and good spec?

Class:

module Sitemap
=begin
Class Name: Cities
Function: This class is responsible for getting the data to create the sitemap 
for each country according to cities. 
=end
  class City
    attr_accessor :country_version, :directory, :country_host, :locale
    def initialize(country_version, directory, country_host,locale)
      @country_version = country_version
      @directory = directory
      @country_host = country_host
      @locale = locale
    end

    def get_data
      ::City.find_each(:conditions => {:country_version_id => @country_version.id}) do |city|
        I18n.locale=(@locale)
        yield entry(city)
      end
    end

    private
    def entry(city)
      {
        :loc => ActionController::Integration::Session.new.url_for(:controller => 'cities', :action => 'show', :city_name => city.name, :host => @country_host.value),
        :changefreq => 0.8,
        :priority => 'monthly',
        :lastmod => city.updated_at
      }
    end
  end
end 

Spec:

require File.join(File.dirname(__FILE__), "/../spec_helper" )
module Sitemap
  describe City do

    before :all do
    @city = City.new :country_version, :directory, :country_host, :locale
    end

    describe "#new" do
      it "takes four parameters and returns a City object" do
        @city.should be_an_instance_of City
      end
    end

    describe "#country_version" do
      it "returns the correct country version" do
        @city.country_version.should eql :country_version
      end
    end

    describe "#directory" do
      it "returns the correct filepath" do
        @city.directory.should eql :directory
      end
    end

    describe "#country_host" do
      it "returns the correct country host" do
        @city.country_host.should eql :country_host
      end
    end

    describe "#locale" do
      it "returns the correct locale for translation" do
        @city.locale.should eql :locale
      end
    end

    describe "#get_data" do
      it "returns collections for xml entries" do
        @city.get_data.should eql ::City
      end
    end 

  end
end
\$\endgroup\$
3
\$\begingroup\$

Here are a few things I thought of:

  1. Use let when you can, it is a pretty versatile function for testing.

    before :all
      @city = City.new :country_version, :directory, :country_host, :locale
    end
    

    Can be rewritten as:

    let(:city) { @city = City.new :country_version, :directory, :country_host, :locale }
    

    This will only call city when you use it.

  2. I don't typically test accessors/intatiation but if I did, I would probably test it with a specify block, it is also good for most tests where the description is "it should do something correctly":

    This is nice for simple tests that explain themselves

    specify { city.should be_an_instance_of City }

  3. When testing, I try to think in terms of cases: Ex:

    it "should act in manner x when no cities are found"
    it "should act in manner y when 1 city is found"
    it "should act in manner z when n cities are found"
    

    I find testing 0, 1, n catches a lot of potential errors

  4. The testing gods expect you to test first, then write code. If you don't :D, then make sure that your test fails in the way you expect it to.

\$\endgroup\$
  • 1
    \$\begingroup\$ Small correction: let(:city) { City.new :country_version, :directory, :country_host, :locale } without @city =. And then use city instead of @city in your code. \$\endgroup\$ – Aliaksei Kliuchnikau Feb 16 '12 at 14:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.