Bug 1447499 - Refactor test helper PreferenceExperiments.withMockExperiment to work like AddonStudies.withStudies. r=Gijs

Differential Revision: https://phabricator.services.mozilla.com/D5609

--HG--
extra : moz-landing-system : lando

toolkit/components/normandy/content/AboutPages.jsm

@@ -61,7 +61,7 @@ var AboutPages = {
     this.aboutStudies.registerParentListeners();
 
     CleanupManager.addCleanupHandler(() => {
-      // Stop loading processs scripts and notify existing scripts to clean up.
+      // Stop loading process scripts and notify existing scripts to clean up.
       Services.ppmm.broadcastAsyncMessage("Shield:ShuttingDown");
       Services.mm.broadcastAsyncMessage("Shield:ShuttingDown");
 

toolkit/components/normandy/lib/PreferenceExperiments.jsm

@@ -90,14 +90,14 @@ const PreferenceBranchType = {
 /**
  * Asynchronously load the JSON file that stores experiment status in the profile.
  */
-let storePromise;
+let gStorePromise;
 function ensureStorage() {
-  if (storePromise === undefined) {
+  if (gStorePromise === undefined) {
     const path = OS.Path.join(OS.Constants.Path.profileDir, EXPERIMENT_FILE);
     const storage = new JSONFile({path});
-    storePromise = storage.load().then(() => storage);
+    gStorePromise = storage.load().then(() => storage);
   }
-  return storePromise;
+  return gStorePromise;
 }
 
 const log = LogManager.getLogger("preference-experiments");
@@ -248,23 +248,30 @@ var PreferenceExperiments = {
    * Test wrapper that temporarily replaces the stored experiment data with fake
    * data for testing.
    */
-  withMockExperiments(testFunction) {
+  withMockExperiments(mockExperiments = []) {
-    return async function inner(...args) {
+    return function wrapper(testFunction) {
-      const oldPromise = storePromise;
+      return async function wrappedTestFunction(...args) {
-      const mockExperiments = {};
+        const data = {};
-      storePromise = Promise.resolve({
+
-        data: mockExperiments,
+        for (const exp of mockExperiments) {
-        saveSoon() { },
+          data[exp.name] = exp;
-      });
+        }
-      const oldObservers = experimentObservers;
+
-      experimentObservers = new Map();
+        const oldPromise = gStorePromise;
-      try {
+        gStorePromise = Promise.resolve({
-        await testFunction(...args, mockExperiments);
+          data,
-      } finally {
+          saveSoon() { },
-        storePromise = oldPromise;
+        });
-        PreferenceExperiments.stopAllObservers();
+        const oldObservers = experimentObservers;
-        experimentObservers = oldObservers;
+        experimentObservers = new Map();
-      }
+        try {
+          await testFunction(...args, mockExperiments);
+        } finally {
+          gStorePromise = oldPromise;
+          PreferenceExperiments.stopAllObservers();
+          experimentObservers = oldObservers;
+        }
+      };
     };
   },
 

toolkit/components/normandy/test/browser/browser_NormandyDriver.js

@@ -173,7 +173,7 @@ decorate_task(
 decorate_task(
   withSandboxManager(Assert),
   withMockPreferences,
-  PreferenceExperiments.withMockExperiments,
+  PreferenceExperiments.withMockExperiments(),
   async function testPreferenceStudies(sandboxManager) {
     const driver = new NormandyDriver(sandboxManager);
     sandboxManager.cloneIntoGlobal("driver", driver, {cloneFunctions: true});

toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js

@@ -28,9 +28,8 @@ function experimentFactory(attrs) {
 
 // clearAllExperimentStorage
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([experimentFactory({ name: "test" })]),
   async function(experiments) {
-    experiments.test = experimentFactory({name: "test"});
     ok(await PreferenceExperiments.has("test"), "Mock experiment is detected.");
     await PreferenceExperiments.clearAllExperimentStorage();
     ok(
@@ -42,10 +41,9 @@ decorate_task(
 
 // start should throw if an experiment with the given name already exists
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([experimentFactory({ name: "test" })]),
   withSendEventStub,
   async function(experiments, sendEventStub) {
-    experiments.test = experimentFactory({name: "test"});
     await Assert.rejects(
       PreferenceExperiments.start({
         name: "test",
@@ -69,10 +67,9 @@ decorate_task(
 
 // start should throw if an experiment for the given preference is active
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([experimentFactory({ name: "test", preferenceName: "fake.preference" })]),
   withSendEventStub,
   async function(experiments, sendEventStub) {
-    experiments.test = experimentFactory({name: "test", preferenceName: "fake.preference"});
     await Assert.rejects(
       PreferenceExperiments.start({
         name: "different",
@@ -96,7 +93,7 @@ decorate_task(
 
 // start should throw if an invalid preferenceBranchType is given
 decorate_task(
-  withMockExperiments,
+  withMockExperiments(),
   withSendEventStub,
   async function(experiments, sendEventStub) {
     await Assert.rejects(
@@ -123,7 +120,7 @@ decorate_task(
 // start should save experiment data, modify the preference, and register a
 // watcher.
 decorate_task(
-  withMockExperiments,
+  withMockExperiments(),
   withMockPreferences,
   withStub(PreferenceExperiments, "startObserver"),
   withSendEventStub,
@@ -139,7 +136,7 @@ decorate_task(
       preferenceBranchType: "default",
       preferenceType: "string",
     });
-    ok("test" in experiments, "start saved the experiment");
+    ok(await PreferenceExperiments.get("test"), "start saved the experiment");
     ok(
       startObserverStub.calledWith("test", "fake.preference", "string", "newvalue"),
       "start registered an observer",
@@ -156,7 +153,8 @@ decorate_task(
       preferenceBranchType: "default",
     };
     const experiment = {};
-    Object.keys(expectedExperiment).forEach(key => experiment[key] = experiments.test[key]);
+    const actualExperiment = await PreferenceExperiments.get("test");
+    Object.keys(expectedExperiment).forEach(key => experiment[key] = actualExperiment[key]);
     Assert.deepEqual(experiment, expectedExperiment, "start saved the experiment");
 
     is(
@@ -179,7 +177,7 @@ decorate_task(
 
 // start should modify the user preference for the user branch type
 decorate_task(
-  withMockExperiments,
+  withMockExperiments(),
   withMockPreferences,
   withStub(PreferenceExperiments, "startObserver"),
   async function(experiments, mockPreferences, startObserver) {
@@ -211,7 +209,8 @@ decorate_task(
     };
 
     const experiment = {};
-    Object.keys(expectedExperiment).forEach(key => experiment[key] = experiments.test[key]);
+    const actualExperiment = await PreferenceExperiments.get("test");
+    Object.keys(expectedExperiment).forEach(key => experiment[key] = actualExperiment[key]);
     Assert.deepEqual(experiment, expectedExperiment, "start saved the experiment");
 
     Assert.notEqual(
@@ -254,7 +253,7 @@ decorate_task(
 // startObserver should throw if an observer for the experiment is already
 // active.
 decorate_task(
-  withMockExperiments,
+  withMockExperiments(),
   async function() {
     PreferenceExperiments.startObserver("test", "fake.preference", "string", "newvalue");
     Assert.throws(
@@ -269,7 +268,7 @@ decorate_task(
 // startObserver should register an observer that calls stop when a preference
 // changes from its experimental value.
 decorate_task(
-  withMockExperiments,
+  withMockExperiments(),
   withMockPreferences,
   async function(mockExperiments, mockPreferences) {
     const tests = [
@@ -300,7 +299,7 @@ decorate_task(
 );
 
 decorate_task(
-  withMockExperiments,
+  withMockExperiments(),
   async function testHasObserver() {
     PreferenceExperiments.startObserver("test", "fake.preference", "string", "experimentValue");
 
@@ -316,7 +315,7 @@ decorate_task(
 
 // stopObserver should throw if there is no observer active for it to stop.
 decorate_task(
-  withMockExperiments,
+  withMockExperiments(),
   async function() {
     Assert.throws(
       () => PreferenceExperiments.stopObserver("neveractive", "another.fake", "othervalue"),
@@ -328,7 +327,7 @@ decorate_task(
 
 // stopObserver should cancel an active observer.
 decorate_task(
-  withMockExperiments,
+  withMockExperiments(),
   withMockPreferences,
   async function(mockExperiments, mockPreferences) {
     const stop = sinon.stub(PreferenceExperiments, "stop");
@@ -357,7 +356,7 @@ decorate_task(
 
 // stopAllObservers
 decorate_task(
-  withMockExperiments,
+  withMockExperiments(),
   withMockPreferences,
   async function(mockExperiments, mockPreferences) {
     const stop = sinon.stub(PreferenceExperiments, "stop");
@@ -390,7 +389,7 @@ decorate_task(
 
 // markLastSeen should throw if it can't find a matching experiment
 decorate_task(
-  withMockExperiments,
+  withMockExperiments(),
   async function() {
     await Assert.rejects(
       PreferenceExperiments.markLastSeen("neveractive"),
@@ -401,14 +400,13 @@ decorate_task(
 );
 
 // markLastSeen should update the lastSeen date
+const oldDate = new Date(1988, 10, 1).toJSON();
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([experimentFactory({ name: "test", lastSeen: oldDate })]),
-  async function(experiments) {
+  async function([experiment]) {
-    const oldDate = new Date(1988, 10, 1).toJSON();
-    experiments.test = experimentFactory({name: "test", lastSeen: oldDate});
     await PreferenceExperiments.markLastSeen("test");
     Assert.notEqual(
-      experiments.test.lastSeen,
+      experiment.lastSeen,
       oldDate,
       "markLastSeen updated the experiment lastSeen date",
     );
@@ -417,7 +415,7 @@ decorate_task(
 
 // stop should throw if an experiment with the given name doesn't exist
 decorate_task(
-  withMockExperiments,
+  withMockExperiments(),
   withSendEventStub,
   async function(experiments, sendEventStub) {
     await Assert.rejects(
@@ -436,10 +434,9 @@ decorate_task(
 
 // stop should throw if the experiment is already expired
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([experimentFactory({ name: "test", expired: true })]),
   withSendEventStub,
   async function(experiments, sendEventStub) {
-    experiments.test = experimentFactory({name: "test", expired: true});
     await Assert.rejects(
       PreferenceExperiments.stop("test"),
       /already expired/,
@@ -457,7 +454,18 @@ decorate_task(
 // stop should mark the experiment as expired, stop its observer, and revert the
 // preference value.
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([
+    experimentFactory({
+      name: "test",
+      expired: false,
+      branch: "fakebranch",
+      preferenceName: "fake.preference",
+      preferenceValue: "experimentvalue",
+      preferenceType: "string",
+      previousPreferenceValue: "oldvalue",
+      preferenceBranchType: "default",
+    }),
+  ]),
   withMockPreferences,
   withSpy(PreferenceExperiments, "stopObserver"),
   withSendEventStub,
@@ -468,21 +476,12 @@ decorate_task(
 
     mockPreferences.set(`${startupPrefs}.fake.preference`, "experimentvalue", "user");
     mockPreferences.set("fake.preference", "experimentvalue", "default");
-    experiments.test = experimentFactory({
-      name: "test",
-      expired: false,
-      branch: "fakebranch",
-      preferenceName: "fake.preference",
-      preferenceValue: "experimentvalue",
-      preferenceType: "string",
-      previousPreferenceValue: "oldvalue",
-      preferenceBranchType: "default",
-    });
     PreferenceExperiments.startObserver("test", "fake.preference", "string", "experimentvalue");
 
     await PreferenceExperiments.stop("test", {reason: "test-reason"});
     ok(stopObserverSpy.calledWith("test"), "stop removed an observer");
-    is(experiments.test.expired, true, "stop marked the experiment as expired");
+    const experiment = await PreferenceExperiments.get("test");
+    is(experiment.expired, true, "stop marked the experiment as expired");
     is(
       DefaultPreferences.get("fake.preference"),
       "oldvalue",
@@ -495,7 +494,7 @@ decorate_task(
 
     Assert.deepEqual(
       sendEventStub.args,
-      [["unenroll", "preference_study", experiments.test.name, {
+      [["unenroll", "preference_study", "test", {
         didResetValue: "true",
         reason: "test-reason",
         branch: "fakebranch",
@@ -509,7 +508,15 @@ decorate_task(
 
 // stop should also support user pref experiments
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([experimentFactory({
+    name: "test",
+    expired: false,
+    preferenceName: "fake.preference",
+    preferenceValue: "experimentvalue",
+    preferenceType: "string",
+    previousPreferenceValue: "oldvalue",
+    preferenceBranchType: "user",
+  })]),
   withMockPreferences,
   withStub(PreferenceExperiments, "stopObserver"),
   withStub(PreferenceExperiments, "hasObserver"),
@@ -517,20 +524,12 @@ decorate_task(
     hasObserver.returns(true);
 
     mockPreferences.set("fake.preference", "experimentvalue", "user");
-    experiments.test = experimentFactory({
-      name: "test",
-      expired: false,
-      preferenceName: "fake.preference",
-      preferenceValue: "experimentvalue",
-      preferenceType: "string",
-      previousPreferenceValue: "oldvalue",
-      preferenceBranchType: "user",
-    });
     PreferenceExperiments.startObserver("test", "fake.preference", "string", "experimentvalue");
 
     await PreferenceExperiments.stop("test");
     ok(stopObserver.calledWith("test"), "stop removed an observer");
-    is(experiments.test.expired, true, "stop marked the experiment as expired");
+    const experiment = await PreferenceExperiments.get("test");
+    is(experiment.expired, true, "stop marked the experiment as expired");
     is(
       Preferences.get("fake.preference"),
       "oldvalue",
@@ -543,20 +542,19 @@ decorate_task(
 
 // stop should remove a preference that had no value prior to an experiment for user prefs
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([experimentFactory({
+    name: "test",
+    expired: false,
+    preferenceName: "fake.preference",
+    preferenceValue: "experimentvalue",
+    preferenceType: "string",
+    previousPreferenceValue: null,
+    preferenceBranchType: "user",
+  })]),
   withMockPreferences,
   async function(experiments, mockPreferences) {
     const stopObserver = sinon.stub(PreferenceExperiments, "stopObserver");
     mockPreferences.set("fake.preference", "experimentvalue", "user");
-    experiments.test = experimentFactory({
-      name: "test",
-      expired: false,
-      preferenceName: "fake.preference",
-      preferenceValue: "experimentvalue",
-      preferenceType: "string",
-      previousPreferenceValue: null,
-      preferenceBranchType: "user",
-    });
 
     await PreferenceExperiments.stop("test");
     ok(
@@ -570,22 +568,21 @@ decorate_task(
 
 // stop should not modify a preference if resetValue is false
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([experimentFactory({
+    name: "test",
+    expired: false,
+    branch: "fakebranch",
+    preferenceName: "fake.preference",
+    preferenceValue: "experimentvalue",
+    preferenceType: "string",
+    previousPreferenceValue: "oldvalue",
+    preferenceBranchType: "default",
+  })]),
   withMockPreferences,
   withStub(PreferenceExperiments, "stopObserver"),
   withSendEventStub,
   async function testStopReset(experiments, mockPreferences, stopObserverStub, sendEventStub) {
     mockPreferences.set("fake.preference", "customvalue", "default");
-    experiments.test = experimentFactory({
-      name: "test",
-      expired: false,
-      branch: "fakebranch",
-      preferenceName: "fake.preference",
-      preferenceValue: "experimentvalue",
-      preferenceType: "string",
-      previousPreferenceValue: "oldvalue",
-      preferenceBranchType: "default",
-    });
 
     await PreferenceExperiments.stop("test", {reason: "test-reason", resetValue: false});
     is(
@@ -595,7 +592,7 @@ decorate_task(
     );
     Assert.deepEqual(
       sendEventStub.args,
-      [["unenroll", "preference_study", experiments.test.name, {
+      [["unenroll", "preference_study", "test", {
         didResetValue: "false",
         reason: "test-reason",
         branch: "fakebranch",
@@ -607,7 +604,7 @@ decorate_task(
 
 // get should throw if no experiment exists with the given name
 decorate_task(
-  withMockExperiments,
+  withMockExperiments(),
   async function() {
     await Assert.rejects(
       PreferenceExperiments.get("neverexisted"),
@@ -619,29 +616,25 @@ decorate_task(
 
 // get
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([experimentFactory({ name: "test" })]),
   async function(experiments) {
-    const experiment = experimentFactory({name: "test"});
+    const experiment = await PreferenceExperiments.get("test");
-    experiments.test = experiment;
+    is(experiment.name, "test", "get fetches the correct experiment");
-
-    const fetchedExperiment = await PreferenceExperiments.get("test");
-    Assert.deepEqual(fetchedExperiment, experiment, "get fetches the correct experiment");
 
     // Modifying the fetched experiment must not edit the data source.
-    fetchedExperiment.name = "othername";
+    experiment.name = "othername";
-    is(experiments.test.name, "test", "get returns a copy of the experiment");
+    const refetched = await PreferenceExperiments.get("test");
+    is(refetched.name, "test", "get returns a copy of the experiment");
   }
 );
 
 // get all
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([
-  async function testGetAll(experiments) {
+    experimentFactory({ name: "experiment1", disabled: false }),
-    const experiment1 = experimentFactory({name: "experiment1"});
+    experimentFactory({ name: "experiment2", disabled: true }),
-    const experiment2 = experimentFactory({name: "experiment2", disabled: true});
+  ]),
-    experiments.experiment1 = experiment1;
+  async function testGetAll([experiment1, experiment2]) {
-    experiments.experiment2 = experiment2;
-
     const fetchedExperiments = await PreferenceExperiments.getAll();
     is(fetchedExperiments.length, 2, "getAll returns a list of all stored experiments");
     Assert.deepEqual(
@@ -663,28 +656,29 @@ decorate_task(
 
 // get all active
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([
-  withMockPreferences,
+    experimentFactory({
-  async function testGetAllActive(experiments) {
-    experiments.active = experimentFactory({
       name: "active",
       expired: false,
-    });
+    }),
-    experiments.inactive = experimentFactory({
+    experimentFactory({
       name: "inactive",
       expired: true,
-    });
+    }),
-
+  ]),
-    const activeExperiments = await PreferenceExperiments.getAllActive();
+  withMockPreferences,
+  async function testGetAllActive([activeExperiment, inactiveExperiment]) {
+    let allActiveExperiments = await PreferenceExperiments.getAllActive();
     Assert.deepEqual(
-      activeExperiments,
+      allActiveExperiments,
-      [experiments.active],
+      [activeExperiment],
       "getAllActive only returns active experiments",
     );
 
-    activeExperiments[0].name = "newfakename";
+    allActiveExperiments[0].name = "newfakename";
+    allActiveExperiments = await PreferenceExperiments.getAllActive();
     Assert.notEqual(
-      experiments.active.name,
+      allActiveExperiments,
       "newfakename",
       "getAllActive returns copies of stored experiments",
     );
@@ -693,9 +687,8 @@ decorate_task(
 
 // has
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([experimentFactory({ name: "test" })]),
   async function(experiments) {
-    experiments.test = experimentFactory({name: "test"});
     ok(await PreferenceExperiments.has("test"), "has returned true for a stored experiment");
     ok(!(await PreferenceExperiments.has("missing")), "has returned false for a missing experiment");
   }
@@ -703,25 +696,22 @@ decorate_task(
 
 // init should register telemetry experiments
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([experimentFactory({
+    name: "test",
+    branch: "branch",
+    preferenceName: "fake.pref",
+    preferenceValue: "experiment value",
+    expired: false,
+    preferenceBranchType: "default",
+  })]),
   withMockPreferences,
   withStub(TelemetryEnvironment, "setExperimentActive"),
   withStub(PreferenceExperiments, "startObserver"),
   async function testInit(experiments, mockPreferences, setActiveStub, startObserverStub) {
     mockPreferences.set("fake.pref", "experiment value");
-
-    experiments.test = experimentFactory({
-      name: "test",
-      branch: "branch",
-      preferenceName: "fake.pref",
-      preferenceValue: "experiment value",
-      expired: false,
-      preferenceBranchType: "default",
-    });
-
     await PreferenceExperiments.init();
     ok(
-      setActiveStub.calledWith("test", "branch", {type: "normandy-exp"}),
+      setActiveStub.calledWith("test", "branch", { type: "normandy-exp" }),
       "Experiment is registered by init",
     );
   },
@@ -729,24 +719,21 @@ decorate_task(
 
 // init should use the provided experiment type
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([experimentFactory({
+    name: "test",
+    branch: "branch",
+    preferenceName: "fake.pref",
+    preferenceValue: "experiment value",
+    experimentType: "pref-test",
+  })]),
   withMockPreferences,
   withStub(TelemetryEnvironment, "setExperimentActive"),
   withStub(PreferenceExperiments, "startObserver"),
   async function testInit(experiments, mockPreferences, setActiveStub, startObserverStub) {
     mockPreferences.set("fake.pref", "experiment value");
-
-    experiments.test = experimentFactory({
-      name: "test",
-      branch: "branch",
-      preferenceName: "fake.pref",
-      preferenceValue: "experiment value",
-      experimentType: "pref-test",
-    });
-
     await PreferenceExperiments.init();
     ok(
-      setActiveStub.calledWith("test", "branch", {type: "normandy-pref-test"}),
+      setActiveStub.calledWith("test", "branch", { type: "normandy-pref-test" }),
       "init should use the provided experiment type",
     );
   },
@@ -754,7 +741,7 @@ decorate_task(
 
 // starting and stopping experiments should register in telemetry
 decorate_task(
-  withMockExperiments,
+  withMockExperiments(),
   withStub(TelemetryEnvironment, "setExperimentActive"),
   withStub(TelemetryEnvironment, "setExperimentInactive"),
   withSendEventStub,
@@ -770,10 +757,10 @@ decorate_task(
 
     Assert.deepEqual(
       setActiveStub.getCall(0).args,
-      ["test", "branch", {type: "normandy-exp"}],
+      ["test", "branch", { type: "normandy-exp" }],
       "Experiment is registered by start()",
     );
-    await PreferenceExperiments.stop("test", {reason: "test-reason"});
+    await PreferenceExperiments.stop("test", { reason: "test-reason" });
     Assert.deepEqual(setInactiveStub.args, [["test"]], "Experiment is unregistered by stop()");
 
     Assert.deepEqual(
@@ -796,7 +783,7 @@ decorate_task(
 
 // starting experiments should use the provided experiment type
 decorate_task(
-  withMockExperiments,
+  withMockExperiments(),
   withStub(TelemetryEnvironment, "setExperimentActive"),
   withStub(TelemetryEnvironment, "setExperimentInactive"),
   withSendEventStub,
@@ -813,7 +800,7 @@ decorate_task(
 
     Assert.deepEqual(
       setActiveStub.getCall(0).args,
-      ["test", "branch", {type: "normandy-pref-test"}],
+      ["test", "branch", { type: "normandy-pref-test" }],
       "start() should register the experiment with the provided type",
     );
 
@@ -834,10 +821,9 @@ decorate_task(
 
 // Experiments shouldn't be recorded by init() in telemetry if they are expired
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([experimentFactory({ name: "expired", branch: "branch", expired: true })]),
   withStub(TelemetryEnvironment, "setExperimentActive"),
   async function testInitTelemetryExpired(experiments, setActiveStub) {
-    experiments.experiment1 = experimentFactory({name: "expired", branch: "branch", expired: true});
     await PreferenceExperiments.init();
     ok(!setActiveStub.called, "Expired experiment is not registered by init");
   },
@@ -845,17 +831,16 @@ decorate_task(
 
 // Experiments should end if the preference has been changed when init() is called
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([experimentFactory({
+    name: "test",
+    preferenceName: "fake.preference",
+    preferenceValue: "experiment value",
+  })]),
   withMockPreferences,
   withStub(PreferenceExperiments, "stop"),
   async function testInitChanges(experiments, mockPreferences, stopStub) {
     mockPreferences.set("fake.preference", "experiment value", "default");
-    experiments.test = experimentFactory({
+    mockPreferences.set("fake.preference", "changed value", "user");
-      name: "test",
-      preferenceName: "fake.preference",
-      preferenceValue: "experiment value",
-    });
-    mockPreferences.set("fake.preference", "changed value");
     await PreferenceExperiments.init();
 
     is(Preferences.get("fake.preference"), "changed value", "Preference value was not changed");
@@ -873,7 +858,11 @@ decorate_task(
 
 // init should register an observer for experiments
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([experimentFactory({
+    name: "test",
+    preferenceName: "fake.preference",
+    preferenceValue: "experiment value",
+  })]),
   withMockPreferences,
   withStub(PreferenceExperiments, "startObserver"),
   withStub(PreferenceExperiments, "stop"),
@@ -882,11 +871,6 @@ decorate_task(
     stop.throws("Stop should not be called");
     mockPreferences.set("fake.preference", "experiment value", "default");
     is(Preferences.get("fake.preference"), "experiment value", "pref shouldn't have a user value");
-    experiments.test = experimentFactory({
-      name: "test",
-      preferenceName: "fake.preference",
-      preferenceValue: "experiment value",
-    });
     await PreferenceExperiments.init();
 
     ok(startObserver.calledOnce, "init should register an observer");
@@ -900,21 +884,24 @@ decorate_task(
 
 // saveStartupPrefs
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([
+    experimentFactory({
+      name: "char",
+      preferenceName: `fake.char`,
+      preferenceValue: "string",
+    }),
+    experimentFactory({
+      name: "int",
+      preferenceName: `fake.int`,
+      preferenceValue: 2,
+    }),
+    experimentFactory({
+      name: "bool",
+      preferenceName: `fake.bool`,
+      preferenceValue: true,
+    }),
+  ]),
   async function testSaveStartupPrefs(experiments) {
-    const experimentPrefs = {
-      char: "string",
-      int: 2,
-      bool: true,
-    };
-
-    for (const [key, value] of Object.entries(experimentPrefs)) {
-      experiments[key] = experimentFactory({
-        preferenceName: `fake.${key}`,
-        preferenceValue: value,
-      });
-    }
-
     Services.prefs.deleteBranch(startupPrefs);
     Services.prefs.setBoolPref(`${startupPrefs}.fake.old`, true);
     await PreferenceExperiments.saveStartupPrefs();
@@ -942,13 +929,12 @@ decorate_task(
 
 // saveStartupPrefs errors for invalid pref type
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([experimentFactory({
+    name: "test",
+    preferenceName: "fake.invalidValue",
+    preferenceValue: new Date(),
+  })]),
   async function testSaveStartupPrefsError(experiments) {
-    experiments.test = experimentFactory({
-      preferenceName: "fake.invalidValue",
-      preferenceValue: new Date(),
-    });
-
     await Assert.rejects(
       PreferenceExperiments.saveStartupPrefs(),
       /invalid preference type/i,
@@ -959,19 +945,21 @@ decorate_task(
 
 // saveStartupPrefs should not store values for user-branch recipes
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([
-  async function testSaveStartupPrefsUserBranch(experiments) {
+    experimentFactory({
-    experiments.defaultBranchRecipe = experimentFactory({
+      name: "defaultBranchRecipe",
       preferenceName: "fake.default",
       preferenceValue: "experiment value",
       branch: "default",
-    });
+    }),
-    experiments.userBranchRecipe = experimentFactory({
+    experimentFactory({
+      name: "userBranchRecipe",
       preferenceName: "fake.user",
       preferenceValue: "experiment value",
       branch: "user",
-    });
+    }),
-
+  ]),
+  async function testSaveStartupPrefsUserBranch(experiments) {
     await PreferenceExperiments.saveStartupPrefs();
 
     is(
@@ -991,7 +979,7 @@ decorate_task(
 
 // test that default branch prefs restore to the right value if the default pref changes
 decorate_task(
-  withMockExperiments,
+  withMockExperiments(),
   withMockPreferences,
   withStub(PreferenceExperiments, "startObserver"),
   withStub(PreferenceExperiments, "stopObserver"),
@@ -1039,7 +1027,7 @@ decorate_task(
 
 // test that default branch prefs restore to the right value if the preference is removed
 decorate_task(
-  withMockExperiments,
+  withMockExperiments(),
   withMockPreferences,
   withStub(PreferenceExperiments, "startObserver"),
   withStub(PreferenceExperiments, "stopObserver"),
@@ -1086,13 +1074,12 @@ decorate_task(
 
 // stop should pass "unknown" to telemetry event for `reason` if none is specified
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([experimentFactory({ name: "test", preferenceName: "fake.preference" })]),
   withMockPreferences,
   withStub(PreferenceExperiments, "stopObserver"),
   withSendEventStub,
   async function testStopUnknownReason(experiments, mockPreferences, stopObserverStub, sendEventStub) {
     mockPreferences.set("fake.preference", "default value", "default");
-    experiments.test = experimentFactory({ name: "test", preferenceName: "fake.preference" });
     await PreferenceExperiments.stop("test");
     is(
       sendEventStub.getCall(0).args[3].reason,
@@ -1104,14 +1091,16 @@ decorate_task(
 
 // stop should pass along the value for resetValue to Telemetry Events as didResetValue
 decorate_task(
-  withMockExperiments,
+  withMockExperiments([
+    experimentFactory({ name: "test1", preferenceName: "fake.preference1" }),
+    experimentFactory({ name: "test2", preferenceName: "fake.preference2" }),
+  ]),
   withMockPreferences,
   withStub(PreferenceExperiments, "stopObserver"),
   withSendEventStub,
   async function testStopResetValue(experiments, mockPreferences, stopObserverStub, sendEventStub) {
     mockPreferences.set("fake.preference1", "default value", "default");
-    experiments.test1 = experimentFactory({ name: "test1", preferenceName: "fake.preference1" });
+    await PreferenceExperiments.stop("test1", { resetValue: true });
-    await PreferenceExperiments.stop("test1", {resetValue: true});
     is(sendEventStub.callCount, 1);
     is(
       sendEventStub.getCall(0).args[3].didResetValue,
@@ -1120,8 +1109,7 @@ decorate_task(
     );
 
     mockPreferences.set("fake.preference2", "default value", "default");
-    experiments.test2 = experimentFactory({ name: "test2", preferenceName: "fake.preference2" });
+    await PreferenceExperiments.stop("test2", { resetValue: false });
-    await PreferenceExperiments.stop("test2", {resetValue: false});
     is(sendEventStub.callCount, 2);
     is(
       sendEventStub.getCall(1).args[3].didResetValue,
@@ -1136,20 +1124,19 @@ decorate_task(
 decorate_task(
   withMockPreferences,
   withSendEventStub,
-  withMockExperiments,
+  withMockExperiments([experimentFactory({
+    name: "test",
+    expired: false,
+    branch: "fakebranch",
+    preferenceName: "fake.preference",
+    preferenceValue: "experimentvalue",
+    preferenceType: "string",
+    previousPreferenceValue: "oldvalue",
+    preferenceBranchType: "default",
+  })]),
   async function testPrefChangeEventTelemetry(mockPreferences, sendEventStub, mockExperiments) {
     is(Preferences.get("fake.preference"), null, "preference should start unset");
     mockPreferences.set("fake.preference", "oldvalue", "default");
-    mockExperiments.test = experimentFactory({
-      name: "test",
-      expired: false,
-      branch: "fakebranch",
-      preferenceName: "fake.preference",
-      preferenceValue: "experimentvalue",
-      preferenceType: "string",
-      previousPreferenceValue: "oldvalue",
-      preferenceBranchType: "default",
-    });
     PreferenceExperiments.startObserver("test", "fake.preference", "string", "experimentvalue");
 
     // setting the preference on the user branch should trigger the observer to stop the experiment