moran742/GeekOnTheRun — pull request #2 by mguterl (2012-07-19 01:53:18+00:00)
Title: Organization and specs

Hey Mike,

There are a few items worth noting in this pull request, while each commit message is fairly descriptive I'll go into a little more detail here.

## Commits

1. Organize everything under lib - this is mostly just habit for me at this point. This is the project structure you'll tend to see in most gems. This will allow for more granular testing in the future, along with the ability to separate the ruby code for parsing the API and the ruby code for powering the web app.
2. Update README - nothing here, move along.
3. Add basic skeleton for minitest/spec - I'm not sure what testing framework you're most familiar with, but I went with minitest/spec which is included in Ruby 1.9. rack/test was added to the project in order to allow for `get '/game/12345'` to be called from within the specs. It might actually be worth adding capybara to be able to do more advanced assertions against the page.
4. Add Rakefile - this is another good practice for ruby projects. You can now run `rake` and all of your specs will be executed.

## Testing Thoughts

Currently the tests actually hit the network and require the external API to be available. This is generally a bad idea in tests because it removes some of the predictability involved. In addition to predictability, because they hit the network, they are also very slow. VCR is a gem that can help with this by recording the HTTP request and response and stubbing them out for better repeatability.

The tests in spec/app_spec.rb are what I would consider integration tests. This is because they are going through multiple layers of your application. In the case of app_spec.rb, you're verifying a few things:

* Routes are correct
* Views are in place and wired up to the instance variables exposed from your routes
* The correct URL is being fetched via HTTP
* XML is being parsed correctly

Like anything, integration tests have advantages and disadvantages. A disadvantage is that all of the variables above are reasons for the tests to break at some point. An advantage is that if your tests exercise every endpoint in your sinatra application, it is unlikely that your app will be severely broken because of misconfiguration. Taking the above into consideration, anytime I'm approaching a project that does not have an existing test suite I start with integration tests. In this case once you have covered the existing functionality with some basic tests, you'll be able to start refactoring the underlying sinatra application. I'm sure all of this is slightly confusing and overwhelming, so here's an example below.

Working through the routes, the next action to put under test prior to refactoring is /hot. From a very high level perspective /hot just contains a list of the top 20 games with a link to each.

```ruby
describe BoardGameGeek::App do
describe '/hot/' do
it 'has a list of 20 hot games' do
visit '/hot'

# verify there are 20 li elements on the page
page.has_css?("ol li", :size => 20).must_equal true

# verify that this game that I saw in the list is on the
# page. This suffers from the fragility I mentioned above, but
# that's okay for now. This test is enough to verify that you're
# correctly parsing the name and id of the game and displaying
# it correctly in the view.

page.has_link?("Descent: Journeys in the Dark (Second Edition)", :href => "/game/104162").must_equal true
end
end
end
```

Now that you have some tests in place it is safe to start refactoring. At this point I'd extract the responsibilities inside of the /hot route into it's own objects. I'm not going to worry about changing your variable names or logic, at this point we just want to encapsulate the code and make sure the tests pass.

```ruby
module BoardGameGeek
class Hot
# class method, note the use of self.

def self.all
url = "http://boardgamegeek.com/xmlapi2/hot?boardgame"
gamesInXml = Nokogiri::XML(open(@url))
list = @gamesInXml.xpath("/items/item").to_a
games = []
list.each do |game|
@h = Hash.new
@h["id"] = game.xpath("@id").to_s
@h["name"] = game.xpath("name/@value").to_s
games.push(@h)
end
games
end
end
end
```

In the next segment I'll talk about integrating VCR and putting BoardGameGeek::Game and BoardGameGeek::Hot under test without capybara being involved.