From ce92ab93641dd2d20fddd27783d30b548fd69cca Mon Sep 17 00:00:00 2001
From: Oscar Tovar <otovar@gitlab.com>
Date: Fri, 23 Feb 2024 18:54:15 +0000
Subject: [PATCH] Fix feature flag valid usage examples in errors
The hash used to reference the example in valid_usage! used symbols to
reference values by key. Since we were passing in a string, the lookup
would always fail, and the example in the error message would fail to
display as a result. To fix this, the `type` attribute is cast to a
symbol so that we use the expected type of key (Symbol). The spec has also
been changed to make a strict comparison, so that a regression for this
is caught in the future.
---
lib/feature/definition.rb | 2 +-
lib/feature/shared.rb | 16 ++++++++--------
spec/lib/feature/definition_spec.rb | 17 ++++++++++++++---
3 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/lib/feature/definition.rb b/lib/feature/definition.rb
index d48d4f45ebb67..9a6b287c61735 100644
--- a/lib/feature/definition.rb
+++ b/lib/feature/definition.rb
@@ -81,7 +81,7 @@ def valid_usage!(type_in_code:)
raise Feature::InvalidFeatureFlagError,
"The given `type: :#{type_in_code}` for `#{key}` is not equal to the " \
":#{type} set in its definition file. Ensure to use a valid type in #{path} or ensure that you use " \
- "a valid syntax: #{TYPES.dig(type, :example)}"
+ "a valid syntax:\n\n#{TYPES.dig(type.to_sym, :example)}"
end
end
diff --git a/lib/feature/shared.rb b/lib/feature/shared.rb
index fcce2642ef6e3..87cb4de9b0e60 100644
--- a/lib/feature/shared.rb
+++ b/lib/feature/shared.rb
@@ -18,7 +18,7 @@ module Shared
optional: false,
rollout_issue: true,
can_be_default_enabled: false,
- example: <<-EOS
+ example: <<~EOS
Feature.enabled?(:my_feature_flag, project, type: :gitlab_com_derisk)
push_frontend_feature_flag(:my_feature_flag, project)
EOS
@@ -28,7 +28,7 @@ module Shared
optional: false,
rollout_issue: false,
can_be_default_enabled: false,
- example: <<-EOS
+ example: <<~EOS
Feature.enabled?(:my_feature_flag, project, type: :wip)
push_frontend_feature_flag(:my_feature_flag, project)
EOS
@@ -39,7 +39,7 @@ module Shared
optional: false,
rollout_issue: true,
can_be_default_enabled: true,
- example: <<-EOS
+ example: <<~EOS
Feature.enabled?(:my_feature_flag, project, type: :beta)
push_frontend_feature_flag(:my_feature_flag, project)
EOS
@@ -49,7 +49,7 @@ module Shared
optional: false,
rollout_issue: true,
can_be_default_enabled: true,
- example: <<-EOS
+ example: <<~EOS
Feature.enabled?(:my_ops_flag, type: :ops)
push_frontend_feature_flag(:my_ops_flag, project, type: :ops)
EOS
@@ -59,7 +59,7 @@ module Shared
optional: true,
rollout_issue: true,
can_be_default_enabled: false,
- example: <<-EOS
+ example: <<~EOS
experiment(:my_experiment, project: project, actor: current_user) { ...variant code... }
EOS
},
@@ -68,10 +68,10 @@ module Shared
optional: true,
rollout_issue: false,
can_be_default_enabled: false,
- example: '<<-EOS
+ example: <<~EOS
Feature.enabled?(:"defer_sidekiq_jobs:AuthorizedProjectsWorker", type: :worker,
- default_enabled_if_undefined: false)
- EOS'
+ default_enabled_if_undefined: false)
+ EOS
},
undefined: {
description: "Feature flags that are undefined in GitLab codebase (should not be used)",
diff --git a/spec/lib/feature/definition_spec.rb b/spec/lib/feature/definition_spec.rb
index c68b37ce4f3aa..826aea981b1a1 100644
--- a/spec/lib/feature/definition_spec.rb
+++ b/spec/lib/feature/definition_spec.rb
@@ -55,11 +55,22 @@
end
describe '#valid_usage!' do
+ let(:attributes) do
+ { name: 'feature_flag',
+ type: 'beta',
+ default_enabled: true }
+ end
+
+ let(:path) { File.join('beta', 'feature_flag.yml') }
+
it 'raises exception for invalid type' do
+ expected_error_message = "The given `type: :invalid` for `feature_flag` is not equal to the " \
+ ":beta set in its definition file. Ensure to use a valid type " \
+ "in beta/feature_flag.yml or ensure that you use a valid syntax:\n\n" \
+ "Feature.enabled?(:my_feature_flag, project, type: :beta)\n" \
+ "push_frontend_feature_flag(:my_feature_flag, project)\n"
expect { definition.valid_usage!(type_in_code: :invalid) }
- .to raise_error(
- /The given `type: :invalid` for `feature_flag` is not equal to the :development set in its definition file./
- )
+ .to raise_error(Feature::InvalidFeatureFlagError, expected_error_message)
end
end
--
GitLab