Backed out changeset 54c8b5fb832f (bug 1773583) for causing browser-c…

…hrome failures in browser/components/newtab/test/browser/browser_aboutwelcome_multistage_experimentAPI.js

browser/components/newtab/test/browser/browser_asrouter_remoteimages.js

@@ -380,11 +380,12 @@ add_task(async function test_remoteImages_prefetch() {
     PREFETCH_FINISHED_TOPIC
   );
 
-  const { doExperimentCleanup } = await ExperimentFakes.enrollmentHelper(
-    recipe
-  );
+  const {
+    enrollmentPromise,
+    doExperimentCleanup,
+  } = ExperimentFakes.enrollmentHelper(recipe);
 
-  await prefetchFinished;
+  await Promise.all([enrollmentPromise, prefetchFinished]);
 
   try {
     await RemoteImages.withDb(async db => {

browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_migrate_v2.js

@@ -1333,7 +1333,7 @@ async function withOnlineExperiment(callback) {
   let {
     enrollmentPromise,
     doExperimentCleanup,
-  } = await ExperimentFakes.enrollmentHelper(
+  } = ExperimentFakes.enrollmentHelper(
     ExperimentFakes.recipe("firefox-suggest-offline-vs-online", {
       active: true,
       branches: [

toolkit/components/nimbus/test/NimbusTestUtils.jsm

@@ -25,6 +25,7 @@ XPCOMUtils.defineLazyModuleGetters(lazy, {
   NormandyUtils: "resource://normandy/lib/NormandyUtils.jsm",
   _RemoteSettingsExperimentLoader:
     "resource://nimbus/lib/RemoteSettingsExperimentLoader.jsm",
+  sinon: "resource://testing-common/Sinon.jsm",
   FeatureManifest: "resource://nimbus/FeatureManifest.js",
   JsonSchema: "resource://gre/modules/JsonSchema.jsm",
 });
@@ -169,7 +170,17 @@ const ExperimentTestUtils = {
 
 const ExperimentFakes = {
   manager(store) {
-    return new lazy._ExperimentManager({ store: store || this.store() });
+    let sandbox = lazy.sinon.createSandbox();
+    let manager = new lazy._ExperimentManager({ store: store || this.store() });
+    // We want calls to `store.addEnrollment` to implicitly validate the
+    // enrollment before saving to store
+    let origAddExperiment = manager.store.addEnrollment.bind(manager.store);
+    sandbox.stub(manager.store, "addEnrollment").callsFake(async enrollment => {
+      await ExperimentTestUtils.validateEnrollment(enrollment);
+      return origAddExperiment(enrollment);
+    });
+
+    return manager;
   },
   store() {
     return new ExperimentStore("FakeStore", {
@@ -249,16 +260,24 @@ const ExperimentFakes = {
         },
       ],
     });
-    let { doExperimentCleanup } = await this.enrollmentHelper(recipe, {
-      manager,
-    });
+    let {
+      enrollmentPromise,
+      doExperimentCleanup,
+    } = this.enrollmentHelper(recipe, { manager });
+
+    await enrollmentPromise;
 
     return doExperimentCleanup;
   },
-  async enrollmentHelper(
-    recipe = {},
-    { manager = lazy.ExperimentManager } = {}
-  ) {
+  enrollmentHelper(recipe = {}, { manager = lazy.ExperimentManager } = {}) {
+    let enrollmentPromise = new Promise(resolve =>
+      manager.store.on(`update:${recipe.slug}`, (event, experiment) => {
+        if (experiment.active) {
+          manager.store._syncToChildren({ flush: true });
+          resolve(experiment);
+        }
+      })
+    );
     let unenrollCompleted = slug =>
       new Promise(resolve =>
         manager.store.on(`update:${slug}`, (event, experiment) => {
@@ -285,10 +304,10 @@ const ExperimentFakes = {
       if (!manager.store._isReady) {
         throw new Error("Manager store not ready, call `manager.onStartup`");
       }
-      await manager.enroll(recipe, "enrollmentHelper");
+      manager.enroll(recipe, "enrollmentHelper");
     }
 
-    return { doExperimentCleanup };
+    return { enrollmentPromise, doExperimentCleanup };
   },
   // Experiment store caches in prefs Enrollments for fast sync access
   cleanupStorePrefCache() {

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

@@ -84,9 +84,12 @@ add_task(async function test_evaluate_active_experiments_activeExperiments() {
   recipe.branches[0].slug = "mochitest-active-foo";
   delete recipe.branches[1];
 
-  const { doExperimentCleanup } = await ExperimentFakes.enrollmentHelper(
-    recipe
-  );
+  let {
+    enrollmentPromise,
+    doExperimentCleanup,
+  } = ExperimentFakes.enrollmentHelper(recipe);
+
+  await enrollmentPromise;
 
   Assert.equal(
     await RemoteSettingsExperimentLoader.evaluateJexl(

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

@@ -76,12 +76,14 @@ const SINGLE_FEATURE_RECIPE = {
 const SYNC_DATA_PREF_BRANCH = "nimbus.syncdatastore.";
 
 add_task(async function test_TODO() {
-  const sandbox = sinon.createSandbox();
-  const stub = sandbox.stub(ExperimentAPI, "recordExposureEvent");
+  let {
+    enrollmentPromise,
+    doExperimentCleanup,
+  } = ExperimentFakes.enrollmentHelper(SINGLE_FEATURE_RECIPE);
+  let sandbox = sinon.createSandbox();
+  let stub = sandbox.stub(ExperimentAPI, "recordExposureEvent");
 
-  const { doExperimentCleanup } = await ExperimentFakes.enrollmentHelper(
-    SINGLE_FEATURE_RECIPE
-  );
+  await enrollmentPromise;
 
   Assert.ok(
     ExperimentAPI.getExperiment({ featureId: "urlbar" }),

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

@@ -21,11 +21,10 @@ const EXPERIMENT_TYPE = "nimbus";
 const EVENT_FILTER = { category: TELEMETRY_CATEGORY };
 
 add_setup(async function() {
-  const sandbox = sinon.createSandbox();
+  let sandbox = sinon.createSandbox();
   // stub the `observe` method to make sure the Experiment Manager
   // pref listener doesn't trigger and cause side effects
   sandbox.stub(ExperimentManager, "observe");
-
   await SpecialPowers.pushPrefEnv({
     set: [["app.shield.optoutstudies.enabled", true]],
   });

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

@@ -32,7 +32,7 @@ function getRecipe(slug) {
 
 add_task(async function test_double_feature_enrollment() {
   let sandbox = sinon.createSandbox();
-  let { doExperimentCleanup } = await ExperimentFakes.enrollmentHelper();
+  let { doExperimentCleanup } = ExperimentFakes.enrollmentHelper();
   let sendFailureTelemetryStub = sandbox.stub(
     ExperimentManager,
     "sendFailureTelemetry"

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

@@ -117,7 +117,6 @@ async function setup(configuration) {
 }
 
 add_task(async function test_remote_fetch_and_ready() {
-  const sandbox = sinon.createSandbox();
   const fooInstance = new ExperimentFeature("foo", FOO_FAKE_FEATURE_MANIFEST);
   const barInstance = new ExperimentFeature("bar", BAR_FAKE_FEATURE_MANIFEST);
 
@@ -126,6 +125,7 @@ add_task(async function test_remote_fetch_and_ready() {
     barInstance
   );
 
+  const sandbox = sinon.createSandbox();
   const setExperimentActiveStub = sandbox.stub(
     TelemetryEnvironment,
     "setExperimentActive"
@@ -267,12 +267,10 @@ add_task(async function test_remote_fetch_and_ready() {
 
   cleanupTestFeatures();
   await cleanup();
-  sandbox.restore();
 });
 
 add_task(async function test_remote_fetch_on_updateRecipes() {
-  const sandbox = sinon.createSandbox();
-
+  let sandbox = sinon.createSandbox();
   let updateRecipesStub = sandbox.stub(
     RemoteSettingsExperimentLoader,
     "updateRecipes"
@@ -307,7 +305,6 @@ add_task(async function test_remote_fetch_on_updateRecipes() {
   Services.prefs.clearUserPref(
     "app.update.lastUpdateTime.rs-experiment-loader-timer"
   );
-  sandbox.restore();
 });
 
 add_task(async function test_finalizeRemoteConfigs_cleanup() {
@@ -349,7 +346,6 @@ add_task(async function test_finalizeRemoteConfigs_cleanup() {
       source: "rs-loader",
     }
   );
-
   let stubFoo = sinon.stub();
   let stubBar = sinon.stub();
   featureFoo.onUpdate(stubFoo);
@@ -415,8 +411,7 @@ add_task(async function test_finalizeRemoteConfigs_cleanup() {
 // If the remote config data returned from the store is not modified
 // this test should not throw
 add_task(async function remote_defaults_no_mutation() {
-  const sandbox = sinon.createSandbox();
-
+  let sandbox = sinon.createSandbox();
   sandbox.stub(ExperimentAPI._store, "getRolloutForFeature").returns(
     Cu.cloneInto(
       {
@@ -439,8 +434,6 @@ add_task(async function remote_defaults_no_mutation() {
 });
 
 add_task(async function remote_defaults_active_remote_defaults() {
-  const sandbox = sinon.createSandbox();
-
   ExperimentAPI._store._deleteForTests("foo");
   ExperimentAPI._store._deleteForTests("bar");
   let barFeature = new ExperimentFeature("bar", {
@@ -493,28 +486,27 @@ add_task(async function remote_defaults_active_remote_defaults() {
     targeting: "'bar' in activeRollouts",
   });
 
-  // Order is important, rollout2 won't match at first.
-  // We cannot use setup() here becuase RS returns records in a
-  // non-deterministic order.
-  sandbox
-    .stub(RemoteSettingsExperimentLoader.remoteSettingsClient, "get")
-    .resolves([rollout2, rollout1]);
+  // Order is important, rollout2 won't match at first
   const { cleanup } = await setup([rollout2, rollout1]);
+  let updatePromise = new Promise(resolve => barFeature.onUpdate(resolve));
   RemoteSettingsExperimentLoader._initialized = true;
   await RemoteSettingsExperimentLoader.updateRecipes("mochitest");
 
+  await updatePromise;
+
   Assert.ok(barFeature.getVariable("enabled"), "Enabled on first sync");
   Assert.ok(!fooFeature.getVariable("enabled"), "Targeting doesn't match");
 
+  let featureUpdate = new Promise(resolve => fooFeature.onUpdate(resolve));
   await RemoteSettingsExperimentLoader.updateRecipes("mochitest");
+  await featureUpdate;
 
   Assert.ok(fooFeature.getVariable("enabled"), "Targeting should match");
   ExperimentAPI._store._deleteForTests("foo");
   ExperimentAPI._store._deleteForTests("bar");
 
   cleanup();
   cleanupTestFeatures();
-  sandbox.restore();
 });
 
 add_task(async function remote_defaults_variables_storage() {

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

@@ -13,3 +13,26 @@ XPCOMUtils.defineLazyModuleGetters(this, {
   ExperimentTestUtils: "resource://testing-common/NimbusTestUtils.jsm",
   ExperimentFakes: "resource://testing-common/NimbusTestUtils.jsm",
 });
+
+add_setup(function() {
+  let sandbox = sinon.createSandbox();
+
+  /* We stub the functions that operate with enrollments and remote rollouts
+   * so that any access to store something is implicitly validated against
+   * the schema and no records have missing (or extra) properties while in tests
+   */
+
+  let origAddExperiment = ExperimentManager.store.addEnrollment.bind(
+    ExperimentManager.store
+  );
+  sandbox
+    .stub(ExperimentManager.store, "addEnrollment")
+    .callsFake(async enrollment => {
+      await ExperimentTestUtils.validateEnrollment(enrollment);
+      return origAddExperiment(enrollment);
+    });
+
+  registerCleanupFunction(() => {
+    sandbox.restore();
+  });
+});

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

@@ -67,7 +67,7 @@ add_task(
 add_task(
   async function test_ExperimentFeature_getAllVariables_prefsOverExperiment() {
     const { sandbox, manager } = await setupForExperimentFeature();
-    const { doExperimentCleanup } = await ExperimentFakes.enrollmentHelper(
+    const { doExperimentCleanup } = ExperimentFakes.enrollmentHelper(
       undefined,
       {
         manager,
@@ -123,7 +123,7 @@ add_task(
   async function test_ExperimentFeature_getAllVariables_experimentOverRemote() {
     Services.prefs.clearUserPref(TEST_FALLBACK_PREF);
     const { manager } = await setupForExperimentFeature();
-    const { doExperimentCleanup } = await ExperimentFakes.enrollmentHelper(
+    const { doExperimentCleanup } = ExperimentFakes.enrollmentHelper(
       undefined,
       {
         manager,

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

@@ -754,17 +754,15 @@ add_task(async function test_featureIds_is_stored() {
   recipe.bucketConfig.count = recipe.bucketConfig.total;
   const store = ExperimentFakes.store();
   const manager = ExperimentFakes.manager(store);
-  const sandbox = sinon.createSandbox();
-  sandbox.spy(store, "addEnrollment");
 
   await manager.onStartup();
 
-  const { doExperimentCleanup } = await ExperimentFakes.enrollmentHelper(
-    recipe,
-    {
-      manager,
-    }
-  );
+  const {
+    enrollmentPromise,
+    doExperimentCleanup,
+  } = ExperimentFakes.enrollmentHelper(recipe, { manager });
+
+  await enrollmentPromise;
 
   Assert.ok(manager.store.addEnrollment.calledOnce, "experiment is stored");
   let [enrollment] = manager.store.addEnrollment.firstCall.args;
@@ -776,7 +774,6 @@ add_task(async function test_featureIds_is_stored() {
   );
 
   await doExperimentCleanup();
-  sandbox.restore();
 });
 
 add_task(async function experiment_and_rollout_enroll_and_cleanup() {

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

@@ -200,7 +200,7 @@ add_task(async function test_unenroll_uploadPref() {
   const recipe = ExperimentFakes.recipe("foo");
 
   await manager.onStartup();
-  await ExperimentFakes.enrollmentHelper(recipe, { manager });
+  await ExperimentFakes.enrollmentHelper(recipe, { manager }).enrollmentPromise;
 
   Assert.equal(
     manager.store.get(recipe.slug).active,

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

@@ -32,11 +32,14 @@ add_task(async function test_enrollmentHelper() {
 
   await manager.onStartup();
 
-  let { doExperimentCleanup } = await ExperimentFakes.enrollmentHelper(recipe, {
-    manager,
-  });
+  let {
+    enrollmentPromise,
+    doExperimentCleanup,
+  } = ExperimentFakes.enrollmentHelper(recipe, { manager });
+
+  await enrollmentPromise;
 
-  Assert.equal(manager.store.getAllActive().length, 1, "Enrolled");
+  Assert.ok(manager.store.getAllActive().length === 1, "Enrolled");
   Assert.equal(
     manager.store.getAllActive()[0].slug,
     recipe.slug,