3 files changed, 78 insertions, 65 deletions

diff --git a/spec/support/redis/redis_helpers.rb b/spec/support/redis/redis_helpers.rb
index 34ac69236ee..2c5ceb2f09e 100644
--- a/spec/support/redis/redis_helpers.rb
+++ b/spec/support/redis/redis_helpers.rb
@@ -6,11 +6,4 @@ module RedisHelpers
       instance_class.with(&:flushdb)
     end
   end
-
-  # Usage: reset cached instance config
-  def redis_clear_raw_config!(instance_class)
-    instance_class.remove_instance_variable(:@_raw_config)
-  rescue NameError
-    # raised if @_raw_config was not set; ignore
-  end
 end
diff --git a/spec/support/redis/redis_new_instance_shared_examples.rb b/spec/support/redis/redis_new_instance_shared_examples.rb
index 943fe0f11ba..0f2de78b2cb 100644
--- a/spec/support/redis/redis_new_instance_shared_examples.rb
+++ b/spec/support/redis/redis_new_instance_shared_examples.rb
@@ -3,27 +3,22 @@
 require 'spec_helper'
 
 RSpec.shared_examples "redis_new_instance_shared_examples" do |name, fallback_class|
+  include TmpdirHelper
+
   let(:instance_specific_config_file) { "config/redis.#{name}.yml" }
   let(:environment_config_file_name) { "GITLAB_REDIS_#{name.upcase}_CONFIG_FILE" }
   let(:fallback_config_file) { nil }
+  let(:rails_root) { mktmpdir }
 
   before do
-    redis_clear_raw_config!(fallback_class)
-
     allow(fallback_class).to receive(:config_file_name).and_return(fallback_config_file)
   end
 
-  after do
-    redis_clear_raw_config!(fallback_class)
-  end
-
   it_behaves_like "redis_shared_examples"
 
   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
@@ -32,10 +27,6 @@ RSpec.shared_examples "redis_new_instance_shared_examples" do |name, fallback_cl
       FileUtils.mkdir_p(File.join(rails_root, 'config'))
     end
 
-    after do
-      FileUtils.rm_rf(rails_root)
-    end
-
     context 'when there is only a resque.yml' do
       before do
         FileUtils.touch(File.join(rails_root, 'config/resque.yml'))
@@ -58,4 +49,49 @@ RSpec.shared_examples "redis_new_instance_shared_examples" do |name, fallback_cl
       end
     end
   end
+
+  describe '#fetch_config' do
+    context 'when redis.yml exists' do
+      subject { described_class.new('test').send(:fetch_config) }
+
+      before do
+        allow(described_class).to receive(:config_file_name).and_call_original
+        allow(described_class).to receive(:redis_yml_path).and_call_original
+        allow(described_class).to receive(:rails_root).and_return(rails_root)
+        FileUtils.mkdir_p(File.join(rails_root, 'config'))
+      end
+
+      context 'when the fallback has a redis.yml entry' do
+        before do
+          File.write(File.join(rails_root, 'config/redis.yml'), {
+            'test' => {
+              described_class.config_fallback.store_name.underscore => { 'fallback redis.yml' => 123 }
+            }
+          }.to_json)
+        end
+
+        it { expect(subject).to eq({ 'fallback redis.yml' => 123 }) }
+
+        context 'and an instance config file exists' do
+          before do
+            File.write(File.join(rails_root, instance_specific_config_file), {
+              'test' => { 'instance specific file' => 456 }
+            }.to_json)
+          end
+
+          it { expect(subject).to eq({ 'instance specific file' => 456 }) }
+
+          context 'and the instance has a redis.yml entry' do
+            before do
+              File.write(File.join(rails_root, 'config/redis.yml'), {
+                'test' => { name => { 'instance redis.yml' => 789 } }
+              }.to_json)
+            end
+
+            it { expect(subject).to eq({ 'instance redis.yml' => 789 }) }
+          end
+        end
+      end
+    end
+  end
 end
diff --git a/spec/support/redis/redis_shared_examples.rb b/spec/support/redis/redis_shared_examples.rb
index 0368fd63357..43c118a362d 100644
--- a/spec/support/redis/redis_shared_examples.rb
+++ b/spec/support/redis/redis_shared_examples.rb
@@ -2,6 +2,7 @@
 
 RSpec.shared_examples "redis_shared_examples" do
   include StubENV
