Backed out changeset 8b3b3c83025d (bug 1797899) for bc failures on browser_AddonStudies.js. CLOSED TREE

This commit is contained in:
Marian-Vasile Laza 2023-02-06 13:45:17 +02:00
parent 66762decd8
commit 4f7f6c0455
18 changed files with 80 additions and 88 deletions

View File

@ -110,13 +110,13 @@ class _ExperimentManager {
Object.defineProperty(context, "activeExperiments", {
get: async () => {
await this.store.ready();
return this.store.getAllActiveExperiments().map(exp => exp.slug);
return this.store.getAllActive().map(exp => exp.slug);
},
});
Object.defineProperty(context, "activeRollouts", {
get: async () => {
await this.store.ready();
return this.store.getAllActiveRollouts().map(rollout => rollout.slug);
return this.store.getAllRollouts().map(rollout => rollout.slug);
},
});
return context;
@ -132,8 +132,8 @@ class _ExperimentManager {
await this.store.init();
this.extraContext = extraContext;
const restoredExperiments = this.store.getAllActiveExperiments();
const restoredRollouts = this.store.getAllActiveRollouts();
const restoredExperiments = this.store.getAllActive();
const restoredRollouts = this.store.getAllRollouts();
for (const experiment of restoredExperiments) {
this.setExperimentActive(experiment);
@ -241,8 +241,8 @@ class _ExperimentManager {
if (!sourceToCheck) {
throw new Error("When calling onFinalize, you must specify a source.");
}
const activeExperiments = this.store.getAllActiveExperiments();
const activeRollouts = this.store.getAllActiveRollouts();
const activeExperiments = this.store.getAllActive();
const activeRollouts = this.store.getAllRollouts();
this._checkUnseenEnrollments(
activeExperiments,
sourceToCheck,
@ -570,10 +570,10 @@ class _ExperimentManager {
*/
observe(aSubject, aTopic, aPrefName) {
if (!this.studiesEnabled) {
for (const { slug } of this.store.getAllActiveExperiments()) {
for (const { slug } of this.store.getAllActive()) {
this.unenroll(slug, "studies-opt-out");
}
for (const { slug } of this.store.getAllActiveRollouts()) {
for (const { slug } of this.store.getAllRollouts()) {
this.unenroll(slug, "studies-opt-out");
}
}

View File

@ -244,12 +244,12 @@ class ExperimentStore extends SharedDataMap {
async init() {
await super.init();
this.getAllActiveExperiments().forEach(({ branch, featureIds }) => {
this.getAllActive().forEach(({ branch, featureIds }) => {
(featureIds || getAllBranchFeatureIds(branch)).forEach(featureId =>
this._emitFeatureUpdate(featureId, "feature-experiment-loaded")
);
});
this.getAllActiveRollouts().forEach(({ featureIds }) => {
this.getAllRollouts().forEach(({ featureIds }) => {
featureIds.forEach(featureId =>
this._emitFeatureUpdate(featureId, "feature-rollout-loaded")
);
@ -269,7 +269,7 @@ class ExperimentStore extends SharedDataMap {
*/
getExperimentForFeature(featureId) {
return (
this.getAllActiveExperiments().find(
this.getAllActive().find(
experiment =>
experiment.featureIds?.includes(featureId) ||
// Supports <v1.3.0, which was when .featureIds was added
@ -309,10 +309,9 @@ class ExperimentStore extends SharedDataMap {
}
/**
* Returns all active experiments
* @returns {Enrollment[]}
*/
getAllActiveExperiments() {
getAllActive() {
return this.getAll().filter(
enrollment => enrollment.active && !enrollment.isRollout
);
@ -320,9 +319,9 @@ class ExperimentStore extends SharedDataMap {
/**
* Returns all active rollouts
* @returns {Enrollment[]}
* @returns {array}
*/
getAllActiveRollouts() {
getAllRollouts() {
return this.getAll().filter(
enrollment => enrollment.active && enrollment.isRollout
);
@ -335,7 +334,7 @@ class ExperimentStore extends SharedDataMap {
*/
getRolloutForFeature(featureId) {
return (
this.getAllActiveRollouts().find(r => r.featureIds.includes(featureId)) ||
this.getAllRollouts().find(r => r.featureIds.includes(featureId)) ||
lazy.syncDataStore.getDefault(featureId)
);
}

View File

@ -331,7 +331,7 @@ const ExperimentFakes = {
await promise;
}
if (manager.store.getAllActiveExperiments().length) {
if (manager.store.getAllActive().length) {
throw new Error("Cleanup failed");
}
},

View File

@ -35,10 +35,7 @@ add_task(async function test_double_feature_enrollment() {
);
await ExperimentAPI.ready();
Assert.ok(
ExperimentManager.store.getAllActiveExperiments().length === 0,
"Clean state"
);
Assert.ok(ExperimentManager.store.getAllActive().length === 0, "Clean state");
let recipe1 = getRecipe("foo" + Math.random());
let recipe2 = getRecipe("foo" + Math.random());
@ -52,7 +49,7 @@ add_task(async function test_double_feature_enrollment() {
ExperimentManager.enroll(recipe2, "test_double_feature_enrollment");
Assert.equal(
ExperimentManager.store.getAllActiveExperiments().length,
ExperimentManager.store.getAllActive().length,
1,
"1 active experiment"
);

View File

@ -513,7 +513,7 @@ add_task(async function test_getActiveBranch() {
add_task(async function test_getActiveBranch_safe() {
const sandbox = sinon.createSandbox();
sandbox.stub(ExperimentAPI._store, "getAllActiveExperiments").throws();
sandbox.stub(ExperimentAPI._store, "getAllActive").throws();
try {
Assert.equal(
@ -544,7 +544,7 @@ add_task(async function test_getActiveBranch_storeFailure() {
// Adding stub later because `addEnrollment` emits update events
const stub = sandbox.stub(store, "emit");
// Call getActiveBranch to trigger an activation event
sandbox.stub(store, "getAllActiveExperiments").throws();
sandbox.stub(store, "getAllActive").throws();
try {
ExperimentAPI.getActiveBranch({ featureId: "green" });
} catch (e) {

View File

@ -238,7 +238,7 @@ add_task(async function test_onUpdate_before_store_ready() {
const stub = sandbox.stub();
const manager = ExperimentFakes.manager();
sandbox.stub(ExperimentAPI, "_store").get(() => manager.store);
sandbox.stub(manager.store, "getAllActiveExperiments").returns([
sandbox.stub(manager.store, "getAllActive").returns([
ExperimentFakes.experiment("foo-experiment", {
branch: {
slug: "control",
@ -293,7 +293,7 @@ add_task(async function test_ExperimentFeature_test_ready_late() {
},
});
sandbox.stub(manager.store, "getAllActiveRollouts").returns([rollout]);
sandbox.stub(manager.store, "getAllRollouts").returns([rollout]);
await manager.onStartup();

View File

@ -14,8 +14,8 @@ add_task(async function test_createTargetingContext() {
const recipe = ExperimentFakes.recipe("foo");
const rollout = ExperimentFakes.rollout("bar");
sandbox.stub(manager.store, "ready").resolves();
sandbox.stub(manager.store, "getAllActiveExperiments").returns([recipe]);
sandbox.stub(manager.store, "getAllActiveRollouts").returns([rollout]);
sandbox.stub(manager.store, "getAllActive").returns([recipe]);
sandbox.stub(manager.store, "getAllRollouts").returns([rollout]);
let context = manager.createTargetingContext();
const activeSlugs = await context.activeExperiments;

View File

@ -192,13 +192,13 @@ function assertExpectedPrefValues(pref, branch, expected, visible, msg) {
*/
function assertEmptyStore(store) {
Assert.deepEqual(
store.getAllActiveExperiments(),
store.getAllActive(),
[],
"There should be no experiments active."
);
Assert.deepEqual(
store.getAllActiveRollouts(),
store.getAllRollouts(),
[],
"There should be no rollouts active"
);
@ -1766,8 +1766,8 @@ add_task(async function test_prefChange() {
);
const enrollments = isRollout
? store.getAllActiveRollouts()
: store.getAllActiveExperiments();
? store.getAllRollouts()
: store.getAllActive();
Assert.equal(
enrollments.length,
@ -2374,8 +2374,8 @@ add_task(async function test_clearUserPref() {
);
const enrollments = isRollout
? store.getAllActiveRollouts()
: store.getAllActiveExperiments();
? store.getAllRollouts()
: store.getAllActive();
Assert.equal(
enrollments.length,
@ -2823,8 +2823,8 @@ add_task(async function test_restorePrefs_manifestChanged() {
});
const enrollments = isRollout
? store.getAllActiveRollouts()
: store.getAllActiveExperiments();
? store.getAllRollouts()
: store.getAllActive();
Assert.equal(
enrollments.length,

View File

@ -394,7 +394,7 @@ add_task(async function test_remove_rollout_onFinalize() {
const manager = ExperimentFakes.manager(store);
const rollout = ExperimentFakes.rollout("foo");
sinon.stub(store, "getAllActiveRollouts").returns([rollout]);
sinon.stub(store, "getAllRollouts").returns([rollout]);
sinon.stub(store, "get").returns(rollout);
sinon.spy(manager, "unenroll");
sinon.spy(manager, "sendFailureTelemetry");
@ -427,7 +427,7 @@ add_task(async function test_rollout_telemetry_events() {
globalSandbox.spy(TelemetryEnvironment, "setExperimentInactive");
globalSandbox.spy(TelemetryEvents, "sendEvent");
sinon.stub(store, "getAllActiveRollouts").returns([rollout]);
sinon.stub(store, "getAllRollouts").returns([rollout]);
sinon.stub(store, "get").returns(rollout);
sinon.spy(manager, "sendFailureTelemetry");

View File

@ -168,7 +168,7 @@ add_task(async function test_hasExperimentForFeature() {
);
});
add_task(async function test_getAll_getAllActiveExperiments() {
add_task(async function test_getAll_getAllActive() {
const store = ExperimentFakes.store();
await store.init();
@ -183,13 +183,13 @@ add_task(async function test_getAll_getAllActiveExperiments() {
".getAll() should return all experiments"
);
Assert.deepEqual(
store.getAllActiveExperiments().map(e => e.slug),
store.getAllActive().map(e => e.slug),
["qux"],
".getAllActiveExperiments() should return all experiments that are active"
".getAllActive() should return all experiments that are active"
);
});
add_task(async function test_getAll_getAllActiveExperiments() {
add_task(async function test_getAll_getAllActive_no_rollouts() {
const store = ExperimentFakes.store();
await store.init();
@ -205,13 +205,13 @@ add_task(async function test_getAll_getAllActiveExperiments() {
".getAll() should return all experiments and rollouts"
);
Assert.deepEqual(
store.getAllActiveExperiments().map(e => e.slug),
store.getAllActive().map(e => e.slug),
["qux"],
".getAllActiveExperiments() should return all experiments that are active and no rollouts"
".getAllActive() should return all experiments that are active and no rollouts"
);
});
add_task(async function test_getAllActiveRollouts() {
add_task(async function test_getAllRollouts() {
const store = ExperimentFakes.store();
await store.init();
@ -226,9 +226,9 @@ add_task(async function test_getAllActiveRollouts() {
".getAll() should return all experiments and rollouts"
);
Assert.deepEqual(
store.getAllActiveRollouts().map(e => e.slug),
store.getAllRollouts().map(e => e.slug),
["foo", "bar", "baz"],
".getAllActiveRollouts() should return all rollouts"
".getAllRollouts() should return all rollouts"
);
});

View File

@ -39,9 +39,9 @@ add_task(async function test_enrollmentHelper() {
await enrollmentPromise;
Assert.ok(manager.store.getAllActiveExperiments().length === 1, "Enrolled");
Assert.ok(manager.store.getAllActive().length === 1, "Enrolled");
Assert.equal(
manager.store.getAllActiveExperiments()[0].slug,
manager.store.getAllActive()[0].slug,
recipe.slug,
"Has expected slug"
);

View File

@ -36,7 +36,7 @@ add_task(async function test_updateRecipes_activeExperiments() {
const onRecipe = sandbox.stub(manager, "onRecipe");
sinon.stub(loader.remoteSettingsClient, "get").resolves([PASS_FILTER_RECIPE]);
sandbox.stub(manager.store, "ready").resolves();
sandbox.stub(manager.store, "getAllActiveExperiments").returns([recipe]);
sandbox.stub(manager.store, "getAllActive").returns([recipe]);
await loader.init();
@ -53,7 +53,7 @@ add_task(async function test_updateRecipes_isFirstRun() {
const onRecipe = sandbox.stub(manager, "onRecipe");
sinon.stub(loader.remoteSettingsClient, "get").resolves([PASS_FILTER_RECIPE]);
sandbox.stub(manager.store, "ready").resolves();
sandbox.stub(manager.store, "getAllActiveExperiments").returns([recipe]);
sandbox.stub(manager.store, "getAllActive").returns([recipe]);
// Pretend to be in the first startup
FirstStartup._state = FirstStartup.IN_PROGRESS;
@ -96,7 +96,7 @@ add_task(async function test_updateRecipes_invalidFeatureId() {
const onRecipe = sandbox.stub(manager, "onRecipe");
sinon.stub(loader.remoteSettingsClient, "get").resolves([badRecipe]);
sandbox.stub(manager.store, "ready").resolves();
sandbox.stub(manager.store, "getAllActiveExperiments").returns([]);
sandbox.stub(manager.store, "getAllActive").returns([]);
await loader.init();
ok(onRecipe.notCalled, "No recipes");
@ -140,7 +140,7 @@ add_task(async function test_updateRecipes_invalidFeatureValue() {
const onRecipe = sandbox.stub(manager, "onRecipe");
sinon.stub(loader.remoteSettingsClient, "get").resolves([badRecipe]);
sandbox.stub(manager.store, "ready").resolves();
sandbox.stub(manager.store, "getAllActiveExperiments").returns([]);
sandbox.stub(manager.store, "getAllActive").returns([]);
await loader.init();
ok(onRecipe.notCalled, "No recipes");
@ -158,7 +158,7 @@ add_task(async function test_updateRecipes_invalidRecipe() {
const onRecipe = sandbox.stub(manager, "onRecipe");
sinon.stub(loader.remoteSettingsClient, "get").resolves([badRecipe]);
sandbox.stub(manager.store, "ready").resolves();
sandbox.stub(manager.store, "getAllActiveExperiments").returns([]);
sandbox.stub(manager.store, "getAllActive").returns([]);
await loader.init();
ok(onRecipe.notCalled, "No recipes");
@ -525,8 +525,8 @@ add_task(async function test_updateRecipes_validationTelemetry() {
sinon.stub(manager, "onRecipe");
sinon.stub(manager.store, "ready").resolves();
sinon.stub(manager.store, "getAllActiveExperiments").returns([]);
sinon.stub(manager.store, "getAllActiveRollouts").returns([]);
sinon.stub(manager.store, "getAllActive").returns([]);
sinon.stub(manager.store, "getAllRollouts").returns([]);
const telemetrySpy = sinon.spy(manager, "sendValidationFailedTelemetry");
@ -619,8 +619,8 @@ add_task(async function test_updateRecipes_validationDisabled() {
sinon.stub(manager, "onRecipe");
sinon.stub(manager.store, "ready").resolves();
sinon.stub(manager.store, "getAllActiveExperiments").returns([]);
sinon.stub(manager.store, "getAllActiveRollouts").returns([]);
sinon.stub(manager.store, "getAllActive").returns([]);
sinon.stub(manager.store, "getAllRollouts").returns([]);
const finalizeStub = sinon.stub(manager, "onFinalize");
const telemetrySpy = sinon.spy(manager, "sendValidationFailedTelemetry");
@ -857,8 +857,8 @@ add_task(async function test_updateRecipes_featureValidationOptOut() {
sinon.stub(manager, "onRecipe");
sinon.stub(manager, "onFinalize");
sinon.stub(manager.store, "ready").resolves();
sinon.stub(manager.store, "getAllActiveExperiments").returns([]);
sinon.stub(manager.store, "getAllActiveRollouts").returns([]);
sinon.stub(manager.store, "getAllActive").returns([]);
sinon.stub(manager.store, "getAllRollouts").returns([]);
await loader.init();
ok(
@ -909,8 +909,8 @@ add_task(async function test_updateRecipes_invalidFeature_mismatch() {
sinon.stub(manager, "onRecipe");
sinon.stub(manager, "onFinalize");
sinon.stub(manager.store, "ready").resolves();
sinon.stub(manager.store, "getAllActiveExperiments").returns([]);
sinon.stub(manager.store, "getAllActiveRollouts").returns([]);
sinon.stub(manager.store, "getAllActive").returns([]);
sinon.stub(manager.store, "getAllRollouts").returns([]);
const telemetrySpy = sinon.stub(manager, "sendValidationFailedTelemetry");
const targetingSpy = sinon.spy(loader, "checkTargeting");

View File

@ -184,7 +184,7 @@ class PreferenceRolloutAction extends BaseAction {
*/
async _verifyRolloutPrefs({ slug, preferences }) {
const existingManagedPrefs = new Set();
for (const rollout of await lazy.PreferenceRollouts.getAllActiveExperiments()) {
for (const rollout of await lazy.PreferenceRollouts.getAllActive()) {
if (rollout.slug === slug) {
continue;
}

View File

@ -164,7 +164,7 @@ var AddonStudies = {
},
async init() {
for (const study of await this.getAllActiveExperiments()) {
for (const study of await this.getAllActive()) {
// If an active study's add-on has been removed since we last ran, stop it.
const addon = await lazy.AddonManager.getAddonByID(study.addonId);
if (!addon) {
@ -247,7 +247,7 @@ var AddonStudies = {
},
async migration02RemoveOldAddonStudyAction() {
const studies = await AddonStudies.getAllActiveExperiments({
const studies = await AddonStudies.getAllActive({
branched: AddonStudies.FILTER_NOT_BRANCHED,
});
if (!studies.length) {
@ -340,7 +340,7 @@ var AddonStudies = {
* Fetch all studies in storage.
* @return {Array<Study>}
*/
async getAllActiveExperiments(options) {
async getAllActive(options) {
return (await this.getAll(options)).filter(study => study.active);
},

View File

@ -212,7 +212,7 @@ var PreferenceExperiments = {
async init() {
CleanupManager.addCleanupHandler(() => this.saveStartupPrefs());
for (const experiment of await this.getAllActiveExperiments()) {
for (const experiment of await this.getAllActive()) {
// Check that the current value of the preference is still what we set it to
for (const [preferenceName, spec] of Object.entries(
experiment.preferences
@ -270,7 +270,7 @@ var PreferenceExperiments = {
// Be careful not to store user branch prefs here, because this
// would cause the default branch to match the user branch,
// causing the user branch pref to get cleared.
const allExperiments = await this.getAllActiveExperiments();
const allExperiments = await this.getAllActive();
const defaultBranchPrefs = allExperiments
.flatMap(exp => Object.entries(exp.preferences))
.filter(
@ -919,7 +919,7 @@ var PreferenceExperiments = {
* Get a list of experiment objects for all active experiments.
* @resolves {Experiment[]}
*/
async getAllActiveExperiments() {
async getAllActive() {
const store = await ensureStorage();
return Object.values(store.data.experiments)
.filter(e => !e.expired)
@ -1051,7 +1051,7 @@ var PreferenceExperiments = {
},
async migration05RemoveOldAction() {
const experiments = await PreferenceExperiments.getAllActiveExperiments();
const experiments = await PreferenceExperiments.getAllActive();
for (const experiment of experiments) {
if (experiment.actionName == "SinglePreferenceExperimentAction") {
try {

View File

@ -139,7 +139,7 @@ var PreferenceRollouts = {
* @param {object} rolloutPrefsChanged Map from pref name to previous pref value
*/
async recordOriginalValues(originalPreferences) {
for (const rollout of await this.getAllActiveExperiments()) {
for (const rollout of await this.getAllActive()) {
let shouldSaveRollout = false;
// Count the number of preferences in this rollout that are now redundant.
@ -177,7 +177,7 @@ var PreferenceRollouts = {
async init() {
lazy.CleanupManager.addCleanupHandler(() => this.saveStartupPrefs());
for (const rollout of await this.getAllActiveExperiments()) {
for (const rollout of await this.getAllActive()) {
if (this.GRADUATION_SET.has(rollout.slug)) {
await this.graduate(rollout, "in-graduation-set");
continue;
@ -322,7 +322,7 @@ var PreferenceRollouts = {
},
/** Get all rollouts in the "active" state. */
async getAllActiveExperiments() {
async getAllActive() {
const rollouts = await this.getAll();
return rollouts.filter(rollout => rollout.state === this.STATE_ACTIVE);
},
@ -337,7 +337,7 @@ var PreferenceRollouts = {
prefBranch.clearUserPref(pref);
}
for (const rollout of await this.getAllActiveExperiments()) {
for (const rollout of await this.getAllActive()) {
for (const prefSpec of rollout.preferences) {
lazy.PrefUtils.setPref(
STARTUP_PREFS_BRANCH + prefSpec.preferenceName,

View File

@ -908,15 +908,15 @@ var dataProviders = {
nimbusRollouts,
] = await Promise.all(
[
NormandyAddonStudies.getAllActiveExperiments(),
NormandyPreferenceRollouts.getAllActiveExperiments(),
NormandyPreferenceStudies.getAllActiveExperiments(),
NormandyAddonStudies.getAllActive(),
NormandyPreferenceRollouts.getAllActive(),
NormandyPreferenceStudies.getAllActive(),
ExperimentManager.store
.ready()
.then(() => ExperimentManager.store.getAllActiveExperiments()),
.then(() => ExperimentManager.store.getAllActive()),
ExperimentManager.store
.ready()
.then(() => ExperimentManager.store.getAllActiveRollouts()),
.then(() => ExperimentManager.store.getAllRollouts()),
].map(promise =>
promise
.catch(error => {

View File

@ -213,17 +213,13 @@ add_task(function normandy() {
add_task(function normandyErrorHandling() {
return NormandyTestUtils.decorate(
NormandyTestUtils.withStub(
PreferenceExperiments,
"getAllActiveExperiments",
{
returnValue: Promise.reject("Expected error - PreferenceExperiments"),
}
),
NormandyTestUtils.withStub(AddonStudies, "getAllActiveExperiments", {
NormandyTestUtils.withStub(PreferenceExperiments, "getAllActive", {
returnValue: Promise.reject("Expected error - PreferenceExperiments"),
}),
NormandyTestUtils.withStub(AddonStudies, "getAllActive", {
returnValue: Promise.reject("Expected error - AddonStudies"),
}),
NormandyTestUtils.withStub(PreferenceRollouts, "getAllActiveExperiments", {
NormandyTestUtils.withStub(PreferenceRollouts, "getAllActive", {
returnValue: Promise.reject("Expected error - PreferenceRollouts"),
}),
async function testNormandyErrorHandling() {