Improve Redis instance test coverage

- add coverage for config file selection
- add coverage for Rails.env-keyed config selection
- consolidate duplicate config file fixtures

lib/gitlab/redis/wrapper.rb

@@ -55,12 +55,14 @@ module Gitlab
           DEFAULT_REDIS_URL
         end
 
-        # Return the absolute path to a Rails configuration file
-        #
-        # We use this instead of `Rails.root` because for certain tasks
-        # utilizing these classes, `Rails` might not be available.
         def config_file_path(filename)
-          File.expand_path("../../../config/#{filename}", __dir__)
+          File.join(rails_root, 'config', filename)
+        end
+
+        # We need this local implementation of Rails.root because MailRoom
+        # doesn't load Rails.
+        def rails_root
+          File.expand_path('../../..', __dir__)
         end
 
         def config_file_name

spec/config/mail_room_spec.rb

@@ -43,7 +43,7 @@ RSpec.describe 'mail_room.yml' do
 
   context 'when both incoming email and service desk email are enabled' do
     let(:gitlab_config_path) { 'spec/fixtures/config/mail_room_enabled.yml' }
-    let(:queues_config_path) { 'spec/fixtures/config/redis_queues_new_format_host.yml' }
+    let(:queues_config_path) { 'spec/fixtures/config/redis_new_format_host.yml' }
     let(:gitlab_redis_queues) { Gitlab::Redis::Queues.new(Rails.env) }
 
     it 'contains the intended configuration' do
@@ -72,7 +72,7 @@ RSpec.describe 'mail_room.yml' do
 
   context 'when both incoming email and service desk email are enabled for Microsoft Graph' do
     let(:gitlab_config_path) { 'spec/fixtures/config/mail_room_enabled_ms_graph.yml' }
-    let(:queues_config_path) { 'spec/fixtures/config/redis_queues_new_format_host.yml' }
+    let(:queues_config_path) { 'spec/fixtures/config/redis_new_format_host.yml' }
     let(:gitlab_redis_queues) { Gitlab::Redis::Queues.new(Rails.env) }
 
     it 'contains the intended configuration' do

spec/fixtures/config/redis_cache_config_with_env.yml

-test:
-  url: <%= ENV['TEST_GITLAB_REDIS_CACHE_URL'] %>

spec/fixtures/config/redis_cache_new_format_host.yml

-# redis://[:password@]host[:port][/db-number][?option=value]
-# more details: http://www.iana.org/assignments/uri-schemes/prov/redis
-development:
-  url: redis://:mynewpassword@localhost:6380/10
-  sentinels:
-    -
-      host: localhost
-      port: 26380 # point to sentinel, not to redis port
-    -
-      host: replica2
-      port: 26380 # point to sentinel, not to redis port
-test:
-  url: redis://:mynewpassword@localhost:6380/10
-  sentinels:
-    -
-      host: localhost
-      port: 26380 # point to sentinel, not to redis port
-    -
-      host: replica2
-      port: 26380 # point to sentinel, not to redis port
-production:
-  url: redis://:mynewpassword@localhost:6380/10
-  sentinels:
-    -
-      host: replica1
-      port: 26380 # point to sentinel, not to redis port
-    -
-      host: replica2
-      port: 26380 # point to sentinel, not to redis port

spec/fixtures/config/redis_cache_new_format_socket.yml

-development:
-  url: unix:/path/to/redis.cache.sock
-test:
-  url: unix:/path/to/redis.cache.sock
-production:
-  url: unix:/path/to/redis.cache.sock

spec/fixtures/config/redis_cache_old_format_host.yml

-# redis://[:password@]host[:port][/db-number][?option=value]
-# more details: http://www.iana.org/assignments/uri-schemes/prov/redis
-development: redis://:mypassword@localhost:6380/10
-test: redis://:mypassword@localhost:6380/10
-production: redis://:mypassword@localhost:6380/10

spec/fixtures/config/redis_cache_old_format_socket.yml

-development: unix:/path/to/old/redis.cache.sock
-test: unix:/path/to/old/redis.cache.sock
-production: unix:/path/to/old/redis.cache.sock

spec/fixtures/config/redis_new_format_host.yml

 # redis://[:password@]host[:port][/db-number][?option=value]
 # more details: http://www.iana.org/assignments/uri-schemes/prov/redis
 development:
