diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-23 15:06:29 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-23 15:06:29 +0000 |
commit | b3f7042d06c53e5d4b8cad42e1b2679d0450f1a7 (patch) | |
tree | 373a039989e0497a71a4844f48e27d5f82f0e198 | |
parent | 09ffaae1328da918056512ddc674913f0bb7b134 (diff) | |
download | gitlab-ce-b3f7042d06c53e5d4b8cad42e1b2679d0450f1a7.tar.gz |
Add latest changes from gitlab-org/gitlab@master
-rw-r--r-- | app/assets/javascripts/pages/sessions/new/index.js | 9 | ||||
-rw-r--r-- | app/controllers/registrations_controller.rb | 4 | ||||
-rw-r--r-- | app/controllers/sessions_controller.rb | 6 | ||||
-rw-r--r-- | changelogs/unreleased/tracking-experimental-signup-flow.yml | 5 | ||||
-rw-r--r-- | doc/ci/yaml/README.md | 18 | ||||
-rw-r--r-- | doc/development/testing_guide/best_practices.md | 21 | ||||
-rw-r--r-- | lib/gitlab/experimentation.rb | 62 | ||||
-rw-r--r-- | spec/controllers/application_controller_spec.rb | 2 | ||||
-rw-r--r-- | spec/controllers/registrations_controller_spec.rb | 92 | ||||
-rw-r--r-- | spec/controllers/sessions_controller_spec.rb | 33 | ||||
-rw-r--r-- | spec/features/users/signup_spec.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/experimentation_spec.rb | 253 | ||||
-rw-r--r-- | spec/requests/api/users_spec.rb | 4 | ||||
-rw-r--r-- | spec/support/helpers/stub_experiments.rb | 14 |
14 files changed, 441 insertions, 84 deletions
diff --git a/app/assets/javascripts/pages/sessions/new/index.js b/app/assets/javascripts/pages/sessions/new/index.js index 55bc93a2b13..c0e798e004f 100644 --- a/app/assets/javascripts/pages/sessions/new/index.js +++ b/app/assets/javascripts/pages/sessions/new/index.js @@ -5,6 +5,7 @@ import NoEmojiValidator from '../../../emoji/no_emoji_validator'; import SigninTabsMemoizer from './signin_tabs_memoizer'; import OAuthRememberMe from './oauth_remember_me'; import preserveUrlFragment from './preserve_url_fragment'; +import Tracking from '~/tracking'; document.addEventListener('DOMContentLoaded', () => { new UsernameValidator(); // eslint-disable-line no-new @@ -19,4 +20,12 @@ document.addEventListener('DOMContentLoaded', () => { // Save the URL fragment from the current window location. This will be present if the user was // redirected to sign-in after attempting to access a protected URL that included a fragment. preserveUrlFragment(window.location.hash); + + if (gon.tracking_data) { + const tab = document.querySelector(".new-session-tabs a[href='#register-pane']"); + const { category, action, ...data } = gon.tracking_data; + tab.addEventListener('click', () => { + Tracking.event(category, action, data); + }); + } }); diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 4a746fc915d..e55156c619b 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -16,6 +16,7 @@ class RegistrationsController < Devise::RegistrationsController def new if experiment_enabled?(:signup_flow) + track_experiment_event(:signup_flow, 'start') # We want this event to be tracked when the user is _in_ the experimental group @resource = build_resource else redirect_to new_user_session_path(anchor: 'register-pane') @@ -23,6 +24,8 @@ class RegistrationsController < Devise::RegistrationsController end def create + track_experiment_event(:signup_flow, 'end') unless experiment_enabled?(:signup_flow) # We want this event to be tracked when the user is _in_ the control group + accept_pending_invitations super do |new_user| @@ -61,6 +64,7 @@ class RegistrationsController < Devise::RegistrationsController result = ::Users::UpdateService.new(current_user, user_params.merge(user: current_user)).execute if result[:status] == :success + track_experiment_event(:signup_flow, 'end') # We want this event to be tracked when the user is _in_ the experimental group set_flash_message! :notice, :signed_up redirect_to stored_location_or_dashboard_or_almost_there_path(current_user) else diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 1c506065b56..00e3be0edfa 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -24,6 +24,7 @@ class SessionsController < Devise::SessionsController before_action :store_unauthenticated_sessions, only: [:new] before_action :save_failed_login, if: :action_new_and_failed_login? before_action :load_recaptcha + before_action :frontend_tracking_data, only: [:new] after_action :log_failed_login, if: :action_new_and_failed_login? @@ -293,6 +294,11 @@ class SessionsController < Devise::SessionsController "standard" end end + + def frontend_tracking_data + # We want tracking data pushed to the frontend when the user is _in_ the control group + frontend_experimentation_tracking_data(:signup_flow, 'start') unless experiment_enabled?(:signup_flow) + end end SessionsController.prepend_if_ee('EE::SessionsController') diff --git a/changelogs/unreleased/tracking-experimental-signup-flow.yml b/changelogs/unreleased/tracking-experimental-signup-flow.yml new file mode 100644 index 00000000000..e0effd396cb --- /dev/null +++ b/changelogs/unreleased/tracking-experimental-signup-flow.yml @@ -0,0 +1,5 @@ +--- +title: Track the starting and stopping of the current signup flow and the experimental signup flow +merge_request: 17521 +author: +type: other diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 36a2d0c4e01..242367db2ef 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -2321,6 +2321,24 @@ staging: branch: stable ``` +It is possible to mirror the status from a triggered pipeline: + +``` +trigger_job: + trigger: + project: my/project + strategy: depend +``` + +It is possible to mirror the status from an upstream pipeline: + +``` +upstream_bridge: + stage: test + needs: + pipeline: other/project +``` + ### `interruptible` > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/23464) in GitLab 12.3. diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index 70945cdaacb..22dab30e362 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -356,17 +356,22 @@ However, if a spec makes direct Redis calls, it should mark itself with the `:clean_gitlab_redis_cache`, `:clean_gitlab_redis_shared_state` or `:clean_gitlab_redis_queues` traits as appropriate. -Sidekiq jobs are typically not run in specs, but this behaviour can be altered -in each spec through the use of `perform_enqueued_jobs` blocks. -Any spec that causes Sidekiq jobs to be pushed to Redis should use the -`:sidekiq_inline` trait, to ensure that they are removed once the spec completes. +#### Background jobs / Sidekiq + +By default, Sidekiq jobs are enqueued into a jobs array and aren't processed. +If a test enqueues Sidekiq jobs and need them to be processed, the +`:sidekiq_inline` trait can be used. The `:sidekiq_might_not_need_inline` trait was added when [Sidekiq inline mode was -changed to fake mode](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31662) -to all the examples that needed Sidekiq to actually process jobs. Examples with +changed to fake mode](https://gitlab.com/gitlab-org/gitlab/merge_requests/15479) +to all the tests that needed Sidekiq to actually process jobs. Tests with this trait should be either fixed to not rely on Sidekiq processing jobs, or their -`:sidekiq_might_not_need_inline` trait should be updated to `:sidekiq_inline` if the -processing of background jobs is needed/expected. +`:sidekiq_might_not_need_inline` trait should be updated to `:sidekiq_inline` if +the processing of background jobs is needed/expected. + +NOTE: **Note:** +The usage of `perform_enqueued_jobs` is currently useless since our +workers aren't inheriting from `ApplicationJob` / `ActiveJob::Base`. #### Filesystem diff --git a/lib/gitlab/experimentation.rb b/lib/gitlab/experimentation.rb index 895755376ee..8b315ae5606 100644 --- a/lib/gitlab/experimentation.rb +++ b/lib/gitlab/experimentation.rb @@ -14,13 +14,15 @@ module Gitlab signup_flow: { feature_toggle: :experimental_separate_sign_up_flow, environment: ::Gitlab.dev_env_or_com?, - enabled_ratio: 0.1 + enabled_ratio: 0.1, + tracking_category: 'Growth::Acquisition::Experiment::SignUpFlow' } }.freeze # Controller concern that checks if an experimentation_subject_id cookie is present and sets it if absent. # Used for A/B testing of experimental features. Exposes the `experiment_enabled?(experiment_name)` method - # to controllers and views. + # to controllers and views. It returns true when the experiment is enabled and the user is selected as part + # of the experimental group. # module ControllerConcern extend ActiveSupport::Concern @@ -41,17 +43,57 @@ module Gitlab end def experiment_enabled?(experiment_key) - Experimentation.enabled?(experiment_key, experimentation_subject_index) + Experimentation.enabled_for_user?(experiment_key, experimentation_subject_index) + end + + def track_experiment_event(experiment_key, action) + track_experiment_event_for(experiment_key, action) do |tracking_data| + ::Gitlab::Tracking.event(tracking_data.delete(:category), tracking_data.delete(:action), tracking_data) + end + end + + def frontend_experimentation_tracking_data(experiment_key, action) + track_experiment_event_for(experiment_key, action) do |tracking_data| + gon.push(tracking_data: tracking_data) + end end private + def experimentation_subject_id + cookies.signed[:experimentation_subject_id] + end + def experimentation_subject_index - experimentation_subject_id = cookies.signed[:experimentation_subject_id] return if experimentation_subject_id.blank? experimentation_subject_id.delete('-').hex % 100 end + + def track_experiment_event_for(experiment_key, action) + return unless Experimentation.enabled?(experiment_key) + + yield experimentation_tracking_data(experiment_key, action) + end + + def experimentation_tracking_data(experiment_key, action) + { + category: tracking_category(experiment_key), + action: action, + property: tracking_group(experiment_key), + label: experimentation_subject_id + } + end + + def tracking_category(experiment_key) + Experimentation.experiment(experiment_key).tracking_category + end + + def tracking_group(experiment_key) + return unless Experimentation.enabled?(experiment_key) + + experiment_enabled?(experiment_key) ? 'experimental_group' : 'control_group' + end end class << self @@ -59,18 +101,20 @@ module Gitlab Experiment.new(EXPERIMENTS[key].merge(key: key)) end - def enabled?(experiment_key, experimentation_subject_index) + def enabled?(experiment_key) return false unless EXPERIMENTS.key?(experiment_key) experiment = experiment(experiment_key) + experiment.feature_toggle_enabled? && experiment.enabled_for_environment? + end - experiment.feature_toggle_enabled? && - experiment.enabled_for_environment? && - experiment.enabled_for_experimentation_subject?(experimentation_subject_index) + def enabled_for_user?(experiment_key, experimentation_subject_index) + enabled?(experiment_key) && + experiment(experiment_key).enabled_for_experimentation_subject?(experimentation_subject_index) end end - Experiment = Struct.new(:key, :feature_toggle, :environment, :enabled_ratio, keyword_init: true) do + Experiment = Struct.new(:key, :feature_toggle, :environment, :enabled_ratio, :tracking_category, keyword_init: true) do def feature_toggle_enabled? return Feature.enabled?(key, default_enabled: true) if feature_toggle.nil? diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index f4ac539136c..94afe741057 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -836,7 +836,7 @@ describe ApplicationController do let(:experiment_enabled) { true } before do - stub_experiment(signup_flow: experiment_enabled) + stub_experiment_for_user(signup_flow: experiment_enabled) end context 'experiment enabled and user with required role' do diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index ebeed94c274..8f147f949d0 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -9,6 +9,49 @@ describe RegistrationsController do stub_feature_flags(invisible_captcha: false) end + describe '#new' do + subject { get :new } + + context 'with the experimental signup flow enabled and the user is part of the experimental group' do + before do + stub_experiment(signup_flow: true) + stub_experiment_for_user(signup_flow: true) + end + + it 'tracks the event with the right parameters' do + expect(Gitlab::Tracking).to receive(:event).with( + 'Growth::Acquisition::Experiment::SignUpFlow', + 'start', + label: anything, + property: 'experimental_group' + ) + subject + end + + it 'renders new template and sets the resource variable' do + expect(subject).to render_template(:new) + expect(assigns(:resource)).to be_a(User) + end + end + + context 'with the experimental signup flow enabled and the user is part of the control group' do + before do + stub_experiment(signup_flow: true) + stub_experiment_for_user(signup_flow: false) + end + + it 'does not track the event' do + expect(Gitlab::Tracking).not_to receive(:event) + subject + end + + it 'renders new template and sets the resource variable' do + subject + expect(response).to redirect_to(new_user_session_path(anchor: 'register-pane')) + end + end + end + describe '#create' do let(:base_user_params) { { name: 'new_user', username: 'new_username', email: 'new@user.com', password: 'Any_password' } } let(:user_params) { { user: base_user_params } } @@ -217,6 +260,37 @@ describe RegistrationsController do end end + describe 'tracking data' do + context 'with the experimental signup flow enabled and the user is part of the control group' do + before do + stub_experiment(signup_flow: true) + stub_experiment_for_user(signup_flow: false) + end + + it 'tracks the event with the right parameters' do + expect(Gitlab::Tracking).to receive(:event).with( + 'Growth::Acquisition::Experiment::SignUpFlow', + 'end', + label: anything, + property: 'control_group' + ) + post :create, params: user_params + end + end + + context 'with the experimental signup flow enabled and the user is part of the experimental group' do + before do + stub_experiment(signup_flow: true) + stub_experiment_for_user(signup_flow: true) + end + + it 'does not track the event' do + expect(Gitlab::Tracking).not_to receive(:event) + post :create, params: user_params + end + end + end + it "logs a 'User Created' message" do stub_feature_flags(registrations_recaptcha: false) @@ -304,4 +378,22 @@ describe RegistrationsController do end end end + + describe '#update_role' do + before do + stub_experiment(signup_flow: true) + stub_experiment_for_user(signup_flow: true) + sign_in(create(:user)) + end + + it 'tracks the event with the right parameters' do + expect(Gitlab::Tracking).to receive(:event).with( + 'Growth::Acquisition::Experiment::SignUpFlow', + 'end', + label: anything, + property: 'experimental_group' + ) + patch :update_role, params: { user: { name: 'New name', role: 'software_developer' } } + end + end end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 2108cf1c8ae..53847e30a5c 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -34,6 +34,39 @@ describe SessionsController do end end end + + describe 'tracking data' do + context 'when the user is part of the experimental group' do + before do + stub_experiment_for_user(signup_flow: true) + end + + it 'doesn\'t pass tracking parameters to the frontend' do + get(:new) + expect(Gon.tracking_data).to be_nil + end + end + + context 'with the experimental signup flow enabled and the user is part of the control group' do + before do + stub_experiment(signup_flow: true) + stub_experiment_for_user(signup_flow: false) + allow_any_instance_of(described_class).to receive(:experimentation_subject_id).and_return('uuid') + end + + it 'passes the right tracking parameters to the frontend' do + get(:new) + expect(Gon.tracking_data).to eq( + { + category: 'Growth::Acquisition::Experiment::SignUpFlow', + action: 'start', + label: 'uuid', + property: 'control_group' + } + ) + end + end + end end describe '#create' do diff --git a/spec/features/users/signup_spec.rb b/spec/features/users/signup_spec.rb index 562d6fcab1b..8a82d3f3bd0 100644 --- a/spec/features/users/signup_spec.rb +++ b/spec/features/users/signup_spec.rb @@ -413,6 +413,7 @@ end describe 'With original flow' do before do stub_experiment(signup_flow: false) + stub_experiment_for_user(signup_flow: false) end it_behaves_like 'Signup' @@ -421,6 +422,7 @@ end describe 'With experimental flow' do before do stub_experiment(signup_flow: true) + stub_experiment_for_user(signup_flow: true) end it_behaves_like 'Signup' diff --git a/spec/lib/gitlab/experimentation_spec.rb b/spec/lib/gitlab/experimentation_spec.rb index 2e5fd16d370..a7d3f628741 100644 --- a/spec/lib/gitlab/experimentation_spec.rb +++ b/spec/lib/gitlab/experimentation_spec.rb @@ -2,81 +2,186 @@ require 'spec_helper' -describe Gitlab::Experimentation::ControllerConcern, type: :controller do - controller(ApplicationController) do - include Gitlab::Experimentation::ControllerConcern +describe Gitlab::Experimentation do + before do + stub_const('Gitlab::Experimentation::EXPERIMENTS', { + test_experiment: { + feature_toggle: feature_toggle, + environment: environment, + enabled_ratio: enabled_ratio, + tracking_category: 'Team' + } + }) - def index - head :ok - end + stub_feature_flags(feature_toggle => true) end - describe '#set_experimentation_subject_id_cookie' do - before do - get :index + let(:feature_toggle) { :test_experiment_toggle } + let(:environment) { Rails.env.test? } + let(:enabled_ratio) { 0.1 } + + describe Gitlab::Experimentation::ControllerConcern, type: :controller do + controller(ApplicationController) do + include Gitlab::Experimentation::ControllerConcern + + def index + head :ok + end end - context 'cookie is present' do + describe '#set_experimentation_subject_id_cookie' do before do - cookies[:experimentation_subject_id] = 'test' + get :index end - it 'does not change the cookie' do - expect(cookies[:experimentation_subject_id]).to eq 'test' + context 'cookie is present' do + before do + cookies[:experimentation_subject_id] = 'test' + end + + it 'does not change the cookie' do + expect(cookies[:experimentation_subject_id]).to eq 'test' + end end - end - context 'cookie is not present' do - it 'sets a permanent signed cookie' do - expect(cookies.permanent.signed[:experimentation_subject_id]).to be_present + context 'cookie is not present' do + it 'sets a permanent signed cookie' do + expect(cookies.permanent.signed[:experimentation_subject_id]).to be_present + end end end - end - describe '#experiment_enabled?' do - context 'cookie is not present' do - it 'calls Gitlab::Experimentation.enabled? with the name of the experiment and an experimentation_subject_index of nil' do - expect(Gitlab::Experimentation).to receive(:enabled?).with(:test_experiment, nil) - controller.experiment_enabled?(:test_experiment) + describe '#experiment_enabled?' do + context 'cookie is not present' do + it 'calls Gitlab::Experimentation.enabled_for_user? with the name of the experiment and an experimentation_subject_index of nil' do + expect(Gitlab::Experimentation).to receive(:enabled_for_user?).with(:test_experiment, nil) # rubocop:disable RSpec/DescribedClass + controller.experiment_enabled?(:test_experiment) + end + end + + context 'cookie is present' do + before do + cookies.permanent.signed[:experimentation_subject_id] = 'abcd-1234' + get :index + end + + it 'calls Gitlab::Experimentation.enabled_for_user? with the name of the experiment and an experimentation_subject_index of the modulo 100 of the hex value of the uuid' do + # 'abcd1234'.hex % 100 = 76 + expect(Gitlab::Experimentation).to receive(:enabled_for_user?).with(:test_experiment, 76) # rubocop:disable RSpec/DescribedClass + controller.experiment_enabled?(:test_experiment) + end end end - context 'cookie is present' do - before do - cookies.permanent.signed[:experimentation_subject_id] = 'abcd-1234' - get :index + describe '#track_experiment_event' do + context 'when the experiment is enabled' do + before do + stub_experiment(test_experiment: true) + end + + context 'the user is part of the experimental group' do + before do + stub_experiment_for_user(test_experiment: true) + end + + it 'tracks the event with the right parameters' do + expect(Gitlab::Tracking).to receive(:event).with( + 'Team', + 'start', + label: nil, + property: 'experimental_group' + ) + controller.track_experiment_event(:test_experiment, 'start') + end + end + + context 'the user is part of the control group' do + before do + stub_experiment_for_user(test_experiment: false) + end + + it 'tracks the event with the right parameters' do + expect(Gitlab::Tracking).to receive(:event).with( + 'Team', + 'start', + label: nil, + property: 'control_group' + ) + controller.track_experiment_event(:test_experiment, 'start') + end + end end - it 'calls Gitlab::Experimentation.enabled? with the name of the experiment and an experimentation_subject_index of the modulo 100 of the hex value of the uuid' do - # 'abcd1234'.hex % 100 = 76 - expect(Gitlab::Experimentation).to receive(:enabled?).with(:test_experiment, 76) - controller.experiment_enabled?(:test_experiment) + context 'when the experiment is disabled' do + before do + stub_experiment(test_experiment: false) + end + + it 'does not track the event' do + expect(Gitlab::Tracking).not_to receive(:event) + controller.track_experiment_event(:test_experiment, 'start') + end end end - end -end -describe Gitlab::Experimentation do - before do - stub_const('Gitlab::Experimentation::EXPERIMENTS', { - test_experiment: { - feature_toggle: feature_toggle, - environment: environment, - enabled_ratio: enabled_ratio - } - }) + describe '#frontend_experimentation_tracking_data' do + context 'when the experiment is enabled' do + before do + stub_experiment(test_experiment: true) + end - stub_feature_flags(feature_toggle => true) - end + context 'the user is part of the experimental group' do + before do + stub_experiment_for_user(test_experiment: true) + end + + it 'pushes the right parameters to gon' do + controller.frontend_experimentation_tracking_data(:test_experiment, 'start') + expect(Gon.tracking_data).to eq( + { + category: 'Team', + action: 'start', + label: nil, + property: 'experimental_group' + } + ) + end + end - let(:feature_toggle) { :test_experiment_toggle } - let(:environment) { Rails.env.test? } - let(:enabled_ratio) { 0.1 } + context 'the user is part of the control group' do + before do + allow_any_instance_of(described_class).to receive(:experiment_enabled?).with(:test_experiment).and_return(false) + end + + it 'pushes the right parameters to gon' do + controller.frontend_experimentation_tracking_data(:test_experiment, 'start') + expect(Gon.tracking_data).to eq( + { + category: 'Team', + action: 'start', + label: nil, + property: 'control_group' + } + ) + end + end + end - describe '.enabled?' do - subject { described_class.enabled?(:test_experiment, experimentation_subject_index) } + context 'when the experiment is disabled' do + before do + stub_experiment(test_experiment: false) + end - let(:experimentation_subject_index) { 9 } + it 'does not push data to gon' do + expect(Gon.method_defined?(:tracking_data)).to be_falsey + controller.track_experiment_event(:test_experiment, 'start') + end + end + end + end + + describe '.enabled?' do + subject { described_class.enabled?(:test_experiment) } context 'feature toggle is enabled, we are on the right environment and we are selected' do it { is_expected.to be_truthy } @@ -84,7 +189,7 @@ describe Gitlab::Experimentation do describe 'experiment is not defined' do it 'returns false' do - expect(described_class.enabled?(:missing_experiment, experimentation_subject_index)).to be_falsey + expect(described_class.enabled?(:missing_experiment)).to be_falsey end end @@ -127,30 +232,52 @@ describe Gitlab::Experimentation do it { is_expected.to be_falsey } end end + end - describe 'enabled ratio' do - context 'enabled ratio is not set' do - let(:enabled_ratio) { nil } + describe '.enabled_for_user?' do + subject { described_class.enabled_for_user?(:test_experiment, experimentation_subject_index) } - it { is_expected.to be_falsey } + let(:experimentation_subject_index) { 9 } + + context 'experiment is disabled' do + before do + allow(described_class).to receive(:enabled?).and_return(false) end - context 'experimentation_subject_index is not set' do - let(:experimentation_subject_index) { nil } + it { is_expected.to be_falsey } + end - it { is_expected.to be_falsey } + context 'experiment is enabled' do + before do + allow(described_class).to receive(:enabled?).and_return(true) end - context 'experimentation_subject_index is an empty string' do - let(:experimentation_subject_index) { '' } + it { is_expected.to be_truthy } + + context 'enabled ratio is not set' do + let(:enabled_ratio) { nil } it { is_expected.to be_falsey } end - context 'experimentation_subject_index outside enabled ratio' do - let(:experimentation_subject_index) { 11 } + describe 'experimentation_subject_index' do + context 'experimentation_subject_index is not set' do + let(:experimentation_subject_index) { nil } - it { is_expected.to be_falsey } + it { is_expected.to be_falsey } + end + + context 'experimentation_subject_index is an empty string' do + let(:experimentation_subject_index) { '' } + + it { is_expected.to be_falsey } + end + + context 'experimentation_subject_index outside enabled ratio' do + let(:experimentation_subject_index) { 11 } + + it { is_expected.to be_falsey } + end end end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index a006ac4571c..50e82ca5481 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -636,7 +636,7 @@ describe API::Users do describe "GET /users/sign_up" do context 'when experimental signup_flow is active' do before do - stub_experiment(signup_flow: true) + stub_experiment_for_user(signup_flow: true) end it "shows sign up page" do @@ -648,7 +648,7 @@ describe API::Users do context 'when experimental signup_flow is not active' do before do - stub_experiment(signup_flow: false) + stub_experiment_for_user(signup_flow: false) end it "redirects to sign in page" do diff --git a/spec/support/helpers/stub_experiments.rb b/spec/support/helpers/stub_experiments.rb index ed868e22c6e..7a5a188ab4d 100644 --- a/spec/support/helpers/stub_experiments.rb +++ b/spec/support/helpers/stub_experiments.rb @@ -9,7 +9,19 @@ module StubExperiments # - `stub_experiment(signup_flow: false)` ... Disable `signup_flow` experiment globally. def stub_experiment(experiments) experiments.each do |experiment_key, enabled| - allow(Gitlab::Experimentation).to receive(:enabled?).with(experiment_key, any_args) { enabled } + allow(Gitlab::Experimentation).to receive(:enabled?).with(experiment_key) { enabled } + end + end + + # Stub Experiment for user with `key: true/false` + # + # @param [Hash] experiment where key is feature name and value is boolean whether enabled or not. + # + # Examples + # - `stub_experiment_for_user(signup_flow: false)` ... Disable `signup_flow` experiment for user. + def stub_experiment_for_user(experiments) + experiments.each do |experiment_key, enabled| + allow(Gitlab::Experimentation).to receive(:enabled_for_user?).with(experiment_key, anything) { enabled } end end end |