Fixed `stub_feature_flag behavior` for `disabled?` flags.

Previous code would not work with `disabled?` because that method would
send two parameters (second always `nil`) which we are not mocking.

Instead of mock yet another state, I decide to fix it where it belongs.

4 files changed, 73 insertions, 1 deletions

diff --git a/spec/lib/feature_spec.rb b/spec/lib/feature_spec.rb
index f313e675654..8bb5a843484 100644
--- a/spec/lib/feature_spec.rb
+++ b/spec/lib/feature_spec.rb
@@ -1,6 +1,13 @@
 require 'spec_helper'
 
 describe Feature do
+  before do
+    # We mock all calls to .enabled? to return true in order to force all
+    # specs to run the feature flag gated behavior, but here we need a clean
+    # behavior from the class
+    allow(described_class).to receive(:enabled?).and_call_original
+  end
+
   describe '.get' do
     let(:feature) { double(:feature) }
     let(:key) { 'my_feature' }
@@ -106,4 +113,63 @@ describe Feature do
       it_behaves_like 'a memoized Flipper instance'
     end
   end
+
+  describe '.enabled?' do
+    it 'returns false for undefined feature' do
+      expect(described_class.enabled?(:some_random_feature_flag)).to be_falsey
+    end
+
+    it 'returns false for existing disabled feature in the database' do
+      described_class.disable(:disabled_feature_flag)
+
+      expect(described_class.enabled?(:disabled_feature_flag)).to be_falsey
+    end
+
+    it 'returns true for existing enabled feature in the database' do
+      described_class.enable(:enabled_feature_flag)
+
+      expect(described_class.enabled?(:enabled_feature_flag)).to be_truthy
+    end
+
+    context 'with an individual actor' do
+      CustomActor = Struct.new(:flipper_id)
+
+      let(:actor) { CustomActor.new(flipper_id: 'CustomActor:5') }
+      let(:another_actor) { CustomActor.new(flipper_id: 'CustomActor:10') }
+
+      before do
+        described_class.enable(:enabled_feature_flag, actor)
+      end
+
+      it 'returns true when same actor is informed' do
+        expect(described_class.enabled?(:enabled_feature_flag, actor)).to be_truthy
+      end
+
+      it 'returns false when different actor is informed' do
+        expect(described_class.enabled?(:enabled_feature_flag, another_actor)).to be_falsey
+      end
+
+      it 'returns false when no actor is informed' do
+        expect(described_class.enabled?(:enabled_feature_flag)).to be_falsey
+      end
+    end
+  end
+
+  describe '.disable?' do
+    it 'returns true for undefined feature' do
+      expect(described_class.disabled?(:some_random_feature_flag)).to be_truthy
+    end
+
+    it 'returns true for existing disabled feature in the database' do
+      described_class.disable(:disabled_feature_flag)
+
+      expect(described_class.disabled?(:disabled_feature_flag)).to be_truthy
+    end
+
+    it 'returns false for existing enabled feature in the database' do
+      described_class.enable(:enabled_feature_flag)
+
+      expect(described_class.disabled?(:enabled_feature_flag)).to be_falsey
+    end
+  end
 end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 8cb706b54b0..6f82a83ddd2 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -2989,6 +2989,7 @@ describe Project do
         # call. This makes testing a bit easier.
         allow(project).to receive(:gitlab_shell).and_return(gitlab_shell)
         allow(project).to receive(:previous_changes).and_return('path' => ['foo'])
+        stub_feature_flags(disable_hashed_storage_upgrade: false)
       end
 
       it 'renames a repository' do
@@ -3160,6 +3161,7 @@ describe Project do
         # call. This makes testing a bit easier.
         allow(project).to receive(:gitlab_shell).and_return(gitlab_shell)
         allow(project).to receive(:previous_changes).and_return('path' => ['foo'])
+        stub_feature_flags(disable_hashed_storage_upgrade: false)
       end
 
       context 'migration to hashed storage' do
diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb
index 2409be80cec..bc8bda1000e 100644
--- a/spec/services/projects/update_service_spec.rb
+++ b/spec/services/projects/update_service_spec.rb
@@ -262,6 +262,7 @@ describe Projects::UpdateService do
         context 'when hashed storage is enabled' do
           before do
             stub_application_setting(hashed_storage_enabled: true)
+            stub_feature_flags(disable_hashed_storage_upgrade: false)
           end
 
           it 'migrates project to a hashed storage instead of renaming the repo to another legacy name' do
@@ -275,7 +276,7 @@ describe Projects::UpdateService do
 
           context 'when disable_hashed_storage_upgrade feature flag is enabled' do
             before do
-              expect(Feature).to receive(:enabled?).with(:disable_hashed_storage_upgrade) { true }
+              stub_feature_flags(disable_hashed_storage_upgrade: true)
             end
 
             it 'renames the project without upgrading it' do
diff --git a/spec/support/helpers/stub_feature_flags.rb b/spec/support/helpers/stub_feature_flags.rb
index b96338bf548..c54a871b157 100644
--- a/spec/support/helpers/stub_feature_flags.rb
+++ b/spec/support/helpers/stub_feature_flags.rb
@@ -1,4 +1,7 @@
 module StubFeatureFlags
+  # Stub Feature flags with `flag_name: true/false`
+  #
+  # @param [Hash] features where key is feature name and value is boolean whether enabled or not
   def stub_feature_flags(features)
     features.each do |feature_name, enabled|
       allow(Feature).to receive(:enabled?).with(feature_name) { enabled }