Merge pull request #556 from riccardoporreca/fix-recheck-failure

Fix recheck of failures whith caching

lib/html-proofer/cache.rb

@@ -120,9 +120,8 @@ def retrieve_urls(external_urls)
       @cache_log.each_pair do |url, cache|
         if within_timeframe?(cache['time'])
           next if cache['message'].empty? # these were successes to skip
-        else
-          urls_to_check[url] = cache['filenames'] # recheck expired links
         end
+        urls_to_check[url] = cache['filenames'] # recheck expired links
       end
       urls_to_check
     end

spec/html-proofer/cache_spec.rb

@@ -110,37 +110,34 @@ def read_cache(cache_file)
 
   context 'within date' do
     let(:cache_file_name) { '.within_date.log' }
-
     it 'does not write file if timestamp is within date' do
       # this is frozen to within 7 days of the log
-      new_time = Time.local(2015, 10, 20, 12, 0, 0)
+      new_time = Time.local(2015, 10, 27, 12, 0, 0)
       Timecop.freeze(new_time)
 
-      log = read_cache(cache_file)
-      current_time = log.values.first['time']
-
       expect_any_instance_of(HTMLProofer::Cache).to receive(:write)
-      run_proofer(['www.github.com'], :links, cache: { timeframe: '30d', cache_file: cache_file_name }.merge(default_cache_options))
 
-      # note that the timestamp did not change
-      log = read_cache(cache_file)
-      new_time = log.values.first['time']
-      expect(current_time).to eq(new_time)
+      # we expect no add since we are within the timeframe
+      expect_any_instance_of(HTMLProofer::Cache).to_not receive(:add)
+
+      run_proofer(['www.github.com'], :links, cache: { timeframe: '30d', cache_file: cache_file_name }.merge(default_cache_options))
 
       Timecop.return
     end
   end
 
   context 'not within date' do
-    let(:cache_file_name) { '.not_within_date.log' }
-
+    let(:cache_file_name) { '.within_date.log' }
     it 'does write file if timestamp is not within date' do
-      # this is frozen to within 20 days after the log
-      new_time = Time.local(2014, 10, 21, 12, 0, 0)
-      Timecop.travel(new_time)
+      # this is frozen to within 7 days of the log
+      new_time = Time.local(2015, 10, 27, 12, 0, 0)
+      Timecop.freeze(new_time)
+
+      expect_any_instance_of(HTMLProofer::Cache).to receive(:write)
+
+      # we expect an add since we are mocking outside the timeframe
+      expect_any_instance_of(HTMLProofer::Cache).to receive(:add).with('www.github.com', nil, 200)
 
-      # since the timestamp changed, we expect an add
-      expect_any_instance_of(HTMLProofer::Cache).to receive(:add)
       run_proofer(['www.github.com'], :links, cache: { timeframe: '4d', cache_file: cache_file_name }.merge(default_cache_options))
 
       Timecop.return
@@ -149,14 +146,13 @@ def read_cache(cache_file)
 
   context 'new url added' do
     let(:cache_file_name) { '.new_url.log' }
-
     it 'does write file if a new URL is added' do
       # this is frozen to within 7 days of the log
       new_time = Time.local(2015, 10, 20, 12, 0, 0)
       Timecop.freeze(new_time)
 
       expect_any_instance_of(HTMLProofer::Cache).to receive(:write)
-      # we expect a new link to be added, but github.com can stay...
+      # we expect one new link to be added, but github.com can stay...
       expect_any_instance_of(HTMLProofer::Cache).to receive(:add).with('www.google.com', nil, 200)
 
       # ...because it's within the 30d time frame
@@ -168,17 +164,17 @@ def read_cache(cache_file)
 
   context 'recheck failure' do
     let(:cache_file_name) { '.recheck_failure.log' }
-
     it 'does recheck failures, regardless of cache' do
       # this is frozen to within 7 days of the log
-      new_time = Time.local(2015, 10, 20, 12, 0, 0)
+      new_time = Time.local(2015, 10, 27, 12, 0, 0)
       Timecop.freeze(new_time)
 
       expect_any_instance_of(HTMLProofer::Cache).to receive(:write)
-      # we expect the same link to be readded...
+
+      # we expect the same link to be re-added, even though we are within the time frame,
+      # because `foofoofoo.biz` was a failure
       expect_any_instance_of(HTMLProofer::Cache).to receive(:add)
 
-      # ...even though we are within the 30d time frame, because it's a failure
       run_proofer(['http://www.foofoofoo.biz'], :links, cache: { timeframe: '30d', cache_file: cache_file_name }.merge(default_cache_options))
 
       Timecop.return

spec/html-proofer/fixtures/cache/.recheck_failure.log

@@ -1 +1 @@
-{"www.foofoofoo.biz":{"time":"2015-10-20 12:00:00 -0700","filenames":null,"status":0,"message":"External link www.foofoofoo.biz failed: 0 "}}
+{"http://www.foofoofoo.biz":{"time":"2015-10-20 12:00:00 -0700","filenames":null,"status":0,"message":"External link www.foofoofoo.biz failed: 0 "}}

spec/html-proofer/fixtures/cache/.within_date.log

@@ -1 +1 @@
-{"www.github.com":{"time":"2015-10-27 12:00:00 -0700","filenames":null,"status":200,"message":""}}
+{"www.github.com":{"time":"2015-10-20 12:00:00 -0700","filenames":null,"status":200,"message":""}}