diff --git a/toolkit/components/nimbus/lib/ExperimentManager.sys.mjs b/toolkit/components/nimbus/lib/ExperimentManager.sys.mjs index 00c8d732ed52..165309d17b2c 100644 --- a/toolkit/components/nimbus/lib/ExperimentManager.sys.mjs +++ b/toolkit/components/nimbus/lib/ExperimentManager.sys.mjs @@ -482,6 +482,10 @@ export class ExperimentManager { * as `recipe` and re-enrollment is prevented. */ async enroll(recipe, source, { reenroll = false, branchSlug } = {}) { + if (typeof source !== "string") { + throw new Error("source is required"); + } + let { slug, branches, bucketConfig, isFirefoxLabsOptIn } = recipe; const enrollment = this.store.get(slug); @@ -573,11 +577,11 @@ export class ExperimentManager { } } - return this._enroll(recipe, branch, source); + return this._enroll(recipe, branch.slug, source); } - async _enroll( - { + async _enroll(recipe, branchSlug, source) { + const { slug, userFacingName, userFacingDescription, @@ -590,10 +594,9 @@ export class ExperimentManager { firefoxLabsDescriptionLinks = null, firefoxLabsGroup, requiresRestart = false, - }, - branch, - source - ) { + } = recipe; + + const branch = recipe.branches.find(b => b.slug === branchSlug); const { prefs, prefsToSet } = this._getPrefsForBranch(branch, isRollout); // Unenroll in any conflicting prefFlips enrollments. @@ -604,7 +607,6 @@ export class ExperimentManager { ); } - /** @type {Enrollment} */ const enrollment = { slug, branch, @@ -634,13 +636,15 @@ export class ExperimentManager { } await this._prefFlips._annotateEnrollment(enrollment); - this.store.addEnrollment(enrollment); - lazy.NimbusTelemetry.recordEnrollment(enrollment); + await this.store._addEnrollmentToDatabase(enrollment, recipe); + this.store.addEnrollment(enrollment); this._setEnrollmentPrefs(prefsToSet); this._updatePrefObservers(enrollment); + lazy.NimbusTelemetry.recordEnrollment(enrollment); + lazy.log.debug( `New ${isRollout ? "rollout" : "experiment"} started: ${slug}, ${ branch.slug @@ -686,7 +690,7 @@ export class ExperimentManager { ...recipe, slug, }, - branch, + branch.slug, lazy.NimbusTelemetry.EnrollmentSource.FORCE_ENROLLMENT ); @@ -872,6 +876,17 @@ export class ExperimentManager { ); } + // TODO(bug 1956082): This is an async method that we are not awaiting. + // + // Changing the entire unenrollment flow to be asynchronous requires changes + // to a lot of tests and it only really matters once we're actually checking + // the database contents. + // + // For now, we're going to return the promise which will make unenroll() + // awaitable in the few contexts that need to synchronize reads and writes + // right now (i.e., tests). + await this.store._deactivateEnrollmentInDatabase(slug, cause.reason); + this.store.updateExperiment(slug, { active: false, unenrollReason: cause.reason, diff --git a/toolkit/components/nimbus/lib/ExperimentStore.sys.mjs b/toolkit/components/nimbus/lib/ExperimentStore.sys.mjs index 65172e586ec2..a943ceb16156 100644 --- a/toolkit/components/nimbus/lib/ExperimentStore.sys.mjs +++ b/toolkit/components/nimbus/lib/ExperimentStore.sys.mjs @@ -7,8 +7,11 @@ import { SharedDataMap } from "resource://nimbus/lib/SharedDataMap.sys.mjs"; const lazy = {}; ChromeUtils.defineESModuleGetters(lazy, { + ExperimentAPI: "resource://nimbus/ExperimentAPI.sys.mjs", NimbusFeatures: "resource://nimbus/ExperimentAPI.sys.mjs", PrefUtils: "resource://normandy/lib/PrefUtils.sys.mjs", + ProfilesDatastoreService: + "moz-src:///toolkit/profile/ProfilesDatastoreService.sys.mjs", }); // This branch is used to store experiment data @@ -470,4 +473,103 @@ export class ExperimentStore extends SharedDataMap { lazy.syncDataStore.deleteDefault(slugOrFeatureId); lazy.syncDataStore.delete(slugOrFeatureId); } + + async _addEnrollmentToDatabase(enrollment, recipe) { + if ( + !Services.prefs.getBoolPref( + "nimbus.profilesdatastoreservice.enabled", + false + ) + ) { + // We are in an xpcshell test that has not initialized the + // ProfilesDatastoreService. + // + // TODO(bug 1967779): require the ProfilesDatastoreService to be initialized + // and remove this check. + return; + } + + const profileId = lazy.ExperimentAPI.profileId; + + const conn = await lazy.ProfilesDatastoreService.getConnection(); + await conn.execute( + ` + INSERT INTO NimbusEnrollments VALUES( + null, + :profileId, + :slug, + :branchSlug, + jsonb(:recipe), + :active, + :unenrollReason, + :lastSeen, + jsonb(:setPrefs), + jsonb(:prefFlips), + :source + ) + ON CONFLICT(profileId, slug) + DO UPDATE SET + branchSlug = excluded.branchSlug, + recipe = excluded.recipe, + active = excluded.active, + unenrollReason = excluded.unenrollReason, + lastSeen = excluded.lastSeen, + setPrefs = excluded.setPrefs, + prefFlips = excluded.setPrefs, + source = excluded.source; + `, + { + profileId, + slug: enrollment.slug, + branchSlug: enrollment.branch.slug, + recipe: recipe ? JSON.stringify(recipe) : null, + active: enrollment.active, + unenrollReason: null, + lastSeen: enrollment.lastSeen, + setPrefs: enrollment.prefs ? JSON.stringify(enrollment.prefs) : null, + prefFlips: enrollment.prefFlips + ? JSON.stringify(enrollment.prefFlips) + : null, + source: enrollment.source, + } + ); + } + + async _deactivateEnrollmentInDatabase(slug, unenrollReason = "unknown") { + if ( + !Services.prefs.getBoolPref( + "nimbus.profilesdatastoreservice.enabled", + false + ) + ) { + // We are in an xpcshell test that has not initialized the + // ProfilesDatastoreService. + // + // TODO(bug 1967779): require the ProfilesDatastoreService to be initialized + // and remove this check. + return; + } + + const profileId = lazy.ExperimentAPI.profileId; + + const conn = await lazy.ProfilesDatastoreService.getConnection(); + await conn.execute( + ` + UPDATE NimbusEnrollments SET + active = false, + unenrollReason = :unenrollReason, + recipe = null, + prefFlips = null, + setPrefs = null + WHERE + profileId = :profileId AND + slug = :slug; + `, + { + slug, + profileId, + unenrollReason, + } + ); + } } diff --git a/toolkit/components/nimbus/lib/PrefFlipsFeature.sys.mjs b/toolkit/components/nimbus/lib/PrefFlipsFeature.sys.mjs index 1fc4d18f980e..a8dce652bcf1 100644 --- a/toolkit/components/nimbus/lib/PrefFlipsFeature.sys.mjs +++ b/toolkit/components/nimbus/lib/PrefFlipsFeature.sys.mjs @@ -518,7 +518,7 @@ export class PrefFlipsFeature { const entry = this.#prefs.get(pref); if (entry.branch !== branch || entry.value !== value) { - throw new PrefFlipsFailedError(pref); + throw new PrefFlipsFailedError(pref, value); } entry.slugs.add(slug); diff --git a/toolkit/components/nimbus/test/NimbusTestUtils.sys.mjs b/toolkit/components/nimbus/test/NimbusTestUtils.sys.mjs index 891a94bc1ac6..c6354871cd15 100644 --- a/toolkit/components/nimbus/test/NimbusTestUtils.sys.mjs +++ b/toolkit/components/nimbus/test/NimbusTestUtils.sys.mjs @@ -16,10 +16,12 @@ ChromeUtils.defineESModuleGetters(lazy, { JsonSchema: "resource://gre/modules/JsonSchema.sys.mjs", NetUtil: "resource://gre/modules/NetUtil.sys.mjs", ExperimentManager: "resource://nimbus/lib/ExperimentManager.sys.mjs", + ObjectUtils: "resource://gre/modules/ObjectUtils.sys.mjs", ProfilesDatastoreService: "moz-src:///toolkit/profile/ProfilesDatastoreService.sys.mjs", RemoteSettingsExperimentLoader: "resource://nimbus/lib/RemoteSettingsExperimentLoader.sys.mjs", + TestUtils: "resource://testing-common/TestUtils.sys.mjs", sinon: "resource://testing-common/Sinon.sys.mjs", }); @@ -133,7 +135,7 @@ export const NimbusTestUtils = { * @param {object} store * The `ExperimentStore`. */ - storeIsEmpty(store) { + async storeIsEmpty(store) { NimbusTestUtils.Assert.deepEqual( store .getAll() @@ -159,11 +161,7 @@ export const NimbusTestUtils = { NimbusTestUtils.cleanupStorePrefCache(); - // TODO(bug 1956082): This is an async method that we are not awaiting. - // - // Only browser tests and tests that otherwise have manually enabled the - // ProfilesDatastoreService need to await the result. - return NimbusTestUtils.cleanupEnrollmentDatabase(); + await NimbusTestUtils.cleanupEnrollmentDatabase(); }, }, @@ -428,13 +426,32 @@ export const NimbusTestUtils = { const conn = await lazy.ProfilesDatastoreService.getConnection(); - // TODO(bug 1956080): This should only delete inactive enrollments, but - // unenrolling does not yet trigger database writes. + const activeSlugs = await conn + .execute( + ` + SELECT + slug + FROM NimbusEnrollments + WHERE + profileId = :profileId AND + active = true; + `, + { profileId } + ) + .then(rows => rows.map(row => row.getResultByName("slug"))); + + NimbusTestUtils.Assert.deepEqual( + activeSlugs, + [], + `No active slugs in NimbusEnrollments for ${profileId}` + ); + await conn.execute( ` DELETE FROM NimbusEnrollments WHERE - profileId = :profileId; + profileId = :profileId AND + active = false; `, { profileId } ); @@ -501,15 +518,9 @@ export const NimbusTestUtils = { experimentManager.store._syncToChildren({ flush: true }); - return function doEnrollmentCleanup() { - // TODO(bug 1956082): This is an async method that we are not awaiting. - // - // Only browser tests and tests that otherwise have manually enabled the - // ProfilesDatastoreService need to await the result. - const promise = experimentManager.unenroll(enrollment.slug); + return async function doEnrollmentCleanup() { + await experimentManager.unenroll(enrollment.slug); experimentManager.store._deleteForTests(enrollment.slug); - - return promise; }; }, @@ -725,6 +736,9 @@ export const NimbusTestUtils = { Services.fog.testResetFOG(); Services.telemetry.clearEvents(); } + + // Remove all migration state. + Services.prefs.deleteBranch("nimbus.migrations."); }, }; @@ -795,6 +809,80 @@ export const NimbusTestUtils = { `Experiment ${experiment.slug} not valid` ); }, + + /** + * Wait for the given slugs to be the only active enrollments in the + * NimbusEnrollments table. + * + * @param {string[]} expectedSlugs The slugs of the only active enrollmetns we + * expect. + */ + async waitForActiveEnrollments(expectedSlugs) { + const profileId = ExperimentAPI.profileId; + + await lazy.TestUtils.waitForCondition(async () => { + const conn = await lazy.ProfilesDatastoreService.getConnection(); + const slugs = await conn + .execute( + ` + SELECT + slug + FROM NimbusEnrollments + WHERE + active = true AND + profileId = :profileId; + `, + { profileId } + ) + .then(rows => rows.map(row => row.getResultByName("slug"))); + + return lazy.ObjectUtils.deepEqual(slugs.sort(), expectedSlugs.sort()); + }, `Waiting for enrollments of ${expectedSlugs} to sync to database`); + }, + + async waitForInactiveEnrollment(slug) { + const profileId = ExperimentAPI.profileId; + + await lazy.TestUtils.waitForCondition(async () => { + const conn = await lazy.ProfilesDatastoreService.getConnection(); + const result = await conn.execute( + ` + SELECT + active + FROM NimbusEnrollments + WHERE + slug = :slug AND + profileId = :profileId; + `, + { profileId, slug } + ); + + return result.length === 1 && !result[0].getResultByName("active"); + }, `Waiting for ${slug} enrollment to exist and be inactive`); + }, + + async waitForAllUnenrollments() { + const profileId = ExperimentAPI.profileId; + + await lazy.TestUtils.waitForCondition(async () => { + const conn = await lazy.ProfilesDatastoreService.getConnection(); + const slugs = await conn + .execute( + ` + SELECT + slug + FROM NimbusEnrollments + WHERE + active = true AND + profileId = :profileId; + `, + { profileId } + ) + .then(rows => rows.map(row => row.getResultByName("slug"))); + + return slugs.length === 0; + }, "Waiting for unenrollments to sync to database"); + }, }; Object.defineProperties(NimbusTestUtils.factories.experiment, { diff --git a/toolkit/components/nimbus/test/browser/browser_experimentapi_child.js b/toolkit/components/nimbus/test/browser/browser_experimentapi_child.js index 1bfe14bad4a8..966d2f8398bb 100644 --- a/toolkit/components/nimbus/test/browser/browser_experimentapi_child.js +++ b/toolkit/components/nimbus/test/browser/browser_experimentapi_child.js @@ -61,7 +61,8 @@ add_task(async function testGetFromChildNewEnrollment() { enabled: true, testInt: 123, }, - }) + }), + "test" ); // Immediately serialize sharedData and broadcast changes to the child processes. @@ -143,7 +144,8 @@ add_task(async function testGetFromChildExistingEnrollment() { enabled: false, testInt: 456, }, - }) + }), + "test" ); // We don't have to wait for this to update in the client, but we *do* have to diff --git a/toolkit/components/nimbus/test/browser/browser_nimbus_telemetry.js b/toolkit/components/nimbus/test/browser/browser_nimbus_telemetry.js index 1bf00d976484..4cf9f971726e 100644 --- a/toolkit/components/nimbus/test/browser/browser_nimbus_telemetry.js +++ b/toolkit/components/nimbus/test/browser/browser_nimbus_telemetry.js @@ -54,7 +54,7 @@ add_task(async function test_experiment_enroll_unenroll_Telemetry() { EVENT_FILTER ); - cleanup(); + await cleanup(); TelemetryTestUtils.assertEvents( [ @@ -98,7 +98,7 @@ add_task(async function test_experiment_expose_Telemetry() { EVENT_FILTER ); - cleanup(); + await cleanup(); }); add_task(async function test_rollout_expose_Telemetry() { @@ -132,5 +132,5 @@ add_task(async function test_rollout_expose_Telemetry() { EVENT_FILTER ); - cleanup(); + await cleanup(); }); diff --git a/toolkit/components/nimbus/test/unit/head.js b/toolkit/components/nimbus/test/unit/head.js index b490e16e710e..437c92740237 100644 --- a/toolkit/components/nimbus/test/unit/head.js +++ b/toolkit/components/nimbus/test/unit/head.js @@ -1,5 +1,8 @@ "use strict"; +/* import-globals-from ../../../../../toolkit/profile/test/xpcshell/head.js */ +/* import-globals-from ../../../../../browser/components/profiles/tests/unit/head.js */ + const { sinon } = ChromeUtils.importESModule( "resource://testing-common/Sinon.sys.mjs" ); @@ -23,8 +26,19 @@ ChromeUtils.defineESModuleGetters(this, { NimbusTestUtils.init(this); -add_setup(function () { +add_setup(async function () { do_get_profile(); + + await initSelectableProfileService(); + + // TODO(bug 1967779): require the ProfilesDatastoreService to be initialized + Services.prefs.setBoolPref("nimbus.profilesdatastoreservice.enabled", true); + registerCleanupFunction(() => { + Services.prefs.setBoolPref( + "nimbus.profilesdatastoreservice.enabled", + false + ); + }); }); /** diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature.js b/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature.js index 1d53b1a13485..607ef57e0c99 100644 --- a/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature.js +++ b/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature.js @@ -64,7 +64,7 @@ add_task(async function test_ExperimentFeature_test_helper_ready() { "set by remote config" ); - cleanupExperiment(); + await cleanupExperiment(); await cleanup(); }); @@ -212,7 +212,7 @@ add_task(async function test_allow_multiple_exposure_events() { // We expect 3 events Assert.equal(3, exposureEvents.length); - doExperimentCleanup(); + await doExperimentCleanup(); await cleanup(); }); 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 ad1eab3ca315..dffee1e7b479 100644 --- a/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature_getVariable.js +++ b/toolkit/components/nimbus/test/unit/test_ExperimentAPI_ExperimentFeature_getVariable.js @@ -1,9 +1,5 @@ "use strict"; -const { AppConstants } = ChromeUtils.importESModule( - "resource://gre/modules/AppConstants.sys.mjs" -); - const FEATURE_ID = "testfeature1"; // Note: this gets deleted at the end of tests const TEST_PREF_BRANCH = "testfeature1."; diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentManager_enroll.js b/toolkit/components/nimbus/test/unit/test_ExperimentManager_enroll.js index cce05168d0ef..063040145b8c 100644 --- a/toolkit/components/nimbus/test/unit/test_ExperimentManager_enroll.js +++ b/toolkit/components/nimbus/test/unit/test_ExperimentManager_enroll.js @@ -135,7 +135,7 @@ add_task(async function test_enroll_optin_recipe_branch_selection() { Assert.ok( manager._enroll.calledOnceWith( optInRecipe, - optInRecipe.branches[0], + optInRecipe.branches[0].slug, "test" ), "should call ._enroll() with the correct arguments" @@ -871,7 +871,7 @@ add_task(async function test_featureIds_is_stored() { "Has expected value" ); - doExperimentCleanup(); + await doExperimentCleanup(); await cleanup(); }); @@ -907,7 +907,7 @@ add_task(async function experiment_and_rollout_enroll_and_cleanup() { ) ); - doExperimentCleanup(); + await doExperimentCleanup(); Assert.ok( !Services.prefs.getBoolPref( @@ -921,7 +921,7 @@ add_task(async function experiment_and_rollout_enroll_and_cleanup() { ) ); - doRolloutCleanup(); + await doRolloutCleanup(); Assert.ok( !Services.prefs.getBoolPref( diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentManager_lifecycle.js b/toolkit/components/nimbus/test/unit/test_ExperimentManager_lifecycle.js index 24642724765e..6d03e2d8c63d 100644 --- a/toolkit/components/nimbus/test/unit/test_ExperimentManager_lifecycle.js +++ b/toolkit/components/nimbus/test/unit/test_ExperimentManager_lifecycle.js @@ -15,6 +15,10 @@ const { UnenrollmentCause } = ChromeUtils.importESModule( "resource://nimbus/lib/ExperimentManager.sys.mjs" ); +const { ProfilesDatastoreService } = ChromeUtils.importESModule( + "moz-src:///toolkit/profile/ProfilesDatastoreService.sys.mjs" +); + /** * onStartup() * - should set call setExperimentActive for each active experiment @@ -364,3 +368,208 @@ add_task(async function test_experimentStore_updateEvent() { await cleanup(); }); + +add_task(async function testDb() { + const conn = await ProfilesDatastoreService.getConnection(); + + function processRow(row) { + const fields = [ + "profileId", + "slug", + "branchSlug", + "recipe", + "active", + "unenrollReason", + "lastSeen", + "setPrefs", + "prefFlips", + "source", + ]; + + const processed = {}; + + for (const field of fields) { + processed[field] = row.getResultByName(field); + } + + processed.recipe = JSON.parse(processed.recipe); + processed.setPrefs = JSON.parse(processed.setPrefs); + processed.prefFlips = JSON.parse(processed.prefFlips); + + return processed; + } + + async function getEnrollment(slug) { + const results = await conn.execute( + ` + SELECT + profileId, + slug, + branchSlug, + json(recipe) AS recipe, + active, + unenrollReason, + lastSeen, + json(setPrefs) AS setPrefs, + json(prefFlips) AS prefFlips, + source + FROM NimbusEnrollments + WHERE + slug = :slug AND + profileId = :profileId; + `, + { slug, profileId: ExperimentAPI.profileId } + ); + + Assert.equal( + results.length, + 1, + `Exactly one enrollment should be returned for ${slug}` + ); + return processRow(results[0]); + } + + async function getEnrollmentSlugs() { + const result = await conn.execute( + ` + SELECT + slug + FROM NimbusEnrollments + WHERE + profileId = :profileId; + `, + { profileId: ExperimentAPI.profileId } + ); + + return result.map(row => row.getResultByName("slug")).sort(); + } + + const { manager, cleanup } = await NimbusTestUtils.setupTest(); + + const experimentRecipe = NimbusTestUtils.factories.recipe("experiment", { + branches: [ + { + ratio: 1, + slug: "control", + features: [ + { + featureId: "no-feature-firefox-desktop", + value: {}, + }, + ], + }, + { + ratio: 0, // Force enrollment in control + slug: "treatment", + features: [ + { + featureId: "no-feature-firefox-desktop", + value: {}, + }, + ], + }, + ], + }); + + const rolloutRecipe = NimbusTestUtils.factories.recipe.withFeatureConfig( + "rollout", + { branchSlug: "rollout", featureId: "no-feature-firefox-desktop" } + ); + + Assert.deepEqual( + await getEnrollmentSlugs(), + [], + "There are no database entries" + ); + + // Enroll in an experiment + await manager.enroll(experimentRecipe, "test"); + Assert.deepEqual( + await getEnrollmentSlugs(), + [experimentRecipe.slug], + "There is one enrollment" + ); + + let experimentEnrollment = await getEnrollment(experimentRecipe.slug); + Assert.ok(experimentEnrollment.active, "experiment enrollment is active"); + Assert.deepEqual( + experimentEnrollment.recipe, + experimentRecipe, + "experiment enrollment has the correct recipe" + ); + Assert.equal( + experimentEnrollment.branchSlug, + manager.store.get(experimentRecipe.slug).branch.slug, + "experiment branch slug matches" + ); + + // Enroll in a rollout. + await manager.enroll(rolloutRecipe, "test"); + Assert.deepEqual( + await getEnrollmentSlugs(), + [experimentRecipe.slug, rolloutRecipe.slug].sort(), + "There are two enrollments" + ); + + let rolloutEnrollment = await getEnrollment(rolloutRecipe.slug); + Assert.ok(rolloutEnrollment.active, "rollout enrollment is active"); + Assert.deepEqual( + rolloutEnrollment.recipe, + rolloutRecipe, + "rollout enrollment has the correct recipe" + ); + Assert.equal( + rolloutEnrollment.branchSlug, + manager.store.get(rolloutRecipe.slug).branch.slug, + "rollout branch slug matches" + ); + + // Unenroll from the rollout. + await manager.unenroll(rolloutRecipe.slug, { reason: "recipe-not-seen" }); + Assert.deepEqual( + await getEnrollmentSlugs(), + [experimentRecipe.slug, rolloutRecipe.slug].sort(), + "There are two enrollments" + ); + + rolloutEnrollment = await getEnrollment(rolloutRecipe.slug); + Assert.ok(!rolloutEnrollment.active, "rollout enrollment is inactive"); + Assert.equal( + rolloutEnrollment.recipe, + null, + "rollout enrollment recipe is null" + ); + Assert.equal( + rolloutEnrollment.unenrollReason, + "recipe-not-seen", + "rollout unenrollReason" + ); + Assert.equal( + rolloutEnrollment.branchSlug, + manager.store.get(rolloutRecipe.slug).branch.slug, + "rollout branch slug matches" + ); + + // Unenroll from the experiment. + await manager.unenroll(experimentEnrollment.slug, { reason: "targeting" }); + + experimentEnrollment = await getEnrollment(experimentRecipe.slug); + Assert.ok(!experimentEnrollment.active, "experiment enrollment is inactive"); + Assert.equal( + experimentEnrollment.recipe, + null, + "experiment enrollment recipe is null" + ); + Assert.equal( + experimentEnrollment.unenrollReason, + "targeting", + "experiment unenrollReason" + ); + Assert.equal( + experimentEnrollment.branchSlug, + manager.store.get(experimentRecipe.slug).branch.slug, + "experiment branch slug matches" + ); + + await cleanup(); +}); diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentManager_prefs.js b/toolkit/components/nimbus/test/unit/test_ExperimentManager_prefs.js index 63760b7b18aa..1f1573a3a2b0 100644 --- a/toolkit/components/nimbus/test/unit/test_ExperimentManager_prefs.js +++ b/toolkit/components/nimbus/test/unit/test_ExperimentManager_prefs.js @@ -8,8 +8,8 @@ const { PrefUtils } = ChromeUtils.importESModule( "resource://normandy/lib/PrefUtils.sys.mjs" ); -const { TelemetryTestUtils } = ChromeUtils.importESModule( - "resource://testing-common/TelemetryTestUtils.sys.mjs" +const { ProfilesDatastoreService } = ChromeUtils.importESModule( + "moz-src:///toolkit/profile/ProfilesDatastoreService.sys.mjs" ); function assertIncludes(array, obj, msg) { @@ -319,7 +319,7 @@ add_task(async function test_enroll_setPref_rolloutsAndExperiments() { } for (const enrollmentKind of unenrollOrder) { - cleanupFns[enrollmentKind](); + await cleanupFns[enrollmentKind](); assertExpectedPrefValues( pref, @@ -1712,9 +1712,6 @@ add_task(async function test_prefChange() { const cleanupFunctions = {}; const slugs = {}; - await manager.store.init(); - await manager.onStartup(); - setPrefs(pref, { defaultBranchValue, userBranchValue }); info(`Enrolling in ${Array.from(Object.keys(configs)).join(", ")} ...`); @@ -1744,6 +1741,10 @@ add_task(async function test_prefChange() { PrefUtils.setPref(pref, OVERWRITE_VALUE, { branch: setBranch }); + await NimbusTestUtils.waitForActiveEnrollments( + expectedEnrollments.map(kind => slugs[kind]) + ); + if (expectedDefault === null) { Assert.ok( !Services.prefs.prefHasDefaultValue(pref), @@ -1783,6 +1784,9 @@ add_task(async function test_prefChange() { for (const enrollmentKind of Object.keys(configs)) { if (!expectedEnrollments.includes(enrollmentKind)) { const slug = slugs[enrollmentKind]; + + await NimbusTestUtils.waitForInactiveEnrollment(slug); + const enrollment = manager.store.get(slug); Assert.ok( @@ -1873,7 +1877,7 @@ add_task(async function test_prefChange() { ); for (const enrollmentKind of expectedEnrollments) { - cleanupFunctions[enrollmentKind](); + await cleanupFunctions[enrollmentKind](); } Services.prefs.deleteBranch(pref); @@ -2322,7 +2326,7 @@ add_task(async function test_deleteBranch() { ); for (const cleanupFn of cleanupFunctions) { - cleanupFn(); + await cleanupFn(); } Services.prefs.deleteBranch(PREFS[USER]); @@ -2405,6 +2409,11 @@ add_task(async function test_clearUserPref() { for (const enrollmentKind of Object.keys(configs)) { const slug = slugs[enrollmentKind]; + + if (!expectedEnrolled) { + await NimbusTestUtils.waitForInactiveEnrollment(slug); + } + const enrollment = manager.store.get(slug); Assert.ok( enrollment !== null, @@ -2438,7 +2447,7 @@ add_task(async function test_clearUserPref() { if (expectedEnrolled) { for (const cleanupFn of Object.values(cleanupFns)) { - cleanupFn(); + await cleanupFn(); } } @@ -2676,7 +2685,7 @@ add_task(async function test_prefChanged_noPrefSet() { ); } - doEnrollmentCleanup(); + await doEnrollmentCleanup(); await cleanup(); Services.prefs.deleteBranch(pref); @@ -3388,7 +3397,7 @@ add_task(async function test_setPref_types() { Assert.deepEqual(json, jsonPrefValue); - experimentCleanup(); + await experimentCleanup(); featureCleanup(); await cleanup(); }); @@ -3481,3 +3490,56 @@ add_task(async function test_setPref_types_restore() { await cleanup(); featureCleanup(); }); + +add_task(async function testDb() { + const { manager, cleanup } = await setupTest(); + + PrefUtils.setPref("nimbus.qa.pref-1", "foo", { branch: DEFAULT }); + + await manager.enroll( + NimbusTestUtils.factories.recipe.withFeatureConfig("slug", { + featureId: "nimbus-qa-1", + value: { value: "hello" }, + }), + "test" + ); + + const conn = await ProfilesDatastoreService.getConnection(); + const [result] = await conn.execute( + ` + SELECT + json(setPrefs) as setPrefs + FROM NimbusEnrollments + WHERE + profileId = :profileId AND + slug = :slug; + `, + { + slug: "slug", + profileId: ExperimentAPI.profileId, + } + ); + + const setPrefs = JSON.parse(result.getResultByName("setPrefs")); + const enrollment = manager.store.get("slug"); + + Assert.deepEqual( + setPrefs, + enrollment.prefs, + "setPrefs stored in the database" + ); + Assert.deepEqual(setPrefs, [ + { + name: "nimbus.qa.pref-1", + branch: "default", + featureId: "nimbus-qa-1", + variable: "value", + originalValue: "foo", + }, + ]); + + await manager.unenroll("slug"); + await cleanup(); + + Services.prefs.deleteBranch("nimbus.qa.pref-1"); +}); diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentManager_unenroll.js b/toolkit/components/nimbus/test/unit/test_ExperimentManager_unenroll.js index ed396b5e165b..1a9cfb51864c 100644 --- a/toolkit/components/nimbus/test/unit/test_ExperimentManager_unenroll.js +++ b/toolkit/components/nimbus/test/unit/test_ExperimentManager_unenroll.js @@ -60,6 +60,8 @@ add_task(async function test_unenroll_opt_out() { Services.prefs.setBoolPref(STUDIES_OPT_OUT_PREF, false); + await NimbusTestUtils.waitForInactiveEnrollment(experiment.slug); + Assert.equal( manager.store.get(experiment.slug).active, false, @@ -119,6 +121,8 @@ add_task(async function test_unenroll_rollout_opt_out() { Services.prefs.setBoolPref(STUDIES_OPT_OUT_PREF, false); + await NimbusTestUtils.waitForInactiveEnrollment(rollout.slug); + Assert.equal( manager.store.get(rollout.slug).active, false, @@ -171,6 +175,8 @@ add_task(async function test_unenroll_uploadPref() { Services.prefs.setBoolPref(UPLOAD_ENABLED_PREF, false); + await NimbusTestUtils.waitForInactiveEnrollment(recipe.slug); + Assert.equal( manager.store.get(recipe.slug).active, false, diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentStore.js b/toolkit/components/nimbus/test/unit/test_ExperimentStore.js index e30fca009747..26c4b92b11f4 100644 --- a/toolkit/components/nimbus/test/unit/test_ExperimentStore.js +++ b/toolkit/components/nimbus/test/unit/test_ExperimentStore.js @@ -102,7 +102,7 @@ add_task(async function test_initOnUpdateEventsFire() { storePath = await NimbusTestUtils.saveStore(store); } - const { sandbox, store, initExperimentAPI, cleanup } = await setupTest({ + const { sandbox, initExperimentAPI, cleanup } = await setupTest({ storePath, init: false, }); @@ -136,13 +136,14 @@ add_task(async function test_initOnUpdateEventsFire() { NimbusFeatures.testFeature.offUpdate(onFeatureUpdate); - store.updateExperiment("testFeature-1", { active: false }); - store.updateExperiment("testFeature-2", { active: false }); - store.updateExperiment("coenroll-1", { active: false }); - store.updateExperiment("coenroll-2", { active: false }); - store.updateExperiment("coenroll-3", { active: false }); - store.updateExperiment("coenroll-4", { active: false }); - + await NimbusTestUtils.cleanupManager([ + "testFeature-1", + "testFeature-2", + "coenroll-1", + "coenroll-2", + "coenroll-3", + "coenroll-4", + ]); await cleanup(); }); @@ -857,8 +858,6 @@ add_task(async function test_restore() { Assert.ok(store.get("experiment")); Assert.ok(store.get("rollout")); - store.updateExperiment("experiment", { active: false }); - store.updateExperiment("rollout", { active: false }); - + await NimbusTestUtils.cleanupManager(["experiment", "rollout"]); await cleanup(); }); diff --git a/toolkit/components/nimbus/test/unit/test_Migrations.js b/toolkit/components/nimbus/test/unit/test_Migrations.js index 60f5d55dce68..a1b444cc732f 100644 --- a/toolkit/components/nimbus/test/unit/test_Migrations.js +++ b/toolkit/components/nimbus/test/unit/test_Migrations.js @@ -1,9 +1,6 @@ /* Any copyright is dedicated to the Public Domain. * http://creativecommons.org/publicdomain/zero/1.0/ */ -/* import-globals-from ../../../../../toolkit/profile/test/xpcshell/head.js */ -/* import-globals-from ../../../../../browser/components/profiles/tests/unit/head.js */ - const { LABS_MIGRATION_FEATURE_MAP, LEGACY_NIMBUS_MIGRATION_PREF, @@ -47,16 +44,6 @@ function getEnabledPrefForFeature(featureId) { add_setup(async function setup() { Services.fog.initializeFOG(); - - Services.prefs.setBoolPref("nimbus.profilesdatastoreservice.enabled", true); - registerCleanupFunction(() => { - Services.prefs.setBoolPref( - "nimbus.profilesdatastoreservice.enabled", - false - ); - }); - - await initSelectableProfileService(); }); /** @@ -97,11 +84,7 @@ async function setupTest({ ); } - const { - initExperimentAPI, - cleanup: baseCleanup, - ...ctx - } = await NimbusTestUtils.setupTest({ + const { initExperimentAPI, ...ctx } = await NimbusTestUtils.setupTest({ init: false, clearTelemetry: true, ...args, @@ -141,13 +124,7 @@ async function setupTest({ ctx.initExperimentAPI = initExperimentAPI; } - return { - ...ctx, - async cleanup() { - await baseCleanup(); - Services.prefs.deleteBranch("nimbus.migrations"); - }, - }; + return ctx; } function makeMigrations(phase, count) { diff --git a/toolkit/components/nimbus/test/unit/test_RemoteSettingsExperimentLoader.js b/toolkit/components/nimbus/test/unit/test_RemoteSettingsExperimentLoader.js index f536a69c196a..19bb0a74557b 100644 --- a/toolkit/components/nimbus/test/unit/test_RemoteSettingsExperimentLoader.js +++ b/toolkit/components/nimbus/test/unit/test_RemoteSettingsExperimentLoader.js @@ -204,13 +204,14 @@ add_task(async function test_checkExperimentSelfReference() { add_task(async function test_optIn_debug_disabled() { info("Testing users cannot opt-in when nimbus.debug is false"); - const recipe = NimbusTestUtils.factories.recipe("foo"); - const { sandbox, loader, initExperimentAPI, cleanup } = + const recipe = NimbusTestUtils.factories.recipe("foo", { + targeting: "false", + }); + const { loader, initExperimentAPI, cleanup } = await NimbusTestUtils.setupTest({ init: false, experiments: [recipe], }); - sandbox.stub(loader, "updateRecipes").resolves(); await initExperimentAPI(); @@ -238,12 +239,12 @@ add_task(async function test_optIn_studies_disabled() { "Testing users cannot opt-in when telemetry is disabled or studies are disabled." ); - const recipe = NimbusTestUtils.factories.recipe("foo"); - const { sandbox, loader, initExperimentAPI, cleanup } = + const recipe = NimbusTestUtils.factories.recipe("foo", { + targeting: "false", + }); + const { loader, initExperimentAPI, cleanup } = await NimbusTestUtils.setupTest({ init: false, experiments: [recipe] }); - sandbox.stub(loader, "updateRecipes").resolves(); - await initExperimentAPI(); Services.prefs.setBoolPref(DEBUG_PREF, true); diff --git a/toolkit/components/nimbus/test/unit/test_RemoteSettingsExperimentLoader_updateRecipes.js b/toolkit/components/nimbus/test/unit/test_RemoteSettingsExperimentLoader_updateRecipes.js index 02c35edc4afe..02d84929bd9a 100644 --- a/toolkit/components/nimbus/test/unit/test_RemoteSettingsExperimentLoader_updateRecipes.js +++ b/toolkit/components/nimbus/test/unit/test_RemoteSettingsExperimentLoader_updateRecipes.js @@ -12,9 +12,6 @@ const { PanelTestProvider } = ChromeUtils.importESModule( const { TelemetryEnvironment } = ChromeUtils.importESModule( "resource://gre/modules/TelemetryEnvironment.sys.mjs" ); -const { TelemetryTestUtils } = ChromeUtils.importESModule( - "resource://testing-common/TelemetryTestUtils.sys.mjs" -); const { UnenrollmentCause } = ChromeUtils.importESModule( "resource://nimbus/lib/ExperimentManager.sys.mjs" ); @@ -957,7 +954,7 @@ add_task(async function test_updateRecipes_rollout_bucketing() { } ); - manager.unenroll(experiment.slug); + await manager.unenroll(experiment.slug); await cleanup(); }); @@ -1008,7 +1005,7 @@ add_task(async function test_reenroll_rollout_resized() { "New enrollment should not have unenroll reason" ); - manager.unenroll(rollout.slug); + await manager.unenroll(rollout.slug); await cleanup(); }); @@ -1025,7 +1022,7 @@ add_task(async function test_experiment_reenroll() { "Should enroll in experiment" ); - manager.unenroll(experiment.slug); + await manager.unenroll(experiment.slug); Assert.ok( !manager.store.getExperimentForFeature("testFeature"), "Should unenroll from experiment" @@ -1162,8 +1159,8 @@ add_task(async function test_active_and_past_experiment_targeting() { ["experiment-a", "experiment-b", "rollout-a", "rollout-b"] ); - manager.unenroll("experiment-c"); - manager.unenroll("rollout-c"); + await manager.unenroll("experiment-c"); + await manager.unenroll("rollout-c"); cleanupFeatures(); await cleanup(); @@ -1329,14 +1326,12 @@ add_task(async function test_enrollment_targeting() { [] ); - for (const slug of [ + await NimbusTestUtils.cleanupManager([ "experiment-b", "experiment-c", "rollout-b", "rollout-c", - ]) { - manager.unenroll(slug); - } + ]); cleanupFeatures(); await cleanup(); @@ -1425,7 +1420,7 @@ add_task( const isReadyEvents = Glean.nimbusEvents.isReady.testGetValue("events"); Assert.equal(isReadyEvents.length, 3); - manager.unenroll(recipe.slug); + await manager.unenroll(recipe.slug); await cleanup(); } @@ -1594,7 +1589,7 @@ add_task(async function test_updateRecipes_optInsStayEnrolled() { await loader.updateRecipes(); Assert.ok(manager.store.get("opt-in")?.active, "Opt-in stayed enrolled"); - manager.unenroll("opt-in"); + await manager.unenroll("opt-in"); await cleanup(); }); @@ -1914,8 +1909,8 @@ add_task(async function test_updateRecipes_enrollmentStatus_telemetry() { }, ]); - manager.unenroll("stays-enrolled"); - manager.unenroll("enrolls"); + await manager.unenroll("stays-enrolled"); + await manager.unenroll("enrolls"); cleanupFeatures(); await cleanup(); @@ -2027,8 +2022,8 @@ add_task(async function test_updateRecipes_enrollmentStatus_notEnrolled() { ] ); - manager.unenroll("enrolled-experiment"); - manager.unenroll("enrolled-rollout"); + await manager.unenroll("enrolled-experiment"); + await manager.unenroll("enrolled-rollout"); cleanupFeatures(); await cleanup(); @@ -2218,8 +2213,8 @@ add_task(async function testUnenrollsFirst() { await loader.updateRecipes(); assertEnrollments(manager.store, ["e3", "r3"], ["e1", "e2", "r1", "r2"]); - manager.unenroll("e3"); - manager.unenroll("r3"); + await manager.unenroll("e3"); + await manager.unenroll("r3"); await cleanup(); }); diff --git a/toolkit/components/nimbus/test/unit/test_localization.js b/toolkit/components/nimbus/test/unit/test_localization.js index 73e77ffeed5d..ec4619fdf4e7 100644 --- a/toolkit/components/nimbus/test/unit/test_localization.js +++ b/toolkit/components/nimbus/test/unit/test_localization.js @@ -4,9 +4,6 @@ const { MatchStatus } = ChromeUtils.importESModule( "resource://nimbus/lib/RemoteSettingsExperimentLoader.sys.mjs" ); -const { TelemetryTestUtils } = ChromeUtils.importESModule( - "resource://testing-common/TelemetryTestUtils.sys.mjs" -); const LOCALIZATIONS = { "en-US": { @@ -213,7 +210,7 @@ add_task(async function test_getLocalizedValue() { "_getLocalizedValue() with a nested localization" ); - doExperimentCleanup(); + await doExperimentCleanup(); await cleanup(); }); @@ -247,6 +244,8 @@ add_task(async function test_getLocalizedValue_unenroll_missingEntry() { "_getLocalizedValue() with a bogus localization" ); + await NimbusTestUtils.waitForInactiveEnrollment(enrollment.slug); + Assert.equal( manager.store.getExperimentForFeature(FEATURE_ID), null, @@ -316,6 +315,8 @@ add_task(async function test_getLocalizedValue_unenroll_missingEntry() { "_getLocalizedValue() with a bogus localization" ); + await NimbusTestUtils.waitForInactiveEnrollment(enrollment.slug); + Assert.equal( manager.store.getExperimentForFeature(FEATURE_ID), null, @@ -393,7 +394,7 @@ add_task(async function test_getVariables() { "getVariable() returns substitutions inside arrays" ); - doExperimentCleanup(); + await doExperimentCleanup(); await cleanup(); }); @@ -645,6 +646,9 @@ add_task(async function test_getVariables_fallback_unenroll() { waldo: ["fallback-waldo-pref-value"], }); + await NimbusTestUtils.waitForInactiveEnrollment("experiment"); + await NimbusTestUtils.waitForInactiveEnrollment("rollout"); + Assert.equal( manager.store.getExperimentForFeature(FEATURE_ID), null, diff --git a/toolkit/components/nimbus/test/unit/test_prefFlips.js b/toolkit/components/nimbus/test/unit/test_prefFlips.js index 827e91474198..180c45d57c4c 100644 --- a/toolkit/components/nimbus/test/unit/test_prefFlips.js +++ b/toolkit/components/nimbus/test/unit/test_prefFlips.js @@ -7,8 +7,9 @@ const { PrefUtils } = ChromeUtils.importESModule( const { JsonSchema } = ChromeUtils.importESModule( "resource://gre/modules/JsonSchema.sys.mjs" ); -const { TelemetryTestUtils } = ChromeUtils.importESModule( - "resource://testing-common/TelemetryTestUtils.sys.mjs" + +const { ProfilesDatastoreService } = ChromeUtils.importESModule( + "moz-src:///toolkit/profile/ProfilesDatastoreService.sys.mjs" ); const USER = "user"; @@ -145,6 +146,7 @@ async function setupTest({ ...args } = {}) { ...ctx, async cleanup() { assertNoObservers(ctx.manager); + await NimbusTestUtils.waitForAllUnenrollments(); await baseCleanup(); }, }; @@ -1606,6 +1608,8 @@ add_task(async function test_prefFlips_unenrollment() { Assert.ok(enrollment.active, `It should still be active`); } + await NimbusTestUtils.waitForActiveEnrollments(expectedEnrollments); + info("Checking expected unenrollments..."); for (const slug of expectedUnenrollments) { const enrollment = manager.store.get(slug); @@ -1614,6 +1618,13 @@ add_task(async function test_prefFlips_unenrollment() { Assert.ok(!enrollment.active, "It should no longer be active"); } + let expectedCurrentEnrollments = new Set(expectedEnrollments).difference( + new Set(expectedUnenrollments) + ); + await NimbusTestUtils.waitForActiveEnrollments( + Array.from(expectedCurrentEnrollments) + ); + if (unenrollmentOrder) { info("Unenrolling from specific experiments before checking prefs..."); for (const slug of unenrollmentOrder ?? []) { @@ -1621,6 +1632,13 @@ add_task(async function test_prefFlips_unenrollment() { } } + expectedCurrentEnrollments = expectedCurrentEnrollments.difference( + new Set(unenrollmentOrder) + ); + await NimbusTestUtils.waitForActiveEnrollments( + Array.from(expectedCurrentEnrollments) + ); + if (expectedPrefs) { info("Checking expected prefs..."); checkExpectedPrefs(expectedPrefs); @@ -1634,6 +1652,8 @@ add_task(async function test_prefFlips_unenrollment() { } } + await NimbusTestUtils.waitForActiveEnrollments([]); + info("Cleaning up prefs..."); Services.prefs.deleteBranch(PREF_FOO); Services.prefs.deleteBranch(PREF_BAR); @@ -2273,6 +2293,15 @@ add_task(async function test_prefFlips_failed() { const { manager, cleanup } = await setupTest(); await manager.enroll(recipe, "test"); + // Unenrolling is triggered by the PrefFlipsFeature in response to the + // enrollment being added to the store and triggering the feature update + // callback. + // + // That callback triggers an async unenroll() without awaiting (because it + // wouldn't block the ExperimentStore anyway) so we have to wait for the + // unenroll to be propagated to the database first. + await NimbusTestUtils.waitForInactiveEnrollment(recipe.slug); + const enrollment = manager.store.get(recipe.slug); Assert.ok(!enrollment.active, "Experiment should not be active"); @@ -2342,6 +2371,15 @@ add_task(async function test_prefFlips_failed_multiple_prefs() { await manager.enroll(recipe, "test"); + // Unenrolling is triggered by the PrefFlipsFeature in response to the + // enrollment being added to the store and triggering the feature update + // callback. + // + // That callback triggers an async unenroll() without awaiting (because it + // wouldn't block the ExperimentStore anyway) so we have to wait for the + // unenroll to be propagated to the database first. + await NimbusTestUtils.waitForInactiveEnrollment(recipe.slug); + const enrollment = manager.store.get(recipe.slug); Assert.ok(!enrollment.active, "Experiment should not be active"); @@ -2470,8 +2508,11 @@ add_task(async function test_prefFlips_failed_experiment_and_rollout_1() { Assert.ok(enrollment.active, `The enrollment for ${slug} is active`); } + await NimbusTestUtils.waitForActiveEnrollments(expectedEnrollments); + info("Checking expected unenrollments..."); for (const slug of expectedUnenrollments) { + await NimbusTestUtils.waitForInactiveEnrollment(slug); const enrollment = manager.store.get(slug); Assert.ok(!enrollment.active, "The enrollment is no longer active."); } @@ -2577,6 +2618,8 @@ add_task(async function test_prefFlips_failed_experiment_and_rollout_2() { ); } + await NimbusTestUtils.waitForActiveEnrollments(expectedEnrollments); + info("Checking expected enrollments..."); for (const slug of expectedEnrollments) { const enrollment = manager.store.get(slug); @@ -2585,6 +2628,7 @@ add_task(async function test_prefFlips_failed_experiment_and_rollout_2() { info("Checking expected unenrollments..."); for (const slug of expectedUnenrollments) { + await NimbusTestUtils.waitForInactiveEnrollment(slug); const enrollment = manager.store.get(slug); Assert.ok(!enrollment.active, "The enrollment is no longer active."); } @@ -2643,6 +2687,8 @@ add_task(async function test_prefFlips_update_failure() { { manager, slug: "experiment" } ); + await NimbusTestUtils.waitForActiveEnrollments(["rollout"]); + const rolloutEnrollment = manager.store.get("rollout"); const experimentEnrollment = manager.store.get("experiment"); @@ -2653,7 +2699,7 @@ add_task(async function test_prefFlips_update_failure() { Assert.equal(Services.prefs.getStringPref("pref.one"), "one"); Assert.equal(Services.prefs.getStringPref("pref.two"), "two"); - cleanupExperiment(); + await cleanupExperiment(); Services.prefs.deleteBranch("pref.one"); Services.prefs.deleteBranch("pref.two"); @@ -2893,6 +2939,9 @@ add_task(async function test_prefFlips_restore_failure_conflict() { const { manager, cleanup } = await setupTest({ storePath }); + await NimbusTestUtils.waitForActiveEnrollments(["rollout-1"]); + await NimbusTestUtils.waitForInactiveEnrollment("rollout-2"); + Assert.ok(manager.store.get("rollout-1").active, "rollout-1 is active"); Assert.ok(!manager.store.get("rollout-2").active, "rollout-2 is not active"); Assert.equal( @@ -2977,7 +3026,12 @@ add_task(async function test_prefFlips_restore_failure_wrong_type() { Services.prefs.setIntPref(PREF_1, 123); - const { manager, cleanup } = await setupTest({ storePath }); + const { manager, cleanup } = await setupTest({ + storePath, + secureExperiments: [recipe], + }); + + await NimbusTestUtils.waitForInactiveEnrollment(recipe.slug); const enrollment = manager.store.get(recipe.slug); @@ -3026,6 +3080,7 @@ add_task( PrefUtils.setPref(PREF, "default-value", { branch: DEFAULT }); await manager.enroll(recipe, "rs-loader"); + await NimbusTestUtils.waitForInactiveEnrollment(recipe.slug); let enrollment = manager.store.get(recipe.slug); @@ -3033,6 +3088,8 @@ add_task( Assert.equal(enrollment.unenrollReason, "prefFlips-failed"); await manager.enroll(recipe, "rs-loader", { reenroll: true }); + await NimbusTestUtils.waitForInactiveEnrollment(recipe.slug); + enrollment = manager.store.get(recipe.slug); Assert.ok(!enrollment.active, "enrollment should not be active"); @@ -3043,3 +3100,59 @@ add_task( await cleanup(); } ); + +add_task(async function testDb() { + const { manager, cleanup } = await setupTest(); + + PrefUtils.setPref("foo.bar.baz", "foo"); + + await manager.enroll( + NimbusTestUtils.factories.recipe.withFeatureConfig("slug", { + featureId: "prefFlips", + value: { + prefs: { + "foo.bar.baz": { + value: "bar", + branch: "user", + }, + }, + }, + }), + "test" + ); + + const conn = await ProfilesDatastoreService.getConnection(); + const [result] = await conn.execute( + ` + SELECT + json(prefFlips) as prefFlips + FROM NimbusEnrollments + WHERE + profileId = :profileId AND + slug = :slug; + `, + { + slug: "slug", + profileId: ExperimentAPI.profileId, + } + ); + + const prefFlips = JSON.parse(result.getResultByName("prefFlips")); + const enrollment = manager.store.get("slug"); + + Assert.deepEqual( + prefFlips, + enrollment.prefFlips, + "prefFlips stored in the database" + ); + Assert.deepEqual(prefFlips, { + originalValues: { + "foo.bar.baz": "foo", + }, + }); + + await manager.unenroll("slug"); + await cleanup(); + + Services.prefs.deleteBranch("foo.bar.baz"); +}); diff --git a/toolkit/components/nimbus/test/unit/xpcshell.toml b/toolkit/components/nimbus/test/unit/xpcshell.toml index 00f476ba622a..a86b7853eb9e 100644 --- a/toolkit/components/nimbus/test/unit/xpcshell.toml +++ b/toolkit/components/nimbus/test/unit/xpcshell.toml @@ -1,5 +1,5 @@ [DEFAULT] -head = "head.js" +head = "../../../../../toolkit/profile/test/xpcshell/head.js ../../../../../browser/components/profiles/tests/unit/head.js head.js" tags = "nimbus" firefox-appdir = "browser" support-files = ["reference_aboutwelcome_experiment_content.json"] @@ -39,7 +39,6 @@ run-sequentially = "very high failure rate in parallel" ["test_FirefoxLabs.js"] ["test_Migrations.js"] -head = "../../../../../toolkit/profile/test/xpcshell/head.js ../../../../../browser/components/profiles/tests/unit/head.js head.js" ["test_NimbusTestUtils.js"]