User cannot belong to all experiments in an experiment group (#11945)

microsoft · May 21, 2020 · 82e1fdd · 82e1fdd
1 parent a91d737
commit 82e1fdd
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 2 deletions.
diff --git a/news/2 Fixes/11943.md b/news/2 Fixes/11943.md
@@ -0,0 +1 @@
+Ensure user cannot belong to all experiments in an experiment group.
diff --git a/src/client/common/experiments.ts b/src/client/common/experiments.ts
@@ -150,6 +150,8 @@ export class ExperimentsManager implements IExperimentsManager {
     public populateUserExperiments(): void {
         this.cleanUpExperimentsOptList();
         if (Array.isArray(this.experimentStorage.value)) {
+            const remainingExpriments: ABExperiments = [];
+            // First process experiments in order of user preference (if they have opted out or opted in).
             for (const experiment of this.experimentStorage.value) {
                 // User cannot belong to NotebookExperiment if they are not using Insiders.
                 if (
@@ -176,13 +178,19 @@ export class ExperimentsManager implements IExperimentsManager {
                             expNameOptedInto: experiment.name
                         });
                         this.userExperiments.push(experiment);
-                    } else if (this.isUserInRange(experiment.min, experiment.max, experiment.salt)) {
-                        this.userExperiments.push(experiment);
+                    } else {
+                        remainingExpriments.push(experiment);
                     }
                 } catch (ex) {
                     traceError(`Failed to populate experiment list for experiment '${experiment.name}'`, ex);
                 }
             }
+
+            // Add users (based on algorithm) to experiments they haven't already opted out of or opted into.
+            remainingExpriments
+                .filter((experiment) => this.isUserInRange(experiment.min, experiment.max, experiment.salt))
+                .filter((experiment) => !this.userExperiments.some((existing) => existing.salt === experiment.salt))
+                .forEach((experiment) => this.userExperiments.push(experiment));
         }
     }
 

diff --git a/src/test/common/experiments.unit.test.ts b/src/test/common/experiments.unit.test.ts
@@ -841,6 +841,17 @@ suite('A/B experiments', () => {
             experimentsOptedInto: ['experiment1'],
             expectedResult: [{ name: 'experiment1', salt: 'salt', min: 79, max: 94 }]
         },
+        {
+            testName:
+                'User experiments list contains the experiment user has opened into and not the control experiment even if user is in the control experiment range',
+            experimentStorageValue: [
+                { name: 'control', salt: 'salt', min: 0, max: 100 },
+                { name: 'experiment', salt: 'salt', min: 0, max: 0 }
+            ],
+            hash: 8187,
+            experimentsOptedInto: ['experiment'],
+            expectedResult: [{ name: 'experiment', salt: 'salt', min: 0, max: 0 }]
+        },
         {
             testName:
                 'User experiments list does not contain the experiment if user has both opted in and out of an experiment',