From 48aff82709769b098321c738f3444b9bdaa694c6 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 21 Oct 2020 07:08:36 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-5-stable-ee --- spec/models/operations/feature_flag_spec.rb | 18 ++- .../operations/feature_flags/strategy_spec.rb | 176 +++++++++++++++++---- 2 files changed, 159 insertions(+), 35 deletions(-) (limited to 'spec/models/operations') diff --git a/spec/models/operations/feature_flag_spec.rb b/spec/models/operations/feature_flag_spec.rb index db432e73355..b4e941f2856 100644 --- a/spec/models/operations/feature_flag_spec.rb +++ b/spec/models/operations/feature_flag_spec.rb @@ -25,7 +25,7 @@ RSpec.describe Operations::FeatureFlag do context 'a version 1 feature flag' do it 'is valid if associated with Operations::FeatureFlagScope models' do project = create(:project) - feature_flag = described_class.create({ name: 'test', project: project, version: 1, + feature_flag = described_class.create!({ name: 'test', project: project, version: 1, scopes_attributes: [{ environment_scope: '*', active: false }] }) expect(feature_flag).to be_valid @@ -33,9 +33,10 @@ RSpec.describe Operations::FeatureFlag do it 'is invalid if associated with Operations::FeatureFlags::Strategy models' do project = create(:project) - feature_flag = described_class.create({ name: 'test', project: project, version: 1, + feature_flag = described_class.new({ name: 'test', project: project, version: 1, strategies_attributes: [{ name: 'default', parameters: {} }] }) + expect(feature_flag.valid?).to eq(false) expect(feature_flag.errors.messages).to eq({ version_associations: ["version 1 feature flags may not have strategies"] }) @@ -45,9 +46,10 @@ RSpec.describe Operations::FeatureFlag do context 'a version 2 feature flag' do it 'is invalid if associated with Operations::FeatureFlagScope models' do project = create(:project) - feature_flag = described_class.create({ name: 'test', project: project, version: 2, + feature_flag = described_class.new({ name: 'test', project: project, version: 2, scopes_attributes: [{ environment_scope: '*', active: false }] }) + expect(feature_flag.valid?).to eq(false) expect(feature_flag.errors.messages).to eq({ version_associations: ["version 2 feature flags may not have scopes"] }) @@ -55,7 +57,7 @@ RSpec.describe Operations::FeatureFlag do it 'is valid if associated with Operations::FeatureFlags::Strategy models' do project = create(:project) - feature_flag = described_class.create({ name: 'test', project: project, version: 2, + feature_flag = described_class.create!({ name: 'test', project: project, version: 2, strategies_attributes: [{ name: 'default', parameters: {} }] }) expect(feature_flag).to be_valid @@ -75,7 +77,7 @@ RSpec.describe Operations::FeatureFlag do it 'defaults to 1 if unspecified' do project = create(:project) - feature_flag = described_class.create(name: 'my_flag', project: project, active: true) + feature_flag = described_class.create!(name: 'my_flag', project: project, active: true) expect(feature_flag).to be_valid expect(feature_flag.version_before_type_cast).to eq(1) @@ -113,14 +115,14 @@ RSpec.describe Operations::FeatureFlag do context 'with a version 1 feature flag' do it 'creates a default scope' do - feature_flag = described_class.create({ name: 'test', project: project, scopes_attributes: [], version: 1 }) + feature_flag = described_class.create!({ name: 'test', project: project, scopes_attributes: [], version: 1 }) expect(feature_flag.scopes.count).to eq(1) expect(feature_flag.scopes.first.environment_scope).to eq('*') end it 'allows specifying the default scope in the parameters' do - feature_flag = described_class.create({ name: 'test', project: project, + feature_flag = described_class.create!({ name: 'test', project: project, scopes_attributes: [{ environment_scope: '*', active: false }, { environment_scope: 'review/*', active: true }], version: 1 }) @@ -131,7 +133,7 @@ RSpec.describe Operations::FeatureFlag do context 'with a version 2 feature flag' do it 'does not create a default scope' do - feature_flag = described_class.create({ name: 'test', project: project, scopes_attributes: [], version: 2 }) + feature_flag = described_class.create!({ name: 'test', project: project, scopes_attributes: [], version: 2 }) expect(feature_flag.scopes).to eq([]) end diff --git a/spec/models/operations/feature_flags/strategy_spec.rb b/spec/models/operations/feature_flags/strategy_spec.rb index 04e3ef26e9d..0ecb49e75f3 100644 --- a/spec/models/operations/feature_flags/strategy_spec.rb +++ b/spec/models/operations/feature_flags/strategy_spec.rb @@ -4,11 +4,12 @@ require 'spec_helper' RSpec.describe Operations::FeatureFlags::Strategy do let_it_be(:project) { create(:project) } + let_it_be(:feature_flag) { create(:operations_feature_flag, project: project) } describe 'validations' do it do is_expected.to validate_inclusion_of(:name) - .in_array(%w[default gradualRolloutUserId userWithId gitlabUserList]) + .in_array(%w[default gradualRolloutUserId flexibleRollout userWithId gitlabUserList]) .with_message('strategy name is invalid') end @@ -19,7 +20,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'skips parameters validation' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: invalid_name, parameters: { bad: 'params' }) @@ -36,7 +36,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must have valid parameters for the strategy' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gradualRolloutUserId', parameters: invalid_parameters) @@ -45,7 +44,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end it 'allows the parameters in any order' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gradualRolloutUserId', parameters: { percentage: '10', groupId: 'mygroup' }) @@ -55,13 +53,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do describe 'percentage' do where(:invalid_value) do - [50, 40.0, { key: "value" }, "garbage", "00", "01", "101", "-1", "-10", "0100", - "1000", "10.0", "5%", "25%", "100hi", "e100", "30m", " ", "\r\n", "\n", "\t", - "\n10", "20\n", "\n100", "100\n", "\n ", nil] + [50, 40.0, { key: "value" }, "garbage", "101", "-1", "-10", "1000", "10.0", "5%", "25%", + "100hi", "e100", "30m", " ", "\r\n", "\n", "\t", "\n10", "20\n", "\n100", "100\n", + "\n ", nil] end with_them do it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gradualRolloutUserId', parameters: { groupId: 'mygroup', percentage: invalid_value }) @@ -75,7 +72,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gradualRolloutUserId', parameters: { groupId: 'mygroup', percentage: valid_value }) @@ -92,7 +88,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must be a string value of up to 32 lowercase characters' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gradualRolloutUserId', parameters: { groupId: invalid_value, percentage: '40' }) @@ -106,7 +101,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must be a string value of up to 32 lowercase characters' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gradualRolloutUserId', parameters: { groupId: valid_value, percentage: '40' }) @@ -117,13 +111,132 @@ RSpec.describe Operations::FeatureFlags::Strategy do end end + context 'when the strategy name is flexibleRollout' do + valid_parameters = { rollout: '40', groupId: 'mygroup', stickiness: 'DEFAULT' } + where(invalid_parameters: [ + nil, + {}, + *valid_parameters.to_a.combination(1).to_a.map { |p| p.to_h }, + *valid_parameters.to_a.combination(2).to_a.map { |p| p.to_h }, + { **valid_parameters, userIds: '4' }, + { **valid_parameters, extra: nil } + ]) + with_them do + it 'must have valid parameters for the strategy' do + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: invalid_parameters) + + expect(strategy.errors[:parameters]).to eq(['parameters are invalid']) + end + end + + [ + [:rollout, '10'], + [:stickiness, 'DEFAULT'], + [:groupId, 'mygroup'] + ].permutation(3).each do |parameters| + it "allows the parameters in the order #{parameters.map { |p| p.first }.join(', ')}" do + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: Hash[parameters]) + + expect(strategy.errors[:parameters]).to be_empty + end + end + + describe 'rollout' do + where(invalid_value: [50, 40.0, { key: "value" }, "garbage", "101", "-1", " ", "-10", + "1000", "10.0", "5%", "25%", "100hi", "e100", "30m", "\r\n", + "\n", "\t", "\n10", "20\n", "\n100", "100\n", "\n ", nil]) + with_them do + it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do + parameters = { stickiness: 'DEFAULT', groupId: 'mygroup', rollout: invalid_value } + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: parameters) + + expect(strategy.errors[:parameters]).to eq([ + 'rollout must be a string between 0 and 100 inclusive' + ]) + end + end + + where(valid_value: %w[0 1 10 38 100 93]) + with_them do + it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do + parameters = { stickiness: 'DEFAULT', groupId: 'mygroup', rollout: valid_value } + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: parameters) + + expect(strategy.errors[:parameters]).to eq([]) + end + end + end + + describe 'groupId' do + where(invalid_value: [nil, 4, 50.0, {}, 'spaces bad', 'bad$', '%bad', '', + '!bad', '.bad', 'Bad', 'bad1', "", " ", "b" * 33, "ba_d", "ba\nd"]) + with_them do + it 'must be a string value of up to 32 lowercase characters' do + parameters = { stickiness: 'DEFAULT', groupId: invalid_value, rollout: '40' } + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: parameters) + + expect(strategy.errors[:parameters]).to eq(['groupId parameter is invalid']) + end + end + + where(valid_value: ["somegroup", "anothergroup", "okay", "g", "a" * 32]) + with_them do + it 'must be a string value of up to 32 lowercase characters' do + parameters = { stickiness: 'DEFAULT', groupId: valid_value, rollout: '40' } + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: parameters) + + expect(strategy.errors[:parameters]).to eq([]) + end + end + end + + describe 'stickiness' do + where(invalid_value: [nil, " ", "default", "DEFAULT\n", "UserId", "USER", "USERID "]) + with_them do + it 'must be a string representing a supported stickiness setting' do + parameters = { stickiness: invalid_value, groupId: 'mygroup', rollout: '40' } + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: parameters) + + expect(strategy.errors[:parameters]).to eq([ + 'stickiness parameter must be DEFAULT, USERID, SESSIONID, or RANDOM' + ]) + end + end + + where(valid_value: %w[DEFAULT USERID SESSIONID RANDOM]) + with_them do + it 'must be a string representing a supported stickiness setting' do + parameters = { stickiness: valid_value, groupId: 'mygroup', rollout: '40' } + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: parameters) + + expect(strategy.errors[:parameters]).to eq([]) + end + end + end + end + context 'when the strategy name is userWithId' do where(:invalid_parameters) do [nil, { userIds: 'sam', percentage: '40' }, { userIds: 'sam', some: 'param' }, { percentage: '40' }, {}] end with_them do it 'must have valid parameters for the strategy' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'userWithId', parameters: invalid_parameters) @@ -140,7 +253,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'is valid with a string of comma separated values' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'userWithId', parameters: { userIds: valid_value }) @@ -155,7 +267,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'is invalid' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'userWithId', parameters: { userIds: invalid_value }) @@ -173,7 +284,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must be empty' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'default', parameters: invalid_value) @@ -183,7 +293,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end it 'must be empty' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'default', parameters: {}) @@ -198,7 +307,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must be empty' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gitlabUserList', parameters: invalid_value) @@ -208,7 +316,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end it 'must be empty' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gitlabUserList', parameters: {}) @@ -221,7 +328,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do describe 'associations' do context 'when name is gitlabUserList' do it 'is valid when associated with a user list' do - feature_flag = create(:operations_feature_flag, project: project) user_list = create(:operations_feature_flag_user_list, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gitlabUserList', @@ -232,7 +338,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end it 'is invalid without a user list' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gitlabUserList', parameters: {}) @@ -242,7 +347,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do it 'is invalid when associated with a user list from another project' do other_project = create(:project) - feature_flag = create(:operations_feature_flag, project: project) user_list = create(:operations_feature_flag_user_list, project: other_project) strategy = described_class.create(feature_flag: feature_flag, name: 'gitlabUserList', @@ -255,7 +359,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do context 'when name is default' do it 'is invalid when associated with a user list' do - feature_flag = create(:operations_feature_flag, project: project) user_list = create(:operations_feature_flag_user_list, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'default', @@ -266,7 +369,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end it 'is valid without a user list' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'default', parameters: {}) @@ -277,7 +379,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do context 'when name is userWithId' do it 'is invalid when associated with a user list' do - feature_flag = create(:operations_feature_flag, project: project) user_list = create(:operations_feature_flag_user_list, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'userWithId', @@ -288,7 +389,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end it 'is valid without a user list' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'userWithId', parameters: { userIds: 'user1' }) @@ -299,7 +399,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do context 'when name is gradualRolloutUserId' do it 'is invalid when associated with a user list' do - feature_flag = create(:operations_feature_flag, project: project) user_list = create(:operations_feature_flag_user_list, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gradualRolloutUserId', @@ -310,7 +409,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end it 'is valid without a user list' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gradualRolloutUserId', parameters: { groupId: 'default', percentage: '10' }) @@ -318,6 +416,30 @@ RSpec.describe Operations::FeatureFlags::Strategy do expect(strategy.errors[:user_list]).to be_empty end end + + context 'when name is flexibleRollout' do + it 'is invalid when associated with a user list' do + user_list = create(:operations_feature_flag_user_list, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + user_list: user_list, + parameters: { groupId: 'default', + rollout: '10', + stickiness: 'DEFAULT' }) + + expect(strategy.errors[:user_list]).to eq(['must be blank']) + end + + it 'is valid without a user list' do + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: { groupId: 'default', + rollout: '10', + stickiness: 'DEFAULT' }) + + expect(strategy.errors[:user_list]).to be_empty + end + end end end end -- cgit v1.2.3