diff options
Diffstat (limited to 'toolkit/components/normandy')
17 files changed, 162 insertions, 46 deletions
diff --git a/toolkit/components/normandy/Normandy.sys.mjs b/toolkit/components/normandy/Normandy.sys.mjs index 653461df48..ce30bb8456 100644 --- a/toolkit/components/normandy/Normandy.sys.mjs +++ b/toolkit/components/normandy/Normandy.sys.mjs @@ -78,7 +78,7 @@ export var Normandy = { await this.finishInit(); }, - async observe(subject, topic, data) { + async observe(subject, topic) { if (topic === UI_AVAILABLE_NOTIFICATION) { Services.obs.removeObserver(this, UI_AVAILABLE_NOTIFICATION); this.uiAvailableNotificationObserved.resolve(); diff --git a/toolkit/components/normandy/actions/BaseAction.sys.mjs b/toolkit/components/normandy/actions/BaseAction.sys.mjs index cb9198a55e..25f64c293f 100644 --- a/toolkit/components/normandy/actions/BaseAction.sys.mjs +++ b/toolkit/components/normandy/actions/BaseAction.sys.mjs @@ -183,7 +183,7 @@ export class BaseAction { * * @param {Recipe} recipe */ - async _run(recipe) { + async _run() { throw new Error("Not implemented"); } diff --git a/toolkit/components/normandy/actions/BranchedAddonStudyAction.sys.mjs b/toolkit/components/normandy/actions/BranchedAddonStudyAction.sys.mjs index d775429ced..b3b111cdf8 100644 --- a/toolkit/components/normandy/actions/BranchedAddonStudyAction.sys.mjs +++ b/toolkit/components/normandy/actions/BranchedAddonStudyAction.sys.mjs @@ -115,7 +115,7 @@ export class BranchedAddonStudyAction extends BaseStudyAction { this.seenRecipeIds = new Set(); } - async _run(recipe) { + async _run() { throw new Error("_run should not be called anymore"); } diff --git a/toolkit/components/normandy/actions/PreferenceExperimentAction.sys.mjs b/toolkit/components/normandy/actions/PreferenceExperimentAction.sys.mjs index 310d1b08fd..0dc416d789 100644 --- a/toolkit/components/normandy/actions/PreferenceExperimentAction.sys.mjs +++ b/toolkit/components/normandy/actions/PreferenceExperimentAction.sys.mjs @@ -160,7 +160,7 @@ export class PreferenceExperimentAction extends BaseStudyAction { } } - async _run(recipe) { + async _run() { throw new Error("_run shouldn't be called anymore"); } diff --git a/toolkit/components/normandy/actions/ShowHeartbeatAction.sys.mjs b/toolkit/components/normandy/actions/ShowHeartbeatAction.sys.mjs index db9a4ec5ff..7e6af63266 100644 --- a/toolkit/components/normandy/actions/ShowHeartbeatAction.sys.mjs +++ b/toolkit/components/normandy/actions/ShowHeartbeatAction.sys.mjs @@ -72,10 +72,7 @@ export class ShowHeartbeatAction extends BaseAction { learnMoreUrl, postAnswerUrl: await this.generatePostAnswerURL(recipe), flowId: lazy.NormandyUtils.generateUuid(), - // Recipes coming from Nimbus won't have a revision_id. - ...(Object.hasOwn(recipe, "revision_id") - ? { surveyVersion: recipe.revision_id } - : {}), + surveyVersion: recipe.revision_id, }); heartbeat.eventEmitter.once( diff --git a/toolkit/components/normandy/lib/AddonStudies.sys.mjs b/toolkit/components/normandy/lib/AddonStudies.sys.mjs index 202bf8ccb3..e13d470bd7 100644 --- a/toolkit/components/normandy/lib/AddonStudies.sys.mjs +++ b/toolkit/components/normandy/lib/AddonStudies.sys.mjs @@ -162,7 +162,7 @@ export var AddonStudies = { }, /** - * These migrations should only be called from `NormandyMigrations.jsm` and + * These migrations should only be called from `NormandyMigrations.sys.mjs` and * tests. */ migrations: { diff --git a/toolkit/components/normandy/lib/LegacyHeartbeat.sys.mjs b/toolkit/components/normandy/lib/LegacyHeartbeat.sys.mjs index 501c9f70af..93c24faf5d 100644 --- a/toolkit/components/normandy/lib/LegacyHeartbeat.sys.mjs +++ b/toolkit/components/normandy/lib/LegacyHeartbeat.sys.mjs @@ -43,6 +43,7 @@ export const LegacyHeartbeat = { capabilities: ["action.show-heartbeat"], filter_expression: "true", use_only_baseline_capabilities: true, + revision_id: "1", // Required for the Heartbeat telemetry ping. }; }, }; diff --git a/toolkit/components/normandy/lib/PreferenceExperiments.sys.mjs b/toolkit/components/normandy/lib/PreferenceExperiments.sys.mjs index c89beff978..92c5e63076 100644 --- a/toolkit/components/normandy/lib/PreferenceExperiments.sys.mjs +++ b/toolkit/components/normandy/lib/PreferenceExperiments.sys.mjs @@ -244,7 +244,7 @@ export var PreferenceExperiments = { const defaultBranchPrefs = allExperiments .flatMap(exp => Object.entries(exp.preferences)) .filter( - ([preferenceName, preferenceInfo]) => + ([, preferenceInfo]) => preferenceInfo.preferenceBranchType === "default" ); for (const [preferenceName, { preferenceValue }] of defaultBranchPrefs) { @@ -906,7 +906,7 @@ export var PreferenceExperiments = { InvalidPreferenceName: class extends Error {}, /** - * These migrations should only be called from `NormandyMigrations.jsm` and tests. + * These migrations should only be called from `NormandyMigrations.sys.mjs` and tests. */ migrations: { /** Move experiments into a specific key. */ diff --git a/toolkit/components/normandy/lib/RecipeRunner.sys.mjs b/toolkit/components/normandy/lib/RecipeRunner.sys.mjs index 7e02a3150a..f9578b37d2 100644 --- a/toolkit/components/normandy/lib/RecipeRunner.sys.mjs +++ b/toolkit/components/normandy/lib/RecipeRunner.sys.mjs @@ -63,13 +63,13 @@ ChromeUtils.defineLazyGetter(lazy, "gRemoteSettingsClient", () => { function cacheProxy(target) { const cache = new Map(); return new Proxy(target, { - get(target, prop, receiver) { + get(target, prop) { if (!cache.has(prop)) { cache.set(prop, target[prop]); } return cache.get(prop); }, - set(target, prop, value, receiver) { + set(target, prop, value) { cache.set(prop, value); return true; }, diff --git a/toolkit/components/normandy/test/browser/browser_ActionsManager.js b/toolkit/components/normandy/test/browser/browser_ActionsManager.js index 8b5772fa26..2667a085cf 100644 --- a/toolkit/components/normandy/test/browser/browser_ActionsManager.js +++ b/toolkit/components/normandy/test/browser/browser_ActionsManager.js @@ -14,7 +14,7 @@ const { ActionSchemas } = ChromeUtils.importESModule( ); // Test life cycle methods for actions -decorate_task(async function (reportActionStub, Stub) { +decorate_task(async function () { let manager = new ActionsManager(); const recipe = { id: 1, action: "test-local-action-used" }; diff --git a/toolkit/components/normandy/test/browser/browser_BaseAction.js b/toolkit/components/normandy/test/browser/browser_BaseAction.js index 240a235346..3a5ebd9d39 100644 --- a/toolkit/components/normandy/test/browser/browser_BaseAction.js +++ b/toolkit/components/normandy/test/browser/browser_BaseAction.js @@ -19,7 +19,7 @@ class NoopAction extends BaseAction { this._testPreExecutionFlag = true; } - _run(recipe) { + _run() { this._testRunFlag = true; } @@ -37,7 +37,7 @@ class FailPreExecutionAction extends NoopAction { } class FailRunAction extends NoopAction { - _run(recipe) { + _run() { throw NoopAction._errorToThrow; } } diff --git a/toolkit/components/normandy/test/browser/browser_LegacyHeartbeat.js b/toolkit/components/normandy/test/browser/browser_LegacyHeartbeat.js index 465e5c1040..8d48298da8 100644 --- a/toolkit/components/normandy/test/browser/browser_LegacyHeartbeat.js +++ b/toolkit/components/normandy/test/browser/browser_LegacyHeartbeat.js @@ -9,6 +9,9 @@ const { BaseAction } = ChromeUtils.importESModule( const { ClientEnvironment } = ChromeUtils.importESModule( "resource://normandy/lib/ClientEnvironment.sys.mjs" ); +const { EventEmitter } = ChromeUtils.importESModule( + "resource://normandy/lib/EventEmitter.sys.mjs" +); const { Heartbeat } = ChromeUtils.importESModule( "resource://normandy/lib/Heartbeat.sys.mjs" ); @@ -27,6 +30,9 @@ const { RecipeRunner } = ChromeUtils.importESModule( const { RemoteSettings } = ChromeUtils.importESModule( "resource://services-settings/remote-settings.sys.mjs" ); +const { JsonSchema } = ChromeUtils.importESModule( + "resource://gre/modules/JsonSchema.sys.mjs" +); const SURVEY = { surveyId: "a survey", @@ -39,9 +45,80 @@ const SURVEY = { repeatOption: "once", }; +// See properties.payload in +// https://github.com/mozilla-services/mozilla-pipeline-schemas/blob/main/schemas/telemetry/heartbeat/heartbeat.4.schema.json + +const PAYLOAD_SCHEMA = { + additionalProperties: false, + anyOf: [ + { + required: ["closedTS"], + }, + { + required: ["windowClosedTS"], + }, + ], + properties: { + closedTS: { + minimum: 0, + type: "integer", + }, + engagedTS: { + minimum: 0, + type: "integer", + }, + expiredTS: { + minimum: 0, + type: "integer", + }, + flowId: { + pattern: + "^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$", + type: "string", + }, + learnMoreTS: { + minimum: 0, + type: "integer", + }, + offeredTS: { + minimum: 0, + type: "integer", + }, + score: { + minimum: 1, + type: "integer", + }, + surveyId: { + type: "string", + }, + surveyVersion: { + pattern: "^([0-9]+|[a-fA-F0-9]{64})$", + type: "string", + }, + testing: { + type: "boolean", + }, + version: { + maximum: 1, + minimum: 1, + type: "number", + }, + votedTS: { + minimum: 0, + type: "integer", + }, + windowClosedTS: { + minimum: 0, + type: "integer", + }, + }, + required: ["version", "flowId", "offeredTS", "surveyId", "surveyVersion"], + type: "object", +}; + function assertSurvey(actual, expected) { for (const key of Object.keys(actual)) { - if (["postAnswerUrl", "flowId"].includes(key)) { + if (["flowId", "postAnswerUrl", "surveyVersion"].includes(key)) { continue; } @@ -52,6 +129,7 @@ function assertSurvey(actual, expected) { ); } + Assert.equal(actual.surveyVersion, "1"); Assert.ok(actual.postAnswerUrl.startsWith(expected.postAnswerUrl)); } @@ -86,3 +164,61 @@ decorate_task( } } ); + +decorate_task( + withClearStorage(), + async function testLegacyHeartbeatPingPayload() { + const sandbox = sinon.createSandbox(); + + const cleanupEnrollment = await ExperimentFakes.enrollWithFeatureConfig({ + featureId: "legacyHeartbeat", + value: { + survey: SURVEY, + }, + }); + + const client = RemoteSettings("normandy-recipes-capabilities"); + sandbox.stub(client, "get").resolves([]); + + // Override Heartbeat so we can get the instance and manipulate it directly. + const heartbeatDeferred = Promise.withResolvers(); + class TestHeartbeat extends Heartbeat { + constructor(...args) { + super(...args); + heartbeatDeferred.resolve(this); + } + } + ShowHeartbeatAction.overrideHeartbeatForTests(TestHeartbeat); + + try { + await RecipeRunner.run(); + const heartbeat = await heartbeatDeferred.promise; + // We are going to simulate the timer timing out, so we do not want it to + // *actually* time out. + heartbeat.endTimerIfPresent("surveyEndTimer"); + const notice = await heartbeat.noticePromise; + await notice.updateComplete; + + const telemetrySentPromise = new Promise(resolve => { + heartbeat.eventEmitter.once("TelemetrySent", payload => + resolve(payload) + ); + }); + + // This method would be triggered when the timer timed out. This will + // trigger telemetry to be submitted. + heartbeat.close(); + + const payload = await telemetrySentPromise; + + const result = JsonSchema.validate(payload, PAYLOAD_SCHEMA); + Assert.ok(result.valid); + Assert.equal(payload.surveyVersion, "1"); + + await cleanupEnrollment(); + } finally { + ShowHeartbeatAction.overrideHeartbeatForTests(); + sandbox.restore(); + } + } +); diff --git a/toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js b/toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js index 4269020975..e50e9bee0e 100644 --- a/toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js +++ b/toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js @@ -312,7 +312,7 @@ decorate_task( // clearAllExperimentStorage decorate_task( withMockExperiments([preferenceStudyFactory({ slug: "test" })]), - async function ({ prefExperiments }) { + async function () { ok(await PreferenceExperiments.has("test"), "Mock experiment is detected."); await PreferenceExperiments.clearAllExperimentStorage(); ok( @@ -434,12 +434,7 @@ decorate_task( withMockPreferences(), withStub(PreferenceExperiments, "startObserver"), withSendEventSpy(), - async function testStart({ - prefExperiments, - mockPreferences, - startObserverStub, - sendEventSpy, - }) { + async function testStart({ mockPreferences, startObserverStub }) { mockPreferences.set("fake.preference", "oldvalue", "default"); mockPreferences.set("fake.preference", "uservalue", "user"); mockPreferences.set("fake.preferenceinteger", 1, "default"); @@ -1193,7 +1188,7 @@ decorate_task(withMockExperiments(), async function () { // get decorate_task( withMockExperiments([preferenceStudyFactory({ slug: "test" })]), - async function ({ prefExperiments }) { + async function () { const experiment = await PreferenceExperiments.get("test"); is(experiment.slug, "test", "get fetches the correct experiment"); @@ -1253,9 +1248,7 @@ decorate_task( }), ]), withMockPreferences(), - async function testGetAllActive({ - prefExperiments: [activeExperiment, inactiveExperiment], - }) { + async function testGetAllActive({ prefExperiments: [activeExperiment] }) { let allActiveExperiments = await PreferenceExperiments.getAllActive(); Assert.deepEqual( allActiveExperiments, @@ -1306,11 +1299,7 @@ decorate_task( withMockPreferences(), withStub(TelemetryEnvironment, "setExperimentActive"), withStub(PreferenceExperiments, "startObserver"), - async function testInit({ - prefExperiments, - mockPreferences, - setExperimentActiveStub, - }) { + async function testInit({ mockPreferences, setExperimentActiveStub }) { mockPreferences.set("fake.pref", "experiment value"); await PreferenceExperiments.init(); ok( diff --git a/toolkit/components/normandy/test/browser/browser_RecipeRunner.js b/toolkit/components/normandy/test/browser/browser_RecipeRunner.js index 00f0f81c51..864c237ff9 100644 --- a/toolkit/components/normandy/test/browser/browser_RecipeRunner.js +++ b/toolkit/components/normandy/test/browser/browser_RecipeRunner.js @@ -216,7 +216,6 @@ decorate_task( withMockNormandyApi(), withStub(ClientEnvironment, "getClientClassification"), async function testClientClassificationCache({ - mockNormandyApi, getClientClassificationStub, }) { getClientClassificationStub.returns(Promise.resolve(false)); @@ -294,7 +293,6 @@ decorate_task( async function testReadFromRemoteSettings({ verifyObjectSignatureStub, processRecipeStub, - finalizeStub, reportRecipeStub, }) { const matchRecipe = { @@ -334,7 +332,7 @@ decorate_task( let recipesFromRS = ( await RecipeRunner._remoteSettingsClientForTesting.get() - ).map(({ recipe, signature }) => recipe); + ).map(({ recipe }) => recipe); // Sort the records by id so that they match the order in the assertion recipesFromRS.sort((a, b) => a.id - b.id); Assert.deepEqual( @@ -518,11 +516,7 @@ decorate_task( withStub(RecipeRunner, "run"), withStub(RecipeRunner, "registerTimer"), withStub(RecipeRunner, "watchPrefs"), - async function testInitFirstRun({ - runStub, - registerTimerStub, - watchPrefsStub, - }) { + async function testInitFirstRun({ runStub, registerTimerStub }) { await RecipeRunner.init(); Assert.deepEqual( runStub.args, @@ -818,7 +812,7 @@ decorate_task( withStub(Uptake, "reportRunner"), withStub(ActionsManager.prototype, "finalize"), NormandyTestUtils.withMockRecipeCollection([]), - async function testRunEvents({ reportRunnerStub, finalizeStub }) { + async function testRunEvents() { const observer = sinon.spy(); Services.obs.addObserver(observer, "recipe-runner:start"); diff --git a/toolkit/components/normandy/test/browser/browser_actions_BranchedAddonStudyAction.js b/toolkit/components/normandy/test/browser/browser_actions_BranchedAddonStudyAction.js index cce82ae89e..b7d4b34754 100644 --- a/toolkit/components/normandy/test/browser/browser_actions_BranchedAddonStudyAction.js +++ b/toolkit/components/normandy/test/browser/browser_actions_BranchedAddonStudyAction.js @@ -719,7 +719,7 @@ decorate_task( ensureAddonCleanup(), AddonStudies.withStudies([branchedAddonStudyFactory({ active: false })]), withSendEventSpy(), - async ({ addonStudies: [study], sendEventSpy }) => { + async ({ addonStudies: [study] }) => { const action = new BranchedAddonStudyAction(); await Assert.rejects( action.unenroll(study.recipeId), diff --git a/toolkit/components/normandy/test/browser/browser_actions_PreferenceExperimentAction.js b/toolkit/components/normandy/test/browser/browser_actions_PreferenceExperimentAction.js index 5359174169..f275487e14 100644 --- a/toolkit/components/normandy/test/browser/browser_actions_PreferenceExperimentAction.js +++ b/toolkit/components/normandy/test/browser/browser_actions_PreferenceExperimentAction.js @@ -831,7 +831,6 @@ decorate_task( withSpy(PreferenceExperiments, "stop"), withStub(PreferenceExperimentAction.prototype, "_considerTemporaryError"), async function testNoRecipes({ - stopSpy, _considerTemporaryErrorStub, prefExperiments: [experiment], }) { diff --git a/toolkit/components/normandy/test/unit/test_NormandyApi.js b/toolkit/components/normandy/test/unit/test_NormandyApi.js index c0c826b045..5b0ede1701 100644 --- a/toolkit/components/normandy/test/unit/test_NormandyApi.js +++ b/toolkit/components/normandy/test/unit/test_NormandyApi.js @@ -199,7 +199,7 @@ decorate_task( // A normal request should send that cookie const cookieExpectedDeferred = Promise.withResolvers(); - function cookieExpectedObserver(aSubject, aTopic, aData) { + function cookieExpectedObserver(aSubject, aTopic) { equal( aTopic, "http-on-modify-request", @@ -223,7 +223,7 @@ decorate_task( // A request through the NormandyApi method should not send that cookie const cookieNotExpectedDeferred = Promise.withResolvers(); - function cookieNotExpectedObserver(aSubject, aTopic, aData) { + function cookieNotExpectedObserver(aSubject, aTopic) { equal( aTopic, "http-on-modify-request", |