fix(experiments): Only enable the experiment defined by `forceExperiment`. (#5447) r=vladikoff

In the functional test we use the forceExperiment and
forceExperimentGroup query parameters to force users
into an experiment. To avoid unexpected behaviors,
within the functional tests experiments are usually
disabled, unless of course, forceExperiment is defined.
The problem is, if forceExperiment is defined, all
experiments are then enabled. Independent experiments
that do not have a meta-experiment to choose between
them, e.g., q3FormChanges, can all be chosen. This
leads to unexpected behaviors in the functional tests.

If forceExperiment is defined, only that experiment should be enabled.

fixes #5446

app/scripts/lib/experiments/grouping-rules/disabled-button-state.js

@@ -30,15 +30,7 @@ define((require, exports, module) => {
         return false;
       }
 
-      let choice;
-
-      if (subject.forceExperiment === this.name && subject.forceExperimentGroup) {
-        choice = subject.forceExperimentGroup;
-      } else {
-        choice = this.uniformChoice(GROUPS, subject.uniqueUserId);
-      }
-
-      return choice;
+      return this.uniformChoice(GROUPS, subject.uniqueUserId);
     }
   };
 });

app/scripts/lib/experiments/grouping-rules/email-first.js

@@ -26,8 +26,6 @@ define((require, exports, module) => {
 
       if (! this._areSubjectPrereqsMet(subject)) {
         return false;
-      } else if (subject.forceExperiment === this.name && subject.forceExperimentGroup) {
-        return subject.forceExperimentGroup;
       } else if (! subject.isEmailFirstSupported) {
         // isEmailFirstSupported is `true` for brokers that support the email-first flow.
         return false;

app/scripts/lib/experiments/grouping-rules/index.js

@@ -40,6 +40,12 @@ define((require, exports, module) => {
     choose (name, subject = {}) {
       const experiment = _.find(this._experimentGroupingRules, (experimentGroupingRule) => experimentGroupingRule.name === name);
       if (experiment) {
+        if (! isExperimentAllowed(experiment, subject)) {
+          return false;
+        } else if (useForceExperimentGroup(experiment, subject)) {
+          return subject.forceExperimentGroup;
+        }
+
         // A reference to `experimentGroupingRules` is passed in
         // `subject` to allow tests to recursively call other tests.
         // Passing via the `subject` instead of the constructor
@@ -53,4 +59,26 @@ define((require, exports, module) => {
       }
     }
   };
+
+  function isExperimentAllowed(experiment, subject) {
+    // Functional tests use the forceExperiment & forceExperimentGroup query parameters
+    // to force entry into a given experiment. When forcing a given experiment,
+    // functional tests do not expect interference from other functional tests.
+    // If forceExperiment and forceExperimentGroup are specified, *only* enter the
+    // user into the experiment whose name matches forceExperimentGroup. Ignore
+    // all other experiments unless they are explicitly allowed.
+    return ! subject.forceExperiment ||
+             allowExperimentWithForceExperiment(experiment, subject.forceExperiment);
+  }
+
+  function allowExperimentWithForceExperiment(experiment, forceExperiment) {
+    return forceExperiment === experiment.name ||
+           forceExperiment === experiment.forceExperimentAllow;
+  }
+
+  function useForceExperimentGroup (experiment, subject) {
+    return subject.forceExperiment &&
+           subject.forceExperimentGroup &&
+           subject.forceExperiment === experiment.name;
+  }
 });

app/scripts/lib/experiments/grouping-rules/q3-form-changes.js

@@ -25,15 +25,7 @@ define((require, exports, module) => {
         return false;
       }
 
-      let choice;
-
-      if (subject.forceExperiment) {
-        choice = subject.forceExperiment;
-      } else {
-        choice = this.uniformChoice(EXPERIMENTS, subject.uniqueUserId);
-      }
-
-      return choice;
+      return this.uniformChoice(EXPERIMENTS, subject.uniqueUserId);
     }
   };
 });

app/scripts/lib/experiments/grouping-rules/send-sms-enabled-for-country.js

@@ -28,6 +28,8 @@ define((require, exports, module) => {
     constructor () {
       super();
       this.name = 'sendSmsEnabledForCountry';
+      // This experiment must be allowed if `sendSms` is forced.
+      this.forceExperimentAllow = 'sendSms';
       this.ENABLED_COUNTRY_LIST = ENABLED_COUNTRY_LIST;
     }
 

app/scripts/lib/experiments/grouping-rules/send-sms-install-link.js

@@ -30,9 +30,7 @@ define((require, exports, module) => {
 
       let choice;
 
-      if (subject.forceExperiment === this.name && subject.forceExperimentGroup) {
-        choice = subject.forceExperimentGroup;
-      } else if (isEmailInSigninCodesGroup(subject.account.get('email'))) {
+      if (isEmailInSigninCodesGroup(subject.account.get('email'))) {
         choice = 'signinCodes';
       } else {
         choice = this.uniformChoice(GROUPS, subject.uniqueUserId);

app/scripts/lib/experiments/grouping-rules/signup-password-confirm.js

@@ -20,10 +20,6 @@ define((require, exports, module) => {
         return false;
       }
 
-      if (subject.forceExperiment === this.name && subject.forceExperimentGroup) {
-        return subject.forceExperimentGroup;
-      }
-
       if (! subject.experimentGroupingRules || subject.experimentGroupingRules.choose('q3FormChanges', subject) !== this.name) {
         return false;
       }

app/tests/spec/lib/experiments/grouping-rules/disabled-button-state.js

@@ -31,16 +31,6 @@ define(function (require, exports, module) {
         assert.isFalse(experiment.choose({}));
       });
 
-      it('returns experiment if forceExperiment', () => {
-        assert.equal(experiment.choose({
-          account,
-          experimentGroupingRules,
-          forceExperiment: 'disabledButtonState',
-          forceExperimentGroup: 'control',
-          uniqueUserId: 'user-id'
-        }), 'control');
-      });
-
       it('returns chooses some experiment ', () => {
         assert.ok(experiment.choose({
           account,

app/tests/spec/lib/experiments/grouping-rules/email-first.js

@@ -46,15 +46,6 @@ define(function (require, exports, module) {
         assert.isFalse(experiment.choose({ uniqueUserId: 'user-id' }));
       });
 
-      it('returns experiment if forceExperiment', () => {
-        assert.equal(experiment.choose({
-          experimentGroupingRules,
-          forceExperiment: 'emailFirst',
-          forceExperimentGroup: 'control',
-          uniqueUserId: 'user-id'
-        }), 'control');
-      });
-
       it('returns `false` if `isEmailFirstSupported=false`', () => {
         assert.isFalse(experiment.choose({
           experimentGroupingRules,

app/tests/spec/lib/experiments/grouping-rules/index.js

@@ -15,6 +15,7 @@ define(function (require, exports, module) {
       let experimentGroupingRules;
       let rule1;
       let rule2;
+      let rule3;
 
       before(() => {
         rule1 = {
@@ -25,12 +26,18 @@ define(function (require, exports, module) {
           choose: sinon.spy(() => 'treatment'),
           name: 'rule2'
         };
+        rule3 = {
+          choose: sinon.spy(() => 'control'),
+          forceExperimentAllow: 'rule2',
+          name: 'rule3'
+        };
 
         experimentGroupingRules = new ExperimentGroupingRules({
           env: 'development',
           experimentGroupingRules: [
             rule1,
-            rule2
+            rule2,
+            rule3
           ]
         });
       });
@@ -39,6 +46,15 @@ define(function (require, exports, module) {
         assert.isUndefined(experimentGroupingRules.choose('does-not-exist', {}));
       });
 
+      it('returns `false` if experiment does not meet forceExperiment requirements', () => {
+        assert.isFalse(experimentGroupingRules.choose('rule1', { forceExperiment: 'rule2', forceExperimentGroup: 'treatment' }));
+        assert.isFalse(experimentGroupingRules.choose('rule3', { forceExperiment: 'rule1', forceExperimentGroup: 'treatment' }));
+      });
+
+      it('returns `forceExperimentGroup` if defined and `forceExperiment === experiment.name', () => {
+        assert.equal(experimentGroupingRules.choose('rule1', { forceExperiment: 'rule1', forceExperimentGroup: 'treatment' }), 'treatment');
+      });
+
       it('delegates to the experimentGroupingRule', () => {
         const subject = { env: 'development', uniqueUserId: 'user-id' };
 
@@ -49,6 +65,10 @@ define(function (require, exports, module) {
         assert.equal(experimentGroupingRules.choose('rule2', subject), 'treatment');
         assert.isTrue(rule2.choose.calledOnce);
         assert.isTrue(rule2.choose.calledWith(subject));
+
+        // rule3 is allowed even if rule2 is forced.
+        subject.forceExperiment = 'rule2';
+        assert.equal(experimentGroupingRules.choose('rule3', subject), 'control');
       });
     });
 

app/tests/spec/lib/experiments/grouping-rules/send-sms-install-link.js

@@ -25,13 +25,6 @@ define(function (require, exports, module) {
         assert.isFalse(experiment.choose({ uniqueUserId: 'user-id' }));
       });
 
-      describe('forceExperiment, forceExperimentGroup set', () => {
-        it('returns `treatment`', () => {
-          account.set('email', 'testuser@testuser.com');
-          assert.equal(experiment.choose({ account, forceExperiment: 'sendSms', forceExperimentGroup: 'treatment', uniqueUserId: 'user-id' }), 'treatment');
-        });
-      });
-
       describe('email forces `signinCodes`', () => {
         it('returns true', () => {
           account.set('email', 'testuser@softvision.com');

app/tests/spec/lib/experiments/grouping-rules/signup-password-confirm.js

@@ -31,41 +31,6 @@ define((require, exports, module) => {
         assert.isTrue(experiment.uniformChoice.calledOnce);
         assert.isTrue(experiment.uniformChoice.calledWith(['control', 'treatment'], 'user-id'));
       });
-
-      describe('forceExperiment set', () => {
-        describe('to name', () => {
-          it('returns group', () => {
-            assert.equal(experiment.choose({
-              experimentGroupingRules,
-              forceExperiment: 'signupPasswordConfirm',
-              forceExperimentGroup: 'treatment',
-              uniqueUserId: 'user-id'
-            }), 'treatment');
-
-            assert.equal(experiment.choose({
-              experimentGroupingRules,
-              forceExperiment: 'signupPasswordConfirm',
-              forceExperimentGroup: 'control',
-              uniqueUserId: 'user-id'
-            }), 'control');
-          });
-        });
-
-        describe('to other experiment', () => {
-          it('returns false', () => {
-            sinon.stub(experiment, 'uniformChoice').callsFake(() => true);
-            experimentGroupingRules.choose = () => 'disabledButtonState';
-
-            assert.isFalse(experiment.choose({
-              experimentGroupingRules,
-              forceExperiment: 'disabledButtonState',
-              forceExperimentGroup: 'treatment',
-              uniqueUserId: 'user-id'
-            }));
-            assert.isFalse(experiment.uniformChoice.called);
-          });
-        });
-      });
     });
   });
 });