Commit 0ec832ec authored by Kamil Trzciński's avatar Kamil Trzciński

Add `AvoidFeatureGet` RuboCop rule

This removes the usage of `Feature.get`
and instead provides a helpers for almost
all possible usages.
parent c22f7b06
...@@ -274,6 +274,9 @@ Cop/ActiveRecordAssociationReload: ...@@ -274,6 +274,9 @@ Cop/ActiveRecordAssociationReload:
- 'spec/**/*' - 'spec/**/*'
- 'ee/spec/**/*' - 'ee/spec/**/*'
Gitlab/AvoidFeatureGet:
Enabled: true
Naming/PredicateName: Naming/PredicateName:
Enabled: true Enabled: true
Exclude: Exclude:
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
module AutoDevopsHelper module AutoDevopsHelper
def show_auto_devops_callout?(project) def show_auto_devops_callout?(project)
Feature.get(:auto_devops_banner_disabled).off? && Feature.disabled?(:auto_devops_banner_disabled) &&
show_callout?('auto_devops_settings_dismissed') && show_callout?('auto_devops_settings_dismissed') &&
can?(current_user, :admin_pipeline, project) && can?(current_user, :admin_pipeline, project) &&
project.has_auto_devops_implicitly_disabled? && project.has_auto_devops_implicitly_disabled? &&
......
...@@ -357,7 +357,7 @@ Feature.enabled?(:my_feature2) # => false ...@@ -357,7 +357,7 @@ Feature.enabled?(:my_feature2) # => false
Feature.enabled?(:my_feature2, project1) # => true Feature.enabled?(:my_feature2, project1) # => true
``` ```
#### `stub_feature_flags` vs `Feature.get/enable` #### `stub_feature_flags` vs `Feature.enable*`
It is preferred to use `stub_feature_flags` for enabling feature flags It is preferred to use `stub_feature_flags` for enabling feature flags
in testing environment. This method provides a simple and well described in testing environment. This method provides a simple and well described
...@@ -378,10 +378,10 @@ stub_feature_flags(my_feature: [project, project2]) ...@@ -378,10 +378,10 @@ stub_feature_flags(my_feature: [project, project2])
Feature.enable(:my_feature_2) Feature.enable(:my_feature_2)
# Good: enable my_feature for 50% of time # Good: enable my_feature for 50% of time
Feature.get(:my_feature_3).enable_percentage_of_time(50) Feature.enable_percentage_of_time(:my_feature_3, 50)
# Good: enable my_feature for 50% of actors/gates/things # Good: enable my_feature for 50% of actors/gates/things
Feature.get(:my_feature_4).enable_percentage_of_actors(50) Feature.enable_percentage_of_actors(:my_feature_4, 50)
``` ```
Each feature flag that has a defined state will be persisted Each feature flag that has a defined state will be persisted
......
...@@ -598,7 +598,7 @@ The banner can be disabled for: ...@@ -598,7 +598,7 @@ The banner can be disabled for:
- By an administrator running the following in a Rails console: - By an administrator running the following in a Rails console:
```ruby ```ruby
Feature.get(:auto_devops_banner_disabled).enable Feature.enable(:auto_devops_banner_disabled)
``` ```
- Through the REST API with an admin access token: - Through the REST API with an admin access token:
......
...@@ -346,7 +346,7 @@ of projects. From the console, enter the following command, replacing `10` with ...@@ -346,7 +346,7 @@ of projects. From the console, enter the following command, replacing `10` with
your desired percentage: your desired percentage:
```ruby ```ruby
Feature.get(:force_autodevops_on_by_default).enable_percentage_of_actors(10) Feature.enable_percentage_of_actors(:force_autodevops_on_by_default, 10)
``` ```
### Deployment strategy ### Deployment strategy
......
...@@ -337,7 +337,7 @@ module EE ...@@ -337,7 +337,7 @@ module EE
filter = user_preference.feature_filter_type.presence || 0 filter = user_preference.feature_filter_type.presence || 0
# We use a 2nd feature flag for control as enabled and percentage_of_time for chatops # We use a 2nd feature flag for control as enabled and percentage_of_time for chatops
flipper_feature = ::Feature.get((feature.to_s + '_control').to_sym) flipper_feature = ::Feature.get((feature.to_s + '_control').to_sym) # rubocop:disable Gitlab/AvoidFeatureGet
percentage ||= flipper_feature.gate_values[:percentage_of_time] || 0 if flipper_feature percentage ||= flipper_feature.gate_values[:percentage_of_time] || 0 if flipper_feature
return false if percentage <= 0 return false if percentage <= 0
......
...@@ -29,6 +29,30 @@ module EE ...@@ -29,6 +29,30 @@ module EE
log_geo_event(key) log_geo_event(key)
end end
override :enable_percentage_of_time
def enable_percentage_of_time(key, percentage)
super
log_geo_event(key)
end
override :disable_percentage_of_time
def disable_percentage_of_time(key)
super
log_geo_event(key)
end
override :enable_percentage_of_actors
def enable_percentage_of_actors(key, percentage)
super
log_geo_event(key)
end
override :disable_percentage_of_actors
def disable_percentage_of_actors(key)
super
log_geo_event(key)
end
private private
def log_geo_event(key) def log_geo_event(key)
......
...@@ -55,8 +55,8 @@ describe OnboardingExperimentHelper, type: :helper do ...@@ -55,8 +55,8 @@ describe OnboardingExperimentHelper, type: :helper do
context 'and experiment_growth_onboarding has been set' do context 'and experiment_growth_onboarding has been set' do
it 'checks if feature is enabled for current_user' do it 'checks if feature is enabled for current_user' do
feature = Feature.get(described_class::EXPERIMENT_GROWTH_ONBOARDING_FEATURE_NAME) Feature.enable_percentage_of_actors(
feature.enable_percentage_of_actors(50) described_class::EXPERIMENT_GROWTH_ONBOARDING_FEATURE_NAME, 50)
expect(helper.allow_access_to_onboarding?).to eq(true) expect(helper.allow_access_to_onboarding?).to eq(true)
end end
......
...@@ -44,7 +44,7 @@ describe RecaptchaExperimentHelper, type: :helper do ...@@ -44,7 +44,7 @@ describe RecaptchaExperimentHelper, type: :helper do
with_them do with_them do
before do before do
# Enable feature to 50% of actors # Enable feature to 50% of actors
Feature.get(feature_name).enable_percentage_of_actors(50) Feature.enable_percentage_of_actors(feature_name, 50)
end end
it "returns expected_result" do it "returns expected_result" do
......
...@@ -977,7 +977,7 @@ describe User do ...@@ -977,7 +977,7 @@ describe User do
end end
it 'returns true when 100% control percentage is provided' do it 'returns true when 100% control percentage is provided' do
Feature.get(:discover_security_control).enable_percentage_of_time(100) Feature.enable_percentage_of_time(:discover_security_control, 100)
expect(experiment_user.ab_feature_enabled?(:discover_security)).to eq(true) expect(experiment_user.ab_feature_enabled?(:discover_security)).to eq(true)
expect(experiment_user.user_preference.feature_filter_type).to eq(UserPreference::FEATURE_FILTER_EXPERIMENT) expect(experiment_user.user_preference.feature_filter_type).to eq(UserPreference::FEATURE_FILTER_EXPERIMENT)
......
...@@ -61,7 +61,7 @@ module API ...@@ -61,7 +61,7 @@ module API
mutually_exclusive :key, :project mutually_exclusive :key, :project
end end
post ':name' do post ':name' do
feature = Feature.get(params[:name]) feature = Feature.get(params[:name]) # rubocop:disable Gitlab/AvoidFeatureGet
targets = gate_targets(params) targets = gate_targets(params)
value = gate_value(params) value = gate_value(params)
key = gate_key(params) key = gate_key(params)
...@@ -92,7 +92,7 @@ module API ...@@ -92,7 +92,7 @@ module API
desc 'Remove the gate value for the given feature' desc 'Remove the gate value for the given feature'
delete ':name' do delete ':name' do
Feature.get(params[:name]).remove Feature.remove(params[:name])
no_content! no_content!
end end
......
...@@ -79,7 +79,7 @@ class Feature ...@@ -79,7 +79,7 @@ class Feature
# for the feature yet and return the default. # for the feature yet and return the default.
return default_enabled unless Gitlab::Database.exists? return default_enabled unless Gitlab::Database.exists?
feature = Feature.get(key) feature = get(key)
# If we're not default enabling the flag or the feature has been set, always evaluate. # If we're not default enabling the flag or the feature has been set, always evaluate.
# `persisted?` can potentially generate DB queries and also checks for inclusion # `persisted?` can potentially generate DB queries and also checks for inclusion
...@@ -109,12 +109,41 @@ class Feature ...@@ -109,12 +109,41 @@ class Feature
get(key).disable_group(group) get(key).disable_group(group)
end end
def enable_percentage_of_time(key, percentage)
get(key).enable_percentage_of_time(percentage)
end
def disable_percentage_of_time(key)
get(key).disable_percentage_of_time
end
def enable_percentage_of_actors(key, percentage)
get(key).enable_percentage_of_actors(percentage)
end
def disable_percentage_of_actors(key)
get(key).disable_percentage_of_actors
end
def remove(key) def remove(key)
return unless persisted_name?(key) return unless persisted_name?(key)
get(key).remove get(key).remove
end end
def reset
Gitlab::SafeRequestStore.delete(:flipper) if Gitlab::SafeRequestStore.active?
@flipper = nil
end
# This method is called from config/initializers/flipper.rb and can be used
# to register Flipper groups.
# See https://docs.gitlab.com/ee/development/feature_flags.html#feature-groups
def register_feature_groups
end
private
def flipper def flipper
if Gitlab::SafeRequestStore.active? if Gitlab::SafeRequestStore.active?
Gitlab::SafeRequestStore[:flipper] ||= build_flipper_instance Gitlab::SafeRequestStore[:flipper] ||= build_flipper_instance
...@@ -123,11 +152,6 @@ class Feature ...@@ -123,11 +152,6 @@ class Feature
end end
end end
def reset
Gitlab::SafeRequestStore.delete(:flipper) if Gitlab::SafeRequestStore.active?
@flipper = nil
end
def build_flipper_instance def build_flipper_instance
active_record_adapter = Flipper::Adapters::ActiveRecord.new( active_record_adapter = Flipper::Adapters::ActiveRecord.new(
feature_class: FlipperFeature, feature_class: FlipperFeature,
...@@ -158,12 +182,6 @@ class Feature ...@@ -158,12 +182,6 @@ class Feature
Gitlab.dev_or_test_env? Gitlab.dev_or_test_env?
end end
# This method is called from config/initializers/flipper.rb and can be used
# to register Flipper groups.
# See https://docs.gitlab.com/ee/development/feature_flags.html#feature-groups
def register_feature_groups
end
def l1_cache_backend def l1_cache_backend
Gitlab::ProcessMemoryCache.cache_backend Gitlab::ProcessMemoryCache.cache_backend
end end
......
...@@ -12,19 +12,21 @@ ...@@ -12,19 +12,21 @@
# #
# To enable the experiment for 10% of the users: # To enable the experiment for 10% of the users:
# #
# chatops: `/chatops run feature set experiment_key_experiment_percentage 10` # chatops: `/chatops run feature set experiment_key_experiment_percentage 10 --actors`
# console: `Feature.get(:experiment_key_experiment_percentage).enable_percentage_of_time(10)` # console: `Feature.enable_percentage_of_actors(:experiment_key_experiment_percentage, 10)`
# #
# To disable the experiment: # To disable the experiment:
# #
# chatops: `/chatops run feature delete experiment_key_experiment_percentage` # chatops: `/chatops run feature delete experiment_key_experiment_percentage`
# console: `Feature.get(:experiment_key_experiment_percentage).remove` # console: `Feature.remove(:experiment_key_experiment_percentage)`
# #
# To check the current rollout percentage: # To check the current rollout percentage:
# #
# chatops: `/chatops run feature get experiment_key_experiment_percentage` # chatops: `/chatops run feature get experiment_key_experiment_percentage`
# console: `Feature.get(:experiment_key_experiment_percentage).percentage_of_time_value` # console: `Feature.get(:experiment_key_experiment_percentage).percentage_of_time_value`
# #
# TODO: rewrite that
module Gitlab module Gitlab
module Experimentation module Experimentation
EXPERIMENTS = { EXPERIMENTS = {
...@@ -178,7 +180,7 @@ module Gitlab ...@@ -178,7 +180,7 @@ module Gitlab
# When a feature does not exist, the `percentage_of_time_value` method will return 0 # When a feature does not exist, the `percentage_of_time_value` method will return 0
def experiment_percentage def experiment_percentage
@experiment_percentage ||= Feature.get(:"#{key}_experiment_percentage").percentage_of_time_value @experiment_percentage ||= Feature.get(:"#{key}_experiment_percentage").percentage_of_time_value # rubocop:disable Gitlab/AvoidFeatureGet
end end
end end
end end
......
...@@ -52,7 +52,7 @@ module Gitlab ...@@ -52,7 +52,7 @@ module Gitlab
end end
def disabled_by_feature(options) def disabled_by_feature(options)
options.with_feature && !::Feature.get(options.with_feature).enabled? options.with_feature && !::Feature.enabled?(options.with_feature)
end end
def build_metric!(type, name, options) def build_metric!(type, name, options)
......
...@@ -19,7 +19,7 @@ module Gitlab ...@@ -19,7 +19,7 @@ module Gitlab
private private
def feature def feature
Feature.get(:sourcegraph) Feature.get(:sourcegraph) # rubocop:disable Gitlab/AvoidFeatureGet
end end
end end
end end
......
...@@ -21,7 +21,7 @@ namespace :gitlab do ...@@ -21,7 +21,7 @@ namespace :gitlab do
Gitlab::Git::RuggedImpl::Repository::FEATURE_FLAGS.each do |flag| Gitlab::Git::RuggedImpl::Repository::FEATURE_FLAGS.each do |flag|
case status case status
when nil when nil
Feature.get(flag).remove Feature.remove(flag)
when true when true
Feature.enable(flag) Feature.enable(flag)
when false when false
......
# frozen_string_literal: true
module RuboCop
module Cop
module Gitlab
# Cop that blacklists the use of `Feature.get`.
class AvoidFeatureGet < RuboCop::Cop::Cop
MSG = 'Use `Feature.enable/disable` methods instead of `Feature.get`. ' \
'See doc/development/testing_guide/best_practices.md#feature-flags-in-tests for more information.'
def_node_matcher :feature_get?, <<~PATTERN
(send (const nil? :Feature) :get ...)
PATTERN
def_node_matcher :global_feature_get?, <<~PATTERN
(send (const (cbase) :Feature) :get ...)
PATTERN
def on_send(node)
return unless feature_get?(node) || global_feature_get?(node)
add_offense(node, location: :selector)
end
end
end
end
end
...@@ -36,10 +36,6 @@ describe SourcegraphDecorator do ...@@ -36,10 +36,6 @@ describe SourcegraphDecorator do
sign_in user if user sign_in user if user
end end
after do
Feature.get(:sourcegraph).disable
end
subject do subject do
get :index, format: format get :index, format: format
......
...@@ -13,7 +13,7 @@ describe AutoDevopsHelper do ...@@ -13,7 +13,7 @@ describe AutoDevopsHelper do
allow(helper).to receive(:can?).with(user, :admin_pipeline, project) { allowed } allow(helper).to receive(:can?).with(user, :admin_pipeline, project) { allowed }
allow(helper).to receive(:current_user) { user } allow(helper).to receive(:current_user) { user }
Feature.get(:auto_devops_banner_disabled).disable stub_feature_flags(auto_devops_banner_disabled: false)
end end
subject { helper.show_auto_devops_callout?(project) } subject { helper.show_auto_devops_callout?(project) }
...@@ -32,7 +32,7 @@ describe AutoDevopsHelper do ...@@ -32,7 +32,7 @@ describe AutoDevopsHelper do
context 'when the banner is disabled by feature flag' do context 'when the banner is disabled by feature flag' do
before do before do
Feature.get(:auto_devops_banner_disabled).enable stub_feature_flags(auto_devops_banner_disabled: true)
end end
it { is_expected.to be_falsy } it { is_expected.to be_falsy }
......
...@@ -142,7 +142,7 @@ describe Feature, stub_feature_flags: false do ...@@ -142,7 +142,7 @@ describe Feature, stub_feature_flags: false do
expect(Flipper).to receive(:new).once.and_call_original expect(Flipper).to receive(:new).once.and_call_original
2.times do 2.times do
described_class.flipper described_class.send(:flipper)
end end
end end
end end
...@@ -151,9 +151,9 @@ describe Feature, stub_feature_flags: false do ...@@ -151,9 +151,9 @@ describe Feature, stub_feature_flags: false do
it 'memoizes the Flipper instance' do it 'memoizes the Flipper instance' do
expect(Flipper).to receive(:new).once.and_call_original expect(Flipper).to receive(:new).once.and_call_original
described_class.flipper described_class.send(:flipper)
described_class.instance_variable_set(:@flipper, nil) described_class.instance_variable_set(:@flipper, nil)
described_class.flipper described_class.send(:flipper)
end end
end end
end end
...@@ -179,21 +179,21 @@ describe Feature, stub_feature_flags: false do ...@@ -179,21 +179,21 @@ describe Feature, stub_feature_flags: false do
expect(described_class.enabled?(:enabled_feature_flag)).to be_truthy expect(described_class.enabled?(:enabled_feature_flag)).to be_truthy
end end
it { expect(described_class.l1_cache_backend).to eq(Gitlab::ProcessMemoryCache.cache_backend) } it { expect(described_class.send(:l1_cache_backend)).to eq(Gitlab::ProcessMemoryCache.cache_backend) }
it { expect(described_class.l2_cache_backend).to eq(Rails.cache) } it { expect(described_class.send(:l2_cache_backend)).to eq(Rails.cache) }
it 'caches the status in L1 and L2 caches', it 'caches the status in L1 and L2 caches',
:request_store, :use_clean_rails_memory_store_caching do :request_store, :use_clean_rails_memory_store_caching do
described_class.enable(:enabled_feature_flag) described_class.enable(:enabled_feature_flag)
flipper_key = "flipper/v1/feature/enabled_feature_flag" flipper_key = "flipper/v1/feature/enabled_feature_flag"
expect(described_class.l2_cache_backend) expect(described_class.send(:l2_cache_backend))
.to receive(:fetch) .to receive(:fetch)
.once .once
.with(flipper_key, expires_in: 1.hour) .with(flipper_key, expires_in: 1.hour)
.and_call_original .and_call_original
expect(described_class.l1_cache_backend) expect(described_class.send(:l1_cache_backend))
.to receive(:fetch) .to receive(:fetch)
.once .once
.with(flipper_key, expires_in: 1.minute) .with(flipper_key, expires_in: 1.minute)
...@@ -215,14 +215,14 @@ describe Feature, stub_feature_flags: false do ...@@ -215,14 +215,14 @@ describe Feature, stub_feature_flags: false do
let(:flag) { :some_feature_flag } let(:flag) { :some_feature_flag }
before do before do
described_class.flipper.memoize = false described_class.send(:flipper).memoize = false
described_class.enabled?(flag) described_class.enabled?(flag)
end end
it 'caches the status in L1 cache for the first minute' do it 'caches the status in L1 cache for the first minute' do
expect do expect do
expect(described_class.l1_cache_backend).to receive(:fetch).once.and_call_original expect(described_class.send(:l1_cache_backend)).to receive(:fetch).once.and_call_original
expect(described_class.l2_cache_backend).not_to receive(:fetch) expect(described_class.send(:l2_cache_backend)).not_to receive(:fetch)
expect(described_class.enabled?(flag)).to be_truthy expect(described_class.enabled?(flag)).to be_truthy
end.not_to exceed_query_limit(0) end.not_to exceed_query_limit(0)
end end
...@@ -230,8 +230,8 @@ describe Feature, stub_feature_flags: false do ...@@ -230,8 +230,8 @@ describe Feature, stub_feature_flags: false do
it 'caches the status in L2 cache after 2 minutes' do it 'caches the status in L2 cache after 2 minutes' do
Timecop.travel 2.minutes do Timecop.travel 2.minutes do
expect do expect do
expect(described_class.l1_cache_backend).to receive(:fetch).once.and_call_original expect(described_class.send(:l1_cache_backend)).to receive(:fetch).once.and_call_original
expect(described_class.l2_cache_backend).to receive(:fetch).once.and_call_original expect(described_class.send(:l2_cache_backend)).to receive(:fetch).once.and_call_original
expect(described_class.enabled?(flag)).to be_truthy expect(described_class.enabled?(flag)).to be_truthy
end.not_to exceed_query_limit(0) end.not_to exceed_query_limit(0)
end end
...@@ -240,8 +240,8 @@ describe Feature, stub_feature_flags: false do ...@@ -240,8 +240,8 @@ describe Feature, stub_feature_flags: false do
it 'fetches the status after an hour' do it 'fetches the status after an hour' do
Timecop.travel 61.minutes do Timecop.travel 61.minutes do
expect do expect do
expect(described_class.l1_cache_backend).to receive(:fetch).once.and_call_original expect(described_class.send(:l1_cache_backend)).to receive(:fetch).once.and_call_original
expect(described_class.l2_cache_backend).to receive(:fetch).once.and_call_original expect(described_class.send(:l2_cache_backend)).to receive(:fetch).once.and_call_original
expect(described_class.enabled?(flag)).to be_truthy expect(described_class.enabled?(flag)).to be_truthy
end.not_to exceed_query_limit(1) end.not_to exceed_query_limit(1)
end end
......
...@@ -11,7 +11,7 @@ describe Gitlab::Experimentation do ...@@ -11,7 +11,7 @@ describe Gitlab::Experimentation do
} }
}) })
Feature.get(:test_experiment_experiment_percentage).enable_percentage_of_time(enabled_percentage) Feature.enable_percentage_of_time(:test_experiment_experiment_percentage, enabled_percentage)
end end
let(:environment) { Rails.env.test? } let(:environment) { Rails.env.test? }
......
...@@ -8,7 +8,6 @@ describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do ...@@ -8,7 +8,6 @@ describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:repository) { project.repository } let(:repository) { project.repository }
let(:feature_flag_name) { 'feature-flag-name' } let(:feature_flag_name) { 'feature-flag-name' }
let(:feature_flag) { Feature.get(feature_flag_name) }
let(:temp_gitaly_metadata_file) { create_temporary_gitaly_metadata_file } let(:temp_gitaly_metadata_file) { create_temporary_gitaly_metadata_file }
before(:all) do before(:all) do
......
...@@ -26,7 +26,7 @@ describe Gitlab::Metrics::MethodCall do ...@@ -26,7 +26,7 @@ describe Gitlab::Metrics::MethodCall do
context 'prometheus instrumentation is enabled' do context 'prometheus instrumentation is enabled' do
before do before do
Feature.get(:prometheus_metrics_method_instrumentation).enable stub_feature_flags(prometheus_metrics_method_instrumentation: true)
end end
around do |example| around do |example|
...@@ -50,7 +50,7 @@ describe Gitlab::Metrics::MethodCall do ...@@ -50,7 +50,7 @@ describe Gitlab::Metrics::MethodCall do
context 'prometheus instrumentation is disabled' do context 'prometheus instrumentation is disabled' do
before do before do
Feature.get(:prometheus_metrics_method_instrumentation).disable stub_feature_flags(prometheus_metrics_method_instrumentation: false)
end end
it 'observes using NullMetric' do it 'observes using NullMetric' do
......
...@@ -104,7 +104,7 @@ describe Gitlab::Metrics::Methods do ...@@ -104,7 +104,7 @@ describe Gitlab::Metrics::Methods do
context 'when feature is enabled' do context 'when feature is enabled' do
before do before do
Feature.get(feature_name).enable stub_feature_flags(feature_name => true)
end end
it "initializes #{metric_type} metric" do it "initializes #{metric_type} metric" do
...@@ -118,7 +118,7 @@ describe Gitlab::Metrics::Methods do ...@@ -118,7 +118,7 @@ describe Gitlab::Metrics::Methods do
context 'when feature is disabled' do context 'when feature is disabled' do
before do before do
Feature.get(feature_name).disable stub_feature_flags(feature_name => false)
end end
it "returns NullMetric" do it "returns NullMetric" do
......
...@@ -4262,7 +4262,7 @@ describe Project do ...@@ -4262,7 +4262,7 @@ describe Project do
describe '#auto_devops_enabled?' do describe '#auto_devops_enabled?' do
before do before do
Feature.get(:force_autodevops_on_by_default).enable_percentage_of_actors(0) Feature.enable_percentage_of_actors(:force_autodevops_on_by_default, 0)
end end
let_it_be(:project, reload: true) { create(:project) } let_it_be(:project, reload: true) { create(:project) }
...@@ -4464,7 +4464,7 @@ describe Project do ...@@ -4464,7 +4464,7 @@ describe Project do
let_it_be(:project, reload: true) { create(:project) } let_it_be(:project, reload: true) { create(:project) }
before do before do
Feature.get(:force_autodevops_on_by_default).enable_percentage_of_actors(0) Feature.enable_percentage_of_actors(:force_autodevops_on_by_default, 0)
end end
context 'when explicitly disabled' do context 'when explicitly disabled' do
...@@ -4510,7 +4510,7 @@ describe Project do ...@@ -4510,7 +4510,7 @@ describe Project do
before do before do
create(:project_auto_devops, project: project, enabled: false) create(:project_auto_devops, project: project, enabled: false)
Feature.get(:force_autodevops_on_by_default).enable_percentage_of_actors(100) Feature.enable_percentage_of_actors(:force_autodevops_on_by_default, 100)
end end
it 'does not have auto devops implicitly disabled' do it 'does not have auto devops implicitly disabled' do
......
...@@ -39,9 +39,9 @@ describe API::Features, stub_feature_flags: false do ...@@ -39,9 +39,9 @@ describe API::Features, stub_feature_flags: false do
end end
before do before do
Feature.get('feature_1').enable Feature.enable('feature_1')
Feature.get('feature_2').disable Feature.disable('feature_2')
Feature.get('feature_3').enable Feature.group(:perf_team) Feature.enable('feature_3', Feature.group(:perf_team))
end end
it 'returns a 401 for anonymous users' do it 'returns a 401 for anonymous users' do
...@@ -227,10 +227,8 @@ describe API::Features, stub_feature_flags: false do ...@@ -227,10 +227,8 @@ describe API::Features, stub_feature_flags: false do
end end
context 'when the feature exists' do context 'when the feature exists' do
let(:feature) { Feature.get(feature_name) }
before do before do
feature.disable # This also persists the feature on the DB Feature.disable(feature_name) # This also persists the feature on the DB
end end
context 'when passed value=true' do context 'when passed value=true' do
...@@ -273,8 +271,8 @@ describe API::Features, stub_feature_flags: false do ...@@ -273,8 +271,8 @@ describe API::Features, stub_feature_flags: false do
context 'when feature is enabled and value=false is passed' do context 'when feature is enabled and value=false is passed' do
it 'disables the feature' do it 'disables the feature' do
feature.enable Feature.enable(feature_name)
expect(feature).to be_enabled expect(Feature.enabled?(feature_name)).to eq(true)
post api("/features/#{feature_name}", admin), params: { value: 'false' } post api("/features/#{feature_name}", admin), params: { value: 'false' }
...@@ -286,8 +284,8 @@ describe API::Features, stub_feature_flags: false do ...@@ -286,8 +284,8 @@ describe API::Features, stub_feature_flags: false do
end end
it 'disables the feature for the given Flipper group when passed feature_group=perf_team' do it 'disables the feature for the given Flipper group when passed feature_group=perf_team' do
feature.enable(Feature.group(:perf_team)) Feature.enable(feature_name, Feature.group(:perf_team))
expect(Feature.get(feature_name).enabled?(admin)).to be_truthy expect(Feature.enabled?(feature_name, admin)).to be_truthy
post api("/features/#{feature_name}", admin), params: { value: 'false', feature_group: 'perf_team' } post api("/features/#{feature_name}", admin), params: { value: 'false', feature_group: 'perf_team' }
...@@ -299,8 +297,8 @@ describe API::Features, stub_feature_flags: false do ...@@ -299,8 +297,8 @@ describe API::Features, stub_feature_flags: false do
end end
it 'disables the feature for the given user when passed user=username' do it 'disables the feature for the given user when passed user=username' do
feature.enable(user) Feature.enable(feature_name, user)
expect(Feature.get(feature_name).enabled?(user)).to be_truthy expect(Feature.enabled?(feature_name, user)).to be_truthy
post api("/features/#{feature_name}", admin), params: { value: 'false', user: user.username } post api("/features/#{feature_name}", admin), params: { value: 'false', user: user.username }
...@@ -314,7 +312,7 @@ describe API::Features, stub_feature_flags: false do ...@@ -314,7 +312,7 @@ describe API::Features, stub_feature_flags: false do
context 'with a pre-existing percentage of time value' do context 'with a pre-existing percentage of time value' do
before do before do
feature.enable_percentage_of_time(50) Feature.enable_percentage_of_time(feature_name, 50)
end end
it 'updates the percentage of time if passed an integer' do it 'updates the percentage of time if passed an integer' do
...@@ -333,7 +331,7 @@ describe API::Features, stub_feature_flags: false do ...@@ -333,7 +331,7 @@ describe API::Features, stub_feature_flags: false do
context 'with a pre-existing percentage of actors value' do context 'with a pre-existing percentage of actors value' do
before do before do
feature.enable_percentage_of_actors(42) Feature.enable_percentage_of_actors(feature_name, 42)
end end
it 'updates the percentage of actors if passed an integer' do it 'updates the percentage of actors if passed an integer' do
...@@ -378,14 +376,17 @@ describe API::Features, stub_feature_flags: false do ...@@ -378,14 +376,17 @@ describe API::Features, stub_feature_flags: false do
context 'when the gate value was set' do context 'when the gate value was set' do
before do before do
Feature.get(feature_name).enable Feature.enable(feature_name)
end end
it 'deletes an enabled feature' do it 'deletes an enabled feature' do
delete api("/features/#{feature_name}", admin) expect do
delete api("/features/#{feature_name}", admin)
Feature.reset
end.to change { Feature.persisted_name?(feature_name) }
.and change { Feature.enabled?(feature_name) }
expect(response).to have_gitlab_http_status(:no_content) expect(response).to have_gitlab_http_status(:no_content)
expect(Feature.get(feature_name)).not_to be_enabled
end end
end end
end end
......
...@@ -52,7 +52,7 @@ describe API::Settings, 'Settings' do ...@@ -52,7 +52,7 @@ describe API::Settings, 'Settings' do
storages = Gitlab.config.repositories.storages storages = Gitlab.config.repositories.storages
.merge({ 'custom' => 'tmp/tests/custom_repositories' }) .merge({ 'custom' => 'tmp/tests/custom_repositories' })
allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
Feature.get(:sourcegraph).enable stub_feature_flags(sourcegraph: true)
end end
it "updates application settings" do it "updates application settings" do
......
...@@ -47,7 +47,7 @@ module StubFeatureFlags ...@@ -47,7 +47,7 @@ module StubFeatureFlags
def stub_feature_flags(features) def stub_feature_flags(features)
features.each do |feature_name, actors| features.each do |feature_name, actors|
# Remove feature flag overwrite # Remove feature flag overwrite
feature = Feature.get(feature_name) feature = Feature.get(feature_name) # rubocop:disable Gitlab/AvoidFeatureGet
feature.remove feature.remove
Array(actors).each do |actor| Array(actors).each do |actor|
......
...@@ -32,8 +32,7 @@ describe 'profiles/preferences/show' do ...@@ -32,8 +32,7 @@ describe 'profiles/preferences/show' do
end end
before do before do
# Can't use stub_feature_flags because we use Feature.get to check if conditinally applied stub_feature_flags(sourcegraph: sourcegraph_feature)
Feature.get(:sourcegraph).enable sourcegraph_feature
stub_application_setting(sourcegraph_enabled: sourcegraph_enabled) stub_application_setting(sourcegraph_enabled: sourcegraph_enabled)
end end
......
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