-  url: redis://:mynewpassword@localhost:6379/99
+  url: redis://:mynewpassword@development-host:6379/99
   sentinels:
     -
-      host: localhost
+      host: development-replica1
       port: 26379 # point to sentinel, not to redis port
     -
-      host: replica2
+      host: development-replica2
       port: 26379 # point to sentinel, not to redis port
 test:
-  url: redis://:mynewpassword@localhost:6379/99
+  url: redis://:mynewpassword@test-host:6379/99
   sentinels:
     -
-      host: localhost
+      host: test-replica1
       port: 26379 # point to sentinel, not to redis port
     -
-      host: replica2
+      host: test-replica2
       port: 26379 # point to sentinel, not to redis port
 production:
-  url: redis://:mynewpassword@localhost:6379/99
+  url: redis://:mynewpassword@production-host:6379/99
   sentinels:
     -
-      host: replica1
+      host: production-replica1
       port: 26379 # point to sentinel, not to redis port
     -
-      host: replica2
+      host: production-replica2
       port: 26379 # point to sentinel, not to redis port

spec/fixtures/config/redis_queues_config_with_env.yml

-test:
-  url: <%= ENV['TEST_GITLAB_REDIS_QUEUES_URL'] %>

spec/fixtures/config/redis_queues_new_format_host.yml

-# redis://[:password@]host[:port][/db-number][?option=value]
-# more details: http://www.iana.org/assignments/uri-schemes/prov/redis
-development:
-  url: redis://:mynewpassword@localhost:6381/11
-  sentinels:
-    -
-      host: localhost
-      port: 26381 # point to sentinel, not to redis port
-    -
-      host: replica2
-      port: 26381 # point to sentinel, not to redis port
-test:
-  url: redis://:mynewpassword@localhost:6381/11
-  sentinels:
-    -
-      host: localhost
-      port: 26381 # point to sentinel, not to redis port
-    -
-      host: replica2
-      port: 26381 # point to sentinel, not to redis port
-production:
-  url: redis://:mynewpassword@localhost:6381/11
-  sentinels:
-    -
-      host: replica1
-      port: 26381 # point to sentinel, not to redis port
-    -
-      host: replica2
-      port: 26381 # point to sentinel, not to redis port

spec/fixtures/config/redis_queues_new_format_socket.yml

-development:
-  url: unix:/path/to/redis.queues.sock
-test:
-  url: unix:/path/to/redis.queues.sock
-production:
-  url: unix:/path/to/redis.queues.sock

spec/fixtures/config/redis_queues_old_format_host.yml

-# redis://[:password@]host[:port][/db-number][?option=value]
-# more details: http://www.iana.org/assignments/uri-schemes/prov/redis
-development: redis://:mypassword@localhost:6381/11
-test: redis://:mypassword@localhost:6381/11
-production: redis://:mypassword@localhost:6381/11

spec/fixtures/config/redis_queues_old_format_socket.yml

-development: unix:/path/to/old/redis.queues.sock
-test: unix:/path/to/old/redis.queues.sock
-production: unix:/path/to/old/redis.queues.sock

spec/fixtures/config/redis_shared_state_config_with_env.yml

-test:
-  url: <%= ENV['TEST_GITLAB_REDIS_SHARED_STATE_URL'] %>

spec/fixtures/config/redis_shared_state_new_format_host.yml

-# redis://[:password@]host[:port][/db-number][?option=value]
-# more details: http://www.iana.org/assignments/uri-schemes/prov/redis
-development:
-  url: redis://:mynewpassword@localhost:6382/12
-  sentinels:
-    -
-      host: localhost
-      port: 26382 # point to sentinel, not to redis port
-    -
-      host: replica2
-      port: 26382 # point to sentinel, not to redis port
-test:
-  url: redis://:mynewpassword@localhost:6382/12
-  sentinels:
-    -
-      host: localhost
-      port: 26382 # point to sentinel, not to redis port
-    -
-      host: replica2
-      port: 26382 # point to sentinel, not to redis port
-production:
-  url: redis://:mynewpassword@localhost:6382/12
-  sentinels:
-    -
-      host: replica1
-      port: 26382 # point to sentinel, not to redis port
-    -
-      host: replica2
-      port: 26382 # point to sentinel, not to redis port

spec/fixtures/config/redis_shared_state_new_format_socket.yml

-development:
-  url: unix:/path/to/redis.shared_state.sock
-test:
-  url: unix:/path/to/redis.shared_state.sock
-production:
-  url: unix:/path/to/redis.shared_state.sock

spec/fixtures/config/redis_shared_state_old_format_host.yml

-# redis://[:password@]host[:port][/db-number][?option=value]
-# more details: http://www.iana.org/assignments/uri-schemes/prov/redis
-development: redis://:mypassword@localhost:6382/12
-test: redis://:mypassword@localhost:6382/12
-production: redis://:mypassword@localhost:6382/12

spec/fixtures/config/redis_shared_state_old_format_socket.yml

-development: unix:/path/to/old/redis.shared_state.sock
-test: unix:/path/to/old/redis.shared_state.sock
-production: unix:/path/to/old/redis.shared_state.sock

spec/lib/gitlab/redis/cache_spec.rb

@@ -3,19 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Redis::Cache do
-  let(:config_file_name) { "config/redis.cache.yml" }
+  let(:instance_specific_config_file) { "config/redis.cache.yml" }
   let(:environment_config_file_name) { "GITLAB_REDIS_CACHE_CONFIG_FILE" }
-  let(:config_old_format_socket) { "spec/fixtures/config/redis_cache_old_format_socket.yml" }
-  let(:config_new_format_socket) { "spec/fixtures/config/redis_cache_new_format_socket.yml" }
-  let(:old_socket_path) {"/path/to/old/redis.cache.sock" }
-  let(:new_socket_path) {"/path/to/redis.cache.sock" }
-  let(:config_old_format_host) { "spec/fixtures/config/redis_cache_old_format_host.yml" }
-  let(:config_new_format_host) { "spec/fixtures/config/redis_cache_new_format_host.yml" }
-  let(:redis_port) { 6380 }
-  let(:redis_database) { 10 }
-  let(:sentinel_port) { redis_port + 20000 }
-  let(:config_with_environment_variable_inside) { "spec/fixtures/config/redis_cache_config_with_env.yml"}
-  let(:config_env_variable_url) {"TEST_GITLAB_REDIS_CACHE_URL"}
   let(:class_redis_url) { Gitlab::Redis::Cache::DEFAULT_REDIS_CACHE_URL }
 
   include_examples "redis_shared_examples"

spec/lib/gitlab/redis/queues_spec.rb

@@ -3,19 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Redis::Queues do
-  let(:config_file_name) { "config/redis.queues.yml" }
+  let(:instance_specific_config_file) { "config/redis.queues.yml" }
   let(:environment_config_file_name) { "GITLAB_REDIS_QUEUES_CONFIG_FILE" }
-  let(:config_old_format_socket) { "spec/fixtures/config/redis_queues_old_format_socket.yml" }
-  let(:config_new_format_socket) { "spec/fixtures/config/redis_queues_new_format_socket.yml" }
-  let(:old_socket_path) {"/path/to/old/redis.queues.sock" }
-  let(:new_socket_path) {"/path/to/redis.queues.sock" }
-  let(:config_old_format_host) { "spec/fixtures/config/redis_queues_old_format_host.yml" }
-  let(:config_new_format_host) { "spec/fixtures/config/redis_queues_new_format_host.yml" }
-  let(:redis_port) { 6381 }
-  let(:redis_database) { 11 }
-  let(:sentinel_port) { redis_port + 20000 }
-  let(:config_with_environment_variable_inside) { "spec/fixtures/config/redis_queues_config_with_env.yml"}
-  let(:config_env_variable_url) {"TEST_GITLAB_REDIS_QUEUES_URL"}
   let(:class_redis_url) { Gitlab::Redis::Queues::DEFAULT_REDIS_QUEUES_URL }
 
   include_examples "redis_shared_examples"

spec/lib/gitlab/redis/shared_state_spec.rb

@@ -3,19 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Redis::SharedState do
-  let(:config_file_name) { "config/redis.shared_state.yml" }
+  let(:instance_specific_config_file) { "config/redis.shared_state.yml" }
   let(:environment_config_file_name) { "GITLAB_REDIS_SHARED_STATE_CONFIG_FILE" }
-  let(:config_old_format_socket) { "spec/fixtures/config/redis_shared_state_old_format_socket.yml" }
-  let(:config_new_format_socket) { "spec/fixtures/config/redis_shared_state_new_format_socket.yml" }
-  let(:old_socket_path) {"/path/to/old/redis.shared_state.sock" }
-  let(:new_socket_path) {"/path/to/redis.shared_state.sock" }
-  let(:config_old_format_host) { "spec/fixtures/config/redis_shared_state_old_format_host.yml" }
-  let(:config_new_format_host) { "spec/fixtures/config/redis_shared_state_new_format_host.yml" }
-  let(:redis_port) { 6382 }
-  let(:redis_database) { 12 }
-  let(:sentinel_port) { redis_port + 20000 }
-  let(:config_with_environment_variable_inside) { "spec/fixtures/config/redis_shared_state_config_with_env.yml"}
-  let(:config_env_variable_url) {"TEST_GITLAB_REDIS_SHARED_STATE_URL"}
   let(:class_redis_url) { Gitlab::Redis::SharedState::DEFAULT_REDIS_SHARED_STATE_URL }
 
   include_examples "redis_shared_examples"

spec/lib/gitlab/redis/wrapper_spec.rb

@@ -3,47 +3,9 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Redis::Wrapper do
-  let(:config_file_name) { "config/resque.yml" }
-  let(:environment_config_file_name) { "GITLAB_REDIS_CONFIG_FILE" }
-  let(:config_old_format_socket) { "spec/fixtures/config/redis_old_format_socket.yml" }
-  let(:config_new_format_socket) { "spec/fixtures/config/redis_new_format_socket.yml" }
-  let(:old_socket_path) {"/path/to/old/redis.sock" }
-  let(:new_socket_path) {"/path/to/redis.sock" }
-  let(:config_old_format_host) { "spec/fixtures/config/redis_old_format_host.yml" }
-  let(:config_new_format_host) { "spec/fixtures/config/redis_new_format_host.yml" }
-  let(:redis_port) { 6379 }
-  let(:redis_database) { 99 }
-  let(:sentinel_port) { redis_port + 20000 }
-  let(:config_with_environment_variable_inside) { "spec/fixtures/config/redis_config_with_env.yml"}
-  let(:config_env_variable_url) {"TEST_GITLAB_REDIS_URL"}
-  let(:class_redis_url) { Gitlab::Redis::Wrapper::DEFAULT_REDIS_URL }
-
-  include_examples "redis_shared_examples" do
-    before do
-      allow(described_class).to receive(:instrumentation_class) do
-        ::Gitlab::Instrumentation::Redis::Cache
-      end
-    end
-  end
-
-  describe '.version' do
-    it 'returns a version' do
-      expect(described_class.version).to be_present
-    end
-  end
-
   describe '.instrumentation_class' do
     it 'raises a NotImplementedError' do
-      expect(described_class).to receive(:instrumentation_class).and_call_original
-
       expect { described_class.instrumentation_class }.to raise_error(NotImplementedError)
     end
   end
-
-  describe '.config_file_path' do
-    it 'returns the absolute path to the configuration file' do
-      expect(described_class.config_file_path('foo.yml'))
-        .to eq Rails.root.join('config', 'foo.yml').to_s
-    end
-  end
 end

spec/support/redis/redis_shared_examples.rb

@@ -4,9 +4,21 @@ RSpec.shared_examples "redis_shared_examples" do
   include StubENV
 
   let(:test_redis_url) { "redis://redishost:#{redis_port}"}
+  let(:config_file_name) { instance_specific_config_file }
+  let(:config_old_format_socket) { "spec/fixtures/config/redis_old_format_socket.yml" }
+  let(:config_new_format_socket) { "spec/fixtures/config/redis_new_format_socket.yml" }
+  let(:old_socket_path) {"/path/to/old/redis.sock" }
+  let(:new_socket_path) {"/path/to/redis.sock" }
+  let(:config_old_format_host) { "spec/fixtures/config/redis_old_format_host.yml" }
+  let(:config_new_format_host) { "spec/fixtures/config/redis_new_format_host.yml" }
+  let(:redis_port) { 6379 }
+  let(:redis_database) { 99 }
+  let(:sentinel_port) { 26379 }
+  let(:config_with_environment_variable_inside) { "spec/fixtures/config/redis_config_with_env.yml"}
+  let(:config_env_variable_url) {"TEST_GITLAB_REDIS_URL"}
 
   before do
-    stub_env(environment_config_file_name, Rails.root.join(config_file_name))
+    allow(described_class).to receive(:config_file_name).and_return(Rails.root.join(config_file_name).to_s)
     clear_raw_config
   end
 
