Commit 5838da3d authored by Gabriel Mazetto's avatar Gabriel Mazetto
Browse files

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.
parent bba971070855
---
title: Feature flag to disable Hashed Storage migration when renaming a repository
merge_request: 21291
author:
type: added
......@@ -57,3 +57,15 @@ end
Features that are developed and are intended to be merged behind a feature flag
should not include a changelog entry. The entry should be added in the merge
request removing the feature flags.
### Specs
In the test environment `Feature.enabled?` is stubbed to always respond to `true`,
so we make sure behavior under feature flag doesn't go untested in some non-specific
contexts.
If you need to test the feature flag in a different state, you need to stub it with:
```ruby
stub_feature_flags(my_feature_flag: false)
```
......@@ -47,7 +47,8 @@ def enabled?(key, thing = nil)
end
def disabled?(key, thing = nil)
!enabled?(key, thing)
# we need to make different method calls to make it easy to mock / define expectations in test mode
thing.nil? ? !enabled?(key) : !enabled?(key, thing)
end
def enable(key, thing = true)
......
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 @@
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
......@@ -2989,6 +2989,7 @@ def enable_lfs
# 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 @@ def enable_lfs
# 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
......
......@@ -262,6 +262,7 @@
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 @@
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
......
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 }
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment