Merge branch '348135-improve-redis-sessions-specs' into 'master'

Improve Redis-Sessions specs

See merge request gitlab-org/gitlab!76821

spec/config/mail_room_spec.rb

@@ -26,11 +26,11 @@ RSpec.describe 'mail_room.yml' do
 
   before do
     stub_env('GITLAB_REDIS_QUEUES_CONFIG_FILE', absolute_path(queues_config_path))
-    clear_queues_raw_config
+    redis_clear_raw_config!(Gitlab::Redis::Queues)
   end
 
   after do
-    clear_queues_raw_config
+    redis_clear_raw_config!(Gitlab::Redis::Queues)
   end
 
   context 'when incoming email is disabled' do
@@ -103,12 +103,6 @@ RSpec.describe 'mail_room.yml' do
     end
   end
 
-  def clear_queues_raw_config
-    Gitlab::Redis::Queues.remove_instance_variable(:@_raw_config)
-  rescue NameError
-    # raised if @_raw_config was not set; ignore
-  end
-
   def absolute_path(path)
     Rails.root.join(path).to_s
   end

spec/lib/gitlab/redis/sessions_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Redis::Sessions do
-  include_examples "redis_new_instance_shared_examples", 'sessions', Gitlab::Redis::SharedState
+  it_behaves_like "redis_new_instance_shared_examples", 'sessions', Gitlab::Redis::SharedState
 
   describe 'redis instance used in connection pool' do
     before do
@@ -42,25 +42,51 @@ RSpec.describe Gitlab::Redis::Sessions do
   end
 
   describe '#store' do
-    subject { described_class.store(namespace: described_class::SESSION_NAMESPACE) }
+    subject(:store) { described_class.store(namespace: described_class::SESSION_NAMESPACE) }
 
     context 'when redis.sessions configuration is NOT provided' do
       it 'instantiates ::Redis instance' do
         expect(described_class).to receive(:config_fallback?).and_return(true)
-        expect(subject).to be_instance_of(::Redis::Store)
+        expect(store).to be_instance_of(::Redis::Store)
       end
     end
 
     context 'when redis.sessions configuration is provided' do
+      let(:config_new_format_host) { "spec/fixtures/config/redis_new_format_host.yml" }
+      let(:config_new_format_socket) { "spec/fixtures/config/redis_new_format_socket.yml" }
+
       before do
+        redis_clear_raw_config!(Gitlab::Redis::Sessions)
+        redis_clear_raw_config!(Gitlab::Redis::SharedState)
         allow(described_class).to receive(:config_fallback?).and_return(false)
       end
 
-      it 'instantiates an instance of MultiStore' do
-        expect(subject).to be_instance_of(::Gitlab::Redis::MultiStore)
+      after do
+        redis_clear_raw_config!(Gitlab::Redis::Sessions)
+        redis_clear_raw_config!(Gitlab::Redis::SharedState)
+      end
+
+      # Check that Gitlab::Redis::Sessions is configured as MultiStore with proper attrs.
+      it 'instantiates an instance of MultiStore', :aggregate_failures do
+        expect(described_class).to receive(:config_file_name).and_return(config_new_format_host)
+        expect(::Gitlab::Redis::SharedState).to receive(:config_file_name).and_return(config_new_format_socket)
+
+        expect(store).to be_instance_of(::Gitlab::Redis::MultiStore)
+
+        expect(store.primary_store.to_s).to eq("Redis Client connected to test-host:6379 against DB 99 with namespace session:gitlab")
+        expect(store.secondary_store.to_s).to eq("Redis Client connected to /path/to/redis.sock against DB 0 with namespace session:gitlab")
+
+        expect(store.instance_name).to eq('Sessions')
+      end
+
+      context 'when MultiStore correctly configured' do
+        before do
+          allow(described_class).to receive(:config_file_name).and_return(config_new_format_host)
+          allow(::Gitlab::Redis::SharedState).to receive(:config_file_name).and_return(config_new_format_socket)
         end
 
         it_behaves_like 'multi store feature flags', :use_primary_and_secondary_stores_for_sessions, :use_primary_store_as_default_for_sessions
       end
     end
+  end
 end

spec/support/redis/redis_helpers.rb

@@ -32,4 +32,11 @@ module RedisHelpers
   def redis_sessions_cleanup!
     Gitlab::Redis::Sessions.with(&:flushdb)
   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

spec/support/redis/redis_new_instance_shared_examples.rb

@@ -8,13 +8,13 @@ RSpec.shared_examples "redis_new_instance_shared_examples" do |name, fallback_cl
   let(:fallback_config_file) { nil }
 
   before do
-    fallback_class.remove_instance_variable(:@_raw_config) rescue nil
+    redis_clear_raw_config!(fallback_class)
 
     allow(fallback_class).to receive(:config_file_name).and_return(fallback_config_file)
   end
 
   after do
-    fallback_class.remove_instance_variable(:@_raw_config) rescue nil
+    redis_clear_raw_config!(fallback_class)
   end
 
   it_behaves_like "redis_shared_examples"

spec/support/redis/redis_shared_examples.rb

@@ -20,11 +20,11 @@ RSpec.shared_examples "redis_shared_examples" do
 
   before do
     allow(described_class).to receive(:config_file_name).and_return(Rails.root.join(config_file_name).to_s)
-    clear_raw_config
+    redis_clear_raw_config!(described_class)
   end
 
   after do
-    clear_raw_config
+    redis_clear_raw_config!(described_class)
   end
 
   describe '.config_file_name' do
@@ -399,12 +399,6 @@ RSpec.shared_examples "redis_shared_examples" do
     end
   end
 
-  def clear_raw_config
-    described_class.remove_instance_variable(:@_raw_config)
-  rescue NameError
-    # raised if @_raw_config was not set; ignore
-  end
-
   def clear_pool
     described_class.remove_instance_variable(:@pool)
   rescue NameError