Bug 1553198: PreferenceExperiments.start takes an actionName r=mythmon a=jcristau
authorEthan Glasser-Camp <ethan@betacantrips.com>
Thu, 23 May 2019 18:23:16 +0000
changeset 536533 bc0971ecb6e6d21b434ba82afcfa1459cb549856
parent 536532 49ab063b752d2698b2ca9f7416c16b9b2c83e85f
child 536534 2841c37353a5c50fdecf0284007308409e0c3474
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmythmon, jcristau
bugs1553198
milestone68.0
Bug 1553198: PreferenceExperiments.start takes an actionName r=mythmon a=jcristau Differential Revision: https://phabricator.services.mozilla.com/D32339
browser/components/tests/browser/browser_urlbar_matchBuckets_migration60.js
toolkit/components/normandy/actions/PreferenceExperimentAction.jsm
toolkit/components/normandy/lib/PreferenceExperiments.jsm
toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js
toolkit/components/normandy/test/browser/browser_actions_PreferenceExperimentAction.js
--- a/browser/components/tests/browser/browser_urlbar_matchBuckets_migration60.js
+++ b/browser/components/tests/browser/browser_urlbar_matchBuckets_migration60.js
@@ -235,16 +235,17 @@ function newExperimentOpts(opts = {}) {
   };
   const preferences = {};
   for (const [prefName, prefInfo] of Object.entries(opts.preferences || defaultPref)) {
     preferences[prefName] = { ...defaultPrefInfo, ...prefInfo };
   }
 
   return Object.assign({
     name: STUDY_NAME,
+    actionName: "SomeAction",
     branch: "branch",
   }, opts, {
     preferences,
   });
 }
 
 async function getNonExpiredExperiment() {
   try {
--- a/toolkit/components/normandy/actions/PreferenceExperimentAction.jsm
+++ b/toolkit/components/normandy/actions/PreferenceExperimentAction.jsm
@@ -73,16 +73,17 @@ class PreferenceExperimentAction extends
         return;
       }
 
       // Otherwise, enroll!
       const branch = await this.chooseBranch(slug, branches);
       const experimentType = isHighPopulation ? "exp-highpop" : "exp";
       await PreferenceExperiments.start({
         name: slug,
+        actionName: this.name,
         branch: branch.slug,
         preferences: branch.preferences,
         experimentType,
       });
     } else {
       // If the experiment exists, and isn't expired, bump the lastSeen date.
       const experiment = await PreferenceExperiments.get(slug);
       if (experiment.expired) {
--- a/toolkit/components/normandy/lib/PreferenceExperiments.jsm
+++ b/toolkit/components/normandy/lib/PreferenceExperiments.jsm
@@ -397,27 +397,31 @@ var PreferenceExperiments = {
     };
     store.saveSoon();
   },
 
   /**
    * Start a new preference experiment.
    * @param {Object} experiment
    * @param {string} experiment.name
+   * @param {string} experiment.actionName  The action who knows about this
+   *   experiment and is responsible for cleaning it up. This should
+   *   correspond to the name of some BaseAction subclass.
    * @param {string} experiment.branch
    * @param {string} experiment.preferenceName
    * @param {string|integer|boolean} experiment.preferenceValue
    * @param {PreferenceBranchType} experiment.preferenceBranchType
    * @rejects {Error}
    *   - If an experiment with the given name already exists
    *   - if an experiment for the given preference is active
    *   - If the given preferenceType does not match the existing stored preference
    */
   async start({
     name,
+    actionName,
     branch,
     preferences,
     experimentType = "exp",
     userFacingName = null,
     userFacingDescription = null,
   }) {
     log.debug(`PreferenceExperiments.start(${name}, ${branch})`);
 
@@ -482,16 +486,17 @@ var PreferenceExperiments = {
       const preferenceBranch = PreferenceBranchType[preferenceBranchType];
       setPref(preferenceBranch, preferenceName, preferenceType, preferenceValue);
     }
     PreferenceExperiments.startObserver(name, preferences);
 
     /** @type {Experiment} */
     const experiment = {
       name,
+      actionName,
       branch,
       expired: false,
       lastSeen: new Date().toJSON(),
       preferences,
       experimentType,
       userFacingName,
       userFacingDescription,
     };
--- a/toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js
+++ b/toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js
@@ -198,16 +198,17 @@ decorate_task(
 // start should throw if an experiment with the given name already exists
 decorate_task(
   withMockExperiments([experimentFactory({ name: "test" })]),
   withSendEventStub,
   async function(experiments, sendEventStub) {
     await Assert.rejects(
       PreferenceExperiments.start({
         name: "test",
+        actionName: "SomeAction",
         branch: "branch",
         preferences: {
           "fake.preference": {
             preferenceValue: "value",
             preferenceType: "string",
             preferenceBranchType: "default",
           },
         },
@@ -226,16 +227,17 @@ decorate_task(
 // preferences are active
 decorate_task(
   withMockExperiments([experimentFactory({ name: "test", preferences: {"fake.preferenceinteger": {}} })]),
   withSendEventStub,
   async function(experiments, sendEventStub) {
     await Assert.rejects(
       PreferenceExperiments.start({
         name: "different",
+        actionName: "SomeAction",
         branch: "branch",
         preferences: {
           "fake.preference": {
             preferenceValue: "value",
             preferenceType: "string",
             preferenceBranchType: "default",
           },
           "fake.preferenceinteger": {
@@ -258,16 +260,17 @@ decorate_task(
 // start should throw if an invalid preferenceBranchType is given
 decorate_task(
   withMockExperiments(),
   withSendEventStub,
   async function(experiments, sendEventStub) {
     await Assert.rejects(
       PreferenceExperiments.start({
         name: "test",
+        actionName: "SomeAction",
         branch: "branch",
         preferences: {
           "fake.preference": {
             preferenceValue: "value",
             preferenceType: "string",
             preferenceBranchType: "invalid",
           },
         },
@@ -292,16 +295,17 @@ decorate_task(
   async function testStart(experiments, mockPreferences, startObserverStub, sendEventStub) {
     mockPreferences.set("fake.preference", "oldvalue", "default");
     mockPreferences.set("fake.preference", "uservalue", "user");
     mockPreferences.set("fake.preferenceinteger", 1, "default");
     mockPreferences.set("fake.preferenceinteger", 101, "user");
 
     const experiment = {
       name: "test",
+      actionName: "SomeAction",
       branch: "branch",
       preferences: {
         "fake.preference": {
           preferenceValue: "newvalue",
           preferenceBranchType: "default",
           preferenceType: "string",
         },
         "fake.preferenceinteger": {
@@ -381,16 +385,17 @@ decorate_task(
   withMockPreferences,
   withStub(PreferenceExperiments, "startObserver"),
   async function(experiments, mockPreferences, startObserver) {
     mockPreferences.set("fake.preference", "olddefaultvalue", "default");
     mockPreferences.set("fake.preference", "oldvalue", "user");
 
     const experiment = {
       name: "test",
+      actionName: "SomeAction",
       branch: "branch",
       preferences: {
         "fake.preference": {
           preferenceValue: "newvalue",
           preferenceType: "string",
           preferenceBranchType: "user",
         },
       },
@@ -435,16 +440,17 @@ decorate_task(
   withMockPreferences,
   withSendEventStub,
   async function(mockPreferences, sendEventStub) {
     mockPreferences.set("fake.type_preference", "oldvalue");
 
     await Assert.rejects(
       PreferenceExperiments.start({
         name: "test",
+        actionName: "SomeAction",
         branch: "branch",
         preferences: {
           "fake.type_preference": {
             preferenceBranchType: "user",
             preferenceValue: 12345,
             preferenceType: "integer",
           },
         },
@@ -997,16 +1003,17 @@ decorate_task(
 decorate_task(
   withMockExperiments(),
   withStub(TelemetryEnvironment, "setExperimentActive"),
   withStub(TelemetryEnvironment, "setExperimentInactive"),
   withSendEventStub,
   async function testStartAndStopTelemetry(experiments, setActiveStub, setInactiveStub, sendEventStub) {
     await PreferenceExperiments.start({
       name: "test",
+      actionName: "SomeAction",
       branch: "branch",
       preferences: {
         "fake.preference": {
           preferenceValue: "value",
           preferenceType: "string",
           preferenceBranchType: "default",
         },
       },
@@ -1040,16 +1047,17 @@ decorate_task(
 decorate_task(
   withMockExperiments(),
   withStub(TelemetryEnvironment, "setExperimentActive"),
   withStub(TelemetryEnvironment, "setExperimentInactive"),
   withSendEventStub,
   async function testInitTelemetryExperimentType(experiments, setActiveStub, setInactiveStub, sendEventStub) {
     await PreferenceExperiments.start({
       name: "test",
+      actionName: "SomeAction",
       branch: "branch",
       preferences: {
         "fake.preference": {
           preferenceValue: "value",
           preferenceType: "string",
           preferenceBranchType: "default",
         },
       },
@@ -1277,16 +1285,17 @@ decorate_task(
   withStub(PreferenceExperiments, "stopObserver"),
   async function testDefaultBranchStop(mockExperiments, mockPreferences) {
     const prefName = "fake.preference";
     mockPreferences.set(prefName, "old version's value", "default");
 
     // start an experiment
     await PreferenceExperiments.start({
       name: "test",
+      actionName: "SomeAction",
       branch: "branch",
       preferences: {
         [prefName]: {
           preferenceValue: "experiment value",
           preferenceBranchType: "default",
           preferenceType: "string",
         },
       },
@@ -1328,16 +1337,17 @@ decorate_task(
   withStub(PreferenceExperiments, "stopObserver"),
   async function testDefaultBranchStop(mockExperiments, mockPreferences) {
     const prefName = "fake.preference";
     mockPreferences.set(prefName, "old version's value", "default");
 
     // start an experiment
     await PreferenceExperiments.start({
       name: "test",
+      actionName: "SomeAction",
       branch: "branch",
       preferences: {
         [prefName]: {
           preferenceValue: "experiment value",
           preferenceBranchType: "default",
           preferenceType: "string",
         },
       },
--- a/toolkit/components/normandy/test/browser/browser_actions_PreferenceExperimentAction.js
+++ b/toolkit/components/normandy/test/browser/browser_actions_PreferenceExperimentAction.js
@@ -128,16 +128,17 @@ decorate_task(
       return branches[0];
     });
 
     await action.runRecipe(recipe);
     await action.finalize();
 
     Assert.deepEqual(startStub.args, [[{
       name: "test",
+      actionName: "PreferenceExperimentAction",
       branch: "branch1",
       preferences: {
         "fake.preference": {
           preferenceValue: "branch1",
           preferenceBranchType: "user",
           preferenceType: "string",
         },
       },