commit 4e6f10a173bdc530f04fb299992ad164433b7f39 parent 52a349bba6a928eb6362b0781dd128015e7715ea Author: Beth Rennie <beth@brennie.ca> Date: Fri, 14 Nov 2025 22:27:59 +0000 Bug 1996359 - Require migrationState in NimbusTestUtils.setupTest if storePath is present r=nimbus-reviewers,relud New aliases have been added to NimbusTestUtils.migrationState: - `UNMIGRATED`, which represents a state with no migrations applied; and - `LATEST`, which represents the most up-to-date set of migrations. Tests that do not care about the specifics of the migration state *but* need to have a pre-existing store should use one of these two states. Differential Revision: https://phabricator.services.mozilla.com/D270255 Diffstat:
8 files changed, 58 insertions(+), 14 deletions(-)
diff --git a/toolkit/components/nimbus/test/NimbusTestUtils.sys.mjs b/toolkit/components/nimbus/test/NimbusTestUtils.sys.mjs @@ -498,6 +498,19 @@ export const NimbusTestUtils = { }, migrationState: { + /** + * A migration state that represents no migrations. + * + * @type {Record<Phase, Number>} + */ + UNMIGRATED: Object.freeze({}), + + /** + * A migration state that represents a successful import into the + * NimbusEnrollments table. + * + * @type {Record<Phase, Number}> + */ get IMPORTED_ENROLLMENTS_TO_SQL() { const { Phase } = lazy.NimbusMigrations; @@ -507,6 +520,15 @@ export const NimbusTestUtils = { [Phase.AFTER_REMOTE_SETTINGS_UPDATE]: "firefox-labs-enrollments", }); }, + + /** + * A migration state that represents all migrations applied. + * + * @type {Record<Phase, Number>} + */ + get LATEST() { + return NimbusTestUtils.migrationState.IMPORTED_ENROLLMENTS_TO_SQL; + }, }, /** @@ -514,6 +536,8 @@ export const NimbusTestUtils = { * * @param {Record<Phase, string>} migrationsByPhase A map of the latest * completed migration by phase. + * + * @returns {Record<Phase, number>} The values to set for each migration pref. */ makeMigrationState(migrationsByPhase) { const state = {}; @@ -1146,6 +1170,9 @@ export const NimbusTestUtils = { * An optional path to an existing ExperimentStore to use for the * ExperimentManager. * + * If provided, the {@link options.migrationState} option must also be + * set. + * * @param {object[]?} options.experiments * If provided, these recipes will be returned by the RemoteSettings * experiments client. @@ -1164,6 +1191,15 @@ export const NimbusTestUtils = { * The value that should be set for the Nimbus migration prefs. If * not provided, the pref will be unset. * + * Required if {@link options.storePath} is also provided. + * + * Most tests will want to use either + * `NimbusTestUtils.migrationState.UNMIGRATED` or + * `NimbusTestUtils.migrationState.LATEST`, depending on whether or not + * they are writing to the `NimbusEnrollments` database table. + * + * @throws {Error} If the the arguments to this function are not consistent. + * * @returns {TestContext} * Everything you need to write a test using Nimbus. */ @@ -1176,6 +1212,10 @@ export const NimbusTestUtils = { features, migrationState, } = {}) { + if (storePath && typeof migrationState === "undefined") { + throw new Error("setupTest: storePath requires migrationState"); + } + NimbusLogging.enableLogging(); const sandbox = lazy.sinon.createSandbox(); diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentManager_lifecycle.js b/toolkit/components/nimbus/test/unit/test_ExperimentManager_lifecycle.js @@ -45,8 +45,7 @@ async function test_onStartup_setExperimentActive_called() { { store, extra: { active: false } } ); }), - migrationState: - NimbusTestUtils.migrationState.IMPORTED_ENROLLMENTS_TO_SQL, + migrationState: NimbusTestUtils.migrationState.LATEST, }); sandbox.stub(NimbusTelemetry, "setExperimentActive"); @@ -97,6 +96,7 @@ async function test_startup_unenroll() { { store, branchSlug: "control" } ); }), + migrationState: NimbusTestUtils.migrationState.UNMIGRATED, }); sandbox.spy(manager, "_unenroll"); diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentManager_prefs.js b/toolkit/components/nimbus/test/unit/test_ExperimentManager_prefs.js @@ -1321,6 +1321,7 @@ add_task(async function test_restorePrefs_experimentAndRollout() { const { sandbox, manager, initExperimentAPI, cleanup } = await setupTest({ init: false, storePath, + migrationState: NimbusTestUtils.migrationState.LATEST, }); const setPrefSpy = sandbox.spy(PrefUtils, "setPref"); @@ -2912,8 +2913,7 @@ async function test_restorePrefs_manifestChanged() { const { manager, cleanup } = await setupTest({ storePath, - migrationState: - NimbusTestUtils.migrationState.IMPORTED_ENROLLMENTS_TO_SQL, + migrationState: NimbusTestUtils.migrationState.LATEST, }); for (const enrollmentKind of expectedEnrollments) { @@ -3615,7 +3615,7 @@ async function test_setPref_types_restore() { const { manager, cleanup } = await setupTest({ storePath, - migrationState: NimbusTestUtils.migrationState.IMPORTED_ENROLLMENTS_TO_SQL, + migrationState: NimbusTestUtils.migrationState.LATEST, }); const defaultBranch = Services.prefs.getDefaultBranch(null); diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentStore.js b/toolkit/components/nimbus/test/unit/test_ExperimentStore.js @@ -120,7 +120,7 @@ async function test_initOnUpdateEventsFire() { const { sandbox, initExperimentAPI, cleanup } = await setupTest({ init: false, storePath, - migrationState: NimbusTestUtils.migrationState.IMPORTED_ENROLLMENTS_TO_SQL, + migrationState: NimbusTestUtils.migrationState.LATEST, }); const onFeatureUpdate = sandbox.stub(); @@ -897,7 +897,7 @@ async function test_restore() { { store } ); }), - migrationState: NimbusTestUtils.migrationState.IMPORTED_ENROLLMENTS_TO_SQL, + migrationState: NimbusTestUtils.migrationState.LATEST, }); Assert.ok(store.get("experiment")); @@ -960,7 +960,7 @@ async function test_restoreDatabaseConsistency(primary = "jsonfile") { const { cleanup } = await NimbusTestUtils.setupTest({ storePath, clearTelemetry: true, - migrationState: NimbusTestUtils.migrationState.IMPORTED_ENROLLMENTS_TO_SQL, + migrationState: NimbusTestUtils.migrationState.LATEST, }); const events = Glean.nimbusEvents.startupDatabaseConsistency diff --git a/toolkit/components/nimbus/test/unit/test_Migrations.js b/toolkit/components/nimbus/test/unit/test_Migrations.js @@ -887,6 +887,7 @@ add_task(async function test_migration_firefoxLabsEnrollments_idempotent() { }); }), experiments: recipes, + migrationState: NimbusTestUtils.migrationState.UNMIGRATED, migrations: { [NimbusMigrations.Phase.AFTER_REMOTE_SETTINGS_UPDATE]: [ FIREFOX_LABS_MIGRATION, @@ -1545,6 +1546,7 @@ async function testMigrateEnrollmentsToSql(primary = "jsonfile") { storePath, experiments, secureExperiments, + migrationState: NimbusTestUtils.migrationState.UNMIGRATED, migrations: { [NimbusMigrations.Phase.AFTER_STORE_INITIALIZED]: [ IMPORT_TO_SQL_MIGRATION, diff --git a/toolkit/components/nimbus/test/unit/test_RemoteSettingsExperimentLoader_updateRecipes.js b/toolkit/components/nimbus/test/unit/test_RemoteSettingsExperimentLoader_updateRecipes.js @@ -488,7 +488,7 @@ async function test_updateRecipes_invalidFeatureAfterUpdate() { featureConfig ), ], - migrationState: NimbusTestUtils.migrationState.IMPORTED_ENROLLMENTS_TO_SQL, + migrationState: NimbusTestUtils.migrationState.LATEST, }); const enrollment = manager.store.get("recipe"); diff --git a/toolkit/components/nimbus/test/unit/test_policy.js b/toolkit/components/nimbus/test/unit/test_policy.js @@ -66,6 +66,7 @@ async function doTest({ } }), experiments: RECIPES, + migrationState: NimbusTestUtils.migrationState.UNMIGRATED, }); sinon.spy(loader, "updateRecipes"); diff --git a/toolkit/components/nimbus/test/unit/test_prefFlips.js b/toolkit/components/nimbus/test/unit/test_prefFlips.js @@ -1,5 +1,6 @@ /* Any copyright is dedicated to the Public Domain. - * http://creativecommons.org/publicdomain/zero/1.0/ */ + * http://creativecommons.org/publicdomain/zero/1.0/ + */ const { PrefUtils } = ChromeUtils.importESModule( "moz-src:///toolkit/modules/PrefUtils.sys.mjs" @@ -2399,7 +2400,7 @@ async function test_prefFlips_restore_unenroll() { Services.prefs.setStringPref("test.pref.please.ignore", "test-value"); }), secureExperiments: [recipe], - migrationState: NimbusTestUtils.migrationState.IMPORTED_ENROLLMENTS_TO_SQL, + migrationState: NimbusTestUtils.migrationState.LATEST, }); const activeEnrollment = manager.store.getExperimentForFeature(FEATURE_ID); @@ -2961,7 +2962,7 @@ async function test_prefFlips_restore() { const { manager, cleanup } = await setupTest({ storePath, - migrationState: NimbusTestUtils.migrationState.IMPORTED_ENROLLMENTS_TO_SQL, + migrationState: NimbusTestUtils.migrationState.LATEST, }); Assert.ok(manager.store.get("rollout-1").active, "rollout-1 is active"); @@ -3106,7 +3107,7 @@ async function test_prefFlips_restore_failure_conflict() { const { manager, cleanup } = await setupTest({ storePath, - migrationState: NimbusTestUtils.migrationState.IMPORTED_ENROLLMENTS_TO_SQL, + migrationState: NimbusTestUtils.migrationState.LATEST, }); await NimbusTestUtils.waitForActiveEnrollments(["rollout-1"]); @@ -3201,7 +3202,7 @@ async function test_prefFlips_restore_failure_wrong_type() { const { manager, cleanup } = await setupTest({ storePath, secureExperiments: [recipe], - migrationState: NimbusTestUtils.migrationState.IMPORTED_ENROLLMENTS_TO_SQL, + migrationState: NimbusTestUtils.migrationState.LATEST, }); await NimbusTestUtils.flushStore(manager.store);