Bug 1556391: Fix experiments that never expire r=mythmon

Add a test that simulates two Normandy "runs" to verify that the fix
actually works.

While we are here, fix a bug in the handling of
userFacingName/userFacingDescription.

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

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

toolkit/components/normandy/actions/PreferenceExperimentAction.jsm

@@ -45,6 +45,8 @@ class PreferenceExperimentAction extends BaseAction {
       isHighPopulation,
       isEnrollmentPaused,
       slug,
+      userFacingName,
+      userFacingDescription,
     } = recipe.arguments;
 
     this.seenExperimentNames.push(slug);
@@ -82,6 +84,8 @@ class PreferenceExperimentAction extends BaseAction {
         branch: branch.slug,
         preferences: branch.preferences,
         experimentType,
+        userFacingName,
+        userFacingDescription,
       });
     } else {
       // If the experiment exists, and isn't expired, bump the lastSeen date.
@@ -119,7 +123,7 @@ class PreferenceExperimentAction extends BaseAction {
   async _finalize() {
     const activeExperiments = await PreferenceExperiments.getAllActive();
     return Promise.all(activeExperiments.map(experiment => {
-      if (this.name != experiment.action) {
+      if (this.name != experiment.actionName) {
         // Another action is responsible for cleaning this one
         // up. Leave it alone.
         return null;

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

@@ -143,6 +143,8 @@ decorate_task(
         },
       },
       experimentType: "exp",
+      userFacingName: "Super Cool Test Experiment",
+      userFacingDescription: "Test experiment from browser_actions_PreferenceExperimentAction.",
     }]]);
   }
 );
@@ -197,8 +199,8 @@ decorate_task(
 decorate_task(
   withStub(PreferenceExperiments, "stop"),
   PreferenceExperiments.withMockExperiments([
-    {name: "seen", expired: false, action: "PreferenceExperimentAction"},
-    {name: "unseen", expired: false, action: "PreferenceExperimentAction"},
+    {name: "seen", expired: false, actionName: "PreferenceExperimentAction"},
+    {name: "unseen", expired: false, actionName: "PreferenceExperimentAction"},
   ]),
   async function stop_experiments_not_seen(stopStub) {
     const action = new PreferenceExperimentAction();
@@ -217,8 +219,8 @@ decorate_task(
 decorate_task(
   withStub(PreferenceExperiments, "stop"),
   PreferenceExperiments.withMockExperiments([
-    {name: "seen", expired: false, action: "SinglePreferenceExperimentAction"},
-    {name: "unseen", expired: false, action: "SinglePreferenceExperimentAction"},
+    {name: "seen", expired: false, actionName: "SinglePreferenceExperimentAction"},
+    {name: "unseen", expired: false, actionName: "SinglePreferenceExperimentAction"},
   ]),
   async function dont_stop_experiments_for_other_action(stopStub) {
     const action = new PreferenceExperimentAction();
@@ -338,3 +340,74 @@ decorate_task(
     Assert.deepEqual(result, branches[1]);
   }
 );
+
+decorate_task(
+  withMockPreferences,
+  PreferenceExperiments.withMockExperiments([]),
+  async function integration_test_enroll_and_unenroll(prefs) {
+    prefs.set("fake.preference", "oldvalue", "user");
+    const recipe = preferenceExperimentFactory({
+      slug: "integration test experiment",
+      branches: [
+        {
+          slug: "branch1",
+          preferences: {
+            "fake.preference": {
+              preferenceBranchType: "user",
+              preferenceValue: "branch1",
+            },
+          },
+          ratio: 1,
+        },
+        {
+          slug: "branch2",
+          preferences: {
+            "fake.preference": {
+              preferenceBranchType: "user",
+              preferenceValue: "branch2",
+            },
+          },
+          ratio: 1,
+        },
+      ],
+      userFacingName: "userFacingName",
+      userFacingDescription: "userFacingDescription",
+    });
+
+    // Session 1: we see the above recipe and enroll in the experiment.
+    const action = new PreferenceExperimentAction();
+    sinon.stub(action, "chooseBranch").callsFake(async function(slug, branches) {
+      return branches[0];
+    });
+    await action.runRecipe(recipe);
+    await action.finalize();
+
+    const activeExperiments = await PreferenceExperiments.getAllActive();
+    ok(activeExperiments.length > 0);
+    Assert.deepEqual(activeExperiments, [{
+      name: "integration test experiment",
+      actionName: "PreferenceExperimentAction",
+      branch: "branch1",
+      preferences: {
+        "fake.preference": {
+          preferenceBranchType: "user",
+          preferenceValue: "branch1",
+          preferenceType: "string",
+          previousPreferenceValue: "oldvalue",
+        },
+      },
+      expired: false,
+      lastSeen: activeExperiments[0].lastSeen,  // can't predict date
+      experimentType: "exp",
+      userFacingName: "userFacingName",
+      userFacingDescription: "userFacingDescription",
+    }]);
+
+    // Session 2: recipe is filtered out and so does not run.
+    const action2 = new PreferenceExperimentAction();
+    await action2.finalize();
+
+    // Experiment should be unenrolled
+    Assert.deepEqual(await PreferenceExperiments.getAllActive(), []);
+  }
+);