From 84df9c807dd6a81b041e72c1ad1b13657f16d709 Mon Sep 17 00:00:00 2001 From: Akhil G Krishnan Date: Mon, 25 Oct 2021 13:13:25 +0530 Subject: [PATCH] chore: Add climate-control gem for handling the test ENV vars (#3267) --- Gemfile | 1 + Gemfile.lock | 2 + .../integrations/apps_controller_spec.rb | 20 +++--- .../api/v1/accounts_controller_spec.rb | 65 ++++++++++--------- spec/controllers/dashboard_controller_spec.rb | 20 +++--- spec/lib/chatwoot_hub_spec.rb | 27 ++++---- .../push_notification_service_spec.rb | 27 ++++---- spec/spec_helper.rb | 4 ++ 8 files changed, 85 insertions(+), 81 deletions(-) diff --git a/Gemfile b/Gemfile index bbd55d4cd..4f640e804 100644 --- a/Gemfile +++ b/Gemfile @@ -155,6 +155,7 @@ group :development, :test do gem 'active_record_query_trace' gem 'bundle-audit', require: false gem 'byebug', platform: :mri + gem 'climate_control' gem 'factory_bot_rails' gem 'faker' gem 'listen' diff --git a/Gemfile.lock b/Gemfile.lock index 472605559..b319c9103 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -133,6 +133,7 @@ GEM bundler (>= 1.2.0, < 3) thor (~> 1.0) byebug (11.1.3) + climate_control (1.0.1) coderay (1.1.3) commonmarker (0.23.2) concurrent-ruby (1.1.9) @@ -657,6 +658,7 @@ DEPENDENCIES bullet bundle-audit byebug + climate_control commonmarker cypress-on-rails (~> 1.0) database_cleaner diff --git a/spec/controllers/api/v1/accounts/integrations/apps_controller_spec.rb b/spec/controllers/api/v1/accounts/integrations/apps_controller_spec.rb index 8f1716c32..2524ac101 100644 --- a/spec/controllers/api/v1/accounts/integrations/apps_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/integrations/apps_controller_spec.rb @@ -27,18 +27,16 @@ RSpec.describe 'Integration Apps API', type: :request do end it 'returns slack app with appropriate redirect url when configured' do - ENV['SLACK_CLIENT_ID'] = 'client_id' - ENV['SLACK_CLIENT_SECRET'] = 'client_secret' - get api_v1_account_integrations_apps_url(account), - headers: agent.create_new_auth_token, - as: :json + with_modified_env SLACK_CLIENT_ID: 'client_id', SLACK_CLIENT_SECRET: 'client_secret' do + get api_v1_account_integrations_apps_url(account), + headers: agent.create_new_auth_token, + as: :json - expect(response).to have_http_status(:success) - apps = JSON.parse(response.body)['payload'] - slack_app = apps.find { |app| app['id'] == 'slack' } - expect(slack_app['action']).to include('client_id=client_id') - ENV['SLACK_CLIENT_ID'] = nil - ENV['SLACK_CLIENT_SECRET'] = nil + expect(response).to have_http_status(:success) + apps = JSON.parse(response.body)['payload'] + slack_app = apps.find { |app| app['id'] == 'slack' } + expect(slack_app['action']).to include('client_id=client_id') + end end end end diff --git a/spec/controllers/api/v1/accounts_controller_spec.rb b/spec/controllers/api/v1/accounts_controller_spec.rb index cba78e56b..810515a03 100644 --- a/spec/controllers/api/v1/accounts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts_controller_spec.rb @@ -12,64 +12,65 @@ RSpec.describe 'Accounts API', type: :request do before do allow(AccountBuilder).to receive(:new).and_return(account_builder) - ENV['ENABLE_ACCOUNT_SIGNUP'] = nil end it 'calls account builder' do - allow(account_builder).to receive(:perform).and_return([user, account]) + with_modified_env ENABLE_ACCOUNT_SIGNUP: nil do + allow(account_builder).to receive(:perform).and_return([user, account]) - params = { account_name: 'test', email: email, user: nil, user_full_name: user_full_name, password: 'Password1!' } + params = { account_name: 'test', email: email, user: nil, user_full_name: user_full_name, password: 'Password1!' } - post api_v1_accounts_url, - params: params, - as: :json + post api_v1_accounts_url, + params: params, + as: :json - expect(AccountBuilder).to have_received(:new).with(params.except(:password).merge(user_password: params[:password])) - expect(account_builder).to have_received(:perform) - expect(response.headers.keys).to include('access-token', 'token-type', 'client', 'expiry', 'uid') + expect(AccountBuilder).to have_received(:new).with(params.except(:password).merge(user_password: params[:password])) + expect(account_builder).to have_received(:perform) + expect(response.headers.keys).to include('access-token', 'token-type', 'client', 'expiry', 'uid') + end end it 'renders error response on invalid params' do - allow(account_builder).to receive(:perform).and_return(nil) + with_modified_env ENABLE_ACCOUNT_SIGNUP: nil do + allow(account_builder).to receive(:perform).and_return(nil) - params = { account_name: nil, email: nil, user: nil, user_full_name: nil } + params = { account_name: nil, email: nil, user: nil, user_full_name: nil } - post api_v1_accounts_url, - params: params, - as: :json + post api_v1_accounts_url, + params: params, + as: :json - expect(AccountBuilder).to have_received(:new).with(params.merge(user_password: params[:password])) - expect(account_builder).to have_received(:perform) - expect(response).to have_http_status(:forbidden) - expect(response.body).to eq({ message: I18n.t('errors.signup.failed') }.to_json) + expect(AccountBuilder).to have_received(:new).with(params.merge(user_password: params[:password])) + expect(account_builder).to have_received(:perform) + expect(response).to have_http_status(:forbidden) + expect(response.body).to eq({ message: I18n.t('errors.signup.failed') }.to_json) + end end end context 'when ENABLE_ACCOUNT_SIGNUP env variable is set to false' do it 'responds 404 on requests' do params = { account_name: 'test', email: email, user_full_name: user_full_name } - ENV['ENABLE_ACCOUNT_SIGNUP'] = 'false' + with_modified_env ENABLE_ACCOUNT_SIGNUP: 'false' do + post api_v1_accounts_url, + params: params, + as: :json - post api_v1_accounts_url, - params: params, - as: :json - - expect(response).to have_http_status(:not_found) - ENV['ENABLE_ACCOUNT_SIGNUP'] = nil + expect(response).to have_http_status(:not_found) + end end end context 'when ENABLE_ACCOUNT_SIGNUP env variable is set to api_only' do it 'does not respond 404 on requests' do params = { account_name: 'test', email: email, user_full_name: user_full_name, password: 'Password1!' } - ENV['ENABLE_ACCOUNT_SIGNUP'] = 'api_only' + with_modified_env ENABLE_ACCOUNT_SIGNUP: 'api_only' do + post api_v1_accounts_url, + params: params, + as: :json - post api_v1_accounts_url, - params: params, - as: :json - - expect(response).to have_http_status(:success) - ENV['ENABLE_ACCOUNT_SIGNUP'] = nil + expect(response).to have_http_status(:success) + end end end end diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb index e797c878a..31187aeb6 100644 --- a/spec/controllers/dashboard_controller_spec.rb +++ b/spec/controllers/dashboard_controller_spec.rb @@ -10,11 +10,11 @@ describe '/app/login', type: :request do context 'with DEFAULT_LOCALE' do it 'renders the dashboard' do - ENV['DEFAULT_LOCALE'] = 'pt_BR' - get '/app/login' - expect(response).to have_http_status(:success) - expect(response.body).to include "selectedLocale: 'pt_BR'" - ENV['DEFAULT_LOCALE'] = 'en' + with_modified_env DEFAULT_LOCALE: 'pt_BR' do + get '/app/login' + expect(response).to have_http_status(:success) + expect(response.body).to include "selectedLocale: 'pt_BR'" + end end end @@ -23,11 +23,11 @@ describe '/app/login', type: :request do # ref : https://stackoverflow.com/a/63584877/939299 context 'with CW_API_ONLY_SERVER true' do it 'returns 404' do - ENV['CW_API_ONLY_SERVER'] = 'true' - Rails.application.reload_routes! - get '/app/login' - expect(response).to have_http_status(:not_found) - ENV['CW_API_ONLY_SERVER'] = nil + with_modified_env CW_API_ONLY_SERVER: 'true' do + Rails.application.reload_routes! + get '/app/login' + expect(response).to have_http_status(:not_found) + end Rails.application.reload_routes! end end diff --git a/spec/lib/chatwoot_hub_spec.rb b/spec/lib/chatwoot_hub_spec.rb index e42feee95..32925492e 100644 --- a/spec/lib/chatwoot_hub_spec.rb +++ b/spec/lib/chatwoot_hub_spec.rb @@ -18,12 +18,12 @@ describe ChatwootHub do it 'will not send instance metrics when telemetry is disabled' do version = '1.1.1' - ENV['DISABLE_TELEMETRY'] = 'true' - allow(RestClient).to receive(:post).and_return({ version: version }.to_json) - expect(described_class.latest_version).to eq version - expect(RestClient).to have_received(:post).with(described_class::PING_URL, - described_class.instance_config.to_json, { content_type: :json, accept: :json }) - ENV['DISABLE_TELEMETRY'] = nil + with_modified_env DISABLE_TELEMETRY: 'true' do + allow(RestClient).to receive(:post).and_return({ version: version }.to_json) + expect(described_class.latest_version).to eq version + expect(RestClient).to have_received(:post).with(described_class::PING_URL, + described_class.instance_config.to_json, { content_type: :json, accept: :json }) + end end it 'returns nil when chatwoot hub is down' do @@ -59,13 +59,14 @@ describe ChatwootHub do end it 'will not send instance events when telemetry is disabled' do - ENV['DISABLE_TELEMETRY'] = 'true' - info = { event_name: event_name, event_data: event_data } - allow(RestClient).to receive(:post) - described_class.emit_event(event_name, event_data) - expect(RestClient).not_to have_received(:post).with(described_class::EVENTS_URL, - info.merge(described_class.instance_config).to_json, { content_type: :json, accept: :json }) - ENV['DISABLE_TELEMETRY'] = nil + with_modified_env DISABLE_TELEMETRY: 'true' do + info = { event_name: event_name, event_data: event_data } + allow(RestClient).to receive(:post) + described_class.emit_event(event_name, event_data) + expect(RestClient).not_to have_received(:post) + .with(described_class::EVENTS_URL, + info.merge(described_class.instance_config).to_json, { content_type: :json, accept: :json }) + end end end end diff --git a/spec/services/notification/push_notification_service_spec.rb b/spec/services/notification/push_notification_service_spec.rb index 2e7911c84..d5217f635 100644 --- a/spec/services/notification/push_notification_service_spec.rb +++ b/spec/services/notification/push_notification_service_spec.rb @@ -14,26 +14,23 @@ describe Notification::PushNotificationService do describe '#perform' do it 'sends webpush notifications for webpush subscription' do - ENV['VAPID_PUBLIC_KEY'] = 'test' - create(:notification_subscription, user: notification.user) + with_modified_env VAPID_PUBLIC_KEY: 'test' do + create(:notification_subscription, user: notification.user) - described_class.new(notification: notification).perform - expect(Webpush).to have_received(:payload_send) - expect(FCM).not_to have_received(:new) - ENV['VAPID_PUBLIC_KEY'] = nil + described_class.new(notification: notification).perform + expect(Webpush).to have_received(:payload_send) + expect(FCM).not_to have_received(:new) + end end it 'sends a fcm notification for firebase subscription' do - ENV['FCM_SERVER_KEY'] = 'test' - ENV['ENABLE_PUSH_RELAY_SERVER'] = 'false' - create(:notification_subscription, user: notification.user, subscription_type: 'fcm') + with_modified_env FCM_SERVER_KEY: 'test', ENABLE_PUSH_RELAY_SERVER: 'false' do + create(:notification_subscription, user: notification.user, subscription_type: 'fcm') - described_class.new(notification: notification).perform - expect(FCM).to have_received(:new) - expect(Webpush).not_to have_received(:payload_send) - - ENV['FCM_SERVER_KEY'] = nil - ENV['ENABLE_PUSH_RELAY_SERVER'] = nil + described_class.new(notification: notification).perform + expect(FCM).to have_received(:new) + expect(Webpush).not_to have_received(:payload_send) + end end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d017eb0da..b5cdea29e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -14,4 +14,8 @@ RSpec.configure do |config| end config.shared_context_metadata_behavior = :apply_to_host_groups + + def with_modified_env(options, &block) + ClimateControl.modify(options, &block) + end end