City class and spec for testing it 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 =&gt; {:country_version_id =&gt; @country_version.id}) do |city|
        I18n.locale=(@locale)
        yield entry(city)
      end
    end

    private
    def entry(city)
      {
        :loc =&gt; ActionController::Integration::Session.new.url_for(:controller =&gt; 'cities', :action =&gt; 'show', :city_name =&gt; city.name, :host =&gt; @country_host.value),
        :changefreq =&gt; 0.8,
        :priority =&gt; 'monthly',
        :lastmod =&gt; 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
Here are a few things I thought of:


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.
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 }
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
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.