refactor bookmark to simplify and improve checks for invalid bookmarks

Author: Hugh McGowan

Diff for: lib/rasta.rb

@@ -15,8 +15,8 @@ def add_choices(builder)
       builder.add_choice(:results_path, :type=>:string, :default=>'rasta_test_results') do | command_line |
         command_line.uses_option("-r", "--results-path PATH", "Location of test results.")
       end
-      builder.add_choice(:continue, :type=>:string, :default=>nil) do | command_line |
-        command_line.uses_option("-c", "--continue BOOKMARK", "Continue spreadsheet from a give bookmark.")
+      builder.add_choice(:bookmark, :type=>:string, :default=>nil) do | command_line |
+        command_line.uses_option("-c", "--continue BOOKMARK", "Continue spreadsheet from a given bookmark.")
       end
       builder.add_choice(:pages, :type=>:integer, :default=>0) do | command_line |
         command_line.uses_option("--pages [COUNT]", "Number of pages to process.")

Diff for: lib/rasta/fixture_runner.rb

@@ -108,17 +108,17 @@ def underscore(camel_cased_word)
 
     def run_test_fixtures
       @options[:spreadsheets].each do |spreadsheet| 
-        roo = Roo::Spreadsheet.open(spreadsheet)
-        Spec::Runner.options.reporter.roo = roo 
-
+        @roo = Roo::Spreadsheet.open(spreadsheet)
+        Spec::Runner.options.reporter.roo = @roo 
+        @bookmark = Rasta::Bookmark.new(@options)
+        check_for_valid_page @bookmark
         @loader = ClassLoader.new(@options[:fixture_path])
         @loader.load_test_fixtures
-        bookmark = Rasta::Bookmark.new(@options)
-        roo.sheets.each do |sheet| 
+        @roo.sheets.each do |sheet| 
           next if sheet =~ /^#/ #skip sheets that are only comments
           begin
