mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 1572479 pt2 - Rename preference experiment name to slug r=rdalal 

Naming this field better matches the intent of the field, other actions, and
the schemas used by the recipes on the server. Overall it makes the code less
confusing, and more consistent.

Differential Revision: https://phabricator.services.mozilla.com/D41296

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Michael Cooper 2019-08-28 19:58:56 +00:00
Родитель 4b7d518ada
Коммит b678dbac63
12 изменённых файлов: 283 добавлений и 196 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

1
toolkit/components/normandy/NormandyMigrations.jsm
Просмотреть файл

@ -58,6 +58,7 @@ const NormandyMigrations = {
PreferenceExperiments.migrations.migration01MoveExperiments,
PreferenceExperiments.migrations.migration02MultiPreference,
PreferenceExperiments.migrations.migration03AddActionName,
PreferenceExperiments.migrations.migration04RenameNameToSlug,
],
};

12
toolkit/components/normandy/actions/PreferenceExperimentAction.jsm
Просмотреть файл

@ -42,7 +42,7 @@ class PreferenceExperimentAction extends BaseStudyAction {
constructor() {
super();
this.seenExperimentNames = [];
this.seenExperimentSlugs = [];
}
async _run(recipe) {
@ -55,7 +55,7 @@ class PreferenceExperimentAction extends BaseStudyAction {
userFacingDescription,
} = recipe.arguments;
this.seenExperimentNames.push(slug);
this.seenExperimentSlugs.push(slug);
// If we're not in the experiment, enroll!
const hasSlug = await PreferenceExperiments.has(slug);
@ -89,7 +89,7 @@ class PreferenceExperimentAction extends BaseStudyAction {
const branch = await this.chooseBranch(slug, branches);
const experimentType = isHighPopulation ? "exp-highpop" : "exp";
await PreferenceExperiments.start({
name: slug,
slug,
actionName: this.name,
branch: branch.slug,
preferences: branch.preferences,
@ -140,15 +140,15 @@ class PreferenceExperimentAction extends BaseStudyAction {
return null;
}
if (this.seenExperimentNames.includes(experiment.name)) {
if (this.seenExperimentSlugs.includes(experiment.slug)) {
return null;
}
return PreferenceExperiments.stop(experiment.name, {
return PreferenceExperiments.stop(experiment.slug, {
resetValue: true,
reason: "recipe-not-seen",
}).catch(e => {
this.log.warn(`Stopping experiment ${experiment.name} failed: ${e}`);
this.log.warn(`Stopping experiment ${experiment.slug} failed: ${e}`);
});
})
);

18
toolkit/components/normandy/content/about-studies/about-studies.js
Просмотреть файл

@ -190,7 +190,7 @@ class StudyList extends React.Component {
study.type === "addon"
? r(AddonStudyListItem, { key: study.slug, study, translations })
: r(PreferenceStudyListItem, {
key: study.name,
key: study.slug,
study,
translations,
})
@ -204,7 +204,7 @@ class StudyList extends React.Component {
study.type === "addon"
? r(AddonStudyListItem, { key: study.slug, study, translations })
: r(PreferenceStudyListItem, {
key: study.name,
key: study.slug,
study,
translations,
})
@ -307,7 +307,7 @@ class PreferenceStudyListItem extends React.Component {
handleClickRemove() {
sendPageEvent("RemovePreferenceStudy", {
experimentName: this.props.study.name,
experimentName: this.props.study.slug,
reason: "individual-opt-out",
});
}
@ -347,7 +347,7 @@ class PreferenceStudyListItem extends React.Component {
"li",
{
className: classnames("study pref-study", { disabled: study.expired }),
"data-study-slug": study.name, // used to identify this row in tests
"data-study-slug": study.slug, // used to identify this row in tests
},
r("div", { className: "study-icon" }, userFacingName),
r(
@ -356,7 +356,11 @@ class PreferenceStudyListItem extends React.Component {
r(
"div",
{ className: "study-header" },
r("span", { className: "study-name" }, study.name),
r(
"span",
{ className: "study-name" },
study.userFacingName || study.slug
),
r("span", {}, "\u2022"), // •
r(
"span",
@ -386,7 +390,9 @@ class PreferenceStudyListItem extends React.Component {
}
PreferenceStudyListItem.propTypes = {
study: PropTypes.shape({
name: PropTypes.string.isRequired,
slug: PropTypes.string.isRequired,
userFacingName: PropTypes.string,
userFacingDescription: PropTypes.string,
expired: PropTypes.bool.isRequired,
preferenceName: PropTypes.string.isRequired,
preferenceValue: PropTypes.oneOf(

10
toolkit/components/normandy/lib/ClientEnvironment.jsm
Просмотреть файл

@ -97,12 +97,12 @@ class ClientEnvironment extends ClientEnvironmentBase {
return (async () => {
const names = { all: [], active: [], expired: [] };
for (const experiment of await PreferenceExperiments.getAll()) {
names.all.push(experiment.name);
if (experiment.expired) {
names.expired.push(experiment.name);
for (const { slug, expired } of await PreferenceExperiments.getAll()) {
names.all.push(slug);
if (expired) {
names.expired.push(slug);
} else {
names.active.push(experiment.name);
names.active.push(slug);
}
}

203
toolkit/components/normandy/lib/PreferenceExperiments.jsm
Просмотреть файл

@ -20,16 +20,16 @@
/**
* Experiments store info about an active or expired preference experiment.
* @typedef {Object} Experiment
* @property {string} name
* Unique name of the experiment
* @property {string} slug
* A string uniquely identifying the experiment. Used for telemetry, and other
* machine-oriented use cases. Used as a display name if `userFacingName` is
* null.
* @property {string|null} userFacingName
* A user-friendly name for the experiment. Null on old-style
* single-preference experiments, which do not have a
* userFacingName.
* A user-friendly name for the experiment. Null on old-style single-preference
* experiments, which do not have a userFacingName.
* @property {string|null} userFacingDescription
* A user-friendly description of the experiment. Null on old-style
* single-preference experiments, which do not have a
* userFacingDescription.
* single-preference experiments, which do not have a userFacingDescription.
* @property {string} branch
* Experiment branch that the user was matched to
* @property {boolean} expired
@ -39,8 +39,8 @@
* recipe server.
* @property {Object} preferences
* An object consisting of all the preferences that are set by this experiment.
* Keys are the name of each preference affected by this experiment.
* Values are Preference Objects, about which see below.
* Keys are the name of each preference affected by this experiment. Values are
* Preference Objects, about which see below.
* @property {string} experimentType
* The type to report to Telemetry's experiment marker API.
*/
@ -246,9 +246,9 @@ var PreferenceExperiments = {
) {
// if not, stop the experiment, and skip the remaining steps
log.info(
`Stopping experiment "${experiment.name}" because its value changed`
`Stopping experiment "${experiment.slug}" because its value changed`
);
await this.stop(experiment.name, {
await this.stop(experiment.slug, {
resetValue: false,
reason: "user-preference-changed-sideload",
});
@ -262,13 +262,13 @@ var PreferenceExperiments = {
// Notify Telemetry of experiments we're running, since they don't persist between restarts
TelemetryEnvironment.setExperimentActive(
experiment.name,
experiment.slug,
experiment.branch,
{ type: EXPERIMENT_TYPE_PREFIX + experiment.experimentType }
);
// Watch for changes to the experiment's preference
this.startObserver(experiment.name, experiment.preferences);
this.startObserver(experiment.slug, experiment.preferences);
}
},
@ -332,7 +332,13 @@ var PreferenceExperiments = {
const experiments = {};
for (const exp of mockExperiments) {
experiments[exp.name] = exp;
if (exp.name) {
throw new Error(
"Preference experiments 'name' field has been replaced by 'slug' and 'userFacingName', please update."
);
}
experiments[exp.slug] = exp;
}
const data = { experiments };
@ -368,7 +374,7 @@ var PreferenceExperiments = {
/**
* Start a new preference experiment.
* @param {Object} experiment
* @param {string} experiment.name
* @param {string} experiment.slug
* @param {string} experiment.actionName The action who knows about this
* experiment and is responsible for cleaning it up. This should
* correspond to the name of some BaseAction subclass.
@ -382,7 +388,8 @@ var PreferenceExperiments = {
* - If the given preferenceType does not match the existing stored preference
*/
async start({
name,
name = null, // To check if old code is still using `name` instead of `slug`, and provide a nice error message
slug,
actionName,
branch,
preferences,
@ -390,15 +397,21 @@ var PreferenceExperiments = {
userFacingName = null,
userFacingDescription = null,
}) {
log.debug(`PreferenceExperiments.start(${name}, ${branch})`);
if (name) {
throw new Error(
"Preference experiments 'name' field has been replaced by 'slug' and 'userFacingName', please update."
);
}
log.debug(`PreferenceExperiments.start(${slug}, ${branch})`);
const store = await ensureStorage();
if (name in store.data.experiments) {
TelemetryEvents.sendEvent("enrollFailed", "preference_study", name, {
if (slug in store.data.experiments) {
TelemetryEvents.sendEvent("enrollFailed", "preference_study", slug, {
reason: "name-conflict",
});
throw new Error(
`A preference experiment named "${name}" already exists.`
`A preference experiment with the slug "${slug}" already exists.`
);
}
@ -414,7 +427,7 @@ var PreferenceExperiments = {
);
if (preferencesWithConflicts.length > 0) {
TelemetryEvents.sendEvent("enrollFailed", "preference_study", name, {
TelemetryEvents.sendEvent("enrollFailed", "preference_study", slug, {
reason: "pref-conflict",
});
throw new Error(
@ -425,7 +438,7 @@ var PreferenceExperiments = {
}
if (experimentType.length > MAX_EXPERIMENT_SUBTYPE_LENGTH) {
TelemetryEvents.sendEvent("enrollFailed", "preference_study", name, {
TelemetryEvents.sendEvent("enrollFailed", "preference_study", slug, {
reason: "experiment-type-too-long",
});
throw new Error(
@ -441,7 +454,7 @@ var PreferenceExperiments = {
const { preferenceBranchType, preferenceType } = preferenceInfo;
const preferenceBranch = PreferenceBranchType[preferenceBranchType];
if (!preferenceBranch) {
TelemetryEvents.sendEvent("enrollFailed", "preference_study", name, {
TelemetryEvents.sendEvent("enrollFailed", "preference_study", slug, {
reason: "invalid-branch",
});
throw new Error(
@ -453,7 +466,7 @@ var PreferenceExperiments = {
const givenPrefType = PREFERENCE_TYPE_MAP[preferenceType];
if (!preferenceType || !givenPrefType) {
TelemetryEvents.sendEvent("enrollFailed", "preference_study", name, {
TelemetryEvents.sendEvent("enrollFailed", "preference_study", slug, {
reason: "invalid-type",
});
throw new Error(
@ -465,7 +478,7 @@ var PreferenceExperiments = {
prevPrefType !== Services.prefs.PREF_INVALID &&
prevPrefType !== givenPrefType
) {
TelemetryEvents.sendEvent("enrollFailed", "preference_study", name, {
TelemetryEvents.sendEvent("enrollFailed", "preference_study", slug, {
reason: "invalid-type",
});
throw new Error(
@ -497,11 +510,11 @@ var PreferenceExperiments = {
preferenceValue
);
}
PreferenceExperiments.startObserver(name, preferences);
PreferenceExperiments.startObserver(slug, preferences);
/** @type {Experiment} */
const experiment = {
name,
slug,
actionName,
branch,
expired: false,
@ -512,13 +525,13 @@ var PreferenceExperiments = {
userFacingDescription,
};
store.data.experiments[name] = experiment;
store.data.experiments[slug] = experiment;
store.saveSoon();
TelemetryEnvironment.setExperimentActive(name, branch, {
TelemetryEnvironment.setExperimentActive(slug, branch, {
type: EXPERIMENT_TYPE_PREFIX + experimentType,
});
TelemetryEvents.sendEvent("enroll", "preference_study", name, {
TelemetryEvents.sendEvent("enroll", "preference_study", slug, {
experimentType,
branch,
});
@ -528,18 +541,18 @@ var PreferenceExperiments = {
/**
* Register a preference observer that stops an experiment when the user
* modifies the preference.
* @param {string} experimentName
* @param {string} experimentSlug
* @param {string} preferenceName
* @param {string|integer|boolean} preferenceValue
* @throws {Error}
* If an observer for the named experiment is already active.
* If an observer for the experiment is already active.
*/
startObserver(experimentName, preferences) {
log.debug(`PreferenceExperiments.startObserver(${experimentName})`);
startObserver(experimentSlug, preferences) {
log.debug(`PreferenceExperiments.startObserver(${experimentSlug})`);
if (experimentObservers.has(experimentName)) {
if (experimentObservers.has(experimentSlug)) {
throw new Error(
`An observer for the preference experiment ${experimentName} is already active.`
`An observer for the preference experiment ${experimentSlug} is already active.`
);
}
@ -553,14 +566,14 @@ var PreferenceExperiments = {
preferenceType
);
if (newValue !== preferenceValue) {
PreferenceExperiments.stop(experimentName, {
PreferenceExperiments.stop(experimentSlug, {
resetValue: false,
reason: "user-preference-changed",
}).catch(Cu.reportError);
}
},
};
experimentObservers.set(experimentName, observerInfo);
experimentObservers.set(experimentSlug, observerInfo);
for (const preferenceName of Object.keys(preferences)) {
Services.prefs.addObserver(preferenceName, observerInfo);
}
@ -568,34 +581,34 @@ var PreferenceExperiments = {
/**
* Check if a preference observer is active for an experiment.
* @param {string} experimentName
* @param {string} experimentSlug
* @return {Boolean}
*/
hasObserver(experimentName) {
log.debug(`PreferenceExperiments.hasObserver(${experimentName})`);
return experimentObservers.has(experimentName);
hasObserver(experimentSlug) {
log.debug(`PreferenceExperiments.hasObserver(${experimentSlug})`);
return experimentObservers.has(experimentSlug);
},
/**
* Disable a preference observer for the named experiment.
* @param {string} experimentName
* Disable a preference observer for an experiment.
* @param {string} experimentSlug
* @throws {Error}
* If there is no active observer for the named experiment.
* If there is no active observer for the experiment.
*/
stopObserver(experimentName) {
log.debug(`PreferenceExperiments.stopObserver(${experimentName})`);
stopObserver(experimentSlug) {
log.debug(`PreferenceExperiments.stopObserver(${experimentSlug})`);
if (!experimentObservers.has(experimentName)) {
if (!experimentObservers.has(experimentSlug)) {
throw new Error(
`No observer for the preference experiment ${experimentName} found.`
`No observer for the preference experiment ${experimentSlug} found.`
);
}
const observer = experimentObservers.get(experimentName);
const observer = experimentObservers.get(experimentSlug);
for (const preferenceName of Object.keys(observer.preferences)) {
Services.prefs.removeObserver(preferenceName, observer);
}
experimentObservers.delete(experimentName);
experimentObservers.delete(experimentSlug);
},
/**
@ -612,30 +625,30 @@ var PreferenceExperiments = {
},
/**
* Update the timestamp storing when Normandy last sent a recipe for the named
* Update the timestamp storing when Normandy last sent a recipe for the
* experiment.
* @param {string} experimentName
* @param {string} experimentSlug
* @rejects {Error}
* If there is no stored experiment with the given name.
* If there is no stored experiment with the given slug.
*/
async markLastSeen(experimentName) {
log.debug(`PreferenceExperiments.markLastSeen(${experimentName})`);
async markLastSeen(experimentSlug) {
log.debug(`PreferenceExperiments.markLastSeen(${experimentSlug})`);
const store = await ensureStorage();
if (!(experimentName in store.data.experiments)) {
if (!(experimentSlug in store.data.experiments)) {
throw new Error(
`Could not find a preference experiment named "${experimentName}"`
`Could not find a preference experiment with the slug "${experimentSlug}"`
);
}
store.data.experiments[experimentName].lastSeen = new Date().toJSON();
store.data.experiments[experimentSlug].lastSeen = new Date().toJSON();
store.saveSoon();
},
/**
* Stop an active experiment, deactivate preference watchers, and optionally
* reset the associated preference to its previous value.
* @param {string} experimentName
* @param {string} experimentSlug
* @param {Object} options
* @param {boolean} [options.resetValue = true]
* If true, reset the preference to its original value prior to
@ -644,45 +657,45 @@ var PreferenceExperiments = {
* Reason that the experiment is ending. Optional, defaults to
* "unknown".
* @rejects {Error}
* If there is no stored experiment with the given name, or if the
* If there is no stored experiment with the given slug, or if the
* experiment has already expired.
*/
async stop(experimentName, { resetValue = true, reason = "unknown" } = {}) {
async stop(experimentSlug, { resetValue = true, reason = "unknown" } = {}) {
log.debug(
`PreferenceExperiments.stop(${experimentName}, {resetValue: ${resetValue}, reason: ${reason}})`
`PreferenceExperiments.stop(${experimentSlug}, {resetValue: ${resetValue}, reason: ${reason}})`
);
if (reason === "unknown") {
log.warn(`experiment ${experimentName} ending for unknown reason`);
log.warn(`experiment ${experimentSlug} ending for unknown reason`);
}
const store = await ensureStorage();
if (!(experimentName in store.data.experiments)) {
if (!(experimentSlug in store.data.experiments)) {
TelemetryEvents.sendEvent(
"unenrollFailed",
"preference_study",
experimentName,
experimentSlug,
{ reason: "does-not-exist" }
);
throw new Error(
`Could not find a preference experiment named "${experimentName}"`
`Could not find a preference experiment with the slug "${experimentSlug}"`
);
}
const experiment = store.data.experiments[experimentName];
const experiment = store.data.experiments[experimentSlug];
if (experiment.expired) {
TelemetryEvents.sendEvent(
"unenrollFailed",
"preference_study",
experimentName,
experimentSlug,
{ reason: "already-unenrolled" }
);
throw new Error(
`Cannot stop preference experiment "${experimentName}" because it is already expired`
`Cannot stop preference experiment "${experimentSlug}" because it is already expired`
);
}
if (PreferenceExperiments.hasObserver(experimentName)) {
PreferenceExperiments.stopObserver(experimentName);
if (PreferenceExperiments.hasObserver(experimentSlug)) {
PreferenceExperiments.stopObserver(experimentSlug);
}
if (resetValue) {
@ -708,7 +721,7 @@ var PreferenceExperiments = {
preferences.clearUserPref(preferenceName);
} else {
log.warn(
`Can't revert pref ${preferenceName} for experiment ${experimentName} ` +
`Can't revert pref ${preferenceName} for experiment ${experimentSlug} ` +
`because it had no default value. ` +
`Preference will be reset at the next restart.`
);
@ -721,8 +734,8 @@ var PreferenceExperiments = {
experiment.expired = true;
store.saveSoon();
TelemetryEnvironment.setExperimentInactive(experimentName);
TelemetryEvents.sendEvent("unenroll", "preference_study", experimentName, {
TelemetryEnvironment.setExperimentInactive(experimentSlug);
TelemetryEvents.sendEvent("unenroll", "preference_study", experimentSlug, {
didResetValue: resetValue ? "true" : "false",
branch: experiment.branch,
reason,
@ -747,22 +760,22 @@ var PreferenceExperiments = {
},
/**
* Get the experiment object for the named experiment.
* @param {string} experimentName
* Get the experiment object for the experiment.
* @param {string} experimentSlug
* @resolves {Experiment}
* @rejects {Error}
* If no preference experiment exists with the given name.
* If no preference experiment exists with the given slug.
*/
async get(experimentName) {
log.debug(`PreferenceExperiments.get(${experimentName})`);
async get(experimentSlug) {
log.debug(`PreferenceExperiments.get(${experimentSlug})`);
const store = await ensureStorage();
if (!(experimentName in store.data.experiments)) {
if (!(experimentSlug in store.data.experiments)) {
throw new Error(
`Could not find a preference experiment named "${experimentName}"`
`Could not find a preference experiment with the slug "${experimentSlug}"`
);
}
return this._cloneExperiment(store.data.experiments[experimentName]);
return this._cloneExperiment(store.data.experiments[experimentSlug]);
},
/**
@ -788,14 +801,14 @@ var PreferenceExperiments = {
},
/**
* Check if an experiment exists with the given name.
* @param {string} experimentName
* Check if an experiment exists with the given slug.
* @param {string} experimentSlug
* @resolves {boolean} True if the experiment exists, false if it doesn't.
*/
async has(experimentName) {
log.debug(`PreferenceExperiments.has(${experimentName})`);
async has(experimentSlug) {
log.debug(`PreferenceExperiments.has(${experimentSlug})`);
const store = await ensureStorage();
return experimentName in store.data.experiments;
return experimentSlug in store.data.experiments;
},
/**
@ -873,5 +886,19 @@ var PreferenceExperiments = {
}
storage.saveSoon();
},
async migration04RenameNameToSlug(storage = null) {
if (!storage) {
storage = await ensureStorage();
}
// Rename "name" to "slug" to match the intended purpose of the field.
for (const experiment of Object.values(storage.data.experiments)) {
if (experiment.name && !experiment.slug) {
experiment.slug = experiment.name;
delete experiment.name;
}
}
storage.saveSoon();
},
},
};

2
toolkit/components/normandy/lib/ShieldPreferences.jsm
Просмотреть файл

@ -61,7 +61,7 @@ var ShieldPreferences = {
if (experiment.expired) {
return null;
}
return PreferenceExperiments.stop(experiment.name, {
return PreferenceExperiments.stop(experiment.slug, {
reason: "general-opt-out",
});
}

13
toolkit/components/normandy/test/NormandyTestUtils.jsm
Просмотреть файл

@ -80,9 +80,20 @@ const NormandyTestUtils = {
preferences[prefName] = { ...defaultPrefInfo, ...prefInfo };
}
// Generate a slug from userFacingName
let {
userFacingName = `Test study ${_preferenceStudyFactoryId++}`,
slug,
} = attrs;
delete attrs.slug;
if (userFacingName && !slug) {
slug = userFacingName.replace(" ", "-").toLowerCase();
}
return Object.assign(
{
name: `Test study ${_preferenceStudyFactoryId++}`,
userFacingName,
slug,
branch: "control",
expired: false,
lastSeen: new Date().toJSON(),

4
toolkit/components/normandy/test/browser/browser_ClientEnvironment.js
Просмотреть файл

@ -133,8 +133,8 @@ add_task(async function testDoNotTrack() {
});
add_task(async function testExperiments() {
const active = { name: "active", expired: false };
const expired = { name: "expired", expired: true };
const active = { slug: "active", expired: false };
const expired = { slug: "expired", expired: true };
const getAll = sinon
.stub(PreferenceExperiments, "getAll")
.callsFake(async () => [active, expired]);

146
toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js
Просмотреть файл

@ -30,7 +30,7 @@ function experimentFactory(attrs) {
return Object.assign(
{
name: "fakename",
slug: "fakeslug",
branch: "fakebranch",
expired: false,
lastSeen: NOW.toJSON(),
@ -173,6 +173,43 @@ const mockV4Data = {
},
};
const mockV5Data = {
experiments: {
hypothetical_experiment: {
slug: "hypothetical_experiment",
branch: "hypo_1",
actionName: "SinglePreferenceExperimentAction",
expired: false,
lastSeen: NOW.toJSON(),
preferences: {
"some.pref": {
preferenceValue: 2,
preferenceType: "integer",
previousPreferenceValue: 1,
preferenceBranchType: "user",
},
},
experimentType: "exp",
},
another_experiment: {
slug: "another_experiment",
branch: "another_4",
actionName: "SinglePreferenceExperimentAction",
expired: true,
lastSeen: NOW.toJSON(),
preferences: {
"another.pref": {
preferenceValue: true,
preferenceType: "boolean",
previousPreferenceValue: false,
preferenceBranchType: "default",
},
},
experimentType: "exp",
},
},
};
/**
* Make a mock `JsonFile` object with a no-op `saveSoon` method and a deep copy
* of the data passed.
@ -203,6 +240,12 @@ add_task(async function test_migrations() {
mockJsonFile = makeMockJsonFile(mockV3Data);
await PreferenceExperiments.migrations.migration03AddActionName(mockJsonFile);
Assert.deepEqual(mockJsonFile.data, mockV4Data);
mockJsonFile = makeMockJsonFile(mockV4Data);
await PreferenceExperiments.migrations.migration04RenameNameToSlug(
mockJsonFile
);
Assert.deepEqual(mockJsonFile.data, mockV5Data);
});
add_task(async function migration03KeepsActionName() {
@ -222,6 +265,7 @@ add_task(async function migrations_are_idempotent() {
[PreferenceExperiments.migrations.migration01MoveExperiments, mockV1Data],
[PreferenceExperiments.migrations.migration02MultiPreference, mockV2Data],
[PreferenceExperiments.migrations.migration03AddActionName, mockV3Data],
[PreferenceExperiments.migrations.migration04RenameNameToSlug, mockV4Data],
];
for (const [migration, mockOldData] of dataVersions) {
const mockJsonFileOnce = makeMockJsonFile(mockOldData);
@ -239,7 +283,7 @@ add_task(async function migrations_are_idempotent() {
// clearAllExperimentStorage
decorate_task(
withMockExperiments([experimentFactory({ name: "test" })]),
withMockExperiments([experimentFactory({ slug: "test" })]),
async function(experiments) {
ok(await PreferenceExperiments.has("test"), "Mock experiment is detected.");
await PreferenceExperiments.clearAllExperimentStorage();
@ -252,12 +296,12 @@ decorate_task(
// start should throw if an experiment with the given name already exists
decorate_task(
withMockExperiments([experimentFactory({ name: "test" })]),
withMockExperiments([experimentFactory({ slug: "test" })]),
withSendEventStub,
async function(experiments, sendEventStub) {
await Assert.rejects(
PreferenceExperiments.start({
name: "test",
slug: "test",
actionName: "SomeAction",
branch: "branch",
preferences: {
@ -283,7 +327,7 @@ decorate_task(
decorate_task(
withMockExperiments([
experimentFactory({
name: "test",
slug: "test",
preferences: { "fake.preferenceinteger": {} },
}),
]),
@ -291,7 +335,7 @@ decorate_task(
async function(experiments, sendEventStub) {
await Assert.rejects(
PreferenceExperiments.start({
name: "different",
slug: "different",
actionName: "SomeAction",
branch: "branch",
preferences: {
@ -329,7 +373,7 @@ decorate_task(withMockExperiments(), withSendEventStub, async function(
) {
await Assert.rejects(
PreferenceExperiments.start({
name: "test",
slug: "test",
actionName: "SomeAction",
branch: "branch",
preferences: {
@ -368,7 +412,7 @@ decorate_task(
mockPreferences.set("fake.preferenceinteger", 101, "user");
const experiment = {
name: "test",
slug: "test",
actionName: "SomeAction",
branch: "branch",
preferences: {
@ -392,7 +436,7 @@ decorate_task(
);
const expectedExperiment = {
name: "test",
slug: "test",
branch: "branch",
expired: false,
preferences: {
@ -464,7 +508,7 @@ decorate_task(
mockPreferences.set("fake.preference", "oldvalue", "user");
const experiment = {
name: "test",
slug: "test",
actionName: "SomeAction",
branch: "branch",
preferences: {
@ -482,7 +526,7 @@ decorate_task(
);
const expectedExperiment = {
name: "test",
slug: "test",
branch: "branch",
expired: false,
preferences: {
@ -528,7 +572,7 @@ decorate_task(withMockPreferences, withSendEventStub, async function(
await Assert.rejects(
PreferenceExperiments.start({
name: "test",
slug: "test",
actionName: "SomeAction",
branch: "branch",
preferences: {
@ -775,7 +819,7 @@ decorate_task(withMockExperiments(), async function() {
// markLastSeen should update the lastSeen date
const oldDate = new Date(1988, 10, 1).toJSON();
decorate_task(
withMockExperiments([experimentFactory({ name: "test", lastSeen: oldDate })]),
withMockExperiments([experimentFactory({ slug: "test", lastSeen: oldDate })]),
async function([experiment]) {
await PreferenceExperiments.markLastSeen("test");
Assert.notEqual(
@ -809,7 +853,7 @@ decorate_task(withMockExperiments(), withSendEventStub, async function(
// stop should throw if the experiment is already expired
decorate_task(
withMockExperiments([experimentFactory({ name: "test", expired: true })]),
withMockExperiments([experimentFactory({ slug: "test", expired: true })]),
withSendEventStub,
async function(experiments, sendEventStub) {
await Assert.rejects(
@ -834,7 +878,7 @@ decorate_task(
decorate_task(
withMockExperiments([
experimentFactory({
name: "test",
slug: "test",
expired: false,
branch: "fakebranch",
preferences: {
@ -912,7 +956,7 @@ decorate_task(
decorate_task(
withMockExperiments([
experimentFactory({
name: "test",
slug: "test",
expired: false,
preferences: {
"fake.preference": {
@ -961,7 +1005,7 @@ decorate_task(
decorate_task(
withMockExperiments([
experimentFactory({
name: "test",
slug: "test",
expired: false,
preferences: {
"fake.preference": {
@ -992,7 +1036,7 @@ decorate_task(
decorate_task(
withMockExperiments([
experimentFactory({
name: "test",
slug: "test",
expired: false,
branch: "fakebranch",
preferences: {
@ -1051,23 +1095,23 @@ decorate_task(withMockExperiments(), async function() {
// get
decorate_task(
withMockExperiments([experimentFactory({ name: "test" })]),
withMockExperiments([experimentFactory({ slug: "test" })]),
async function(experiments) {
const experiment = await PreferenceExperiments.get("test");
is(experiment.name, "test", "get fetches the correct experiment");
is(experiment.slug, "test", "get fetches the correct experiment");
// Modifying the fetched experiment must not edit the data source.
experiment.name = "othername";
experiment.slug = "othername";
const refetched = await PreferenceExperiments.get("test");
is(refetched.name, "test", "get returns a copy of the experiment");
is(refetched.slug, "test", "get returns a copy of the experiment");
}
);
// get all
decorate_task(
withMockExperiments([
experimentFactory({ name: "experiment1", disabled: false }),
experimentFactory({ name: "experiment2", disabled: true }),
experimentFactory({ slug: "experiment1", disabled: false }),
experimentFactory({ slug: "experiment2", disabled: true }),
]),
async function testGetAll([experiment1, experiment2]) {
const fetchedExperiments = await PreferenceExperiments.getAll();
@ -1077,12 +1121,12 @@ decorate_task(
"getAll returns a list of all stored experiments"
);
Assert.deepEqual(
fetchedExperiments.find(e => e.name === "experiment1"),
fetchedExperiments.find(e => e.slug === "experiment1"),
experiment1,
"getAll returns a list with the correct experiments"
);
const fetchedExperiment2 = fetchedExperiments.find(
e => e.name === "experiment2"
e => e.slug === "experiment2"
);
Assert.deepEqual(
fetchedExperiment2,
@ -1090,9 +1134,9 @@ decorate_task(
"getAll returns a list with the correct experiments, including disabled ones"
);
fetchedExperiment2.name = "othername";
fetchedExperiment2.slug = "otherslug";
is(
experiment2.name,
experiment2.slug,
"experiment2",
"getAll returns copies of the experiments"
);
@ -1103,11 +1147,11 @@ decorate_task(
decorate_task(
withMockExperiments([
experimentFactory({
name: "active",
slug: "active",
expired: false,
}),
experimentFactory({
name: "inactive",
slug: "inactive",
expired: true,
}),
]),
@ -1120,7 +1164,7 @@ decorate_task(
"getAllActive only returns active experiments"
);
allActiveExperiments[0].name = "newfakename";
allActiveExperiments[0].slug = "newfakename";
allActiveExperiments = await PreferenceExperiments.getAllActive();
Assert.notEqual(
allActiveExperiments,
@ -1132,7 +1176,7 @@ decorate_task(
// has
decorate_task(
withMockExperiments([experimentFactory({ name: "test" })]),
withMockExperiments([experimentFactory({ slug: "test" })]),
async function(experiments) {
ok(
await PreferenceExperiments.has("test"),
@ -1149,7 +1193,7 @@ decorate_task(
decorate_task(
withMockExperiments([
experimentFactory({
name: "test",
slug: "test",
branch: "branch",
preferences: {
"fake.pref": {
@ -1182,7 +1226,7 @@ decorate_task(
decorate_task(
withMockExperiments([
experimentFactory({
name: "test",
slug: "test",
branch: "branch",
preferences: {
"fake.pref": {
@ -1225,7 +1269,7 @@ decorate_task(
sendEventStub
) {
await PreferenceExperiments.start({
name: "test",
slug: "test",
actionName: "SomeAction",
branch: "branch",
preferences: {
@ -1286,7 +1330,7 @@ decorate_task(
sendEventStub
) {
await PreferenceExperiments.start({
name: "test",
slug: "test",
actionName: "SomeAction",
branch: "branch",
preferences: {
@ -1326,7 +1370,7 @@ decorate_task(
// Experiments shouldn't be recorded by init() in telemetry if they are expired
decorate_task(
withMockExperiments([
experimentFactory({ name: "expired", branch: "branch", expired: true }),
experimentFactory({ slug: "expired", branch: "branch", expired: true }),
]),
withStub(TelemetryEnvironment, "setExperimentActive"),
async function testInitTelemetryExpired(experiments, setActiveStub) {
@ -1339,7 +1383,7 @@ decorate_task(
decorate_task(
withMockExperiments([
experimentFactory({
name: "test",
slug: "test",
preferences: {
"fake.preference": {
preferenceValue: "experiment value",
@ -1378,7 +1422,7 @@ decorate_task(
decorate_task(
withMockExperiments([
experimentFactory({
name: "test",
slug: "test",
preferences: {
"fake.preference": {
preferenceValue: "experiment value",
@ -1428,7 +1472,7 @@ decorate_task(
decorate_task(
withMockExperiments([
experimentFactory({
name: "char",
slug: "char",
preferences: {
"fake.char": {
preferenceValue: "string",
@ -1436,7 +1480,7 @@ decorate_task(
},
}),
experimentFactory({
name: "int",
slug: "int",
preferences: {
"fake.int": {
preferenceValue: 2,
@ -1444,7 +1488,7 @@ decorate_task(
},
}),
experimentFactory({
name: "bool",
slug: "bool",
preferences: {
"fake.bool": {
preferenceValue: true,
@ -1482,7 +1526,7 @@ decorate_task(
decorate_task(
withMockExperiments([
experimentFactory({
name: "test",
slug: "test",
preferences: {
"fake.invalidValue": {
preferenceValue: new Date(),
@ -1503,7 +1547,7 @@ decorate_task(
decorate_task(
withMockExperiments([
experimentFactory({
name: "defaultBranchRecipe",
slug: "defaultBranchRecipe",
preferences: {
"fake.default": {
preferenceValue: "experiment value",
@ -1512,7 +1556,7 @@ decorate_task(
},
}),
experimentFactory({
name: "userBranchRecipe",
slug: "userBranchRecipe",
preferences: {
"fake.user": {
preferenceValue: "experiment value",
@ -1565,7 +1609,7 @@ decorate_task(
// start an experiment
await PreferenceExperiments.start({
name: "test",
slug: "test",
actionName: "SomeAction",
branch: "branch",
preferences: {
@ -1619,7 +1663,7 @@ decorate_task(
// start an experiment
await PreferenceExperiments.start({
name: "test",
slug: "test",
actionName: "SomeAction",
branch: "branch",
preferences: {
@ -1661,7 +1705,7 @@ decorate_task(
// stop should pass "unknown" to telemetry event for `reason` if none is specified
decorate_task(
withMockExperiments([
experimentFactory({ name: "test", preferences: { "fake.preference": {} } }),
experimentFactory({ slug: "test", preferences: { "fake.preference": {} } }),
]),
withMockPreferences,
withStub(PreferenceExperiments, "stopObserver"),
@ -1686,11 +1730,11 @@ decorate_task(
decorate_task(
withMockExperiments([
experimentFactory({
name: "test1",
slug: "test1",
preferences: { "fake.preference1": {} },
}),
experimentFactory({
name: "test2",
slug: "test2",
preferences: { "fake.preference2": {} },
}),
]),
@ -1730,7 +1774,7 @@ decorate_task(
withSendEventStub,
withMockExperiments([
experimentFactory({
name: "test",
slug: "test",
expired: false,
branch: "fakebranch",
preferences: {

4
toolkit/components/normandy/test/browser/browser_ShieldPreferences.js
Просмотреть файл

@ -72,8 +72,8 @@ decorate_task(
await stoppedBoth;
Assert.deepEqual(stopArgs, [
[study1.name, { reason: "general-opt-out" }],
[study2.name, { reason: "general-opt-out" }],
[study1.slug, { reason: "general-opt-out" }],
[study2.slug, { reason: "general-opt-out" }],
]);
}
);

48
toolkit/components/normandy/test/browser/browser_about_studies.js
Просмотреть файл

@ -111,17 +111,20 @@ decorate_task(
]),
PreferenceExperiments.withMockExperiments([
preferenceStudyFactory({
name: "D Fake Preference Study",
slug: "fake-study-d",
userFacingName: "D Fake Preference Study",
lastSeen: new Date(2018, 0, 3),
expired: false,
}),
preferenceStudyFactory({
name: "E Fake Preference Study",
slug: "fake-study-e",
userFacingName: "E Fake Preference Study",
lastSeen: new Date(2018, 0, 5),
expired: true,
}),
preferenceStudyFactory({
name: "F Fake Preference Study",
slug: "fake-study-f",
userFacingName: "F Fake Preference Study",
lastSeen: new Date(2018, 0, 6),
expired: false,
}),
@ -151,16 +154,16 @@ decorate_task(
Assert.deepEqual(
activeNames,
[
prefStudies[2].name,
prefStudies[2].slug,
addonStudies[0].slug,
prefStudies[0].name,
prefStudies[0].slug,
addonStudies[2].slug,
],
"Active studies are grouped by enabled status, and sorted by date"
);
Assert.deepEqual(
inactiveNames,
[prefStudies[1].name, addonStudies[1].slug],
[prefStudies[1].slug, addonStudies[1].slug],
"Inactive studies are grouped by enabled status, and sorted by date"
);
@ -199,7 +202,7 @@ decorate_task(
"Inactive studies do not show a remove button"
);
const activePrefStudy = getStudyRow(doc, prefStudies[0].name);
const activePrefStudy = getStudyRow(doc, prefStudies[0].slug);
const preferenceName = Object.keys(prefStudies[0].preferences)[0];
ok(
activePrefStudy
@ -216,15 +219,8 @@ decorate_task(
activePrefStudy.querySelector(".remove-button"),
"Active studies show a remove button"
);
is(
activePrefStudy
.querySelector(".study-icon")
.textContent.toLowerCase(),
"d",
"Study icons use the first letter of the study name."
);
const inactivePrefStudy = getStudyRow(doc, prefStudies[1].name);
const inactivePrefStudy = getStudyRow(doc, prefStudies[1].slug);
is(
inactivePrefStudy.querySelector(".study-status").textContent,
"Complete",
@ -246,10 +242,10 @@ decorate_task(
activePrefStudy.querySelector(".remove-button").click();
await ContentTaskUtils.waitForCondition(() =>
getStudyRow(doc, prefStudies[0].name).matches(".study.disabled")
getStudyRow(doc, prefStudies[0].slug).matches(".study.disabled")
);
ok(
getStudyRow(doc, prefStudies[0].name).matches(".study.disabled"),
getStudyRow(doc, prefStudies[0].slug).matches(".study.disabled"),
"Clicking the remove button updates the UI to show that the study has been disabled."
);
}
@ -262,7 +258,7 @@ decorate_task(
);
const updatedPrefStudy = await PreferenceExperiments.get(
prefStudies[0].name
prefStudies[0].slug
);
ok(
updatedPrefStudy.expired,
@ -306,7 +302,8 @@ decorate_task(
]),
PreferenceExperiments.withMockExperiments([
preferenceStudyFactory({
name: "B Fake Preference Study",
slug: "fake-pref-study",
userFacingName: "B Fake Preference Study",
lastSeen: new Date(2018, 0, 5),
expired: true,
}),
@ -387,7 +384,8 @@ decorate_task(
]),
PreferenceExperiments.withMockExperiments([
preferenceStudyFactory({
name: "Fake Preference Study",
slug: "fake-pref-study",
userFacingName: "Fake Preference Study",
lastSeen: new Date(2018, 0, 3),
expired: false,
}),
@ -397,7 +395,7 @@ decorate_task(
// The content page has already loaded. Disabling the studies here shouldn't
// affect it, since it doesn't live-update.
await AddonStudies.markAsEnded(addonStudy, "disabled-automatically-test");
await PreferenceExperiments.stop(prefStudy.name, {
await PreferenceExperiments.stop(prefStudy.slug, {
resetValue: false,
reason: "disabled-automatically-test",
});
@ -424,7 +422,7 @@ decorate_task(
Assert.deepEqual(
activeNames,
[addonStudy.slug, prefStudy.name],
[addonStudy.slug, prefStudy.slug],
"Both studies should be listed as active, even though they have been disabled outside of the page"
);
Assert.deepEqual(
@ -434,7 +432,7 @@ decorate_task(
);
const activeAddonStudy = getStudyRow(doc, addonStudy.slug);
const activePrefStudy = getStudyRow(doc, prefStudy.name);
const activePrefStudy = getStudyRow(doc, prefStudy.slug);
activeAddonStudy.querySelector(".remove-button").click();
await ContentTaskUtils.waitForCondition(() =>
@ -447,10 +445,10 @@ decorate_task(
activePrefStudy.querySelector(".remove-button").click();
await ContentTaskUtils.waitForCondition(() =>
getStudyRow(doc, prefStudy.name).matches(".study.disabled")
getStudyRow(doc, prefStudy.slug).matches(".study.disabled")
);
ok(
getStudyRow(doc, prefStudy.name).matches(".study.disabled"),
getStudyRow(doc, prefStudy.slug).matches(".study.disabled"),
"Clicking the remove button updates the UI to show that the study has been disabled."
);

18
toolkit/components/normandy/test/browser/browser_actions_PreferenceExperimentAction.js
Просмотреть файл

@ -154,7 +154,7 @@ decorate_task(
Assert.deepEqual(startStub.args, [
[
{
name: "test",
slug: "test",
actionName: "PreferenceExperimentAction",
branch: "branch1",
preferences: {
@ -177,7 +177,7 @@ decorate_task(
decorate_task(
withStudiesEnabled,
withStub(PreferenceExperiments, "markLastSeen"),
PreferenceExperiments.withMockExperiments([{ name: "test", expired: false }]),
PreferenceExperiments.withMockExperiments([{ slug: "test", expired: false }]),
async function markSeen_if_experiment_active(markLastSeenStub) {
const action = new PreferenceExperimentAction();
const recipe = preferenceExperimentFactory({
@ -194,7 +194,7 @@ decorate_task(
decorate_task(
withStudiesEnabled,
withStub(PreferenceExperiments, "markLastSeen"),
PreferenceExperiments.withMockExperiments([{ name: "test", expired: true }]),
PreferenceExperiments.withMockExperiments([{ slug: "test", expired: true }]),
async function dont_markSeen_if_experiment_expired(markLastSeenStub) {
const action = new PreferenceExperimentAction();
const recipe = preferenceExperimentFactory({
@ -228,9 +228,9 @@ decorate_task(
withStudiesEnabled,
withStub(PreferenceExperiments, "stop"),
PreferenceExperiments.withMockExperiments([
{ name: "seen", expired: false, actionName: "PreferenceExperimentAction" },
{ slug: "seen", expired: false, actionName: "PreferenceExperimentAction" },
{
name: "unseen",
slug: "unseen",
expired: false,
actionName: "PreferenceExperimentAction",
},
@ -255,12 +255,12 @@ decorate_task(
withStub(PreferenceExperiments, "stop"),
PreferenceExperiments.withMockExperiments([
{
name: "seen",
slug: "seen",
expired: false,
actionName: "SinglePreferenceExperimentAction",
},
{
name: "unseen",
slug: "unseen",
expired: false,
actionName: "SinglePreferenceExperimentAction",
},
@ -288,7 +288,7 @@ decorate_task(
withStub(Uptake, "reportRecipe"),
PreferenceExperiments.withMockExperiments([
{
name: "conflict",
slug: "conflict",
preferences: {
"conflict.pref": {},
},
@ -447,7 +447,7 @@ decorate_task(
ok(activeExperiments.length > 0);
Assert.deepEqual(activeExperiments, [
{
name: "integration test experiment",
slug: "integration test experiment",
actionName: "PreferenceExperimentAction",
branch: "branch1",
preferences: {