Backed out changeset 2ee3856bd2ae (bug 1693301) for failures on browser_remotesettingsexperimentloader_init.js. CLOSED TREE

browser/components/newtab/test/browser/browser_aboutwelcome_multistage.js

@@ -232,34 +232,30 @@ add_task(async function setup() {
  */
 add_task(async function test_multistage_zeroOnboarding_experimentAPI() {
   await setAboutWelcomePref(true);
+  let updatePromise = ExperimentFakes.waitForExperimentUpdate(ExperimentAPI, {
+    slug: "mochitest-1-aboutwelcome",
+  });
+  ExperimentAPI._store.addExperiment({
+    slug: "mochitest-1-aboutwelcome",
+    branch: {
+      slug: "mochitest-1-aboutwelcome",
+      feature: {
+        enabled: false,
+        featureId: "aboutwelcome",
+        value: null,
+      },
+    },
+    active: true,
+  });
 
-  let {
+  await updatePromise;
-    enrollmentPromise,
+  ExperimentAPI._store._syncToChildren({ flush: true });
-    doExperimentCleanup,
-  } = ExperimentFakes.enrollmentHelper(
-    ExperimentFakes.recipe("mochitest-1-aboutwelcome", {
-      branches: [
-        {
-          slug: "mochitest-1-aboutwelcome",
-          feature: {
-            enabled: false,
-            featureId: "aboutwelcome",
-            value: null,
-          },
-        },
-      ],
-      active: true,
-    })
-  );
-
-  await enrollmentPromise;
 
   let tab = await BrowserTestUtils.openNewForegroundTab(
     gBrowser,
     "about:welcome",
     true
   );
-
   registerCleanupFunction(() => {
     BrowserTestUtils.removeTab(tab);
   });
@@ -275,7 +271,7 @@ add_task(async function test_multistage_zeroOnboarding_experimentAPI() {
     ["div.onboardingContainer", "main.AW_STEP1"]
   );
 
-  await doExperimentCleanup();
+  ExperimentAPI._store._deleteForTests("mochitest-1-aboutwelcome");
   Assert.equal(ExperimentAPI._store.getAll().length, 0, "Cleanup done");
 });
 
@@ -284,27 +280,25 @@ add_task(async function test_multistage_zeroOnboarding_experimentAPI() {
  */
 add_task(async function test_multistage_aboutwelcome_experimentAPI() {
   await setAboutWelcomePref(true);
+  await setAboutWelcomeMultiStage({});
+  let updatePromise = ExperimentFakes.waitForExperimentUpdate(ExperimentAPI, {
+    slug: "mochitest-aboutwelcome",
+  });
+  ExperimentAPI._store.addExperiment({
+    slug: "mochitest-aboutwelcome",
+    branch: {
+      slug: "mochitest-aboutwelcome",
+      feature: {
+        enabled: true,
+        featureId: "aboutwelcome",
+        value: TEST_MULTISTAGE_CONTENT,
+      },
+    },
+    active: true,
+  });
 
-  let {
+  await updatePromise;
-    enrollmentPromise,
+  ExperimentAPI._store._syncToChildren({ flush: true });
-    doExperimentCleanup,
-  } = ExperimentFakes.enrollmentHelper(
-    ExperimentFakes.recipe("mochitest-aboutwelcome", {
-      branches: [
-        {
-          slug: "mochitest-aboutwelcome-branch",
-          feature: {
-            enabled: true,
-            featureId: "aboutwelcome",
-            value: TEST_MULTISTAGE_CONTENT,
-          },
-        },
-      ],
-      active: true,
-    })
-  );
-
-  await enrollmentPromise;
 
   let tab = await BrowserTestUtils.openNewForegroundTab(
     gBrowser,
@@ -383,7 +377,7 @@ add_task(async function test_multistage_aboutwelcome_experimentAPI() {
     ["div.onboardingContainer"]
   );
 
-  await doExperimentCleanup();
+  ExperimentAPI._store._deleteForTests("mochitest-aboutwelcome");
   Assert.equal(ExperimentAPI._store.getAll().length, 0, "Cleanup done");
 });
 

toolkit/components/nimbus/docs/index.rst

@@ -13,4 +13,3 @@ Learn more
   :maxdepth: 2
 
   integration.md
-  testing.md

toolkit/components/nimbus/docs/testing.md

@@ -1,61 +0,0 @@
-# Nimbus Testing Helpers
-
-In order to make testing easier we created some helpers that can be accessed by including
-
-```js
-const { ExperimentFakes } = ChromeUtils.import(
-  "resource://testing-common/NimbusTestUtils.jsm"
-);
-```
-
-## Testing your feature integrating with Nimbus
-
-1. You need to create a recipe
-
-```js
-let recipe = ExperimentFakes.recipe("my-cool-experiment", {
-  branches: [
-    {
-      slug: "treatment-branch",
-      ratio: 1,
-      feature: {
-        featureId: "<YOUR FEATURE>",
-        // The feature is on
-        enabled: true,
-        // If you defined `variables` in the MANIFEST
-        // the `value` should match that schema
-        value: null,
-      },
-    },
-  ],
-  bucketConfig: {
-    start: 0,
-    // Ensure 100% enrollment
-    count: 10000,
-    total: 10000,
-    namespace: "my-mochitest",
-    randomizationUnit: "normandy_id",
-  },
-});
-```
-
-2. Now with the newly created recipe you want the test to enroll in the experiment
-
-```js
-let {
-  enrollmentPromise,
-  doExperimentCleanup,
-} = ExperimentFakes.enrollmentHelper(recipe);
-
-// Await for enrollment to complete
-
-await enrollmentPromise;
-
-// Now you can assume the feature is enabled so you can
-// test and that it's doing the right thing
-
-// Assert.ok(It works!)
-
-// Finishing up
-await doExperimentCleanup();
-```

toolkit/components/nimbus/lib/ExperimentStore.jsm

@@ -179,8 +179,8 @@ class ExperimentStore extends SharedDataMap {
       );
     }
     this.set(experiment.slug, experiment);
-    this._updateSyncStore(experiment);
     this._emitExperimentUpdates(experiment);
+    this._updateSyncStore(experiment);
   }
 
   /**

toolkit/components/nimbus/test/NimbusTestUtils.jsm

@@ -11,7 +11,6 @@ const { XPCOMUtils } = ChromeUtils.import(
 
 XPCOMUtils.defineLazyModuleGetters(this, {
   _ExperimentManager: "resource://nimbus/lib/ExperimentManager.jsm",
-  ExperimentManager: "resource://nimbus/lib/ExperimentManager.jsm",
   ExperimentStore: "resource://nimbus/lib/ExperimentStore.jsm",
   NormandyUtils: "resource://normandy/lib/NormandyUtils.jsm",
   FileTestUtils: "resource://testing-common/FileTestUtils.jsm",
@@ -68,45 +67,6 @@ const ExperimentFakes = {
 
     return new Promise(resolve => ExperimentAPI.on("update", options, resolve));
   },
-  enrollmentHelper(recipe = {}, { manager = ExperimentManager } = {}) {
-    let enrollmentPromise = new Promise(resolve =>
-      manager.store.on(`update:${recipe.slug}`, (event, experiment) => {
-        if (experiment.active) {
-          resolve(experiment);
-        }
-      })
-    );
-    let unenrollCompleted = slug =>
-      new Promise(resolve =>
-        manager.store.on(`update:${slug}`, (event, experiment) => {
-          if (!experiment.active) {
-            // Removes recipe from file storage which
-            // (normally the users archive of past experiments)
-            manager.store._deleteForTests(recipe.slug);
-            resolve();
-          }
-        })
-      );
-    let doExperimentCleanup = async () => {
-      for (let experiment of manager.store.getAllActive()) {
-        let promise = unenrollCompleted(experiment.slug);
-        manager.unenroll(experiment.slug, "cleanup");
-        await promise;
-      }
-      if (manager.store.getAllActive().length) {
-        throw new Error("Cleanup failed");
-      }
-    };
-
-    if (recipe.slug) {
-      if (!manager.store._isReady) {
-        throw new Error("Manager store not ready, call `manager.onStartup`");
-      }
-      manager.enroll(recipe);
-    }
-
-    return { enrollmentPromise, doExperimentCleanup };
-  },
   childStore() {
     return new ExperimentStore("FakeStore", { isParent: false });
   },

toolkit/components/nimbus/test/browser/browser_remotesettingsexperimentloader_init.js

@@ -18,16 +18,47 @@ const { ExperimentFakes } = ChromeUtils.import(
 const { ExperimentManager } = ChromeUtils.import(
   "resource://nimbus/lib/ExperimentManager.jsm"
 );
-const { ExperimentAPI } = ChromeUtils.import(
-  "resource://nimbus/ExperimentAPI.jsm"
-);
 const { BrowserTestUtils } = ChromeUtils.import(
   "resource://testing-common/BrowserTestUtils.jsm"
 );
 const { sinon } = ChromeUtils.import("resource://testing-common/Sinon.jsm");
 
-function getRecipe(slug) {
+let rsClient;
-  return ExperimentFakes.recipe(slug, {
+
+add_task(async function setup() {
+  await SpecialPowers.pushPrefEnv({
+    set: [
+      ["messaging-system.log", "all"],
+      ["app.shield.optoutstudies.enabled", true],
+    ],
+  });
+  rsClient = RemoteSettings("nimbus-desktop-experiments");
+
+  registerCleanupFunction(async () => {
+    await SpecialPowers.popPrefEnv();
+    await rsClient.db.clear();
+  });
+});
+
+add_task(async function test_double_feature_enrollment() {
+  let sandbox = sinon.createSandbox();
+  // We want to prevent this because it would start a recipe
+  // update outside of our asserts
+  sandbox.stub(RemoteSettingsExperimentLoader, "setTimer");
+  let sendFailureTelemetryStub = sandbox.stub(
+    ExperimentManager,
+    "sendFailureTelemetry"
+  );
+
+  for (let experiment of ExperimentManager.store.getAllActive()) {
+    ExperimentManager.unenroll(experiment.slug, "cleanup");
+    ExperimentManager.store._deleteForTests(experiment.slug);
+  }
+  await BrowserTestUtils.waitForCondition(
+    () => ExperimentManager.store.getAllActive().length === 0
+  );
+
+  const recipe1 = ExperimentFakes.recipe("foo" + Date.now(), {
     bucketConfig: {
       start: 0,
       // Make sure the experiment enrolls
@@ -36,84 +67,47 @@ function getRecipe(slug) {
       namespace: "mochitest",
       randomizationUnit: "normandy_id",
     },
-    targeting: "true",
   });
-}
+  const recipe2 = ExperimentFakes.recipe("foo" + Date.now(), {
-
+    bucketConfig: {
-add_task(async function setup() {
+      start: 0,
-  let rsClient = RemoteSettings("nimbus-desktop-experiments");
+      // Make sure the experiment enrolls
-
+      count: 10000,
-  await rsClient.db.importChanges(
+      total: 10000,
-    {},
+      namespace: "mochitest",
-    42,
+      randomizationUnit: "normandy_id",
-    [getRecipe("foo" + Math.random()), getRecipe("bar" + Math.random())],
+    },
-    {
-      clear: true,
-    }
-  );
-
-  registerCleanupFunction(async () => {
-    await rsClient.db.clear();
-  });
-});
-
-async function setup() {
-  let sandbox = sinon.createSandbox();
-  // We want to prevent this because it would start a recipe
-  // update as soon as we turn on the optoutstudies pref
-  sandbox.stub(RemoteSettingsExperimentLoader, "setTimer");
-  sandbox.stub(RemoteSettingsExperimentLoader, "onEnabledPrefChange");
-  RemoteSettingsExperimentLoader._updating = true;
-
-  await SpecialPowers.pushPrefEnv({
-    set: [
-      ["messaging-system.log", "all"],
-      ["app.shield.optoutstudies.enabled", true],
-    ],
   });
 
-  registerCleanupFunction(async () => {
+  await rsClient.db.importChanges({}, 42, [recipe1, recipe2], {
-    await SpecialPowers.popPrefEnv();
+    clear: true,
-    sandbox.restore();
   });
-}
 
-add_task(async function test_double_feature_enrollment() {
-  let { doExperimentCleanup } = ExperimentFakes.enrollmentHelper();
-  await doExperimentCleanup();
-  await setup();
   RemoteSettingsExperimentLoader.uninit();
-  let sandbox = sinon.createSandbox();
+  let enrolledPromise = new Promise(resolve =>
-  let sendFailureTelemetryStub = sandbox.stub(
+    ExperimentManager.store.on("update:test-feature", resolve)
-    ExperimentManager,
-    "sendFailureTelemetry"
   );
-
-  Assert.ok(ExperimentManager.store.getAllActive().length === 0, "Clean state");
-
   await RemoteSettingsExperimentLoader.init();
-
+  await enrolledPromise;
-  await ExperimentFakes.waitForExperimentUpdate(ExperimentAPI, {
-    featureId: "test-feature",
-  });
 
   Assert.ok(
     RemoteSettingsExperimentLoader._initialized,
     "It should initialize and process the recipes"
   );
 
-  Assert.equal(
+  BrowserTestUtils.waitForCondition(
-    ExperimentManager.store.getAllActive().length,
+    () => sendFailureTelemetryStub.calledOnce,
-    1,
-    "1 active experiment"
-  );
-
-  await BrowserTestUtils.waitForCondition(
-    () => sendFailureTelemetryStub.callCount,
     "Expected to fail one of the recipes"
   );
 
-  await doExperimentCleanup();
+  for (let experiment of ExperimentManager.store.getAllActive()) {
+    ExperimentManager.unenroll(experiment.slug, "cleanup");
+    ExperimentManager.store._deleteForTests(experiment.slug);
+  }
+  await BrowserTestUtils.waitForCondition(
+    () => ExperimentManager.store.getAllActive().length === 0
+  );
   await SpecialPowers.popPrefEnv();
+  await rsClient.db.clear();
   sandbox.restore();
 });

toolkit/components/nimbus/test/unit/test_NimbusTestUtils.js

@@ -11,40 +11,3 @@ add_task(async function test_recipe_fake_validates() {
     "should produce a valid experiment recipe"
   );
 });
-
-add_task(async function test_enrollmentHelper() {
-  let recipe = ExperimentFakes.recipe("bar");
-  recipe.branches.forEach(branch => {
-    // Use a feature that will set the sync pref cache
-    branch.feature.featureId = "aboutwelcome";
-  });
-  let manager = ExperimentFakes.manager();
-
-  await manager.onStartup();
-
-  let {
-    enrollmentPromise,
-    doExperimentCleanup,
-  } = ExperimentFakes.enrollmentHelper(recipe, { manager });
-
-  await enrollmentPromise;
-
-  Assert.ok(manager.store.getAllActive().length === 1, "Enrolled");
-  Assert.equal(
-    manager.store.getAllActive()[0].slug,
-    recipe.slug,
-    "Has expected slug"
-  );
-  Assert.ok(
-    Services.prefs.prefHasUserValue("nimbus.syncdatastore.aboutwelcome"),
-    "Sync pref cache set"
-  );
-
-  await doExperimentCleanup();
-
-  Assert.ok(manager.store.getAll().length === 0, "Cleanup done");
-  Assert.ok(
-    !Services.prefs.prefHasUserValue("nimbus.syncdatastore.aboutwelcome"),
-    "Sync pref cache is cleared"
-  );
-});