Organization and specs #2

Open
wants to merge 5 commits into
from

Conversation

Projects
None yet
2 participants

mguterl commented Jul 19, 2012

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.

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.

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.

mguterl commented Oct 17, 2013

Hey @moran742 I was just reviewing my open pull requests and noticed this was still out there. I haven't seen you at Gaslight coffee in awhile. I hope all is well.

Owner

moran742 commented Dec 16, 2013

Hey Michael,

Thanks for the message. Yeah I haven't been around. I got really busy with work at the end of last year, my mother-in-law got really sick and ended up having a double transplant, and then we had another kid so life has been busy. Hope you are doing well. I check the Gaslight blog every once in a while and noticed you are working there. Congratulations.

mguterl commented Dec 21, 2013

Hey Mike,

Congratulations on having another kid! I’m sorry to hear about your mother-in-law, I hope she’s doing okay. Yeah, I’ve been working at Gaslight for almost a year now. Hope all is well, maybe I’ll see you at Friday coffee again some time.

On Dec 16, 2013, at 2:53 PM, Mike notifications@github.com wrote:

Hey Michael,

Thanks for the message. Yeah I haven't been around. I got really busy with work at the end of last year, my mother-in-law got really sick and ended up having a double transplant, and then we had another kid so life has been busy. Hope you are doing well. I check the Gaslight blog every once in a while and noticed you are working there. Congratulations.


Reply to this email directly or view it on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment