diff options
Diffstat (limited to 'spec/experiments')
| -rw-r--r-- | spec/experiments/application_experiment/cache_spec.rb | 12 | ||||
| -rw-r--r-- | spec/experiments/application_experiment_spec.rb | 147 | ||||
| -rw-r--r-- | spec/experiments/members/invite_email_experiment_spec.rb | 37 | ||||
| -rw-r--r-- | spec/experiments/new_project_readme_experiment_spec.rb | 93 | ||||
| -rw-r--r-- | spec/experiments/strategy/round_robin_spec.rb | 68 |
5 files changed, 327 insertions, 30 deletions
diff --git a/spec/experiments/application_experiment/cache_spec.rb b/spec/experiments/application_experiment/cache_spec.rb index a420d557155..4caa91e6ac4 100644 --- a/spec/experiments/application_experiment/cache_spec.rb +++ b/spec/experiments/application_experiment/cache_spec.rb @@ -51,16 +51,4 @@ RSpec.describe ApplicationExperiment::Cache do 'invalid call to clear a non-hash cache key' ) end - - context "when the :caching_experiments feature is disabled" do - before do - stub_feature_flags(caching_experiments: false) - end - - it "doesn't write to the cache" do - subject.write(key_field, 'value') - - expect(subject.read(key_field)).to be_nil - end - end end diff --git a/spec/experiments/application_experiment_spec.rb b/spec/experiments/application_experiment_spec.rb index ece52d37351..501d344e920 100644 --- a/spec/experiments/application_experiment_spec.rb +++ b/spec/experiments/application_experiment_spec.rb @@ -2,8 +2,60 @@ require 'spec_helper' -RSpec.describe ApplicationExperiment do - subject { described_class.new(:stub) } +RSpec.describe ApplicationExperiment, :experiment do + subject { described_class.new('namespaced/stub') } + + let(:feature_definition) do + { name: 'namespaced_stub', type: 'experiment', group: 'group::adoption', default_enabled: false } + end + + around do |example| + Feature::Definition.definitions[:namespaced_stub] = Feature::Definition.new('namespaced_stub.yml', feature_definition) + example.run + Feature::Definition.definitions.delete(:namespaced_stub) + end + + before do + allow(subject).to receive(:enabled?).and_return(true) + end + + it "naively assumes a 1x1 relationship to feature flags for tests" do + expect(Feature).to receive(:persist_used!).with('namespaced_stub') + + described_class.new('namespaced/stub') + end + + describe "enabled" do + before do + allow(subject).to receive(:enabled?).and_call_original + + allow(Feature::Definition).to receive(:get).and_return('_instance_') + allow(Gitlab).to receive(:dev_env_or_com?).and_return(true) + allow(Feature).to receive(:get).and_return(double(state: :on)) + end + + it "is enabled when all criteria are met" do + expect(subject).to be_enabled + end + + it "isn't enabled if the feature definition doesn't exist" do + expect(Feature::Definition).to receive(:get).with('namespaced_stub').and_return(nil) + + expect(subject).not_to be_enabled + end + + it "isn't enabled if we're not in dev or dotcom environments" do + expect(Gitlab).to receive(:dev_env_or_com?).and_return(false) + + expect(subject).not_to be_enabled + end + + it "isn't enabled if the feature flag state is :off" do + expect(Feature).to receive(:get).with('namespaced_stub').and_return(double(state: :off)) + + expect(subject).not_to be_enabled + end + end describe "publishing results" do it "tracks the assignment" do @@ -16,9 +68,9 @@ RSpec.describe ApplicationExperiment do expect(Gon.global).to receive(:push).with( { experiment: { - 'stub' => { # string key because it can be namespaced - experiment: 'stub', - key: 'e8f65fd8d973f9985dc7ea3cf1614ae1', + 'namespaced/stub' => { # string key because it can be namespaced + experiment: 'namespaced/stub', + key: '86208ac54ca798e11f127e8b23ec396a', variant: 'control' } } @@ -31,8 +83,8 @@ RSpec.describe ApplicationExperiment do end describe "tracking events", :snowplow do - it "doesn't track if excluded" do - subject.exclude { true } + it "doesn't track if we shouldn't track" do + allow(subject).to receive(:should_track?).and_return(false) subject.track(:action) @@ -45,7 +97,7 @@ RSpec.describe ApplicationExperiment do ]) expect_snowplow_event( - category: 'stub', + category: 'namespaced/stub', action: 'action', property: '_property_', context: [ @@ -55,7 +107,7 @@ RSpec.describe ApplicationExperiment do }, { schema: 'iglu:com.gitlab/gitlab_experiment/jsonschema/0-3-0', - data: { experiment: 'stub', key: 'e8f65fd8d973f9985dc7ea3cf1614ae1', variant: 'control' } + data: { experiment: 'namespaced/stub', key: '86208ac54ca798e11f127e8b23ec396a', variant: 'control' } } ] ) @@ -63,18 +115,77 @@ RSpec.describe ApplicationExperiment do end describe "variant resolution" do - it "returns nil when not rolled out" do - stub_feature_flags(stub: false) + context "when using the default feature flag percentage rollout" do + it "uses the default value as specified in the yaml" do + expect(Feature).to receive(:enabled?).with('namespaced_stub', subject, type: :experiment, default_enabled: :yaml) + + expect(subject.variant.name).to eq('control') + end + + it "returns nil when not rolled out" do + stub_feature_flags(namespaced_stub: false) + + expect(subject.variant.name).to eq('control') + end + + context "when rolled out to 100%" do + it "returns the first variant name" do + subject.try(:variant1) {} + subject.try(:variant2) {} - expect(subject.variant.name).to eq('control') + expect(subject.variant.name).to eq('variant1') + end + end end - context "when rolled out to 100%" do - it "returns the first variant name" do - subject.try(:variant1) {} - subject.try(:variant2) {} + context "when using the round_robin strategy", :clean_gitlab_redis_shared_state do + context "when variants aren't supplied" do + subject :inheriting_class do + Class.new(described_class) do + def rollout_strategy + :round_robin + end + end.new('namespaced/stub') + end + + it "raises an error" do + expect { inheriting_class.variants }.to raise_error(NotImplementedError) + end + end + + context "when variants are supplied" do + let(:inheriting_class) do + Class.new(described_class) do + def rollout_strategy + :round_robin + end + + def variants + %i[variant1 variant2 control] + end + end + end + + it "proves out round robin in variant selection", :aggregate_failures do + instance_1 = inheriting_class.new('namespaced/stub') + allow(instance_1).to receive(:enabled?).and_return(true) + instance_2 = inheriting_class.new('namespaced/stub') + allow(instance_2).to receive(:enabled?).and_return(true) + instance_3 = inheriting_class.new('namespaced/stub') + allow(instance_3).to receive(:enabled?).and_return(true) + + instance_1.try {} + + expect(instance_1.variant.name).to eq('variant2') + + instance_2.try {} + + expect(instance_2.variant.name).to eq('control') + + instance_3.try {} - expect(subject.variant.name).to eq('variant1') + expect(instance_3.variant.name).to eq('variant1') + end end end end @@ -105,7 +216,7 @@ RSpec.describe ApplicationExperiment do # every control variant assigned, we'd inflate the cache size and # wouldn't be able to roll out to subjects that we'd already assigned to # the control. - stub_feature_flags(stub: false) # simulate being not rolled out + stub_feature_flags(namespaced_stub: false) # simulate being not rolled out expect(subject.variant.name).to eq('control') # if we ask, it should be control diff --git a/spec/experiments/members/invite_email_experiment_spec.rb b/spec/experiments/members/invite_email_experiment_spec.rb new file mode 100644 index 00000000000..4376c021385 --- /dev/null +++ b/spec/experiments/members/invite_email_experiment_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Members::InviteEmailExperiment do + subject :invite_email do + experiment('members/invite_email', actor: double('Member', created_by: double('User', avatar_url: '_avatar_url_'))) + end + + before do + allow(invite_email).to receive(:enabled?).and_return(true) + end + + describe "#rollout_strategy" do + it "resolves to round_robin" do + expect(invite_email.rollout_strategy).to eq(:round_robin) + end + end + + describe "#variants" do + it "has all the expected variants" do + expect(invite_email.variants).to match(%i[avatar permission_info control]) + end + end + + describe "exclusions", :experiment do + it "excludes when created by is nil" do + expect(experiment('members/invite_email')).to exclude(actor: double(created_by: nil)) + end + + it "excludes when avatar_url is nil" do + member_without_avatar_url = double('Member', created_by: double('User', avatar_url: nil)) + + expect(experiment('members/invite_email')).to exclude(actor: member_without_avatar_url) + end + end +end diff --git a/spec/experiments/new_project_readme_experiment_spec.rb b/spec/experiments/new_project_readme_experiment_spec.rb new file mode 100644 index 00000000000..17e28cf6e7f --- /dev/null +++ b/spec/experiments/new_project_readme_experiment_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe NewProjectReadmeExperiment, :experiment do + subject { described_class.new(actor: actor) } + + let(:actor) { User.new(id: 42, created_at: Time.current) } + + before do + stub_experiments(new_project_readme: :control) + end + + describe "exclusions" do + let(:threshold) { described_class::MAX_ACCOUNT_AGE } + + it { is_expected.to exclude(actor: User.new(created_at: (threshold + 1.minute).ago)) } + it { is_expected.not_to exclude(actor: User.new(created_at: (threshold - 1.minute).ago)) } + end + + describe "the control behavior" do + subject { described_class.new(actor: actor).run(:control) } + + it { is_expected.to be false } + end + + describe "the candidate behavior" do + subject { described_class.new(actor: actor).run(:candidate) } + + it { is_expected.to be true } + end + + context "when tracking initial writes" do + let!(:project) { create(:project, :repository) } + + def stub_gitaly_count(count = 1) + allow(Gitlab::GitalyClient).to receive(:call).and_call_original + allow(Gitlab::GitalyClient).to receive(:call).with(anything, :commit_service, :count_commits, anything, anything) + .and_return(double(count: count)) + end + + before do + stub_gitaly_count + end + + it "tracks an event for the first commit on a project with a repository" do + expect(subject).to receive(:track).with(:write, property: project.created_at.to_s, value: 1).and_call_original + + subject.track_initial_writes(project) + end + + it "tracks an event for the second commit on a project with a repository" do + stub_gitaly_count(2) + + expect(subject).to receive(:track).with(:write, property: project.created_at.to_s, value: 2).and_call_original + + subject.track_initial_writes(project) + end + + it "doesn't track if the repository has more then 2 commits" do + stub_gitaly_count(3) + + expect(subject).not_to receive(:track) + + subject.track_initial_writes(project) + end + + it "doesn't track when we generally shouldn't" do + allow(subject).to receive(:should_track?).and_return(false) + + expect(subject).not_to receive(:track) + + subject.track_initial_writes(project) + end + + it "doesn't track if the project is older" do + expect(project).to receive(:created_at).and_return(described_class::EXPERIMENT_START_DATE - 1.minute) + + expect(subject).not_to receive(:track) + + subject.track_initial_writes(project) + end + + it "handles exceptions by logging them" do + allow(Gitlab::GitalyClient).to receive(:call).with(anything, :commit_service, :count_commits, anything, anything) + .and_raise(e = StandardError.new('_message_')) + + expect(Gitlab::ErrorTracking).to receive(:track_exception).with(e, experiment: 'new_project_readme') + + subject.track_initial_writes(project) + end + end +end diff --git a/spec/experiments/strategy/round_robin_spec.rb b/spec/experiments/strategy/round_robin_spec.rb new file mode 100644 index 00000000000..f837a4701b2 --- /dev/null +++ b/spec/experiments/strategy/round_robin_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Strategy::RoundRobin, :clean_gitlab_redis_shared_state do + subject(:round_robin) { described_class.new('_key_', %i[variant1 variant2]) } + + describe "execute" do + context "when there are 2 variants" do + it "proves out round robin in selection", :aggregate_failures do + expect(round_robin.execute).to eq :variant2 + expect(round_robin.execute).to eq :variant1 + expect(round_robin.execute).to eq :variant2 + end + end + + context "when there are more than 2 variants" do + subject(:round_robin) { described_class.new('_key_', %i[variant1 variant2 variant3]) } + + it "proves out round robin in selection", :aggregate_failures do + expect(round_robin.execute).to eq :variant2 + expect(round_robin.execute).to eq :variant3 + expect(round_robin.execute).to eq :variant1 + + expect(round_robin.execute).to eq :variant2 + expect(round_robin.execute).to eq :variant3 + expect(round_robin.execute).to eq :variant1 + end + end + + context "when writing to cache fails" do + subject(:round_robin) { described_class.new('_key_', []) } + + it "raises an error and logs" do + allow(Gitlab::Redis::SharedState).to receive(:with).and_raise(Strategy::RoundRobin::CacheError) + expect(Gitlab::AppLogger).to receive(:warn) + + expect { round_robin.execute }.to raise_error(Strategy::RoundRobin::CacheError) + end + end + end + + describe "#counter_expires_in" do + it 'displays the expiration time in seconds' do + round_robin.execute + + expect(round_robin.counter_expires_in).to be_between(0, described_class::COUNTER_EXPIRE_TIME) + end + end + + describe '#value' do + it 'get the count' do + expect(round_robin.counter_value).to eq(0) + + round_robin.execute + + expect(round_robin.counter_value).to eq(1) + end + end + + describe '#reset!' do + it 'resets the count down to zero' do + 3.times { round_robin.execute } + + expect { round_robin.reset! }.to change { round_robin.counter_value }.from(3).to(0) + end + end +end |