From 165c44c056bb2d56e7f94d79b59667148168be25 Mon Sep 17 00:00:00 2001 From: Ryan Lue Date: Fri, 6 Mar 2020 18:44:59 +0800 Subject: [PATCH] Testing: Prevent synchronization issues with VCR === Background: What is VCR? VCR caches HTTP network traffic for tests that call out to third-party services (e.g., the Twitter REST API). It "records" to a YAML file the first time a test is run, and we save those files to the repo so they can be "replayed" later. === The problem & the naive approach The ways Zammad uses third-party services can be time-sensitive. For instance, Zammad assumes that when it fetches new tweets, it's getting an up-to-date response from the Twitter API, and ignores all tweets 15+ days old. Of course, if we're using VCR, that's not the case-- in the test, Zammad receives a listing of tweets that's frozen in time. Thus, 15 days after the test was first run, it won't import any tweets at all. Initially, the fix was to add the following to the spec file: before { travel_to '2020-02-06 13:37 +0100' } This hardcodes the date when the tests were first written, and it has significant drawbacks: 1. It leads to hard-to-diagnose bugs when adding new test cases. Your new test case will not have a VCR cassette associated with it; the first time you run it, it will travel way back in time and try to interact with the Twitter REST API. The Twitter REST API requires an OAuth signature based on a current timestamp, and will reject the one generated during the test: Twitter::Error::Unauthorized: Timestamp out of bounds. Because RSpec, VCR, and Channel#fetch all suppress error output in different ways, this error message takes some work to find. 2. If you succeed in adding new test cases, they won't match the timestamp above. === A better solution This commit adds a new option to the :use_vcr metadata tag: it 'does something', use_vcr: :time_sensitive Examples tagged in this way will automatically travel back to exactly when a VCR cassette was recorded prior to using it. === Discussion You may notice that the VCR auto-record logic was moved from a helper module to a simple block. The helper module was intended to improve organization and clean up the config code, but due to context/binding issues, it was not possible to call `travel_to` from inside the helper module. --- spec/models/channel/driver/twitter_spec.rb | 5 +-- spec/support/vcr.rb | 40 ++++++++++++---------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/spec/models/channel/driver/twitter_spec.rb b/spec/models/channel/driver/twitter_spec.rb index 001a909a1..0e2ce832e 100644 --- a/spec/models/channel/driver/twitter_spec.rb +++ b/spec/models/channel/driver/twitter_spec.rb @@ -773,7 +773,7 @@ RSpec.describe Channel::Driver::Twitter do end end - describe '#fetch', :use_vcr do + describe '#fetch', use_vcr: :time_sensitive do describe 'Twitter API authentication' do let(:consumer_credentials) do { @@ -807,9 +807,6 @@ RSpec.describe Channel::Driver::Twitter do end describe 'Twitter API activity' do - # travel back in time when VCR was recorded - before { travel_to '2020-02-06 13:37 +0100' } - it 'sets successful status attributes' do expect { channel.fetch(true) } .to change { channel.reload.attributes } diff --git a/spec/support/vcr.rb b/spec/support/vcr.rb index e805226ac..975697ddd 100644 --- a/spec/support/vcr.rb +++ b/spec/support/vcr.rb @@ -20,24 +20,6 @@ VCR.configure do |config| end end -module VCRHelper - def self.auto_record(example) - spec_path = Pathname.new(example.file_path).realpath - cassette_path = spec_path.relative_path_from(Rails.root.join('spec')).sub(/_spec\.rb$/, '') - cassette_name = "#{example.example_group.description} #{example.description}".gsub(/[^0-9A-Za-z.\-]+/, '_').downcase - request_profile = case example.metadata[:use_vcr] - when true - %i[method uri] - when :with_oauth_headers - %i[method uri oauth_headers] - end - - VCR.use_cassette(cassette_path.join(cassette_name), match_requests_on: request_profile) do - example.run - end - end -end - module RSpec VCR_ADVISORY = <<~MSG.freeze If this test is failing unexpectedly, the VCR cassette may be to blame. @@ -87,7 +69,27 @@ module RSpec end RSpec.configure do |config| - config.around(:each, use_vcr: true, &VCRHelper.method(:auto_record)) + config.around(:each, use_vcr: true) do |example| + vcr_options = Array(example.metadata[:use_vcr]) + + spec_path = Pathname.new(example.file_path).realpath + cassette_path = spec_path.relative_path_from(Rails.root.join('spec')).sub(/_spec\.rb$/, '') + cassette_name = "#{example.example_group.description} #{example.description}".gsub(/[^0-9A-Za-z.\-]+/, '_').downcase + request_profile = [ + :method, + :uri, + vcr_options.include?(:with_oauth_headers) ? :oauth_headers : nil + ].compact + + VCR.use_cassette(cassette_path.join(cassette_name), match_requests_on: request_profile) do |cassette| + if vcr_options.include?(:time_sensitive) && !cassette.recording? + travel_to(cassette.http_interactions.interactions.first.recorded_at) + end + + example.run + end + end + config.around(:each, use_vcr: true, &RSpec::Support::VCRHelper.method(:inject_advisory)) config.around(:each, use_vcr: true, &RSpec::Expectations::VCRHelper.method(:inject_advisory)) end