Bug 1436701 - Handle resetValue / didResetValue correctly in Normany preference experiments r=Gijs
authorMike Cooper <mcooper@mozilla.com>
Thu, 08 Feb 2018 11:35:39 -0800
changeset 403158 79a09953294995a722db47bb93905f95fbb85731
parent 403157 716404c7d83afca689b23159777e851a7ed76578
child 403159 63ff291a2433f16ca847277564a011b68df5b9ac
push id33415
push userccoroiu@mozilla.com
push dateFri, 09 Feb 2018 21:58:41 +0000
treeherdermozilla-central@919570891f1c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1436701
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1436701 - Handle resetValue / didResetValue correctly in Normany preference experiments r=Gijs MozReview-Commit-ID: 1UfvmpgvaIx
browser/extensions/shield-recipe-client/lib/PreferenceExperiments.jsm
browser/extensions/shield-recipe-client/test/browser/browser_PreferenceExperiments.js
--- a/browser/extensions/shield-recipe-client/lib/PreferenceExperiments.jsm
+++ b/browser/extensions/shield-recipe-client/lib/PreferenceExperiments.jsm
@@ -182,17 +182,17 @@ this.PreferenceExperiments = {
     CleanupManager.addCleanupHandler(this.saveStartupPrefs.bind(this));
 
     for (const experiment of await this.getAllActive()) {
       // Check that the current value of the preference is still what we set it to
       if (getPref(UserPreferences, experiment.preferenceName, experiment.preferenceType) !== experiment.preferenceValue) {
         // if not, stop the experiment, and skip the remaining steps
         log.info(`Stopping experiment "${experiment.name}" because its value changed`);
         await this.stop(experiment.name, {
-          didResetValue: false,
+          resetValue: false,
           reason: "user-preference-changed-sideload",
         });
         continue;
       }
 
       // Notify Telemetry of experiments we're running, since they don't persist between restarts
       TelemetryEnvironment.setExperimentActive(
         experiment.name,
@@ -461,17 +461,17 @@ this.PreferenceExperiments = {
    * @param {String} [options.reason = "unknown"]
    *   Reason that the experiment is ending. Optional, defaults to
    *   "unknown".
    * @rejects {Error}
    *   If there is no stored experiment with the given name, or if the
    *   experiment has already expired.
    */
   async stop(experimentName, {resetValue = true, reason = "unknown"} = {}) {
-    log.debug(`PreferenceExperiments.stop(${experimentName})`);
+    log.debug(`PreferenceExperiments.stop(${experimentName}, {resetValue: ${resetValue}, reason: ${reason}})`);
     if (reason === "unknown") {
       log.warn(`experiment ${experimentName} ending for unknown reason`);
     }
 
     const store = await ensureStorage();
     if (!(experimentName in store.data)) {
       throw new Error(`Could not find a preference experiment named "${experimentName}"`);
     }
--- a/browser/extensions/shield-recipe-client/test/browser/browser_PreferenceExperiments.js
+++ b/browser/extensions/shield-recipe-client/test/browser/browser_PreferenceExperiments.js
@@ -789,28 +789,36 @@ decorate_task(
   },
 );
 
 // Experiments should end if the preference has been changed when init() is called
 decorate_task(
   withMockExperiments,
   withMockPreferences,
   withStub(PreferenceExperiments, "stop"),
-  withStub(TelemetryEvents, "sendEvent"),
-  async function testInitChanges(experiments, mockPreferences, stopStub, sendEventStub) {
+  async function testInitChanges(experiments, mockPreferences, stopStub) {
     mockPreferences.set("fake.preference", "experiment value", "default");
     experiments.test = experimentFactory({
       name: "test",
       preferenceName: "fake.preference",
       preferenceValue: "experiment value",
     });
     mockPreferences.set("fake.preference", "changed value");
     await PreferenceExperiments.init();
-    ok(stopStub.calledWith("test"), "Experiment is stopped because value changed");
+
     is(Preferences.get("fake.preference"), "changed value", "Preference value was not changed");
+
+    Assert.deepEqual(
+      stopStub.getCall(0).args,
+      ["test", {
+        resetValue: false,
+        reason: "user-preference-changed-sideload",
+      }],
+      "Experiment is stopped correctly because value changed"
+    );
   },
 );
 
 // init should register an observer for experiments
 decorate_task(
   withMockExperiments,
   withMockPreferences,
   withStub(PreferenceExperiments, "startObserver"),
@@ -1001,8 +1009,37 @@ decorate_task(
     await PreferenceExperiments.stop("test");
     is(
       sendEventStub.getCall(0).args[3].reason,
       "unknown",
       "PreferenceExperiments.stop() should use unknown as the default reason",
     );
   }
 );
+
+// stop should pass along the value for resetValue to Telemetry Events as didResetValue
+decorate_task(
+  withMockExperiments,
+  withMockPreferences,
+  withStub(PreferenceExperiments, "stopObserver"),
+  withStub(TelemetryEvents, "sendEvent"),
+  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});
+    is(sendEventStub.callCount, 1);
+    is(
+      sendEventStub.getCall(0).args[3].didResetValue,
+      "true",
+      "PreferenceExperiments.stop() should pass true values of resetValue as didResetValue",
+    );
+
+    mockPreferences.set("fake.preference2", "default value", "default");
+    experiments.test2 = experimentFactory({ name: "test2", preferenceName: "fake.preference2" });
+    await PreferenceExperiments.stop("test2", {resetValue: false});
+    is(sendEventStub.callCount, 2);
+    is(
+      sendEventStub.getCall(1).args[3].didResetValue,
+      "false",
+      "PreferenceExperiments.stop() should pass false values of resetValue as didResetValue",
+    );
+  }
+);