-            roo.default_sheet = sheet
-            base_sheet_name = roo.default_sheet.gsub(/#.*/, '') 
+            @roo.default_sheet = sheet
+            base_sheet_name = @roo.default_sheet.gsub(/#.*/, '') 
             classname = @loader.find_class_by_name(base_sheet_name)
             fixture = classname.new
           rescue ArgumentError => e
@@ -127,13 +127,21 @@ def run_test_fixtures
           if @options[:extend_fixture]
             fixture.extend( @options[:extend_fixture])
           end
-          fixture.initialize_fixture(roo, bookmark)
+          fixture.initialize_fixture(@roo, @bookmark)
           fixture.execute_worksheet
         end
       end  
     end
     private :run_test_fixtures
 
+    def check_for_valid_page
+      if @bookmark.page
+        raise BookmarkError, "Unable to find worksheet: #{@bookmark.page}" unless @bookmark.exists?(@roo)
+      end
+      @bookmark
+    end
+    private :check_for_valid_page
+    
     def stop_rspec
       Spec::Runner.options.reporter.original_dump if Spec::Runner.options
       Spec::Runner.options.clear_format_options

Diff for: spec/bookmark_spec.rb

@@ -15,7 +15,7 @@
     @bookmark.max_page_count.should == 0
     @bookmark.record_count.should == 0
     @bookmark.max_record_count.should == 0
-    @bookmark.continue.should == false
+    @bookmark.bookmark.should be_nil
   end
   it 'should always find the page' do
     @bookmark.found_page?('foo').should == true
@@ -30,29 +30,29 @@
 
 describe 'Bookmark with commandline options' do
   it 'should be able to find a bookmarked page' do
-    @bookmark = Rasta::Bookmark.new({:continue => 'MyPage'})
+    @bookmark = Rasta::Bookmark.new({:bookmark => 'MyPage'})
     @bookmark.found_page?('foo').should == false
     @bookmark.found_page?('MyPage').should == true
     @bookmark.found_record?(3).should == true
     @bookmark.exceeded_max_records?.should == false
   end
   it 'should be able to find a bookmarked page with record' do
-    @bookmark = Rasta::Bookmark.new({:continue => 'MyPage[D]'})
+    @bookmark = Rasta::Bookmark.new({:bookmark => 'MyPage[D]'})
     @bookmark.found_page?('foo').should == false
     @bookmark.found_page?('MyPage').should == true
     @bookmark.found_record?(3).should == false
     @bookmark.found_record?(4).should == true
     @bookmark.exceeded_max_records?.should == false
   end
-  it 'should be able to continue given number of pages' do
+  it 'should be able to bookmark given number of pages' do
     @bookmark = Rasta::Bookmark.new({:pages => 1})
     @bookmark.found_page?('MyPage').should == true
     @bookmark.found_record?(4).should == true
     @bookmark.exceeded_max_records?.should == false # pages = 1
     @bookmark.page_count += 1
     @bookmark.exceeded_max_records?.should == true # pages = 2
   end
-  it 'should be able to continue a given number of records' do
+  it 'should be able to bookmark a given number of records' do
     @bookmark = Rasta::Bookmark.new({:records => 1})
     @bookmark.found_page?('MyPage').should == true
     @bookmark.found_record?(4).should == true
@@ -66,36 +66,52 @@
   before :all do
     @bookmark = Rasta::Bookmark.new
   end
-  it 'should parse an empty bookmark' do
-    @bookmark.parse_bookmark(nil).should == [nil,nil]
-  end
   it 'should parse a page without a record' do
-    @bookmark.parse_bookmark('MyPage').should == ['MyPage',nil]
+    @bookmark.send 'bookmark=', 'MyPage'
+    @bookmark.read_bookmark
+    @bookmark.page.should == 'MyPage'
+    @bookmark.record.should == nil
   end
   it 'should parse a page with a numeric record' do
-    @bookmark.parse_bookmark('MyPage[1]').should == ['MyPage',1]
+    @bookmark.send 'bookmark=', 'MyPage[1]'
+    @bookmark.read_bookmark
+    @bookmark.page.should == 'MyPage'
+    @bookmark.record.should == 1
   end
   it 'should parse a page with an alpha record' do
-    @bookmark.parse_bookmark('MyPage[A]').should == ['MyPage',1]
+    @bookmark.send 'bookmark=', 'MyPage[A]'
+    @bookmark.read_bookmark
+    @bookmark.page.should == 'MyPage'
+    @bookmark.record.should == 1
   end
   it 'should parse a page with an extended alpha record' do
-    @bookmark.parse_bookmark('MyPage[AA]').should == ['MyPage',27]
+    @bookmark.send 'bookmark=', 'MyPage[AA]'
+    @bookmark.read_bookmark
+    @bookmark.page.should == 'MyPage'
+    @bookmark.record.should == 27
   end
   it 'should handle lowercase records' do
-    @bookmark.parse_bookmark('MyPage[aa]').should == ['MyPage',27]
+    @bookmark.send 'bookmark=', 'MyPage[aa]'
+    @bookmark.read_bookmark
+    @bookmark.page.should == 'MyPage'
+    @bookmark.record.should == 27
   end
   it 'should parse a page with an embedded comment' do
-    @bookmark.parse_bookmark('MyPage#comment').should == ['MyPage#comment',nil]
+    @bookmark.send 'bookmark=', 'MyPage#comment'
+    @bookmark.read_bookmark
+    @bookmark.page.should == 'MyPage#comment'
+    @bookmark.record.should == nil
   end
   it 'should raise an exception for an invalid bookmark' do
-    lambda{ @bookmark.parse_bookmark("[1]") }.should raise_error(Rasta::BookmarkError)
+    lambda{
+      @bookmark.send 'bookmark=', '[]'
+      @bookmark.read_bookmark
+    }.should raise_error(Rasta::BookmarkError)
   end
   it 'should raise an exception for an invalid bookmark record' do
-    lambda{ @bookmark.parse_bookmark("MyPage[foo1]") }.should raise_error(Rasta::BookmarkError)
+    lambda{
+      @bookmark.send 'bookmark=', 'MyPage[foo1]'
+      @bookmark.read_bookmark
+    }.should raise_error(Rasta::BookmarkError)
   end
 end
-
-describe 'Bookmark parsing on worksheet' do
-  it 'should handle bookmarks to invalid worksheets'
-  it 'should handle bookmarks to invalid records'
-end

Diff for: spec/options_spec.rb

@@ -59,7 +59,7 @@ def stop_rspec
   end
 end
 
-describe 'continue from bookmark' do
+describe 'bookmark from bookmark' do
   it_should_behave_like 'rasta_options'
 
   it 'Should count the correct number of records when parsing' do 
@@ -69,7 +69,7 @@ def stop_rspec
     @test_fixture.rasta_metrics['MathFunctions#pending'].record_count.should == 7
   end
   it 'Should be able to start from a tab' do 
-    @options.merge!(:continue=>'StringFunctions')
+    @options.merge!(:bookmark=>'StringFunctions')
     Rasta::FixtureRunner.new(@options).execute
     @test_fixture.rasta_metrics['MathFunctions'].record_count.should == 0
     @test_fixture.rasta_metrics['StringFunctions'].record_count.should == 3
@@ -83,7 +83,7 @@ def stop_rspec
     @test_fixture.rasta_metrics['MathFunctions#pending'].record_count.should == 0
   end
   it 'Should be able to do n pages from a tab' do 
-    @options.merge!(:pages=>2, :continue=>'StringFunctions')
+    @options.merge!(:pages=>2, :bookmark=>'StringFunctions')
     Rasta::FixtureRunner.new(@options).execute
     @test_fixture.rasta_metrics['MathFunctions'].record_count.should == 0
     @test_fixture.rasta_metrics['StringFunctions'].record_count.should == 3
@@ -97,52 +97,56 @@ def stop_rspec
     @test_fixture.rasta_metrics['MathFunctions#pending'].record_count.should == 0
   end
   it 'Should be able to do n records from a page' do 
-    @options.merge!(:records=>3, :continue=>'MathFunctions#pending')
+    @options.merge!(:records=>3, :bookmark=>'MathFunctions#pending')
     Rasta::FixtureRunner.new(@options).execute
     @test_fixture.rasta_metrics['MathFunctions'].record_count.should == 0
     @test_fixture.rasta_metrics['StringFunctions'].record_count.should == 0
     @test_fixture.rasta_metrics['MathFunctions#pending'].record_count.should == 3
   end
-  it 'Should be able to continue from a page row' do 
-    @options.merge!(:continue=>'MathFunctions#pending[5]')
+  it 'Should be able to bookmark from a page row' do 
+    @options.merge!(:bookmark=>'MathFunctions#pending[5]')
     Rasta::FixtureRunner.new(@options).execute
     @test_fixture.rasta_metrics['MathFunctions'].record_count.should == 0
     @test_fixture.rasta_metrics['StringFunctions'].record_count.should == 0
     @test_fixture.rasta_metrics['MathFunctions#pending'].record_count.should == 4
   end
-  it 'Should be able to continue from a page row with n records' do 
-    @options.merge!(:continue=>'MathFunctions#pending[5]', :records=>2)
+  it 'Should be able to bookmark from a page row with n records' do 
+    @options.merge!(:bookmark=>'MathFunctions#pending[5]', :records=>2)
     Rasta::FixtureRunner.new(@options).execute
     @test_fixture.rasta_metrics['MathFunctions'].record_count.should == 0
     @test_fixture.rasta_metrics['StringFunctions'].record_count.should == 0
     @test_fixture.rasta_metrics['MathFunctions#pending'].record_count.should == 2
   end
-  it 'Should be able to continue from a page row with n pages' do 
-    @options.merge!(:continue=>'MathFunctions[5]', :pages=>2)
+  it 'Should be able to bookmark from a page row with n pages' do 
+    @options.merge!(:bookmark=>'MathFunctions[5]', :pages=>2)
     Rasta::FixtureRunner.new(@options).execute
     @test_fixture.rasta_metrics['MathFunctions'].record_count.should == 4
     @test_fixture.rasta_metrics['StringFunctions'].record_count.should == 3
     @test_fixture.rasta_metrics['MathFunctions#pending'].record_count.should == 0
   end
-  it 'Should be able to continue from a page column' do 
-    @options.merge!(:continue=>'StringFunctions[D]')
+  it 'Should be able to bookmark from a page column' do 
+    @options.merge!(:bookmark=>'StringFunctions[D]')
     Rasta::FixtureRunner.new(@options).execute
     @test_fixture.rasta_metrics['MathFunctions'].record_count.should == 0
     @test_fixture.rasta_metrics['StringFunctions'].record_count.should == 1
     @test_fixture.rasta_metrics['MathFunctions#pending'].record_count.should == 7 
   end
-  it 'Should be able to continue from a page column with n records' do 
-    @options.merge!(:continue=>'StringFunctions[D]', :records=>3)
+  it 'Should be able to bookmark from a page column with n records' do 
+    @options.merge!(:bookmark=>'StringFunctions[D]', :records=>3)
     Rasta::FixtureRunner.new(@options).execute
     @test_fixture.rasta_metrics['MathFunctions'].record_count.should == 0
     @test_fixture.rasta_metrics['StringFunctions'].record_count.should == 1
     @test_fixture.rasta_metrics['MathFunctions#pending'].record_count.should == 2 
   end
-  it 'Should be able to continue from a page column with n pages' do 
-    @options.merge!(:continue=>'StringFunctions[D]', :pages=>1)
+  it 'Should be able to bookmark from a page column with n pages' do 
+    @options.merge!(:bookmark=>'StringFunctions[D]', :pages=>1)
     Rasta::FixtureRunner.new(@options).execute
     @test_fixture.rasta_metrics['MathFunctions'].record_count.should == 0
     @test_fixture.rasta_metrics['StringFunctions'].record_count.should == 1
     @test_fixture.rasta_metrics['MathFunctions#pending'].record_count.should == 0
    end
+   it 'Should fail gracefully when the bookmark page is invalid' do 
+     @options.merge!(:bookmark=>'MissingPage')
+     lambda{ Rasta::FixtureRunner.new(@options).execute }.should raise_error
+   end
 end