From 821ff0f223ff6c413c017cb9d07c0d914cc6d16c Mon Sep 17 00:00:00 2001 From: riccardoporreca Date: Fri, 7 Feb 2020 01:22:23 +0100 Subject: [PATCH 1/5] Fix and simplify cache tests on existing URL. * Rely on mocking :within_timeframe? instead of time travel (previous dates were not consistent) and fix expectations on :add. * Also fix the log for the recheck failure test: it did not contain the actual URL, which was then of added as new URL => This reveals a bug in handling failure re-checks. --- spec/html-proofer/cache_spec.rb | 43 +++++++++---------- .../fixtures/cache/.recheck_failure.log | 2 +- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/spec/html-proofer/cache_spec.rb b/spec/html-proofer/cache_spec.rb index 857c51fb..40b120ed 100644 --- a/spec/html-proofer/cache_spec.rb +++ b/spec/html-proofer/cache_spec.rb @@ -112,22 +112,17 @@ def read_cache(cache_file) 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) - 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... + expect_any_instance_of(HTMLProofer::Cache).to_not receive(:add) + # ...since we are mocking within_timeframe? to be true + within = true + expect_any_instance_of(HTMLProofer::Cache).to receive(:within_timeframe?).and_return(within) + + run_proofer(['www.github.com'], :links, cache: { timeframe: '30d', cache_file: cache_file_name }.merge(default_cache_options)) - Timecop.return end end @@ -135,15 +130,18 @@ def read_cache(cache_file) let(:cache_file_name) { '.not_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) - # since the timestamp changed, we expect an add - expect_any_instance_of(HTMLProofer::Cache).to receive(:add) + # mock within_timeframe + expect_any_instance_of(HTMLProofer::Cache).to receive(:write) + + # we expect an add... + expect_any_instance_of(HTMLProofer::Cache).to receive(:add).with('www.github.com', nil, 200) + # ...since we are mocking within_timeframe? to be true + within = false + expect_any_instance_of(HTMLProofer::Cache).to receive(:within_timeframe?).and_return(within) + run_proofer(['www.github.com'], :links, cache: { timeframe: '4d', cache_file: cache_file_name }.merge(default_cache_options)) - Timecop.return end end @@ -170,18 +168,17 @@ def read_cache(cache_file) 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) - Timecop.freeze(new_time) expect_any_instance_of(HTMLProofer::Cache).to receive(:write) + # we expect the same link to be readded... expect_any_instance_of(HTMLProofer::Cache).to receive(:add) + # ...even though we are within the time frame + within = true + expect_any_instance_of(HTMLProofer::Cache).to receive(:within_timeframe?).and_return(within) - # ...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 end end end diff --git a/spec/html-proofer/fixtures/cache/.recheck_failure.log b/spec/html-proofer/fixtures/cache/.recheck_failure.log index 4aa0509e..06b73003 100644 --- a/spec/html-proofer/fixtures/cache/.recheck_failure.log +++ b/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 "}} \ No newline at end of file +{"http://www.foofoofoo.biz":{"time":"2015-10-20 12:00:00 -0700","filenames":null,"status":0,"message":"External link www.foofoofoo.biz failed: 0 "}} From d0016ca6a9e0e5f4536bf8ad1d4c6bac381d1fea Mon Sep 17 00:00:00 2001 From: riccardoporreca Date: Fri, 7 Feb 2020 01:26:03 +0100 Subject: [PATCH 2/5] Fix recheck of failures. --- lib/html-proofer/cache.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/html-proofer/cache.rb b/lib/html-proofer/cache.rb index 1eaa2852..6351dd23 100644 --- a/lib/html-proofer/cache.rb +++ b/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 From e8f81ddd691a687ccf27e45aad0812e366c46150 Mon Sep 17 00:00:00 2001 From: riccardoporreca Date: Fri, 7 Feb 2020 01:41:30 +0100 Subject: [PATCH 3/5] No extra empty line detected at block body beginning/end. --- spec/html-proofer/cache_spec.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/spec/html-proofer/cache_spec.rb b/spec/html-proofer/cache_spec.rb index 40b120ed..916a6143 100644 --- a/spec/html-proofer/cache_spec.rb +++ b/spec/html-proofer/cache_spec.rb @@ -110,9 +110,7 @@ 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 - expect_any_instance_of(HTMLProofer::Cache).to receive(:write) # we expect no add... @@ -122,15 +120,12 @@ def read_cache(cache_file) expect_any_instance_of(HTMLProofer::Cache).to receive(:within_timeframe?).and_return(within) run_proofer(['www.github.com'], :links, cache: { timeframe: '30d', cache_file: cache_file_name }.merge(default_cache_options)) - end end context 'not within date' do let(:cache_file_name) { '.not_within_date.log' } - it 'does write file if timestamp is not within date' do - # mock within_timeframe expect_any_instance_of(HTMLProofer::Cache).to receive(:write) @@ -141,13 +136,11 @@ def read_cache(cache_file) expect_any_instance_of(HTMLProofer::Cache).to receive(:within_timeframe?).and_return(within) run_proofer(['www.github.com'], :links, cache: { timeframe: '4d', cache_file: cache_file_name }.merge(default_cache_options)) - end end 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) @@ -166,9 +159,7 @@ def read_cache(cache_file) context 'recheck failure' do let(:cache_file_name) { '.recheck_failure.log' } - it 'does recheck failures, regardless of cache' do - expect_any_instance_of(HTMLProofer::Cache).to receive(:write) # we expect the same link to be readded... @@ -178,7 +169,6 @@ def read_cache(cache_file) expect_any_instance_of(HTMLProofer::Cache).to receive(:within_timeframe?).and_return(within) run_proofer(['http://www.foofoofoo.biz'], :links, cache: { timeframe: '30d', cache_file: cache_file_name }.merge(default_cache_options)) - end end end From 29ed4e74858889eb221ea46ac4d7d178136c9a31 Mon Sep 17 00:00:00 2001 From: riccardoporreca Date: Fri, 7 Feb 2020 02:13:02 +0100 Subject: [PATCH 4/5] minor - improve comments --- spec/html-proofer/cache_spec.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/html-proofer/cache_spec.rb b/spec/html-proofer/cache_spec.rb index 916a6143..4a24820c 100644 --- a/spec/html-proofer/cache_spec.rb +++ b/spec/html-proofer/cache_spec.rb @@ -115,7 +115,7 @@ def read_cache(cache_file) # we expect no add... expect_any_instance_of(HTMLProofer::Cache).to_not receive(:add) - # ...since we are mocking within_timeframe? to be true + # ...since we are mocking :within_timeframe? to be true within = true expect_any_instance_of(HTMLProofer::Cache).to receive(:within_timeframe?).and_return(within) @@ -126,12 +126,11 @@ def read_cache(cache_file) context 'not within date' do let(:cache_file_name) { '.not_within_date.log' } it 'does write file if timestamp is not within date' do - # mock within_timeframe expect_any_instance_of(HTMLProofer::Cache).to receive(:write) # we expect an add... expect_any_instance_of(HTMLProofer::Cache).to receive(:add).with('www.github.com', nil, 200) - # ...since we are mocking within_timeframe? to be true + # ...since we are mocking :within_timeframe? to be false within = false expect_any_instance_of(HTMLProofer::Cache).to receive(:within_timeframe?).and_return(within) From 555045051be53b4df522fd42467c0de724f740e6 Mon Sep 17 00:00:00 2001 From: "Garen J. Torikian" Date: Tue, 11 Feb 2020 12:28:51 +0000 Subject: [PATCH 5/5] Provide Timecop updates --- spec/html-proofer/cache_spec.rb | 38 ++++++---- .../fixtures/cache/.not_within_date.log | 1 - .../fixtures/cache/.within_date.log | 2 +- .../cache_log_level_error_type_links_.yml | 75 +++++++++++++++++++ 4 files changed, 100 insertions(+), 16 deletions(-) delete mode 100644 spec/html-proofer/fixtures/cache/.not_within_date.log create mode 100644 spec/html-proofer/fixtures/vcr_cassettes/www_github_com_cache_timeframe_4d_cache_file_within_date_log_storage_dir_spec/html-proofer/fixtures/cache_log_level_error_type_links_.yml diff --git a/spec/html-proofer/cache_spec.rb b/spec/html-proofer/cache_spec.rb index 4a24820c..54cb54c7 100644 --- a/spec/html-proofer/cache_spec.rb +++ b/spec/html-proofer/cache_spec.rb @@ -111,30 +111,36 @@ 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, 27, 12, 0, 0) + Timecop.freeze(new_time) + expect_any_instance_of(HTMLProofer::Cache).to receive(:write) - # we expect no add... + # we expect no add since we are within the timeframe expect_any_instance_of(HTMLProofer::Cache).to_not receive(:add) - # ...since we are mocking :within_timeframe? to be true - within = true - expect_any_instance_of(HTMLProofer::Cache).to receive(:within_timeframe?).and_return(within) 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 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... + # 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 we are mocking :within_timeframe? to be false - within = false - expect_any_instance_of(HTMLProofer::Cache).to receive(:within_timeframe?).and_return(within) run_proofer(['www.github.com'], :links, cache: { timeframe: '4d', cache_file: cache_file_name }.merge(default_cache_options)) + + Timecop.return end end @@ -146,7 +152,7 @@ def read_cache(cache_file) 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 @@ -159,15 +165,19 @@ 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, 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 time frame - within = true - expect_any_instance_of(HTMLProofer::Cache).to receive(:within_timeframe?).and_return(within) run_proofer(['http://www.foofoofoo.biz'], :links, cache: { timeframe: '30d', cache_file: cache_file_name }.merge(default_cache_options)) + + Timecop.return end end end diff --git a/spec/html-proofer/fixtures/cache/.not_within_date.log b/spec/html-proofer/fixtures/cache/.not_within_date.log deleted file mode 100644 index 31cb822b..00000000 --- a/spec/html-proofer/fixtures/cache/.not_within_date.log +++ /dev/null @@ -1 +0,0 @@ -{"www.github.com":{"time":"2015-10-01 16:18:15 -0700","filenames":null,"status":200,"message":""}} \ No newline at end of file diff --git a/spec/html-proofer/fixtures/cache/.within_date.log b/spec/html-proofer/fixtures/cache/.within_date.log index 4c840aaa..aac87996 100644 --- a/spec/html-proofer/fixtures/cache/.within_date.log +++ b/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":""}} diff --git a/spec/html-proofer/fixtures/vcr_cassettes/www_github_com_cache_timeframe_4d_cache_file_within_date_log_storage_dir_spec/html-proofer/fixtures/cache_log_level_error_type_links_.yml b/spec/html-proofer/fixtures/vcr_cassettes/www_github_com_cache_timeframe_4d_cache_file_within_date_log_storage_dir_spec/html-proofer/fixtures/cache_log_level_error_type_links_.yml new file mode 100644 index 00000000..5448746c --- /dev/null +++ b/spec/html-proofer/fixtures/vcr_cassettes/www_github_com_cache_timeframe_4d_cache_file_within_date_log_storage_dir_spec/html-proofer/fixtures/cache_log_level_error_type_links_.yml @@ -0,0 +1,75 @@ +--- +http_interactions: +- request: + method: head + uri: www.github.com + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - Mozilla/5.0 (compatible; HTML Proofer/3.15.1; +https://github.com/gjtorikian/html-proofer) + Accept: + - application/xml,application/xhtml+xml,text/html;q=0.9, text/plain;q=0.8,image/png,*/*;q=0.5 + Expect: + - '' + response: + status: + code: 200 + message: OK + headers: + Date: + - Tue, 11 Feb 2020 12:26:04 GMT + Content-Type: + - text/html; charset=utf-8 + Server: + - GitHub.com + Status: + - 200 OK + Vary: + - X-PJAX + - Accept-Encoding, Accept + ETag: + - W/"a7cb4d93d4ab298ba08e5b7bfde81bb1" + Cache-Control: + - max-age=0, private, must-revalidate + Set-Cookie: + - _octo=GH1.1.141947230.1581423964; domain=.github.com; path=/; expires=Thu, + 11 Feb 2021 12:26:04 -0000 + - logged_in=no; domain=.github.com; path=/; expires=Thu, 11 Feb 2021 12:26:04 + -0000; secure; HttpOnly + - _gh_sess=Q3lZVkJDU0NrcTEwdUNLUVlzY3VZQkJndWhnakxnUWZIcDY0OExmcXcrd3ZHOXc4RUlRWGIzT3l1SHZwQlZRZmQyZmVJNWJNczN0aUNaVkxGMHBqdnpqcWx4VXFNQSt2RlExUDRtdElyTGtaYTY5OG5rTmVZalVicTc4ekhkSW1OZUJmQ3pKUjlxQld1WHU1cWErczhnPT0tLXJ2ZGIrL25LcWxJUmlkR0srN29Pa1E9PQ%3D%3D--d4aaeed450346ec90679984457d338c49bb643c6; + path=/; secure; HttpOnly + Strict-Transport-Security: + - max-age=31536000; includeSubdomains; preload + X-Frame-Options: + - deny + X-Content-Type-Options: + - nosniff + X-XSS-Protection: + - 1; mode=block + Referrer-Policy: + - origin-when-cross-origin, strict-origin-when-cross-origin + Expect-CT: + - max-age=2592000, report-uri="https://api.github.com/_private/browser/errors" + Content-Security-Policy: + - 'default-src ''none''; base-uri ''self''; block-all-mixed-content; connect-src + ''self'' uploads.github.com www.githubstatus.com collector.githubapp.com api.github.com + www.google-analytics.com github-cloud.s3.amazonaws.com github-production-repository-file-5c1aeb.s3.amazonaws.com + github-production-upload-manifest-file-7fdce7.s3.amazonaws.com github-production-user-asset-6210df.s3.amazonaws.com + wss://live.github.com; font-src github.githubassets.com; form-action ''self'' + github.com gist.github.com; frame-ancestors ''none''; frame-src render.githubusercontent.com; + img-src ''self'' data: github.githubassets.com identicons.github.com collector.githubapp.com + github-cloud.s3.amazonaws.com *.githubusercontent.com customer-stories-feed.github.com + spotlights-feed.github.com; manifest-src ''self''; media-src ''none''; script-src + github.githubassets.com; style-src ''unsafe-inline'' github.githubassets.com' + X-GitHub-Request-Id: + - C3F6:2E6DB:DC9A09:14C1086:5E429D5B + body: + encoding: UTF-8 + string: '' + http_version: '1.1' + adapter_metadata: + effective_url: https://github.com/ + recorded_at: Sat, 21 Oct 2017 11:00:00 GMT +recorded_with: VCR 2.9.3