Skip to content

Commit

Permalink
Refactoring: Automatic RSpec VCR cassette name helper
Browse files Browse the repository at this point in the history
This commit was prepared to support upcoming additions to the test suite
(specifically, better coverage for existing Twitter functionality).

These upcoming changes will depend heavily on VCR.[0]
(VCR is a Ruby gem that makes it easier to write and run tests
that call out to external services over HTTP
by "recording" HTTP transactions to a YAML file
and "replaying" them later.)

VCR is widely-used (4600 GitHub stars), but its API is a little clumsy--
You have to manually specify the name of a "cassette" file every time:

    it 'does something' do
      VCR.use_cassette('path/to/cassette') do
        ...
      end
    end

This commit adds an RSpec metadata config option
as a shorthand for the syntax above:

    it 'does something', :use_vcr do
      ...
    end

This config option automatically generates a cassette filename
based on the description of the example it's applied to.

=== Analysis of alternative approaches

Ideally, these auto-generated cassette filenames should be unique:
if filenames collide, multiple examples will share the same cassette.

A first attempt generated names based on `example.full_description`,
but that led to errors:

    Errno::ENAMETOOLONG:
      File name too long @ rb_sysopen - /opt/zammad/test/data/vcr_cassettes/models/ticket/article/ticket_article_callbacks_observers_async_transactions_-_auto-setting_of_outgoing_twitter_article_attributes_via_bg_jobs_when_the_original_channel_specified_in_ticket_preferences_was_deleted_but_a_new_one_with_the_same_screen_name_exists_sets_appropriate_status_attributes_on_the_new_channel.yml

Another idea was to use MD5 digests of the above,
but in fact both of these approaches share another problem:
even minor changes to the description could break tests
(unless the committer remembers to rename the cassette file to match):
an altered description means VCR will record a new cassette file
instead of replaying from the original.

(Normally, this would only slow down the test instead of breaking it,
but sometimes we modify tests and cassettes after recording them
to hide sensitive data like API keys or login credentials.)

The approach taken by this commit was to use partial descriptions,
combining the parent `describe`/`context` label with the `it` label.
This does not guarantee uniqueness--
even in the present refactoring, it produced a filename collision--
but it's a good middle ground.

[0]: https://relishapp.com/vcr/vcr/docs
  • Loading branch information
Ryan Lue authored and thorsteneckel committed Nov 13, 2019
1 parent 6d684c7 commit 1ebddff
Show file tree
Hide file tree
Showing 36 changed files with 14,220 additions and 113 deletions.
7 changes: 1 addition & 6 deletions spec/lib/import/exchange/folder_spec.rb
Expand Up @@ -3,7 +3,7 @@
RSpec.describe Import::Exchange::Folder do
# see https://github.com/zammad/zammad/issues/2152

describe '#display_path (#2152)' do
describe '#display_path (#2152)', :use_vcr do
let(:subject) { described_class.new(ews_connection) }
let(:ews_connection) { Viewpoint::EWSClient.new(endpoint, user, pass) }
let(:endpoint) { 'https://exchange.example.com/EWS/Exchange.asmx' }
Expand All @@ -12,11 +12,6 @@
let(:grandchild_of_root) { ews_connection.get_folder_by_name('Inbox') }
let(:child_of_root) { ews_connection.get_folder(grandchild_of_root.parent_folder_id) }

around do |example|
cassette_name = example.description.gsub(/[^0-9A-Za-z.\-]+/, '_')
VCR.use_cassette("lib/import/exchange/folder/#{cassette_name}") { example.run }
end

context 'when server returns valid UTF-8' do
context 'and target folder is in root directory' do
it 'returns the display name of the folder' do
Expand Down
10 changes: 1 addition & 9 deletions spec/models/channel_spec.rb
@@ -1,17 +1,11 @@
require 'rails_helper'

RSpec.describe Channel do
describe '#fetch' do
around do |example|
VCR.use_cassette(cassette, match_requests_on: %i[method uri oauth_headers]) { example.run }
end

describe '#fetch', use_vcr: :with_oauth_headers do
context 'for Twitter driver' do
subject(:twitter_channel) { create(:twitter_channel) }

context 'with invalid token' do
let(:cassette) { 'models/channel/driver/twitter/fetch_channel_invalid' }

it 'returns false' do
expect(twitter_channel.fetch(true)).to be(false)
end
Expand All @@ -30,8 +24,6 @@
end

context 'with valid token' do
let(:cassette) { 'models/channel/driver/twitter/fetch_channel_valid' }

it 'returns true' do
expect(twitter_channel.fetch(true)).to be(true)
end
Expand Down
14 changes: 2 additions & 12 deletions spec/models/ticket/article_spec.rb
Expand Up @@ -299,20 +299,11 @@
end
end

describe 'Auto-setting of outgoing Twitter article attributes (via bg jobs):' do
describe 'Auto-setting of outgoing Twitter article attributes (via bg jobs):', use_vcr: :with_oauth_headers do
subject!(:twitter_article) { create(:twitter_article, sender_name: 'Agent') }

let(:channel) { Channel.find(twitter_article.ticket.preferences[:channel_id]) }

let(:run_bg_jobs) do
lambda do
VCR.use_cassette(cassette, match_requests_on: %i[method uri oauth_headers]) do
Scheduler.worker(true)
end
end
end

let(:cassette) { 'models/channel/driver/twitter/article_to_tweet' }
let(:run_bg_jobs) { -> { Scheduler.worker(true) } }

it 'sets #from to sender’s Twitter handle' do
expect(&run_bg_jobs)
Expand Down Expand Up @@ -382,7 +373,6 @@

