Bug 1436701 - Handle resetValue / didResetValue correctly in Normany preference experiments r?Gijs
MozReview-Commit-ID: 1UfvmpgvaIx
--- 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",
+ );
+ }
+);