diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-03-16 21:18:33 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-03-16 21:18:33 +0300 |
commit | f64a639bcfa1fc2bc89ca7db268f594306edfd7c (patch) | |
tree | a2c3c2ebcc3b45e596949db485d6ed18ffaacfa1 /spec/experiments | |
parent | bfbc3e0d6583ea1a91f627528bedc3d65ba4b10f (diff) |
Add latest changes from gitlab-org/gitlab@13-10-stable-eev13.10.0-rc40
Diffstat (limited to 'spec/experiments')
-rw-r--r-- | spec/experiments/application_experiment/cache_spec.rb | 54 | ||||
-rw-r--r-- | spec/experiments/application_experiment_spec.rb | 144 | ||||
-rw-r--r-- | spec/experiments/members/invite_email_experiment_spec.rb | 41 |
3 files changed, 84 insertions, 155 deletions
diff --git a/spec/experiments/application_experiment/cache_spec.rb b/spec/experiments/application_experiment/cache_spec.rb deleted file mode 100644 index 4caa91e6ac4..00000000000 --- a/spec/experiments/application_experiment/cache_spec.rb +++ /dev/null @@ -1,54 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ApplicationExperiment::Cache do - let(:key_name) { 'experiment_name' } - let(:field_name) { 'abc123' } - let(:key_field) { [key_name, field_name].join(':') } - let(:shared_state) { Gitlab::Redis::SharedState } - - around do |example| - shared_state.with { |r| r.del(key_name) } - example.run - shared_state.with { |r| r.del(key_name) } - end - - it "allows reading, writing and deleting", :aggregate_failures do - # we test them all together because they are largely interdependent - - expect(subject.read(key_field)).to be_nil - expect(shared_state.with { |r| r.hget(key_name, field_name) }).to be_nil - - subject.write(key_field, 'value') - - expect(subject.read(key_field)).to eq('value') - expect(shared_state.with { |r| r.hget(key_name, field_name) }).to eq('value') - - subject.delete(key_field) - - expect(subject.read(key_field)).to be_nil - expect(shared_state.with { |r| r.hget(key_name, field_name) }).to be_nil - end - - it "handles the fetch with a block behavior (which is read/write)" do - expect(subject.fetch(key_field) { 'value1' }).to eq('value1') # rubocop:disable Style/RedundantFetchBlock - expect(subject.fetch(key_field) { 'value2' }).to eq('value1') # rubocop:disable Style/RedundantFetchBlock - end - - it "can clear a whole experiment cache key" do - subject.write(key_field, 'value') - subject.clear(key: key_field) - - expect(shared_state.with { |r| r.get(key_name) }).to be_nil - end - - it "doesn't allow clearing a key from the cache that's not a hash (definitely not an experiment)" do - shared_state.with { |r| r.set(key_name, 'value') } - - expect { subject.clear(key: key_name) }.to raise_error( - ArgumentError, - 'invalid call to clear a non-hash cache key' - ) - end -end diff --git a/spec/experiments/application_experiment_spec.rb b/spec/experiments/application_experiment_spec.rb index 501d344e920..2481ee5a806 100644 --- a/spec/experiments/application_experiment_spec.rb +++ b/spec/experiments/application_experiment_spec.rb @@ -58,30 +58,38 @@ RSpec.describe ApplicationExperiment, :experiment do end describe "publishing results" do + it "doesn't track or push data to the client if we shouldn't track", :snowplow do + allow(subject).to receive(:should_track?).and_return(false) + expect(Gon).not_to receive(:push) + + subject.publish(:action) + + expect_no_snowplow_event + end + it "tracks the assignment" do expect(subject).to receive(:track).with(:assignment) - subject.publish(nil) + subject.publish end - it "pushes the experiment knowledge into the client using Gon.global" do - expect(Gon.global).to receive(:push).with( - { - experiment: { - 'namespaced/stub' => { # string key because it can be namespaced - experiment: 'namespaced/stub', - key: '86208ac54ca798e11f127e8b23ec396a', - variant: 'control' - } - } - }, - true - ) + it "pushes the experiment knowledge into the client using Gon" do + expect(Gon).to receive(:push).with({ experiment: { 'namespaced/stub' => subject.signature } }, true) + + subject.publish + end - subject.publish(nil) + it "handles when Gon raises exceptions (like when it can't be pushed into)" do + expect(Gon).to receive(:push).and_raise(NoMethodError) + + expect { subject.publish }.not_to raise_error end end + it "can exclude from within the block" do + expect(described_class.new('namespaced/stub') { |e| e.exclude! }).to be_excluded + end + describe "tracking events", :snowplow do it "doesn't track if we shouldn't track" do allow(subject).to receive(:should_track?).and_return(false) @@ -115,91 +123,36 @@ RSpec.describe ApplicationExperiment, :experiment do end describe "variant resolution" do - 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 "uses the default value as specified in the yaml" do + expect(Feature).to receive(:enabled?).with('namespaced_stub', subject, type: :experiment, default_enabled: :yaml) - 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('variant1') - end - end + expect(subject.variant.name).to eq('control') end - 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 + context "when rolled out to 100%" do + before do + stub_feature_flags(namespaced_stub: true) 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') + it "returns the first variant name" do + subject.try(:variant1) {} + subject.try(:variant2) {} - instance_2.try {} - - expect(instance_2.variant.name).to eq('control') - - instance_3.try {} - - expect(instance_3.variant.name).to eq('variant1') - end + expect(subject.variant.name).to eq('variant1') end end end context "when caching" do - let(:cache) { ApplicationExperiment::Cache.new } + let(:cache) { Gitlab::Experiment::Configuration.cache } before do + allow(Gitlab::Experiment::Configuration).to receive(:cache).and_call_original + cache.clear(key: subject.name) subject.use { } # setup the control subject.try { } # setup the candidate - - allow(Gitlab::Experiment::Configuration).to receive(:cache).and_return(cache) end it "caches the variant determined by the variant resolver" do @@ -207,7 +160,7 @@ RSpec.describe ApplicationExperiment, :experiment do subject.run - expect(cache.read(subject.cache_key)).to eq('candidate') + expect(subject.cache.read).to eq('candidate') end it "doesn't cache a variant if we don't explicitly provide one" do @@ -222,7 +175,7 @@ RSpec.describe ApplicationExperiment, :experiment do subject.run - expect(cache.read(subject.cache_key)).to be_nil + expect(subject.cache.read).to be_nil end it "caches a control variant if we assign it specifically" do @@ -232,7 +185,26 @@ RSpec.describe ApplicationExperiment, :experiment do # write code that would specify a different variant. subject.run(:control) - expect(cache.read(subject.cache_key)).to eq('control') + expect(subject.cache.read).to eq('control') + end + + context "arbitrary attributes" do + before do + subject.cache.store.clear(key: subject.name + '_attrs') + end + + it "sets and gets attributes about an experiment" do + subject.cache.attr_set(:foo, :bar) + + expect(subject.cache.attr_get(:foo)).to eq('bar') + end + + it "increments a value for an experiment" do + expect(subject.cache.attr_get(:foo)).to be_nil + + expect(subject.cache.attr_inc(:foo)).to eq(1) + expect(subject.cache.attr_inc(:foo)).to eq(2) + end end end end diff --git a/spec/experiments/members/invite_email_experiment_spec.rb b/spec/experiments/members/invite_email_experiment_spec.rb index 4376c021385..539230e39b9 100644 --- a/spec/experiments/members/invite_email_experiment_spec.rb +++ b/spec/experiments/members/invite_email_experiment_spec.rb @@ -3,26 +3,14 @@ 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 + subject(:invite_email) { experiment('members/invite_email', **context) } + + let(:context) { { actor: double('Member', created_by: double('User', avatar_url: '_avatar_url_')) } } 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)) @@ -34,4 +22,27 @@ RSpec.describe Members::InviteEmailExperiment do expect(experiment('members/invite_email')).to exclude(actor: member_without_avatar_url) end end + + describe "variant resolution", :clean_gitlab_redis_shared_state do + it "proves out round robin in variant selection", :aggregate_failures do + instance_1 = described_class.new('members/invite_email', **context) + allow(instance_1).to receive(:enabled?).and_return(true) + instance_2 = described_class.new('members/invite_email', **context) + allow(instance_2).to receive(:enabled?).and_return(true) + instance_3 = described_class.new('members/invite_email', **context) + allow(instance_3).to receive(:enabled?).and_return(true) + + instance_1.try { } + + expect(instance_1.variant.name).to eq('permission_info') + + instance_2.try { } + + expect(instance_2.variant.name).to eq('control') + + instance_3.try { } + + expect(instance_3.variant.name).to eq('avatar') + end + end end |