context 'when the original channel (specified in ticket.preferences) was deleted' do
context 'but a new one with the same screen_name exists' do
let(:cassette) { 'models/channel/driver/twitter/article_to_tweet_channel_replace' }
let(:new_channel) { create(:twitter_channel) }

before do
Expand Down
42 changes: 14 additions & 28 deletions spec/requests/external_credentials_spec.rb
Expand Up @@ -89,10 +89,8 @@
end

context 'with invalid credentials, via request params' do
it 'returns 200 with remote (Facebook auth) error' do
VCR.use_cassette('request/external_credentials/facebook/app_verify_invalid_credentials_with_not_created') do
post '/api/v1/external_credentials/facebook/app_verify', params: invalid_credentials, as: :json
end
it 'returns 200 with remote (Facebook auth) error', :use_vcr do
post '/api/v1/external_credentials/facebook/app_verify', params: invalid_credentials, as: :json

expect(response).to have_http_status(:ok)
expect(json_response).to include('error' => 'type: OAuthException, code: 101, message: Error validating application. Cannot get application info due to a system error. [HTTP 400]')
Expand All @@ -102,10 +100,8 @@
context 'with invalid credentials, via ExternalCredential record' do
before { create(:facebook_credential, credentials: invalid_credentials) }

it 'returns 200 with remote (Facebook auth) error' do
VCR.use_cassette('request/external_credentials/facebook/app_verify_invalid_credentials_with_created') do
post '/api/v1/external_credentials/facebook/app_verify', as: :json
end
it 'returns 200 with remote (Facebook auth) error', :use_vcr do
post '/api/v1/external_credentials/facebook/app_verify', as: :json

expect(response).to have_http_status(:ok)
expect(json_response).to include('error' => 'type: OAuthException, code: 101, message: Error validating application. Cannot get application info due to a system error. [HTTP 400]')
Expand Down Expand Up @@ -137,10 +133,8 @@
context 'with invalid credentials, via ExternalCredential record' do
before { create(:facebook_credential, credentials: invalid_credentials) }

it 'returns 500 with remote (Facebook auth) error' do
VCR.use_cassette('request/external_credentials/facebook/link_account_with_invalid_credential') do
get '/api/v1/external_credentials/facebook/link_account', as: :json
end
it 'returns 500 with remote (Facebook auth) error', :use_vcr do
get '/api/v1/external_credentials/facebook/link_account', as: :json

expect(response).to have_http_status(:internal_server_error)
expect(json_response).to include('error' => 'type: OAuthException, code: 101, message: Error validating application. Cannot get application info due to a system error. [HTTP 400]')
Expand Down Expand Up @@ -172,10 +166,8 @@
context 'with invalid credentials, via ExternalCredential record' do
before { create(:facebook_credential, credentials: invalid_credentials) }

it 'returns 500 with remote (Facebook auth) error' do
VCR.use_cassette('request/external_credentials/facebook/callback_invalid_credentials') do
get '/api/v1/external_credentials/facebook/callback', as: :json
end
it 'returns 500 with remote (Facebook auth) error', :use_vcr do
get '/api/v1/external_credentials/facebook/callback', as: :json

expect(response).to have_http_status(:internal_server_error)
expect(json_response).to include('error' => 'type: OAuthException, code: 101, message: Error validating application. Cannot get application info due to a system error. [HTTP 400]')
Expand Down Expand Up @@ -212,10 +204,8 @@
end

context 'with invalid credentials, via request params' do
it 'returns 200 with remote (Twitter auth) error' do
VCR.use_cassette('request/external_credentials/twitter/app_verify_invalid_credentials_with_not_created') do
post '/api/v1/external_credentials/twitter/app_verify', params: invalid_credentials, as: :json
end
it 'returns 200 with remote (Twitter auth) error', :use_vcr do
post '/api/v1/external_credentials/twitter/app_verify', params: invalid_credentials, as: :json

expect(response).to have_http_status(:ok)
expect(json_response).to include('error' => '401 Authorization Required')
Expand All @@ -225,10 +215,8 @@
context 'with invalid credentials, via existing ExternalCredential record' do
before { create(:twitter_credential, credentials: invalid_credentials) }

it 'returns 200 with remote (Twitter auth) error' do
VCR.use_cassette('request/external_credentials/twitter/app_verify_invalid_credentials_with_created') do
post '/api/v1/external_credentials/twitter/app_verify', as: :json
end
it 'returns 200 with remote (Twitter auth) error', :use_vcr do
post '/api/v1/external_credentials/twitter/app_verify', as: :json

expect(response).to have_http_status(:ok)
expect(json_response).to include('error' => '401 Authorization Required')
Expand Down Expand Up @@ -260,10 +248,8 @@
context 'with invalid credentials, via ExternalCredential record' do
before { create(:twitter_credential, credentials: invalid_credentials) }

it 'returns 500 with remote (Twitter auth) error' do
VCR.use_cassette('request/external_credentials/twitter/link_account_with_invalid_credential') do
get '/api/v1/external_credentials/twitter/link_account', as: :json
end
it 'returns 500 with remote (Twitter auth) error', :use_vcr do
get '/api/v1/external_credentials/twitter/link_account', as: :json

expect(response).to have_http_status(:internal_server_error)
expect(json_response).to include('error' => '401 Authorization Required')
Expand Down
18 changes: 18 additions & 0 deletions spec/support/vcr.rb
Expand Up @@ -19,3 +19,21 @@
r2.headers['Authorization']&.map(&without_onetime_oauth_params)
end
end

RSpec.configure do |config|
config.around(:each, use_vcr: true) do |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

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 1ebddff

Please sign in to comment.