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 => {: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

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.

StackExchange Code Review Q#7452, answer score: 3