Jan Svitok wrote: > Hi, > > This is how I'd do it. I don't know the details, so your actual code > will be different. This code was not run at all, so probably there are > many typos/errors. > You'd certainly choose another names than I've chosen. > > First of all, if you want to test, it's good to split the code in > small chunks so that each does a little bit of work. Thus you can test > them separately. I tend to put the code inside some class as well... > you know, the OOP ;-) > > While adding tests, you want to change the code as little as possible, > to not broke it. When you have the tests in place, you can safely > change stuff (=refactor). > > The final code could be a bit different from what is written in the > mail, as the code was written before the text, and I was too lazy to > fix it ;-) > > Now the steps to the code below: > > 1. minor corrections: > - at the end of the code 'end' is missing > - change |records| to |result| as you are iterating over Result(s) > - change each to collect as you seem to collect results (but I may be > wrong) > > 2. move the whole each/collect block inside Result class as: > class Result < ... > def self.get_records > Result.find(:all).each do |result| > ... > end > end > end > (you can omit the Result. before find) > We have a function that returns something for all Results > > what remains at the bottom is > records = Result.get_records > > 3. The block doesn't return anything meaningful so: > - add just after class Result < ...: > BuildStatus = Struct.new(:module_name, :build_status_since, > :last_failure, :last_success, :build_number) > - add BuildStatus.new(module_name, build_status_since, last_failure, > last_success, build_number) at the end of the block (inside it) > > 4. Now we can add the first unit test at the very bottom: > comment out the records = ... line > and add: > > if __FILE__ == $0 > require 'test/unit' > > class TestResult < Test::Unit::TestCase > def test_get_records > assert_equal [], Result.get_records > end > end > end > > This test will fail if there are any results, but we have SOMETHING now. > > 5. Let's make the test succeed: > change the inside of the test: > > def test_get_records > results = Result.find(:all) > records =Result.get_records > assert_equal results.size, records.size > records.each do |record| > assert_equal Result::BuildStatus, record.class > end ........... .......... ........... ......... thanks for the long post Jan, youve given me a good insight into the way that my code should be tested. I think that its hard to see how it works exactly when you look at tests being done on other peoples code but when its your own, it makes a bit more sense. I was dreading the idea that Id have refactor it. Not something i've done before as im very new to ruby code at the moment. In my original post i actually omitted a lot of the code which might have been vital to the way that you refactored it and now im a bit lost. Heres the full code (only another few lines extra): require 'net/http' require 'uri' require 'rexml/document' require 'rubygems' require_gem 'activerecord' include REXML def fetch(uri_str, limit=10) fail 'http redirect too deep' if limit.zero? puts "Scraping: #{uri_str}" response = Net::HTTP.get_response(URI.parse(uri_str)) case response when Net::HTTPSuccess response when NetHTTPRedirection fetch(response['location'], limit-1) else response.error! end end #Connect to the database ActiveRecord::Base.establish_connection( :adapter => "mysql", :username => "root", :host => "localhost", :password => "", :database => "build" ) puts "connected to build database" class Result < ActiveRecord::Base end records = Result.find(:all).each do |records| response = fetch(records.build_url) scraped_data = response.body table_start_pos = scraped_data.index('<table class="index" width="100%">') table_end_pos = scraped_data.index('</table>') + 9 height = table_end_pos - table_start_pos #pick out the table gathered_data = response.body[table_start_pos,height] #convert Data to REXML converted_data = REXML::Document.new gathered_data module_name = XPath.first(converted_data, "//td[@class='data']/a/]") module_name = module_name build_status_since = XPath.first(converted_data, "//td[2]/em") build_status_since = build_status_since.text build_status_since = build_status_since.slice(/(\d+):(\d+)/) last_failure = XPath.first(converted_data, "//tbody/tr/td[3]") last_failure = last_failure.text last_success = XPath.first(converted_data, "//tbody/tr/td[4]") last_success = last_success.text build_number = XPath.first(converted_data, "//tbody/tr/td[5]") build_number = build_number.text #modify current entry for the build. Result.find(:all,:conditions => ["build_url = ?", records.build_url]).each do |b| b.build_status_since = build_status_since b.last_failure = last_failure b.last_success = last_success b.build_number = build_number b.save puts '#{module_name} successfully scraped.' end end I think I can nail the testing now but might need a hand with refactoring it. It was silly of me to leave that out of the original post, apologies. -- Posted via http://www.ruby-forum.com/.