+  include TmpdirHelper
 
   let(:test_redis_url) { "redis://redishost:#{redis_port}" }
   let(:test_cluster_config) { { cluster: [{ host: "redis://redishost", port: redis_port }] } }
@@ -18,15 +19,11 @@ RSpec.shared_examples "redis_shared_examples" do
   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" }
-  let(:rails_root) { Dir.mktmpdir('redis_shared_examples') }
+  let(:rails_root) { mktmpdir }
 
   before do
     allow(described_class).to receive(:config_file_name).and_return(Rails.root.join(config_file_name).to_s)
-    redis_clear_raw_config!(described_class)
-  end
-
-  after do
-    redis_clear_raw_config!(described_class)
+    allow(described_class).to receive(:redis_yml_path).and_return('/dev/null')
   end
 
   describe '.config_file_name' do
@@ -40,10 +37,6 @@ RSpec.shared_examples "redis_shared_examples" do
       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 }
 
@@ -250,26 +243,6 @@ RSpec.shared_examples "redis_shared_examples" do
     end
   end
 
-  describe '._raw_config' do
-    subject { described_class._raw_config }
-
-    let(:config_file_name) { '/var/empty/doesnotexist' }
-
-    it 'is frozen' do
-      expect(subject).to be_frozen
-    end
-
-    it 'returns false when the file does not exist' do
-      expect(subject).to eq(false)
-    end
-
-    it "returns false when the filename can't be determined" do
-      expect(described_class).to receive(:config_file_name).and_return(nil)
-
-      expect(subject).to eq(false)
-    end
-  end
-
   describe '.with' do
     let(:config_file_name) { config_old_format_socket }
 
@@ -313,10 +286,6 @@ RSpec.shared_examples "redis_shared_examples" do
         allow(described_class).to receive(:rails_root).and_return(rails_root)
       end
 
-      after do
-        FileUtils.rm_rf(rails_root)
-      end
-
       it 'can run an empty block' do
         expect { described_class.with { nil } }.not_to raise_error
       end
@@ -408,9 +377,7 @@ RSpec.shared_examples "redis_shared_examples" do
     context 'when sentinels are not defined' do
       let(:config_file_name) { config_old_format_host }
 
-      it 'returns false' do
-        is_expected.to be_falsey
-      end
+      it { expect(subject).to eq(nil) }
     end
 
     context 'when cluster is defined' do
@@ -435,22 +402,39 @@ RSpec.shared_examples "redis_shared_examples" do
   end
 
   describe '#fetch_config' do
-    it 'returns false when no config file is present' do
-      allow(described_class).to receive(:_raw_config) { false }
+    it 'raises an exception when the config file contains invalid yaml' do
+      Tempfile.open('bad.yml') do |file|
+        file.write('{"not":"yaml"')
+        file.flush
+        allow(described_class).to receive(:config_file_name) { file.path }
 
-      expect(subject.send(:fetch_config)).to eq false
+        expect { subject.send(:fetch_config) }.to raise_error(Psych::SyntaxError)
+      end
     end
 
-    it 'returns false when config file is present but has invalid YAML' do
-      allow(described_class).to receive(:_raw_config) { "# development: true" }
+    it 'has a value for the legacy default URL' do
+      allow(subject).to receive(:fetch_config) { nil }
 
-      expect(subject.send(:fetch_config)).to eq false
+      expect(subject.send(:raw_config_hash)).to include(url: a_string_matching(%r{\Aredis://localhost:638[012]\Z}))
     end
 
-    it 'has a value for the legacy default URL' do
-      allow(subject).to receive(:fetch_config) { false }
+    context 'when redis.yml exists' do
+      subject { described_class.new('test').send(:fetch_config) }
 
-      expect(subject.send(:raw_config_hash)).to include(url: a_string_matching(%r{\Aredis://localhost:638[012]\Z}))
+      before do
+        allow(described_class).to receive(:config_file_name).and_call_original
+        allow(described_class).to receive(:redis_yml_path).and_call_original
+        allow(described_class).to receive(:rails_root).and_return(rails_root)
+        FileUtils.mkdir_p(File.join(rails_root, 'config'))
+      end
+
+      it 'uses config/redis.yml' do
+        File.write(File.join(rails_root, 'config/redis.yml'), {
+          'test' => { described_class.store_name.underscore => { 'foobar' => 123 } }
+        }.to_json)
+
+        expect(subject).to eq({ 'foobar' => 123 })
+      end
     end
   end