@@ -14,8 +26,72 @@ RSpec.shared_examples "redis_shared_examples" do
     clear_raw_config
   end
 
+  describe '.config_file_name' do
+    subject { described_class.config_file_name }
+
+    let(:rails_root) { Dir.mktmpdir('redis_shared_examples') }
+
+    before do
+      # Undo top-level stub of config_file_name because we are testing that method now.
+      allow(described_class).to receive(:config_file_name).and_call_original
+
+      allow(described_class).to receive(:rails_root).and_return(rails_root)
+      FileUtils.mkdir_p(File.join(rails_root, 'config'))
+    end
+
+    after do
+      FileUtils.rm_rf(rails_root)
+    end
+
+    context 'when there is no config file anywhere' do
+      it { expect(subject).to be_nil }
+
+      context 'but resque.yml exists' do
+        before do
+          FileUtils.touch(File.join(rails_root, 'config', 'resque.yml'))
+        end
+
+        it { expect(subject).to eq("#{rails_root}/config/resque.yml") }
+
+        it 'returns a path that exists' do
+          expect(File.file?(subject)).to eq(true)
+        end
+
+        context 'and there is a global env override' do
+          before do
+            stub_env('GITLAB_REDIS_CONFIG_FILE', 'global override')
+          end
+
+          it { expect(subject).to eq('global override') }
+
+          context 'and there is an instance specific config file' do
+            before do
+              FileUtils.touch(File.join(rails_root, instance_specific_config_file))
+            end
+
+            it { expect(subject).to eq("#{rails_root}/#{instance_specific_config_file}") }
+
+            it 'returns a path that exists' do
+              expect(File.file?(subject)).to eq(true)
+            end
+
+            context 'and there is a specific env override' do
+              before do
+                stub_env(environment_config_file_name, 'instance specific override')
+              end
+
+              it { expect(subject).to eq('instance specific override') }
+            end
+          end
+        end
+      end
+    end
+  end
+
   describe '.params' do
-    subject { described_class.params }
+    subject { described_class.new(rails_env).params }
+
+    let(:rails_env) { 'development' }
 
     it 'withstands mutation' do
       params1 = described_class.params
@@ -58,13 +134,23 @@ RSpec.shared_examples "redis_shared_examples" do
       context 'with new format' do
         let(:config_file_name) { config_new_format_host }
 
+        where(:rails_env, :host) do
+          [
+            %w[development development-host],
+            %w[test test-host],
+            %w[production production-host]
+          ]
+        end
+
+        with_them do
           it 'returns hash with host, port, db, and password' do
-          is_expected.to include(host: 'localhost', password: 'mynewpassword', port: redis_port, db: redis_database)
+            is_expected.to include(host: host, password: 'mynewpassword', port: redis_port, db: redis_database)
             is_expected.not_to have_key(:url)
           end
         end
       end
     end
+  end
 
   describe '.url' do
     it 'withstands mutation' do
@@ -88,6 +174,12 @@ RSpec.shared_examples "redis_shared_examples" do
     end
   end
 
+  describe '.version' do
+    it 'returns a version' do
+      expect(described_class.version).to be_present
+    end
+  end
+
   describe '._raw_config' do
     subject { described_class._raw_config }
 
@@ -143,14 +235,26 @@ RSpec.shared_examples "redis_shared_examples" do
   end
 
   describe '#sentinels' do
-    subject { described_class.new(Rails.env).sentinels }
+    subject { described_class.new(rails_env).sentinels }
+
+    let(:rails_env) { 'development' }
 
     context 'when sentinels are defined' do
       let(:config_file_name) { config_new_format_host }
 
+      where(:rails_env, :hosts) do
+        [
+          ['development', %w[development-replica1 development-replica2]],
+          ['test', %w[test-replica1 test-replica2]],
+          ['production', %w[production-replica1 production-replica2]]
+        ]
+      end
+
+      with_them do
         it 'returns an array of hashes with host and port keys' do
-        is_expected.to include(host: 'localhost', port: sentinel_port)
-        is_expected.to include(host: 'replica2', port: sentinel_port)
+          is_expected.to include(host: hosts[0], port: sentinel_port)
+          is_expected.to include(host: hosts[1], port: sentinel_port)
+        end
       end
     end