From 1fb5cc957695cdac8762b9237ee367d46e8d691f Mon Sep 17 00:00:00 2001 From: Sandor Molnar Date: Mon, 26 Jul 2021 01:17:51 +0300 Subject: [PATCH] Backed out changeset e5cfc59f9063 (bug 1716736) for causing bc failures in browser_setDefaultBrowser. CLOSED TREE --- .../test/about/browser_aboutSupport.js | 4 +- .../shell/test/browser_doesAppNeedPin.js | 11 +- .../shell/test/browser_setDefaultBrowser.js | 12 +- .../browser_browserGlue_upgradeDialog.js | 12 +- toolkit/components/nimbus/ExperimentAPI.jsm | 6 +- toolkit/components/nimbus/moz.build | 1 - .../ExperimentFeatureRemote.schema.json | 68 ------------ .../schemas/NimbusEnrollment.schema.json | 91 ---------------- .../nimbus/test/NimbusTestUtils.jsm | 103 ++++-------------- .../browser_experiment_evaluate_jexl.js | 26 +---- ...ser_remotesettingsexperimentloader_init.js | 10 +- ...ettingsexperimentloader_remote_defaults.js | 69 +++++------- .../components/nimbus/test/browser/head.js | 52 --------- toolkit/components/nimbus/test/unit/head.js | 6 - .../nimbus/test/unit/test_ExperimentAPI.js | 32 +++--- .../test_ExperimentAPI_ExperimentFeature.js | 50 ++++----- ...ntAPI_ExperimentFeature_getAllVariables.js | 6 +- ...xperimentAPI_ExperimentFeature_getValue.js | 6 +- ...rimentAPI_ExperimentFeature_getVariable.js | 6 + .../unit/test_ExperimentAPI_NimbusFeatures.js | 8 +- .../unit/test_ExperimentManager_enroll.js | 51 +++------ .../unit/test_ExperimentManager_lifecycle.js | 21 ++-- .../unit/test_ExperimentManager_unenroll.js | 13 ++- .../nimbus/test/unit/test_ExperimentStore.js | 3 + .../nimbus/test/unit/test_FeatureManifest.js | 3 + .../nimbus/test/unit/test_SharedDataMap.js | 3 + 26 files changed, 180 insertions(+), 493 deletions(-) delete mode 100644 toolkit/components/nimbus/schemas/NimbusEnrollment.schema.json diff --git a/browser/base/content/test/about/browser_aboutSupport.js b/browser/base/content/test/about/browser_aboutSupport.js index e56eb8a88e2a..81fd272a53c0 100644 --- a/browser/base/content/test/about/browser_aboutSupport.js +++ b/browser/base/content/test/about/browser_aboutSupport.js @@ -111,8 +111,8 @@ add_task(async function test_remote_configuration() { feature: NimbusFeatures.aboutwelcome, configuration: { slug: "about:studies-configuration-slug", - variables: { enabled: true }, - targeting: "true", + enabled: true, + variables: {}, }, }); diff --git a/browser/components/shell/test/browser_doesAppNeedPin.js b/browser/components/shell/test/browser_doesAppNeedPin.js index 52435acc4899..f2a32b6e577a 100644 --- a/browser/components/shell/test/browser_doesAppNeedPin.js +++ b/browser/components/shell/test/browser_doesAppNeedPin.js @@ -26,11 +26,7 @@ add_task(async function remote_disable() { await ExperimentFakes.remoteDefaultsHelper({ feature: NimbusFeatures.shellService, - configuration: { - slug: "shellService_remoteDisable", - variables: { disablePin: true, enabled: true }, - targeting: "true", - }, + configuration: { variables: { disablePin: true } }, }); Assert.equal( @@ -46,7 +42,10 @@ add_task(async function restore_default() { return; } - ExperimentAPI._store._deleteForTests("shellService"); + await ExperimentFakes.remoteDefaultsHelper({ + feature: NimbusFeatures.shellService, + configuration: {}, + }); Assert.equal( await ShellService.doesAppNeedPin(), diff --git a/browser/components/shell/test/browser_setDefaultBrowser.js b/browser/components/shell/test/browser_setDefaultBrowser.js index 2a335f1b1cf4..4330b20d8066 100644 --- a/browser/components/shell/test/browser_setDefaultBrowser.js +++ b/browser/components/shell/test/browser_setDefaultBrowser.js @@ -48,12 +48,7 @@ add_task(async function remote_disable() { setDefaultStub.resetHistory(); await ExperimentFakes.remoteDefaultsHelper({ feature: NimbusFeatures.shellService, - configuration: { - variables: { - setDefaultBrowserUserChoice: false, - enabled: true, - }, - }, + configuration: { variables: { setDefaultBrowserUserChoice: false } }, }); ShellService.setDefaultBrowser(); @@ -73,7 +68,10 @@ add_task(async function restore_default() { userChoiceStub.resetHistory(); setDefaultStub.resetHistory(); - ExperimentAPI._store._deleteForTests("shellService"); + await ExperimentFakes.remoteDefaultsHelper({ + feature: NimbusFeatures.shellService, + configuration: {}, + }); ShellService.setDefaultBrowser(); diff --git a/browser/components/tests/browser/browser_browserGlue_upgradeDialog.js b/browser/components/tests/browser/browser_browserGlue_upgradeDialog.js index a34ed2ffdece..cbf0631e87cf 100644 --- a/browser/components/tests/browser/browser_browserGlue_upgradeDialog.js +++ b/browser/components/tests/browser/browser_browserGlue_upgradeDialog.js @@ -360,11 +360,7 @@ add_task(async function remote_disabled() { await ExperimentAPI.ready(); await ExperimentFakes.remoteDefaultsHelper({ feature: NimbusFeatures.upgradeDialog, - configuration: { - slug: "upgradeDialog_remoteDisabled", - variables: { enabled: false }, - targeting: "true", - }, + configuration: { enabled: false, variables: {} }, }); // Simulate starting from a previous version. @@ -384,11 +380,7 @@ add_task(async function remote_disabled() { // Re-enable back await ExperimentFakes.remoteDefaultsHelper({ feature: NimbusFeatures.upgradeDialog, - configuration: { - slug: "upgradeDialog_remoteEnabled", - variables: { enabled: true }, - targeting: "true", - }, + configuration: { enabled: true, variables: {} }, }); }); diff --git a/toolkit/components/nimbus/ExperimentAPI.jsm b/toolkit/components/nimbus/ExperimentAPI.jsm index f56d8a69b9e0..1b9801ed0efe 100644 --- a/toolkit/components/nimbus/ExperimentAPI.jsm +++ b/toolkit/components/nimbus/ExperimentAPI.jsm @@ -183,7 +183,9 @@ const ExperimentAPI = { } let fullEventName = `${eventName}:${options.slug || options.featureId}`; - if (this._store._isReady) { + // The update event will always fire after the event listener is added, either + // immediately if it is already ready, or on ready + this._store.ready().then(() => { let experiment = this.getExperiment(options); // Only if we have an experiment that matches what the caller requested if (experiment) { @@ -192,7 +194,7 @@ const ExperimentAPI = { // are attached later than the `update` events. callback(fullEventName, experiment); } - } + }); this._store.on(fullEventName, callback); }, diff --git a/toolkit/components/nimbus/moz.build b/toolkit/components/nimbus/moz.build index fef864c07327..580e283a44de 100644 --- a/toolkit/components/nimbus/moz.build +++ b/toolkit/components/nimbus/moz.build @@ -30,7 +30,6 @@ SPHINX_TREES["docs"] = "docs" TESTING_JS_MODULES += [ "schemas/ExperimentFeatureManifest.schema.json", "schemas/ExperimentFeatureRemote.schema.json", - "schemas/NimbusEnrollment.schema.json", "schemas/NimbusExperiment.schema.json", "test/NimbusTestUtils.jsm", ] diff --git a/toolkit/components/nimbus/schemas/ExperimentFeatureRemote.schema.json b/toolkit/components/nimbus/schemas/ExperimentFeatureRemote.schema.json index a43d7d06af10..c4527fc6541e 100644 --- a/toolkit/components/nimbus/schemas/ExperimentFeatureRemote.schema.json +++ b/toolkit/components/nimbus/schemas/ExperimentFeatureRemote.schema.json @@ -79,74 +79,6 @@ }, "required": ["id", "configurations"], "additionalProperties": false - }, - "RemoteFeatureConfiguration": { - "type": "object", - "properties": { - "slug": { - "type": "string", - "description": "Configuration identifier that will be included in Telemetry." - }, - "isEarlyStartup": { - "type": "boolean", - "description": "If the feature values should be cached in prefs for fast early startup." - }, - "variables": { - "type": "object", - "description": "Key value pairs that should match the feature manifest definition.", - "properties": { - "enabled": { - "type": "boolean" - } - }, - "required": ["enabled"] - }, - "targeting": { - "type": "string", - "description": "Target the configuration only to specific clients." - }, - "bucketConfig": { - "type": "object", - "properties": { - "randomizationUnit": { - "type": "string", - "description": "A unique, stable identifier for the user used as an input to bucket hashing" - }, - "namespace": { - "type": "string", - "description": "Additional inputs to the hashing function" - }, - "start": { - "type": "number", - "description": "Index of start of the range of buckets" - }, - "count": { - "type": "number", - "description": "Number of buckets to check" - }, - "total": { - "type": "number", - "description": "Total number of buckets", - "default": 10000 - } - }, - "required": [ - "randomizationUnit", - "namespace", - "start", - "count", - "total" - ], - "additionalProperties": false, - "description": "Bucketing configuration" - }, - "description": { - "type": "string", - "description": "Explanation for configuration and targeting" - } - }, - "required": ["variables", "targeting", "slug"], - "additionalProperties": false } } } diff --git a/toolkit/components/nimbus/schemas/NimbusEnrollment.schema.json b/toolkit/components/nimbus/schemas/NimbusEnrollment.schema.json deleted file mode 100644 index 765e8552b349..000000000000 --- a/toolkit/components/nimbus/schemas/NimbusEnrollment.schema.json +++ /dev/null @@ -1,91 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "$ref": "#/definitions/NimbusExperiment", - "definitions": { - "NimbusExperiment": { - "type": "object", - "properties": { - "slug": { - "type": "string" - }, - "branch": { - "type": "object", - "properties": { - "feature": { - "type": "object", - "properties": { - "featureId": { - "type": "string", - "description": "The identifier for the feature flag" - }, - "value": { - "anyOf": [ - { - "type": "object", - "additionalProperties": {} - }, - { - "type": "null" - } - ], - "description": "Optional extra params for the feature (this should be validated against a schema)" - }, - "enabled": { - "type": "boolean", - "description": "(deprecated)" - } - }, - "required": ["featureId", "value"], - "additionalProperties": false - } - }, - "required": ["feature"] - }, - "active": { - "type": "boolean", - "description": "Experiment status" - }, - "enrollmentId": { - "type": "string", - "description": "Unique identifier used in telemetry" - }, - "experimentType": { - "type": "string" - }, - "isEnrollmentPaused": { - "type": "boolean" - }, - "source": { - "type": "string", - "description": "What triggered the enrollment" - }, - "userFacingName": { - "type": "string" - }, - "userFacingDescription": { - "type": "string" - }, - "lastSeen": { - "type": "string", - "description": "When was the enrollment made" - }, - "force": { - "type": "boolean", - "description": "(debug) If the enrollment happened naturally or through devtools" - } - }, - "required": [ - "slug", - "branch", - "active", - "enrollmentId", - "experimentType", - "source", - "userFacingName", - "userFacingDescription" - ], - "additionalProperties": false, - "description": "The experiment definition accessible to:\n1. The Nimbus SDK via Remote Settings\n2. Jetstream via the Experimenter API" - } - } -} diff --git a/toolkit/components/nimbus/test/NimbusTestUtils.jsm b/toolkit/components/nimbus/test/NimbusTestUtils.jsm index 7e04da404ba7..df1ff7e3ef50 100644 --- a/toolkit/components/nimbus/test/NimbusTestUtils.jsm +++ b/toolkit/components/nimbus/test/NimbusTestUtils.jsm @@ -20,94 +20,47 @@ XPCOMUtils.defineLazyModuleGetters(this, { _RemoteSettingsExperimentLoader: "resource://nimbus/lib/RemoteSettingsExperimentLoader.jsm", Ajv: "resource://testing-common/ajv-4.1.1.js", - sinon: "resource://testing-common/Sinon.jsm", }); const { SYNC_DATA_PREF_BRANCH, SYNC_DEFAULTS_PREF_BRANCH } = ExperimentStore; const PATH = FileTestUtils.getTempFile("shared-data-map").path; -async function fetchSchema(url) { - const response = await fetch(url); +XPCOMUtils.defineLazyGetter(this, "fetchExperimentSchema", async () => { + const response = await fetch( + "resource://testing-common/NimbusExperiment.schema.json" + ); const schema = await response.json(); if (!schema) { - throw new Error(`Failed to load ${url}`); + throw new Error("Failed to load NimbusSchema"); } - return schema.definitions; -} + return schema.definitions.NimbusExperiment; +}); const EXPORTED_SYMBOLS = ["ExperimentTestUtils", "ExperimentFakes"]; const ExperimentTestUtils = { - _validator(schema, value, errorMsg) { - const ajv = new Ajv({ async: "co*", allErrors: true }); - const validator = ajv.compile(schema); - validator(value); - if (validator.errors?.length) { - throw new Error( - `${errorMsg}: ${JSON.stringify(validator.errors, undefined, 2)}` - ); - } - return value; - }, - /** * Checks if an experiment is valid acording to existing schema + * @param {NimbusExperiment} experiment */ async validateExperiment(experiment) { - const schema = ( - await fetchSchema( - "resource://testing-common/NimbusExperiment.schema.json" - ) - ).NimbusExperiment; - - return this._validator( - schema, - experiment, - `Experiment ${experiment.slug} not valid` - ); - }, - async validateEnrollment(enrollment) { - const schema = ( - await fetchSchema( - "resource://testing-common/NimbusEnrollment.schema.json" - ) - ).NimbusExperiment; - - return this._validator( - schema, - enrollment, - `Enrollment ${enrollment.slug} is not valid` - ); - }, - async validateRollouts(rollout) { - const schema = ( - await fetchSchema( - "resource://testing-common/ExperimentFeatureRemote.schema.json" - ) - ).RemoteFeatureConfiguration; - - return this._validator( - schema, - rollout, - `Rollout configuration ${rollout.slug} is not valid` - ); + const schema = await fetchExperimentSchema; + const ajv = new Ajv({ async: "co*", allErrors: true }); + const validator = ajv.compile(schema); + validator(experiment); + if (validator.errors?.length) { + throw new Error( + "Experiment not valid:" + JSON.stringify(validator.errors, undefined, 2) + ); + } + return experiment; }, }; const ExperimentFakes = { manager(store) { - let sandbox = sinon.createSandbox(); - let manager = new _ExperimentManager({ store: store || this.store() }); - // We want calls to `store.addExperiment` to implicitly validate the - // enrollment before saving to store - let origAddExperiment = manager.store.addExperiment.bind(manager.store); - sandbox.stub(manager.store, "addExperiment").callsFake(async enrollment => { - await ExperimentTestUtils.validateEnrollment(enrollment); - return origAddExperiment(enrollment); - }); - - return manager; + return new _ExperimentManager({ store: store || this.store() }); }, store() { return new ExperimentStore("FakeStore", { path: PATH, isParent: true }); @@ -119,7 +72,7 @@ const ExperimentFakes = { return new Promise(resolve => ExperimentAPI.on("update", options, resolve)); }, - async remoteDefaultsHelper({ + remoteDefaultsHelper({ feature, store = ExperimentManager.store, configuration, @@ -127,14 +80,9 @@ const ExperimentFakes = { if (!store._isReady) { throw new Error("Store not ready, need to `await ExperimentAPI.ready()`"); } - - await ExperimentTestUtils.validateRollouts(configuration); - // After storing the remote configuration to store and updating the feature - // we want to flush so that NimbusFeature usage in content process also - // receives the update store.updateRemoteConfigs(feature.featureId, configuration); - await feature.ready(); - store._syncToChildren({ flush: true }); + + return feature.ready().then(() => store._syncToChildren({ flush: true })); }, async enrollWithFeatureConfig( featureConfig, @@ -204,7 +152,7 @@ const ExperimentFakes = { if (!manager.store._isReady) { throw new Error("Manager store not ready, call `manager.onStartup`"); } - manager.enroll(recipe, "enrollmentHelper"); + manager.enroll(recipe); } return { enrollmentPromise, doExperimentCleanup }; @@ -245,11 +193,8 @@ const ExperimentFakes = { }, ...props, }, - source: "NimbusTestUtils", + source: "test", isEnrollmentPaused: true, - experimentType: "NimbusTestUtils", - userFacingName: "NimbusTestUtils", - userFacingDescription: "NimbusTestUtils", ...props, }; }, diff --git a/toolkit/components/nimbus/test/browser/browser_experiment_evaluate_jexl.js b/toolkit/components/nimbus/test/browser/browser_experiment_evaluate_jexl.js index c08ceafa92d9..9ff46b6595d4 100644 --- a/toolkit/components/nimbus/test/browser/browser_experiment_evaluate_jexl.js +++ b/toolkit/components/nimbus/test/browser/browser_experiment_evaluate_jexl.js @@ -75,26 +75,10 @@ add_task(async function test_evaluate_active_experiments_activeExperiments() { const slug = "foo" + Math.random(); // Init the store before we use it await ExperimentManager.onStartup(); - let { - enrollmentPromise, - doExperimentCleanup, - } = ExperimentFakes.enrollmentHelper( - ExperimentFakes.recipe(slug, { - branches: [ - { - slug: "mochitest-active-foo", - feature: { - enabled: true, - featureId: "foo", - value: null, - }, - }, - ], - active: true, - }) - ); - - await enrollmentPromise; + ExperimentManager.store.addExperiment(ExperimentFakes.experiment(slug)); + registerCleanupFunction(() => { + ExperimentManager.store._deleteForTests(slug); + }); Assert.equal( await RemoteSettingsExperimentLoader.evaluateJexl( @@ -113,6 +97,4 @@ add_task(async function test_evaluate_active_experiments_activeExperiments() { false, "should not find an experiment that doesn't exist" ); - - await doExperimentCleanup(); }); diff --git a/toolkit/components/nimbus/test/browser/browser_remotesettingsexperimentloader_init.js b/toolkit/components/nimbus/test/browser/browser_remotesettingsexperimentloader_init.js index 21edaf04c188..273bdb8589e1 100644 --- a/toolkit/components/nimbus/test/browser/browser_remotesettingsexperimentloader_init.js +++ b/toolkit/components/nimbus/test/browser/browser_remotesettingsexperimentloader_init.js @@ -48,10 +48,14 @@ add_task(async function test_double_feature_enrollment() { let enrollPromise1 = ExperimentFakes.waitForExperimentUpdate(ExperimentAPI, { slug: recipe1.slug, }); + let enrollPromise2 = ExperimentFakes.waitForExperimentUpdate(ExperimentAPI, { + slug: recipe2.slug, + }); - ExperimentManager.enroll(recipe1, "test_double_feature_enrollment"); - await enrollPromise1; - ExperimentManager.enroll(recipe2, "test_double_feature_enrollment"); + ExperimentManager.enroll(recipe1); + ExperimentManager.enroll(recipe2); + + await Promise.any([enrollPromise1, enrollPromise2]); Assert.equal( ExperimentManager.store.getAllActive().length, diff --git a/toolkit/components/nimbus/test/browser/browser_remotesettingsexperimentloader_remote_defaults.js b/toolkit/components/nimbus/test/browser/browser_remotesettingsexperimentloader_remote_defaults.js index 25dc54b282ca..98c4f6f31434 100644 --- a/toolkit/components/nimbus/test/browser/browser_remotesettingsexperimentloader_remote_defaults.js +++ b/toolkit/components/nimbus/test/browser/browser_remotesettingsexperimentloader_remote_defaults.js @@ -23,6 +23,9 @@ const { const { BrowserTestUtils } = ChromeUtils.import( "resource://testing-common/BrowserTestUtils.jsm" ); +const { ExperimentFakes } = ChromeUtils.import( + "resource://testing-common/NimbusTestUtils.jsm" +); const { TelemetryEnvironment } = ChromeUtils.import( "resource://gre/modules/TelemetryEnvironment.jsm" ); @@ -33,7 +36,8 @@ const REMOTE_CONFIGURATION_AW = { configurations: [ { slug: "a", - variables: { remoteValue: 24, enabled: false }, + variables: { remoteValue: 24 }, + enabled: false, targeting: "false", bucketConfig: { namespace: "nimbus-test-utils", @@ -45,7 +49,8 @@ const REMOTE_CONFIGURATION_AW = { }, { slug: "b", - variables: { remoteValue: 42, enabled: true }, + variables: { remoteValue: 42 }, + enabled: true, targeting: "true", bucketConfig: { namespace: "nimbus-test-utils", @@ -63,7 +68,8 @@ const REMOTE_CONFIGURATION_NEWTAB = { configurations: [ { slug: "a", - variables: { remoteValue: 1, enabled: false }, + variables: { remoteValue: 1 }, + enabled: false, targeting: "false", bucketConfig: { namespace: "nimbus-test-utils", @@ -75,7 +81,8 @@ const REMOTE_CONFIGURATION_NEWTAB = { }, { slug: "b", - variables: { remoteValue: 3, enabled: true }, + variables: { remoteValue: 3 }, + enabled: true, targeting: "true", bucketConfig: { namespace: "nimbus-test-utils", @@ -87,7 +94,8 @@ const REMOTE_CONFIGURATION_NEWTAB = { }, { slug: "c", - variables: { remoteValue: 2, enabled: false }, + variables: { remoteValue: 2 }, + enabled: false, targeting: "false", bucketConfig: { namespace: "nimbus-test-utils", @@ -479,8 +487,7 @@ add_task(async function remote_defaults_no_mutation() { add_task(async function remote_defaults_active_experiments_check() { let barFeature = new ExperimentFeature("bar", { - description: "mochitest", - variables: { enabled: { type: "boolean" } }, + bar: { description: "mochitest" }, }); let experimentOnlyRemoteDefault = { id: "bar", @@ -488,12 +495,14 @@ add_task(async function remote_defaults_active_experiments_check() { configurations: [ { slug: "a", - variables: { enabled: false }, + variables: {}, + enabled: false, targeting: "'mochitest-active-foo' in activeExperiments", }, { slug: "b", - variables: { enabled: true }, + variables: {}, + enabled: true, targeting: "true", }, ], @@ -541,12 +550,10 @@ add_task(async function remote_defaults_active_remote_defaults() { ExperimentAPI._store._deleteForTests("foo"); ExperimentAPI._store._deleteForTests("bar"); let barFeature = new ExperimentFeature("bar", { - description: "mochitest", - variables: { enabled: { type: "boolean" } }, + bar: { description: "mochitest" }, }); let fooFeature = new ExperimentFeature("foo", { - description: "mochitest", - variables: { enabled: { type: "boolean" } }, + foo: { description: "mochitest" }, }); let remoteDefaults = [ { @@ -555,7 +562,8 @@ add_task(async function remote_defaults_active_remote_defaults() { configurations: [ { slug: "a", - variables: { enabled: true }, + variables: {}, + enabled: true, targeting: "true", }, ], @@ -566,7 +574,8 @@ add_task(async function remote_defaults_active_remote_defaults() { configurations: [ { slug: "b", - variables: { enabled: true }, + variables: {}, + enabled: true, targeting: "'bar' in activeRemoteDefaults", }, ], @@ -638,12 +647,14 @@ add_task(async function test_remote_defaults_no_bucketConfig() { configurations: [ { slug: "a", - variables: { remoteValue: 24, enabled: false }, + variables: { remoteValue: 24 }, + enabled: false, targeting: "false", }, { slug: "b", - variables: { remoteValue: 42, enabled: true }, + variables: { remoteValue: 42 }, + enabled: true, targeting: "true", }, ], @@ -677,23 +688,7 @@ add_task(async function test_remote_defaults_no_bucketConfig() { add_task(async function remote_defaults_variables_storage() { let barFeature = new ExperimentFeature("bar", { - bar: { - description: "mochitest", - variables: { - storage: { - type: "int", - }, - object: { - type: "json", - }, - string: { - type: "string", - }, - bool: { - type: "boolean", - }, - }, - }, + bar: { description: "mochitest" }, }); let remoteDefaults = [ { @@ -708,8 +703,8 @@ add_task(async function remote_defaults_variables_storage() { object: { foo: "foo" }, string: "string", bool: true, - enabled: true, }, + enabled: true, targeting: "true", }, ], @@ -720,10 +715,6 @@ add_task(async function remote_defaults_variables_storage() { await RemoteDefaultsLoader.syncRemoteDefaults("mochitest"); await barFeature.ready(); - Assert.ok( - Services.prefs.getStringPref(`${SYNC_DEFAULTS_PREF_BRANCH}bar`, ""), - "Experiment stored in prefs" - ); Assert.ok( Services.prefs.getIntPref(`${SYNC_DEFAULTS_PREF_BRANCH}bar.storage`, 0), "Stores variable in separate pref" diff --git a/toolkit/components/nimbus/test/browser/head.js b/toolkit/components/nimbus/test/browser/head.js index 71343d8a7ede..32bb9fd89aed 100644 --- a/toolkit/components/nimbus/test/browser/head.js +++ b/toolkit/components/nimbus/test/browser/head.js @@ -5,55 +5,3 @@ // Globals const { sinon } = ChromeUtils.import("resource://testing-common/Sinon.jsm"); -const { XPCOMUtils } = ChromeUtils.import( - "resource://gre/modules/XPCOMUtils.jsm" -); -XPCOMUtils.defineLazyModuleGetters(this, { - ExperimentManager: "resource://nimbus/lib/ExperimentManager.jsm", - Ajv: "resource://testing-common/ajv-4.1.1.js", - ExperimentTestUtils: "resource://testing-common/NimbusTestUtils.jsm", - RemoteDefaultsLoader: - "resource://nimbus/lib/RemoteSettingsExperimentLoader.jsm", - ExperimentFakes: "resource://testing-common/NimbusTestUtils.jsm", -}); - -add_task(function setup() { - 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.addExperiment.bind( - ExperimentManager.store - ); - let origOnUpdatesReady = RemoteDefaultsLoader._onUpdatesReady.bind( - RemoteDefaultsLoader - ); - sandbox - .stub(ExperimentManager.store, "addExperiment") - .callsFake(async enrollment => { - await ExperimentTestUtils.validateEnrollment(enrollment); - return origAddExperiment(enrollment); - }); - // Unlike `addExperiment` the method to store remote rollouts is syncronous - // and our validation method would turn it async. If we had changed to `await` - // for remote configs storage it would have changed the code logic so we are - // going up one level to the function that receives the RS records and do - // the validation there. - sandbox - .stub(RemoteDefaultsLoader, "_onUpdatesReady") - .callsFake(async (remoteDefaults, reason) => { - for (let remoteDefault of remoteDefaults) { - for (let config of remoteDefault.configurations) { - await ExperimentTestUtils.validateRollouts(config); - } - } - return origOnUpdatesReady(remoteDefaults, reason); - }); - - registerCleanupFunction(() => { - sandbox.restore(); - }); -}); diff --git a/toolkit/components/nimbus/test/unit/head.js b/toolkit/components/nimbus/test/unit/head.js index 69bed6f153d6..085386dc49e1 100644 --- a/toolkit/components/nimbus/test/unit/head.js +++ b/toolkit/components/nimbus/test/unit/head.js @@ -3,9 +3,3 @@ const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm"); const { sinon } = ChromeUtils.import("resource://testing-common/Sinon.jsm"); -const { XPCOMUtils } = ChromeUtils.import( - "resource://gre/modules/XPCOMUtils.jsm" -); -XPCOMUtils.defineLazyModuleGetters(this, { - ExperimentFakes: "resource://testing-common/NimbusTestUtils.jsm", -}); diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentAPI.js b/toolkit/components/nimbus/test/unit/test_ExperimentAPI.js index dfe31c35756d..abddc3784a90 100644 --- a/toolkit/components/nimbus/test/unit/test_ExperimentAPI.js +++ b/toolkit/components/nimbus/test/unit/test_ExperimentAPI.js @@ -27,7 +27,7 @@ add_task(async function test_getExperiment_fromChild_slug() { sandbox.stub(ExperimentAPI, "_store").get(() => ExperimentFakes.childStore()); - await manager.store.addExperiment(expected); + manager.store.addExperiment(expected); // Wait to sync to child await TestUtils.waitForCondition( @@ -59,7 +59,7 @@ add_task(async function test_getExperiment_fromParent_slug() { sandbox.stub(ExperimentAPI, "_store").get(() => manager.store); await ExperimentAPI.ready(); - await manager.store.addExperiment(expected); + manager.store.addExperiment(expected); Assert.equal( ExperimentAPI.getExperiment({ slug: "foo" }).slug, @@ -80,7 +80,7 @@ add_task(async function test_getExperimentMetaData() { sandbox.stub(ExperimentAPI, "_store").get(() => manager.store); await ExperimentAPI.ready(); - await manager.store.addExperiment(expected); + manager.store.addExperiment(expected); let metadata = ExperimentAPI.getExperimentMetaData({ slug: expected.slug }); @@ -140,7 +140,7 @@ add_task(async function test_getExperiment_feature() { branch: { slug: "treatment", value: { title: "hi" }, - feature: { featureId: "cfr", enabled: true, value: null }, + feature: { featureId: "cfr", enabled: true }, }, }); @@ -149,7 +149,7 @@ add_task(async function test_getExperiment_feature() { sandbox.stub(ExperimentAPI, "_store").get(() => ExperimentFakes.childStore()); let exposureStub = sandbox.stub(ExperimentAPI, "recordExposureEvent"); - await manager.store.addExperiment(expected); + manager.store.addExperiment(expected); // Wait to sync to child await TestUtils.waitForCondition( @@ -268,7 +268,7 @@ add_task(async function test_addExperiment_eventEmit_add() { const experiment = ExperimentFakes.experiment("foo", { branch: { slug: "variant", - feature: { featureId: "purple", enabled: true, value: null }, + feature: { featureId: "purple", enabled: true }, }, }); const store = ExperimentFakes.store(); @@ -280,7 +280,7 @@ add_task(async function test_addExperiment_eventEmit_add() { ExperimentAPI.on("update", { slug: "foo" }, slugStub); ExperimentAPI.on("update", { featureId: "purple" }, featureStub); - await store.addExperiment(experiment); + store.addExperiment(experiment); Assert.equal( slugStub.callCount, @@ -303,7 +303,7 @@ add_task(async function test_updateExperiment_eventEmit_add_and_update() { const experiment = ExperimentFakes.experiment("foo", { branch: { slug: "variant", - feature: { featureId: "purple", enabled: true, value: null }, + feature: { featureId: "purple", enabled: true }, }, }); const store = ExperimentFakes.store(); @@ -312,7 +312,7 @@ add_task(async function test_updateExperiment_eventEmit_add_and_update() { await store.init(); await ExperimentAPI.ready(); - await store.addExperiment(experiment); + store.addExperiment(experiment); ExperimentAPI.on("update", { slug: "foo" }, slugStub); ExperimentAPI.on("update", { featureId: "purple" }, featureStub); @@ -337,7 +337,7 @@ add_task(async function test_updateExperiment_eventEmit_off() { const experiment = ExperimentFakes.experiment("foo", { branch: { slug: "variant", - feature: { featureId: "purple", enabled: true, value: null }, + feature: { featureId: "purple", enabled: true }, }, }); const store = ExperimentFakes.store(); @@ -349,7 +349,7 @@ add_task(async function test_updateExperiment_eventEmit_off() { ExperimentAPI.on("update", { slug: "foo" }, slugStub); ExperimentAPI.on("update", { featureId: "purple" }, featureStub); - await store.addExperiment(experiment); + store.addExperiment(experiment); ExperimentAPI.off("update:foo", slugStub); ExperimentAPI.off("update:purple", featureStub); @@ -367,12 +367,12 @@ add_task(async function test_activateBranch() { const experiment = ExperimentFakes.experiment("foo", { branch: { slug: "variant", - feature: { featureId: "green", enabled: true, value: null }, + feature: { featureId: "green", enabled: true }, }, }); await store.init(); - await store.addExperiment(experiment); + store.addExperiment(experiment); Assert.deepEqual( ExperimentAPI.activateBranch({ featureId: "green" }), @@ -412,7 +412,7 @@ add_task(async function test_activateBranch_activationEvent() { }); await store.init(); - await store.addExperiment(experiment); + store.addExperiment(experiment); // Adding stub later because `addExperiment` emits update events const stub = sandbox.stub(ExperimentAPI, "recordExposureEvent"); ExperimentAPI.activateBranch({ featureId: "green" }); @@ -449,7 +449,7 @@ add_task(async function test_activateBranch_storeFailure() { }); await store.init(); - await store.addExperiment(experiment); + store.addExperiment(experiment); // Adding stub later because `addExperiment` emits update events const stub = sandbox.stub(store, "emit"); // Call activateBranch to trigger an activation event @@ -476,7 +476,7 @@ add_task(async function test_activateBranch_noActivationEvent() { }); await store.init(); - await store.addExperiment(experiment); + store.addExperiment(experiment); // Adding stub later because `addExperiment` emits update events const stub = sandbox.stub(store, "emit"); // Call activateBranch to trigger an activation event diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature.js b/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature.js index 9975392c18c6..012bb73e1279 100644 --- a/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature.js +++ b/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature.js @@ -40,7 +40,6 @@ const FAKE_FEATURE_MANIFEST = { }, }; const FAKE_FEATURE_REMOTE_VALUE = { - slug: "default-remote-value", variables: { enabled: true, }, @@ -69,7 +68,7 @@ add_task(async function test_ExperimentFeature_ready() { }, }); - await manager.store.addExperiment(expected); + manager.store.addExperiment(expected); await readyPromise; @@ -122,7 +121,7 @@ add_task( }, }); - await manager.store.addExperiment(expected); + manager.store.addExperiment(expected); setDefaultBranch(TEST_FALLBACK_PREF, `{"bar": 123}`); @@ -227,10 +226,7 @@ add_task(async function test_ExperimentFeature_test_helper_ready() { await ExperimentFakes.remoteDefaultsHelper({ feature: featureInstance, store: manager.store, - configuration: { - ...FAKE_FEATURE_REMOTE_VALUE, - variables: { remoteValue: "mochitest", enabled: true }, - }, + configuration: { variables: { remoteValue: "mochitest" }, enabled: true }, }); Assert.equal(featureInstance.isEnabled(), true, "enabled by remote config"); @@ -257,7 +253,7 @@ add_task( await manager.store.ready(); - await manager.store.addExperiment(expected); + manager.store.addExperiment(expected); const exposureSpy = sandbox.spy(ExperimentAPI, "recordExposureEvent"); manager.store.updateRemoteConfigs("foo", { @@ -309,7 +305,7 @@ add_task(async function test_ExperimentFeature_isEnabled_no_exposure() { sandbox.stub(ExperimentAPI, "_store").get(() => manager.store); - await manager.store.addExperiment(expected); + manager.store.addExperiment(expected); const exposureSpy = sandbox.spy(ExperimentAPI, "recordExposureEvent"); @@ -338,7 +334,7 @@ add_task(async function test_record_exposure_event() { "should not emit an exposure event when no experiment is active" ); - await manager.store.addExperiment( + manager.store.addExperiment( ExperimentFakes.experiment("blah", { branch: { slug: "treatment", @@ -367,7 +363,7 @@ add_task(async function test_record_exposure_event_once() { const exposureSpy = sandbox.spy(ExperimentAPI, "recordExposureEvent"); sandbox.stub(ExperimentAPI, "_store").get(() => manager.store); - await manager.store.addExperiment( + manager.store.addExperiment( ExperimentFakes.experiment("blah", { branch: { slug: "treatment", @@ -395,7 +391,7 @@ add_task(async function test_prevent_double_exposure_getValue() { const exposureSpy = sandbox.spy(ExperimentAPI, "recordExposureEvent"); sandbox.stub(ExperimentAPI, "_store").get(() => manager.store); - await manager.store.addExperiment( + manager.store.addExperiment( ExperimentFakes.experiment("blah", { branch: { slug: "treatment", @@ -426,7 +422,7 @@ add_task(async function test_prevent_double_exposure_isEnabled() { const exposureSpy = sandbox.spy(ExperimentAPI, "recordExposureEvent"); sandbox.stub(ExperimentAPI, "_store").get(() => manager.store); - await manager.store.addExperiment( + manager.store.addExperiment( ExperimentFakes.experiment("blah", { branch: { slug: "treatment", @@ -456,15 +452,13 @@ add_task(async function test_set_remote_before_ready() { sandbox.stub(ExperimentAPI, "_store").get(() => manager.store); const feature = new ExperimentFeature("foo", FAKE_FEATURE_MANIFEST); - await Assert.rejects( - ExperimentFakes.remoteDefaultsHelper({ - feature, - store: manager.store, - configuration: { - ...FAKE_FEATURE_REMOTE_VALUE, - variables: { test: true, enabled: true }, - }, - }), + Assert.throws( + () => + ExperimentFakes.remoteDefaultsHelper({ + feature, + store: manager.store, + configuration: { variables: { test: true } }, + }), /Store not ready/, "Throws if used before init finishes" ); @@ -474,10 +468,7 @@ add_task(async function test_set_remote_before_ready() { await ExperimentFakes.remoteDefaultsHelper({ feature, store: manager.store, - configuration: { - ...FAKE_FEATURE_REMOTE_VALUE, - variables: { test: true, enabled: true }, - }, + configuration: { variables: { test: true } }, }); Assert.ok(feature.getValue().test, "Successfully set"); @@ -503,15 +494,12 @@ add_task(async function test_isEnabled_backwards_compatible() { await ExperimentFakes.remoteDefaultsHelper({ feature, store: manager.store, - configuration: { - ...FAKE_FEATURE_REMOTE_VALUE, - variables: { enabled: false }, - }, + configuration: { variables: {}, enabled: false }, }); Assert.ok(!feature.isEnabled(), "Disabled based on remote configs"); - await manager.store.addExperiment( + manager.store.addExperiment( ExperimentFakes.experiment("blah", { branch: { slug: "treatment", diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature_getAllVariables.js b/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature_getAllVariables.js index add55ed01817..d6b6ebff3536 100644 --- a/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature_getAllVariables.js +++ b/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature_getAllVariables.js @@ -11,6 +11,10 @@ const { TestUtils } = ChromeUtils.import( "resource://testing-common/TestUtils.jsm" ); +const { XPCOMUtils } = ChromeUtils.import( + "resource://gre/modules/XPCOMUtils.jsm" +); + const { cleanupStorePrefCache } = ExperimentFakes; async function setupForExperimentFeature() { @@ -84,7 +88,7 @@ add_task( }, }); - await manager.store.addExperiment(recipe); + manager.store.addExperiment(recipe); const featureInstance = new ExperimentFeature( FEATURE_ID, diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature_getValue.js b/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature_getValue.js index 7a2d69eaa17d..1c46ecb3641d 100644 --- a/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature_getValue.js +++ b/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature_getValue.js @@ -11,6 +11,10 @@ const { TestUtils } = ChromeUtils.import( "resource://testing-common/TestUtils.jsm" ); +const { XPCOMUtils } = ChromeUtils.import( + "resource://gre/modules/XPCOMUtils.jsm" +); + const { cleanupStorePrefCache } = ExperimentFakes; async function setupForExperimentFeature() { @@ -78,7 +82,7 @@ add_task(async function test_ExperimentFeature_getValue_prefsOverExperiment() { }, }); - await manager.store.addExperiment(recipe); + manager.store.addExperiment(recipe); const featureInstance = new ExperimentFeature( FEATURE_ID, diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature_getVariable.js b/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature_getVariable.js index 47edc464be39..235a5d1039e4 100644 --- a/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature_getVariable.js +++ b/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature_getVariable.js @@ -4,9 +4,15 @@ const { ExperimentAPI, _ExperimentFeature: ExperimentFeature, } = ChromeUtils.import("resource://nimbus/ExperimentAPI.jsm"); +const { ExperimentFakes } = ChromeUtils.import( + "resource://testing-common/NimbusTestUtils.jsm" +); const { TestUtils } = ChromeUtils.import( "resource://testing-common/TestUtils.jsm" ); +const { XPCOMUtils } = ChromeUtils.import( + "resource://gre/modules/XPCOMUtils.jsm" +); const { AppConstants } = ChromeUtils.import( "resource://gre/modules/AppConstants.jsm" ); diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentAPI_NimbusFeatures.js b/toolkit/components/nimbus/test/unit/test_ExperimentAPI_NimbusFeatures.js index 410caaa11e56..a14479c95085 100644 --- a/toolkit/components/nimbus/test/unit/test_ExperimentAPI_NimbusFeatures.js +++ b/toolkit/components/nimbus/test/unit/test_ExperimentAPI_NimbusFeatures.js @@ -5,11 +5,17 @@ const { NimbusFeatures, _ExperimentFeature: ExperimentFeature, } = ChromeUtils.import("resource://nimbus/ExperimentAPI.jsm"); +const { ExperimentFakes } = ChromeUtils.import( + "resource://testing-common/NimbusTestUtils.jsm" +); const { TestUtils } = ChromeUtils.import( "resource://testing-common/TestUtils.jsm" ); -const { Ajv } = ChromeUtils.import("resource://testing-common/ajv-4.1.1.js"); +const { Ajv } = ChromeUtils.import("resource://testing-common/ajv-4.1.1.js"); +const { XPCOMUtils } = ChromeUtils.import( + "resource://gre/modules/XPCOMUtils.jsm" +); Cu.importGlobalProperties(["fetch"]); XPCOMUtils.defineLazyGetter(this, "fetchSchema", async () => { diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentManager_enroll.js b/toolkit/components/nimbus/test/unit/test_ExperimentManager_enroll.js index 28d822a21bb4..59b6b40958c7 100644 --- a/toolkit/components/nimbus/test/unit/test_ExperimentManager_enroll.js +++ b/toolkit/components/nimbus/test/unit/test_ExperimentManager_enroll.js @@ -1,5 +1,8 @@ "use strict"; +const { ExperimentFakes } = ChromeUtils.import( + "resource://testing-common/NimbusTestUtils.jsm" +); const { NormandyTestUtils } = ChromeUtils.import( "resource://testing-common/NormandyTestUtils.jsm" ); @@ -28,14 +31,10 @@ const { SYNC_DATA_PREF_BRANCH } = ExperimentStore; add_task(async function test_add_to_store() { const manager = ExperimentFakes.manager(); const recipe = ExperimentFakes.recipe("foo"); - const enrollPromise = new Promise(resolve => - manager.store.on("update:foo", resolve) - ); await manager.onStartup(); - await manager.enroll(recipe, "test_add_to_store"); - await enrollPromise; + await manager.enroll(recipe); const experiment = manager.store.get("foo"); Assert.ok(experiment, "should add an experiment with slug foo"); @@ -54,9 +53,6 @@ add_task( async function test_setExperimentActive_sendEnrollmentTelemetry_called() { const manager = ExperimentFakes.manager(); const sandbox = sinon.createSandbox(); - const enrollPromise = new Promise(resolve => - manager.store.on("update:foo", resolve) - ); sandbox.spy(manager, "setExperimentActive"); sandbox.spy(manager, "sendEnrollmentTelemetry"); @@ -64,11 +60,7 @@ add_task( await manager.onStartup(); - await manager.enroll( - ExperimentFakes.recipe("foo"), - "test_setExperimentActive_sendEnrollmentTelemetry_called" - ); - await enrollPromise; + await manager.enroll(ExperimentFakes.recipe("foo")); const experiment = manager.store.get("foo"); Assert.equal( @@ -99,10 +91,10 @@ add_task(async function test_failure_name_conflict() { await manager.onStartup(); // simulate adding a previouly enrolled experiment - await manager.store.addExperiment(ExperimentFakes.experiment("foo")); + manager.store.addExperiment(ExperimentFakes.experiment("foo")); await Assert.rejects( - manager.enroll(ExperimentFakes.recipe("foo"), "test_failure_name_conflict"), + manager.enroll(ExperimentFakes.recipe("foo")), /An experiment with the slug "foo" already exists/, "should throw if a conflicting experiment exists" ); @@ -129,15 +121,15 @@ add_task(async function test_failure_group_conflict() { // These should not be allowed to exist simultaneously. const existingBranch = { slug: "treatment", - feature: { featureId: "pink", enabled: true, value: {} }, + feature: { featureId: "pink", enabled: true }, }; const newBranch = { slug: "treatment", - feature: { featureId: "pink", enabled: true, value: {} }, + feature: { featureId: "pink", enabled: true }, }; // simulate adding an experiment with a conflicting group "pink" - await manager.store.addExperiment( + manager.store.addExperiment( ExperimentFakes.experiment("foo", { branch: existingBranch, }) @@ -147,8 +139,7 @@ add_task(async function test_failure_group_conflict() { sandbox.stub(manager, "chooseBranch").returns(newBranch); Assert.equal( await manager.enroll( - ExperimentFakes.recipe("bar", { branches: [newBranch] }), - "test_failure_group_conflict" + ExperimentFakes.recipe("bar", { branches: [newBranch] }) ), null, "should not enroll if there is a feature conflict" @@ -241,12 +232,8 @@ add_task(async function enroll_in_reference_aw_experiment() { recipe.bucketConfig.count = recipe.bucketConfig.total; const manager = ExperimentFakes.manager(); - const enrollPromise = new Promise(resolve => - manager.store.on("update:reference-aw", resolve) - ); await manager.onStartup(); - await manager.enroll(recipe, "enroll_in_reference_aw_experiment"); - await enrollPromise; + await manager.enroll(recipe); Assert.ok(manager.store.get("reference-aw"), "Successful onboarding"); let prefValue = Services.prefs.getStringPref( @@ -264,19 +251,13 @@ add_task(async function enroll_in_reference_aw_experiment() { add_task(async function test_forceEnroll_cleanup() { const manager = ExperimentFakes.manager(); const sandbox = sinon.createSandbox(); - const fooEnrollPromise = new Promise(resolve => - manager.store.on("update:foo", resolve) - ); - const barEnrollPromise = new Promise(resolve => - manager.store.on("update:optin-bar", resolve) - ); let unenrollStub = sandbox.spy(manager, "unenroll"); let existingRecipe = ExperimentFakes.recipe("foo", { branches: [ { slug: "treatment", ratio: 1, - feature: { featureId: "force-enrollment", enabled: true, value: {} }, + feature: { featureId: "force-enrollment", enabled: true }, }, ], }); @@ -285,18 +266,16 @@ add_task(async function test_forceEnroll_cleanup() { { slug: "treatment", ratio: 1, - feature: { featureId: "force-enrollment", enabled: true, value: {} }, + feature: { featureId: "force-enrollment", enabled: true }, }, ], }); await manager.onStartup(); - await manager.enroll(existingRecipe, "test_forceEnroll_cleanup"); - await fooEnrollPromise; + await manager.enroll(existingRecipe); let setExperimentActiveSpy = sandbox.spy(manager, "setExperimentActive"); manager.forceEnroll(forcedRecipe, forcedRecipe.branches[0]); - await barEnrollPromise; Assert.ok(unenrollStub.called, "Unenrolled from existing experiment"); Assert.equal( diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentManager_lifecycle.js b/toolkit/components/nimbus/test/unit/test_ExperimentManager_lifecycle.js index f84744581de5..432d305abcf4 100644 --- a/toolkit/components/nimbus/test/unit/test_ExperimentManager_lifecycle.js +++ b/toolkit/components/nimbus/test/unit/test_ExperimentManager_lifecycle.js @@ -6,6 +6,9 @@ const { _ExperimentManager } = ChromeUtils.import( const { ExperimentStore } = ChromeUtils.import( "resource://nimbus/lib/ExperimentStore.jsm" ); +const { ExperimentFakes } = ChromeUtils.import( + "resource://testing-common/NimbusTestUtils.jsm" +); const { Sampling } = ChromeUtils.import( "resource://gre/modules/components-utils/Sampling.jsm" ); @@ -160,13 +163,8 @@ add_task(async function test_onRecipe_isEnrollmentPaused() { const updatedRecipe = ExperimentFakes.recipe("foo", { isEnrollmentPaused: true, }); - let enrollmentPromise = new Promise(resolve => - manager.store.on(`update:${fooRecipe.slug}`, resolve) - ); - await manager.enroll(fooRecipe, "test"); - await enrollmentPromise; + await manager.enroll(fooRecipe); await manager.onRecipe(updatedRecipe, "test"); - console.log("XXX", manager.updateEnrollment.callCount); Assert.equal( manager.updateEnrollment.calledWith(updatedRecipe), true, @@ -188,15 +186,10 @@ add_task(async function test_onFinalize_unenroll() { // Add an experiment to the store without calling .onRecipe // This simulates an enrollment having happened in the past. - let recipe0 = ExperimentFakes.experiment("foo", { - experimentType: "unittest", - userFacingName: "foo", - userFacingDescription: "foo", - lastSeen: Date.now().toLocaleString(), - source: "test", - }); - await manager.store.addExperiment(recipe0); + manager.store.addExperiment(ExperimentFakes.experiment("foo")); + // Simulate adding some other recipes + await manager.onStartup(); const recipe1 = ExperimentFakes.recipe("bar"); // Unique features to prevent overlap recipe1.branches[0].feature.featureId = "red"; diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentManager_unenroll.js b/toolkit/components/nimbus/test/unit/test_ExperimentManager_unenroll.js index 3b4a262dc4a5..c3eb1e253a78 100644 --- a/toolkit/components/nimbus/test/unit/test_ExperimentManager_unenroll.js +++ b/toolkit/components/nimbus/test/unit/test_ExperimentManager_unenroll.js @@ -1,5 +1,8 @@ "use strict"; +const { ExperimentFakes } = ChromeUtils.import( + "resource://testing-common/NimbusTestUtils.jsm" +); const { NormandyTestUtils } = ChromeUtils.import( "resource://testing-common/NormandyTestUtils.jsm" ); @@ -28,7 +31,7 @@ add_task(async function test_set_inactive() { const manager = ExperimentFakes.manager(); await manager.onStartup(); - await manager.store.addExperiment(ExperimentFakes.experiment("foo")); + manager.store.addExperiment(ExperimentFakes.experiment("foo")); manager.unenroll("foo", "some-reason"); @@ -46,7 +49,7 @@ add_task(async function test_unenroll_opt_out() { const experiment = ExperimentFakes.experiment("foo"); await manager.onStartup(); - await manager.store.addExperiment(experiment); + manager.store.addExperiment(experiment); Services.prefs.setBoolPref(STUDIES_OPT_OUT_PREF, false); @@ -80,7 +83,7 @@ add_task(async function test_setExperimentInactive_called() { const experiment = ExperimentFakes.experiment("foo"); await manager.onStartup(); - await manager.store.addExperiment(experiment); + manager.store.addExperiment(experiment); manager.unenroll("foo", "some-reason"); @@ -96,7 +99,7 @@ add_task(async function test_send_unenroll_event() { const experiment = ExperimentFakes.experiment("foo"); await manager.onStartup(); - await manager.store.addExperiment(experiment); + manager.store.addExperiment(experiment); manager.unenroll("foo", "some-reason"); @@ -123,7 +126,7 @@ add_task(async function test_undefined_reason() { const experiment = ExperimentFakes.experiment("foo"); await manager.onStartup(); - await manager.store.addExperiment(experiment); + manager.store.addExperiment(experiment); manager.unenroll("foo"); diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentStore.js b/toolkit/components/nimbus/test/unit/test_ExperimentStore.js index ca6fe06a43bf..44a6c00c5f72 100644 --- a/toolkit/components/nimbus/test/unit/test_ExperimentStore.js +++ b/toolkit/components/nimbus/test/unit/test_ExperimentStore.js @@ -1,5 +1,8 @@ "use strict"; +const { ExperimentFakes } = ChromeUtils.import( + "resource://testing-common/NimbusTestUtils.jsm" +); const { ExperimentStore } = ChromeUtils.import( "resource://nimbus/lib/ExperimentStore.jsm" ); diff --git a/toolkit/components/nimbus/test/unit/test_FeatureManifest.js b/toolkit/components/nimbus/test/unit/test_FeatureManifest.js index 8862031ae598..2eb3d39c6200 100644 --- a/toolkit/components/nimbus/test/unit/test_FeatureManifest.js +++ b/toolkit/components/nimbus/test/unit/test_FeatureManifest.js @@ -4,6 +4,9 @@ const { FeatureManifest } = ChromeUtils.import( "resource://nimbus/FeatureManifest.js" ); const { Ajv } = ChromeUtils.import("resource://testing-common/ajv-4.1.1.js"); +const { XPCOMUtils } = ChromeUtils.import( + "resource://gre/modules/XPCOMUtils.jsm" +); Cu.importGlobalProperties(["fetch"]); XPCOMUtils.defineLazyGetter(this, "fetchSchema", async () => { diff --git a/toolkit/components/nimbus/test/unit/test_SharedDataMap.js b/toolkit/components/nimbus/test/unit/test_SharedDataMap.js index ce37564ace82..1597d38bd655 100644 --- a/toolkit/components/nimbus/test/unit/test_SharedDataMap.js +++ b/toolkit/components/nimbus/test/unit/test_SharedDataMap.js @@ -7,6 +7,9 @@ const { FileTestUtils } = ChromeUtils.import( const { TestUtils } = ChromeUtils.import( "resource://testing-common/TestUtils.jsm" ); +const { ExperimentFakes } = ChromeUtils.import( + "resource://testing-common/NimbusTestUtils.jsm" +); const PATH = FileTestUtils.getTempFile("shared-data-map").path;