From d16e84bb52bbc25296ece8751b21c9ddf062035a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Thu, 30 Aug 2018 18:44:49 +0300 Subject: [PATCH 01/28] Added latest version of pioneer-utils as dependency --- package-lock.json | 5 +++++ package.json | 1 + 2 files changed, 6 insertions(+) diff --git a/package-lock.json b/package-lock.json index 85d8043..a33dc69 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7770,6 +7770,11 @@ "integrity": "sha512-NqWvrQD/GpY78ybiNBzi/dg8ylERhDo6nB33j5sfCKpUmWLc3lYzeoBjyRoCMvEpDpL9lmH6ufRd0jw6rcd1pQ==", "dev": true }, + "pioneer-utils": { + "version": "1.0.10", + "resolved": "https://registry.npmjs.org/pioneer-utils/-/pioneer-utils-1.0.10.tgz", + "integrity": "sha1-7xK5xH/E8Va3oRMYwnEDbg1ClZ4=" + }, "pluralize": { "version": "7.0.0", "resolved": "https://registry.npmjs.org/pluralize/-/pluralize-7.0.0.tgz", diff --git a/package.json b/package.json index f533484..edd972c 100644 --- a/package.json +++ b/package.json @@ -13,6 +13,7 @@ "ajv": "^6.5.0", "commander": "^2.15.1", "fs-extra": "^6.0.1", + "pioneer-utils": "^1.0.10", "shield-study-schemas": "^0.8.3" }, "devDependencies": { From 1e553ee65c7c511724f5ee667603dfa2e7ab7e26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Thu, 30 Aug 2018 18:45:22 +0300 Subject: [PATCH 02/28] Made the tests run twice, one for each shieldType (shield, pioneer) --- examples/small-study/src/studySetup.js | 2 +- test-addon/src/studySetup.js | 9 +- test/functional/browser.study.api.js | 109 ++++++++++-------- test/functional/test-addon.js | 18 ++- testUtils/ui.js | 26 ++--- webExtensionApis/study/api.md | 9 ++ webExtensionApis/study/schema.json | 8 ++ webExtensionApis/study/schema.yaml | 10 ++ webExtensionApis/study/src/index.js | 4 + .../study/src/testingOverrides.js | 7 ++ 10 files changed, 136 insertions(+), 66 deletions(-) diff --git a/examples/small-study/src/studySetup.js b/examples/small-study/src/studySetup.js index 4d8a125..b853a0b 100644 --- a/examples/small-study/src/studySetup.js +++ b/examples/small-study/src/studySetup.js @@ -22,7 +22,7 @@ const baseStudySetup = { // used for activeExperiments tagging (telemetryEnvironment.setActiveExperiment) activeExperimentName: browser.runtime.id, - // uses shield sampling and telemetry semantics. Future: will support "pioneer" + // use either "shield" or "pioneer" telemetry semantics and data pipelines studyType: "shield", // telemetry diff --git a/test-addon/src/studySetup.js b/test-addon/src/studySetup.js index 4d8a125..2dd2f3d 100644 --- a/test-addon/src/studySetup.js +++ b/test-addon/src/studySetup.js @@ -22,8 +22,8 @@ const baseStudySetup = { // used for activeExperiments tagging (telemetryEnvironment.setActiveExperiment) activeExperimentName: browser.runtime.id, - // uses shield sampling and telemetry semantics. Future: will support "pioneer" - studyType: "shield", + // use either "shield" or "pioneer" telemetry semantics and data pipelines + studyType: null, // set by internal test override below in getStudySetup() // telemetry telemetry: { @@ -137,5 +137,10 @@ async function getStudySetup() { firstRunTimestamp: testingOverrides.firstRunTimestamp, expired: testingOverrides.expired, }; + + // internal testing override necessary to be able to test all study types + const internalTestingOverrides = await browser.studyDebug.getInternalTestingOverrides(); + studySetup.studyType = internalTestingOverrides.studyType; + return studySetup; } diff --git a/test/functional/browser.study.api.js b/test/functional/browser.study.api.js index ccb1d78..d2ddbde 100644 --- a/test/functional/browser.study.api.js +++ b/test/functional/browser.study.api.js @@ -4,7 +4,7 @@ const KEEPOPEN = process.env.KEEPOPEN; /** Complete list of tests for testing * - * - the public api for `browser.study` + * - the public api for `browser.study` not specific to any add-on background logic */ /** About webdriver extension based tests @@ -56,55 +56,56 @@ function merge(...sources) { return Object.assign({}, ...sources); } -/** return a studySetup, shallow merged from overrides - * - * @return {object} mergedStudySetup - */ -function studySetupForTests(...overrides) { - // Minimal configuration to pass schema validation - const studySetup = { - activeExperimentName: "shield-utils-test-addon@shield.mozilla.org", - studyType: "shield", - endings: { - ineligible: { - baseUrls: [ - "https://qsurvey.mozilla.com/s3/Shield-Study-Example-Survey/?reason=ineligible", - ], +const publicApiTests = function(studyType) { + /** return a studySetup, shallow merged from overrides + * + * @return {object} mergedStudySetup + */ + function studySetupForTests(...overrides) { + // Minimal configuration to pass schema validation + const studySetup = { + activeExperimentName: `shield-utils-test-addon@${studyType}.mozilla.org`, + studyType, + endings: { + ineligible: { + baseUrls: [ + "https://qsurvey.mozilla.com/s3/Shield-Study-Example-Survey/?reason=ineligible", + ], + }, + BrowserStudyApiEnding: { + baseUrls: [ + "https://qsurvey.mozilla.com/s3/Shield-Study-Example-Survey/?reason=BrowserStudyApiEnding", + ], + }, }, - BrowserStudyApiEnding: { - baseUrls: [ - "https://qsurvey.mozilla.com/s3/Shield-Study-Example-Survey/?reason=BrowserStudyApiEnding", - ], + telemetry: { + send: false, // assumed false. Actually send pings if true + removeTestingFlag: false, // Marks pings to be discarded, set true for to have the pings processed in the pipeline }, - }, - telemetry: { - send: false, // assumed false. Actually send pings if true - removeTestingFlag: false, // Marks pings to be discarded, set true for to have the pings processed in the pipeline - }, - logLevel: 10, - weightedVariations: [ - { - name: "control", - weight: 1, + logLevel: 10, + weightedVariations: [ + { + name: "control", + weight: 1, + }, + ], + expire: { + days: 14, }, - ], - expire: { - days: 14, - }, - // Dynamic study configuration flags - allowEnroll: true, - testing: {}, - }; - - return merge(studySetup, ...overrides); -} + // Dynamic study configuration flags + allowEnroll: true, + testing: {}, + }; + + return merge(studySetup, ...overrides); + } -describe("PUBLIC API `browser.study` (not specific to any add-on background logic)", function() { // This gives Firefox time to start, and us a bit longer during some of the tests. this.timeout(15000 + KEEPOPEN * 1000 * 2); let driver; let addonId; + // run in the extension page let addonExec; @@ -112,7 +113,7 @@ describe("PUBLIC API `browser.study` (not specific to any add-on background logi driver = await utils.setupWebdriver.promiseSetupDriver( utils.FIREFOX_PREFERENCES, ); - addonId = await utils.setupWebdriver.installAddon(driver); + installAddon(); await utils.ui.openBrowserConsole(driver); // make a shorter alias @@ -122,9 +123,11 @@ describe("PUBLIC API `browser.study` (not specific to any add-on background logi ); } - async function reinstallAddon() { - await utils.setupWebdriver.uninstallAddon(driver, addonId); - await utils.setupWebdriver.installAddon(driver); + async function installAddon() { + if (addonId) { + await utils.setupWebdriver.uninstallAddon(driver, addonId); + } + addonId = await utils.setupWebdriver.installAddon(driver); } before(createAddonExec); @@ -590,7 +593,7 @@ describe("PUBLIC API `browser.study` (not specific to any add-on background logi }; before(async function reinstallSetupDoTelemetryAndWait() { - await reinstallAddon(); + await installAddon(); studyInfo = await addonExec(async (_studySetupForTests, callback) => { // Ensure we have a configured study and are supposed to run our feature browser.study.onReady.addListener(async _studyInfo => { @@ -796,7 +799,7 @@ describe("PUBLIC API `browser.study` (not specific to any add-on background logi }; before(async function reinstallSetupAndAwaitEndStudy() { - await reinstallAddon(); + await installAddon(); endingResult = await addonExec( async (_studySetupForTests, callback) => { // Ensure we have a configured study and are supposed to run our feature @@ -906,7 +909,7 @@ describe("PUBLIC API `browser.study` (not specific to any add-on background logi }; before(async function reinstallSetupAndAwaitEndStudy() { - await reinstallAddon(); + await installAddon(); endingResult = await addonExec( async (_studySetupForTests, callback) => { // Ensure we have a configured study and are supposed to run our feature @@ -1021,7 +1024,7 @@ describe("PUBLIC API `browser.study` (not specific to any add-on background logi }; before(async function reinstallSetupAndConfigureAlarm() { - await reinstallAddon(); + await installAddon(); endingResult = await addonExec( async (_studySetupForTests, callback) => { console.log( @@ -1206,4 +1209,12 @@ describe("PUBLIC API `browser.study` (not specific to any add-on background logi it("log level works?"); }); }); +}; + +describe("PUBLIC API `browser.study` (studyType: shield)", function() { + publicApiTests.bind(this)("shield"); +}); + +describe("PUBLIC API `browser.study` (studyType: pioneer)", function() { + publicApiTests.bind(this)("pioneer"); }); diff --git a/test/functional/test-addon.js b/test/functional/test-addon.js index 6256024..87c5e7c 100644 --- a/test/functional/test-addon.js +++ b/test/functional/test-addon.js @@ -12,7 +12,7 @@ const KEEPOPEN = process.env.KEEPOPEN; const assert = require("assert"); const utils = require("./utils"); -describe("Tests verifying that the test add-on works as expected", function() { +const testAddonTests = function(studyType) { // This gives Firefox time to start, and us a bit longer during some of the tests. this.timeout(15000 + KEEPOPEN * 1000 * 3); @@ -22,6 +22,14 @@ describe("Tests verifying that the test add-on works as expected", function() { driver = await utils.setupWebdriver.promiseSetupDriver( utils.FIREFOX_PREFERENCES, ); + const widgetId = utils.ui.makeWidgetId( + "shield-utils-test-addon@shield.mozilla.org", + ); + await utils.preferences.set( + driver, + `extensions.${widgetId}.test.studyType`, + studyType, + ); await utils.setupWebdriver.installAddon(driver); await utils.ui.openBrowserConsole(driver); }); @@ -147,4 +155,12 @@ describe("Tests verifying that the test add-on works as expected", function() { */ }); }); +}; + +describe("Tests verifying that the test add-on works as expected (studyType=shield)", function() { + testAddonTests.bind(this)("shield"); +}); + +describe("Tests verifying that the test add-on works as expected (studyType=pioneer)", function() { + testAddonTests.bind(this)("pioneer"); }); diff --git a/testUtils/ui.js b/testUtils/ui.js index e1cda83..75827df 100644 --- a/testUtils/ui.js +++ b/testUtils/ui.js @@ -26,6 +26,18 @@ module.exports.ui = { return JSON.parse(manifestJson); }, + /** + * From firefox/browser/components/extensions/ExtensionPopups.jsm + * + * @param {string} id Id to modify + * @returns {string} widgetId canonical widget id with replaced bits. + */ + makeWidgetId: id => { + id = id.toLowerCase(); + // FIXME: This allows for collisions. + return id.replace(/[^a-z0-9_-]/g, "_"); + }, + /** * The widget id is used to identify add-on specific chrome elements. Examples: * - Browser action - {addonWidgetId}-browser-action @@ -34,20 +46,8 @@ module.exports.ui = { * @returns {Promise} name of the made widget */ addonWidgetId: async () => { - /** - * From firefox/browser/components/extensions/ExtensionPopups.jsm - * - * @param {string} id Id to modify - * @returns {string} widgetId canonical widget id with replaced bits. - */ - function makeWidgetId(id) { - id = id.toLowerCase(); - // FIXME: This allows for collisions. - return id.replace(/[^a-z0-9_-]/g, "_"); - } - const manifest = await module.exports.ui.promiseManifest(); - return makeWidgetId(manifest.applications.gecko.id); + return module.exports.ui.makeWidgetId(manifest.applications.gecko.id); }, openBrowserConsole: async driver => { diff --git a/webExtensionApis/study/api.md b/webExtensionApis/study/api.md index 7f59d2d..922b945 100644 --- a/webExtensionApis/study/api.md +++ b/webExtensionApis/study/api.md @@ -1011,6 +1011,15 @@ About `this._internals`: **Parameters** +### `browser.studyDebug.getInternalTestingOverrides( )` + +Returns an object with the following keys: +studyType - to be able to test add-ons with different studyType configurations +Used to override study testing flags in getStudySetup(). +The values are set by the corresponding preference under the `extensions.${widgetId}.test.*` preference branch. + +**Parameters** + ## Events (None) diff --git a/webExtensionApis/study/schema.json b/webExtensionApis/study/schema.json index 88f0737..8363798 100644 --- a/webExtensionApis/study/schema.json +++ b/webExtensionApis/study/schema.json @@ -866,6 +866,14 @@ "description": "Return `_internals` of the studyUtils object.\n\nUse this for debugging state.\n\nAbout `this._internals`:\n- variation: (chosen variation, `setup` )\n- isEnding: bool `endStudy`\n- isSetup: bool `setup`\n- isFirstRun: bool `setup`, based on pref\n- studySetup: bool `setup` the config\n- seenTelemetry: object of lists of seen telemetry by bucket\n- prefs: object of all created prefs and their names\n", "parameters": [] + }, + { + "name": "getInternalTestingOverrides", + "type": "function", + "async": true, + "description": + "Returns an object with the following keys:\n studyType - to be able to test add-ons with different studyType configurations\nUsed to override study testing flags in getStudySetup().\nThe values are set by the corresponding preference under the `extensions.${widgetId}.test.*` preference branch.\n", + "parameters": [] } ] } diff --git a/webExtensionApis/study/schema.yaml b/webExtensionApis/study/schema.yaml index 944e9a8..7c4f80d 100644 --- a/webExtensionApis/study/schema.yaml +++ b/webExtensionApis/study/schema.yaml @@ -698,3 +698,13 @@ - prefs: object of all created prefs and their names parameters: [] + + - name: getInternalTestingOverrides + type: 'function' + async: true + description: | + Returns an object with the following keys: + studyType - to be able to test add-ons with different studyType configurations + Used to override study testing flags in getStudySetup(). + The values are set by the corresponding preference under the `extensions.${widgetId}.test.*` preference branch. + parameters: [] diff --git a/webExtensionApis/study/src/index.js b/webExtensionApis/study/src/index.js index 50955ed..ea2abf3 100644 --- a/webExtensionApis/study/src/index.js +++ b/webExtensionApis/study/src/index.js @@ -448,6 +448,10 @@ this.study = class extends ExtensionAPI { async getInternals() { return studyUtils._internals; }, + + getInternalTestingOverrides: async function getInternalTestingOverrides() { + return testingOverrides.getInternalTestingOverrides(widgetId); + }, }, }; } diff --git a/webExtensionApis/study/src/testingOverrides.js b/webExtensionApis/study/src/testingOverrides.js index 8769a4e..1b85132 100644 --- a/webExtensionApis/study/src/testingOverrides.js +++ b/webExtensionApis/study/src/testingOverrides.js @@ -25,3 +25,10 @@ export function listPreferences(widgetId) { `extensions.${widgetId}.test.expired`, ]; } + +export function getInternalTestingOverrides(widgetId) { + const internalTestingOverrides = {}; + internalTestingOverrides.studyType = + Preferences.get(`extensions.${widgetId}.test.studyType`) || null; + return internalTestingOverrides; +} From 56523d294f7e0cc2f68898d380d49eb1ef750e76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Fri, 31 Aug 2018 22:21:59 +0300 Subject: [PATCH 03/28] Moved shield-study-type-specifics to separate class, added corresponding stub class for pioneer --- .../study/src/studyTypes/pioneer.js | 35 +++++++++++++++ .../study/src/studyTypes/shield.js | 44 +++++++++++++++++++ webExtensionApis/study/src/studyUtils.js | 29 ++++++------ 3 files changed, 93 insertions(+), 15 deletions(-) create mode 100644 webExtensionApis/study/src/studyTypes/pioneer.js create mode 100644 webExtensionApis/study/src/studyTypes/shield.js diff --git a/webExtensionApis/study/src/studyTypes/pioneer.js b/webExtensionApis/study/src/studyTypes/pioneer.js new file mode 100644 index 0000000..2c9d9d1 --- /dev/null +++ b/webExtensionApis/study/src/studyTypes/pioneer.js @@ -0,0 +1,35 @@ +/* eslint-env commonjs */ +/* eslint no-unused-vars: ["error", { "varsIgnorePattern": "(Pioneer)" }]*/ + +const { PioneerUtils } = require("pioneer-utils/dist/PioneerUtils.jsm"); + +class PioneerStudyType { + /** + * @param {object} studyUtils The studyUtils instance from where this class was instantiated + */ + constructor(studyUtils) { + console.log("studyUtils", studyUtils); + console.log("PioneerUtils", PioneerUtils); + } + + /** + * @returns {Promise<*>} + */ + async getTelemetryId() { + const id = "foo"; + return id; + } + + /** + * @param bucket + * @param payload + * @returns {*|Promise} + */ + sendTelemetry(bucket, payload) { + console.log("bucket", bucket); + console.log("payload", payload); + return true; + } +} + +export default PioneerStudyType; diff --git a/webExtensionApis/study/src/studyTypes/shield.js b/webExtensionApis/study/src/studyTypes/shield.js new file mode 100644 index 0000000..902694a --- /dev/null +++ b/webExtensionApis/study/src/studyTypes/shield.js @@ -0,0 +1,44 @@ +/* eslint-env commonjs */ + +const { TelemetryController } = ChromeUtils.import( + "resource://gre/modules/TelemetryController.jsm", + null, +); +const CID = ChromeUtils.import("resource://gre/modules/ClientID.jsm", {}); +// ChromeUtils.import("resource://gre/modules/ExtensionUtils.jsm"); + +// eslint-disable-next-line no-undef +// const { ExtensionError } = ExtensionUtils; + +class ShieldStudyType { + /** + * @param {object} studyUtils The studyUtils instance from where this class was instantiated + */ + constructor(studyUtils) { + console.log("studyUtils", studyUtils); + } + + /** + * @returns {Promise<*>} + */ + async getTelemetryId() { + const id = TelemetryController.clientID; + /* istanbul ignore next */ + if (id === undefined) { + return CID.ClientIDImpl._doLoadClientID(); + } + return id; + } + + /** + * @param bucket + * @param payload + * @returns {*|Promise} + */ + sendTelemetry(bucket, payload) { + const telOptions = { addClientId: true, addEnvironment: true }; + return TelemetryController.submitExternalPing(bucket, payload, telOptions); + } +} + +export default ShieldStudyType; diff --git a/webExtensionApis/study/src/studyUtils.js b/webExtensionApis/study/src/studyUtils.js index 4757616..3979938 100644 --- a/webExtensionApis/study/src/studyUtils.js +++ b/webExtensionApis/study/src/studyUtils.js @@ -5,6 +5,8 @@ import sampling from "./sampling"; import { utilsLogger } from "./logger"; import makeWidgetId from "./makeWidgetId"; +import ShieldStudyType from "./studyTypes/shield"; +import PioneerStudyType from "./studyTypes/pioneer"; /* * Supports the `browser.study` webExtensionExperiment api. @@ -49,11 +51,6 @@ const { ExtensionUtils } = ChromeUtils.import( const { ExtensionError } = ExtensionUtils; // telemetry utils -const CID = ChromeUtils.import("resource://gre/modules/ClientID.jsm", {}); -const { TelemetryController } = ChromeUtils.import( - "resource://gre/modules/TelemetryController.jsm", - null, -); const { TelemetryEnvironment } = ChromeUtils.import( "resource://gre/modules/TelemetryEnvironment.jsm", null, @@ -269,6 +266,14 @@ class StudyUtils { } guard.it("studySetup", studySetup, "(in studySetup)"); + // Different study types treat data and configuration differently + if (studySetup.studyType === "shield") { + this.studyType = new ShieldStudyType(this); + } + if (studySetup.studyType === "pioneer") { + this.studyType = new PioneerStudyType(this); + } + function getVariationByName(name, variations) { if (!name) return null; const chosen = variations.filter(x => x.name === name)[0]; @@ -318,6 +323,7 @@ class StudyUtils { */ reset() { this._internals = this._createInternals(); + this.studyType = null; this.resetFirstRunTimestamp(); } @@ -399,12 +405,7 @@ class StudyUtils { * @returns {string} - the telemetry client ID */ async getTelemetryId() { - const id = TelemetryController.clientID; - /* istanbul ignore next */ - if (id === undefined) { - return CID.ClientIDImpl._doLoadClientID(); - } - return id; + return this.studyType.getTelemetryId(); } /** @@ -457,8 +458,7 @@ class StudyUtils { weightedVariations, fraction = null, ) { - // this is the standard arm choosing method - // TODO, allow 'pioneer' algorithm + // this is the standard arm choosing method, used by both shield and pioneer studies if (fraction === null) { // hash the studyName and telemetryId to get the same branch every time. const clientId = await this.getTelemetryId(); @@ -744,8 +744,7 @@ class StudyUtils { return false; } - const telOptions = { addClientId: true, addEnvironment: true }; - return TelemetryController.submitExternalPing(bucket, payload, telOptions); + return this.studyType.sendTelemetry(bucket, payload); } /** From 82391f274b2e6d62e3e48e820ffca9920ef806b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Fri, 31 Aug 2018 23:30:26 +0300 Subject: [PATCH 04/28] Imported relevant parts from pioneer-utils 1.0.10, no longer requiring pioneer-utils as a dependency --- package-lock.json | 10 +- package.json | 2 +- webExtensionApis/study/src/dataPermissions.js | 39 +++ .../study/src/studyTypes/pioneer.js | 324 +++++++++++++++++- .../src/studyTypes/pioneer.public_keys.json | 20 ++ .../study/src/studyTypes/shield.js | 2 +- webExtensionApis/study/src/studyUtils.js | 2 +- 7 files changed, 380 insertions(+), 19 deletions(-) create mode 100644 webExtensionApis/study/src/dataPermissions.js create mode 100644 webExtensionApis/study/src/studyTypes/pioneer.public_keys.json diff --git a/package-lock.json b/package-lock.json index a33dc69..c928fb6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5484,6 +5484,11 @@ "integrity": "sha1-LPn7rkbYB0/Ba33gBxyO/rykc6Y=", "dev": true }, + "jose-jwe-jws": { + "version": "0.1.6", + "resolved": "https://registry.npmjs.org/jose-jwe-jws/-/jose-jwe-jws-0.1.6.tgz", + "integrity": "sha512-sZgrf5u15NmfiRAy3TAzQkIDsbXJ4zE11/rRTbQlGNFFy57u3B98U9JwH864+WfWM8BuJPqyCDDfDJJXUdWhiw==" + }, "js-select": { "version": "0.6.0", "resolved": "https://registry.npmjs.org/js-select/-/js-select-0.6.0.tgz", @@ -7770,11 +7775,6 @@ "integrity": "sha512-NqWvrQD/GpY78ybiNBzi/dg8ylERhDo6nB33j5sfCKpUmWLc3lYzeoBjyRoCMvEpDpL9lmH6ufRd0jw6rcd1pQ==", "dev": true }, - "pioneer-utils": { - "version": "1.0.10", - "resolved": "https://registry.npmjs.org/pioneer-utils/-/pioneer-utils-1.0.10.tgz", - "integrity": "sha1-7xK5xH/E8Va3oRMYwnEDbg1ClZ4=" - }, "pluralize": { "version": "7.0.0", "resolved": "https://registry.npmjs.org/pluralize/-/pluralize-7.0.0.tgz", diff --git a/package.json b/package.json index edd972c..9460cce 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "ajv": "^6.5.0", "commander": "^2.15.1", "fs-extra": "^6.0.1", - "pioneer-utils": "^1.0.10", + "jose-jwe-jws": "0.1.6", "shield-study-schemas": "^0.8.3" }, "devDependencies": { diff --git a/webExtensionApis/study/src/dataPermissions.js b/webExtensionApis/study/src/dataPermissions.js new file mode 100644 index 0000000..984b127 --- /dev/null +++ b/webExtensionApis/study/src/dataPermissions.js @@ -0,0 +1,39 @@ +const { Services } = ChromeUtils.import( + "resource://gre/modules/Services.jsm", + {}, +); +const { AddonManager } = ChromeUtils.import( + "resource://gre/modules/AddonManager.jsm", + {}, +); + +/** + * Checks to see if SHIELD is enabled for a user. + * + * @returns {Boolean} + * A boolean to indicate SHIELD opt-in status. + */ +export function isShieldEnabled() { + return Services.prefs.getBoolPref("app.shield.optoutstudies.enabled", true); +} + +/** + * Checks to see if the user has opted in to Pioneer. This is + * done by checking that the opt-in addon is installed and active. + * + * @returns {Boolean} + * A boolean to indicate opt-in status. + */ +export async function isUserOptedInToPioneer() { + const addon = await AddonManager.getAddonByID("pioneer-opt-in@mozilla.org"); + return isShieldEnabled() && addon !== null && addon.isActive; +} + +export async function getDataPermissions() { + const shield = isShieldEnabled(); + const pioneer = await isUserOptedInToPioneer(); + return { + shield, + pioneer, + }; +} diff --git a/webExtensionApis/study/src/studyTypes/pioneer.js b/webExtensionApis/study/src/studyTypes/pioneer.js index 2c9d9d1..08bbfeb 100644 --- a/webExtensionApis/study/src/studyTypes/pioneer.js +++ b/webExtensionApis/study/src/studyTypes/pioneer.js @@ -1,34 +1,336 @@ /* eslint-env commonjs */ /* eslint no-unused-vars: ["error", { "varsIgnorePattern": "(Pioneer)" }]*/ -const { PioneerUtils } = require("pioneer-utils/dist/PioneerUtils.jsm"); +import { utilsLogger } from "../logger"; +import * as dataPermissions from "../dataPermissions"; + +const { Services } = ChromeUtils.import( + "resource://gre/modules/Services.jsm", + {}, +); +const { TelemetryController } = ChromeUtils.import( + "resource://gre/modules/TelemetryController.jsm", + {}, +); + +const { generateUUID } = Cc["@mozilla.org/uuid-generator;1"].getService( + Ci.nsIUUIDGenerator, +); + +import { + setCrypto as joseSetCrypto, + Jose, + JoseJWE, +} from "jose-jwe-jws/dist/jose-commonjs.js"; + +// The public keys used for encryption +import * as PUBLIC_KEYS from "./pioneer.public_keys.json"; + +const PIONEER_ID_PREF = "extensions.pioneer.cachedClientID"; + +const EVENTS = { + INELIGIBLE: "ineligible", + EXPIRED: "expired", + USER_DISABLE: "user-disable", + ENDED_POSITIVE: "ended-positive", + ENDED_NEUTRAL: "ended-neutral", + ENDED_NEGATIVE: "ended-negative", +}; + +// Make crypto available and make jose use it. +Cu.importGlobalProperties(["crypto"]); +joseSetCrypto(crypto); + +/** + * @typedef {Object} Config + * @property {String} studyName + * Unique name of the study. + * + * @property {String?} telemetryEnv + * Optional. Which telemetry environment to send data to. Should be + * either ``"prod"`` or ``"stage"``. Defaults to ``"prod"``. + */ + +/** + * Utilities for making Pioneer Studies. + */ +class PioneerUtils { + /** + * @param {Config} config + */ + constructor(config) { + this.config = config; + this.encrypter = null; + this._logger = null; + } + + /** + * @returns {Object} A public key + */ + getPublicKey() { + const env = this.config.telemetryEnv || "prod"; + return PUBLIC_KEYS[env]; + } + + /** */ + setupEncrypter() { + if (this.encrypter === null) { + const pk = this.getPublicKey(); + const rsa_key = Jose.Utils.importRsaPublicKey(pk.key, "RSA-OAEP"); + const cryptographer = new Jose.WebCryptographer(); + this.encrypter = new JoseJWE.Encrypter(cryptographer, rsa_key); + } + } + + /** + * @returns {String} Unique ID for a Pioneer user. + */ + getPioneerId() { + let id = Services.prefs.getCharPref(PIONEER_ID_PREF, ""); + + if (!id) { + // generateUUID adds leading and trailing "{" and "}". strip them off. + id = generateUUID() + .toString() + .slice(1, -1); + Services.prefs.setCharPref(PIONEER_ID_PREF, id); + } + + return id; + } + + /** + * Calculate the size of a ping. + * + * @param {Object} payload + * The data payload of the ping. + * + * @returns {Number} + * The total size of the ping. + */ + getPingSize(payload) { + const converter = Cc[ + "@mozilla.org/intl/scriptableunicodeconverter" + ].createInstance(Ci.nsIScriptableUnicodeConverter); + converter.charset = "UTF-8"; + let utf8Payload = converter.ConvertFromUnicode(JSON.stringify(payload)); + utf8Payload += converter.Finish(); + return utf8Payload.length; + } + + /** + * @private + * @param {String} data The data to encrypt + * @returns {String} + */ + async encryptData(data) { + this.setupEncrypter(); + return this.encrypter.encrypt(data); + } + + /** + * Constructs a payload object with encrypted data. + * + * @param {String} schemaName + * The name of the schema to be used for validation. + * + * @param {int} schemaVersion + * The version of the schema to be used for validation. + * + * @param {Object} data + * An object containing data to be encrypted and submitted. + * + * @returns {Object} + * A Telemetry payload object with the encrypted data. + */ + async buildEncryptedPayload(schemaName, schemaVersion, data) { + const pk = this.getPublicKey(); + + return { + encryptedData: await this.encryptData(JSON.stringify(data)), + encryptionKeyId: pk.id, + pioneerId: this.getPioneerId(), + studyName: this.config.studyName, + schemaName, + schemaVersion, + }; + } + + /** + * Calculate the size of a ping that has Pioneer encrypted data. + * + * @param {String} schemaName + * The name of the schema to be used for validation. + * + * @param {int} schemaVersion + * The version of the schema to be used for validation. + * + * @param {Object} data + * An object containing data to be encrypted and submitted. + * + * @returns {Number} + * The total size of the ping. + */ + async getEncryptedPingSize(schemaName, schemaVersion, data) { + return this.getPingSize( + await this.buildEncryptedPayload(schemaName, schemaVersion, data), + ); + } + + /** + * Encrypts the given data and submits a properly formatted + * Pioneer ping to Telemetry. + * + * @param {String} schemaName + * The name of the schema to be used for validation. + * + * @param {int} schemaVersion + * The version of the schema to be used for validation. + * + * @param {Object} data + * A object containing data to be encrypted and submitted. + * + * @param {Object} options + * An object with additional options for the function. + * + * @param {Boolean} options.force + * A boolean to indicate whether to force submission of the ping. + * + * @returns {String} + * The ID of the ping that was submitted + */ + async submitEncryptedPing(schemaName, schemaVersion, data, options = {}) { + // If the user is no longer opted in we should not be submitting pings. + const isUserOptedIn = await dataPermissions.isUserOptedInToPioneer(); + if (!isUserOptedIn && !options.force) { + return null; + } + + const payload = await this.buildEncryptedPayload( + schemaName, + schemaVersion, + data, + ); + + const telOptions = { + addClientId: true, + addEnvironment: true, + }; + + return TelemetryController.submitExternalPing( + "pioneer-study", + payload, + telOptions, + ); + } + + /** + * Gets an object that is a mapping of all the available events. + * + * @returns {Object} + * An object with all the available events. + */ + getAvailableEvents() { + return EVENTS; + } + + /** + * Submits an encrypted event ping. + * + * @param {String} eventId + * The ID of the event that occured. + * + * @param {Object} options + * An object of options to be passed through to submitEncryptedPing + * + * @returns {String} + * The ID of the event ping that was submitted. + */ + async submitEventPing(eventId, options = {}) { + if (!Object.values(EVENTS).includes(eventId)) { + throw new Error("Invalid event ID."); + } + return this.submitEncryptedPing("event", 1, { eventId }, options); + } +} class PioneerStudyType { /** * @param {object} studyUtils The studyUtils instance from where this class was instantiated */ constructor(studyUtils) { - console.log("studyUtils", studyUtils); - console.log("PioneerUtils", PioneerUtils); + const studySetup = studyUtils._internals.studySetup; + const Config = { + studyName: studySetup.activeExperimentName, + telemetryEnv: studySetup.telemetry.removeTestingFlag ? "prod" : "stage", + }; + this.pioneerUtils = new PioneerUtils(Config); } /** - * @returns {Promise<*>} + * @returns {Promise} The ID of the event ping that was submitted. + */ + async notifyNotEligible() { + return this.notifyEndStudy(this.EVENTS.INELIGIBLE); + } + + /** + * @param {String?} eventId The ID of the event that occured. + * @returns {Promise} The ID of the event ping that was submitted. + */ + async notifyEndStudy(eventId = EVENTS.ENDED_NEUTRAL) { + return this.pioneerUtils.submitEventPing(eventId, { force: true }); + } + + /** + * @returns {Promise} Unique ID for a Pioneer user. */ async getTelemetryId() { - const id = "foo"; - return id; + return this.pioneerUtils.getPioneerId(); } /** * @param bucket * @param payload - * @returns {*|Promise} + * @returns {Promise<*>} + */ + async sendTelemetry(bucket, payload) { + return this._telemetry(bucket, 1, payload); + } + + /** + * Encrypts the given data and submits a properly formatted + * Pioneer ping to Telemetry. + * + * @param {String} schemaName + * The name of the schema to be used for validation. + * + * @param {int} schemaVersion + * The version of the schema to be used for validation. + * + * @param {Object} payload + * A object containing data to be encrypted and submitted. + * + * @returns {Promise} The ID of the ping that was submitted + * @private */ - sendTelemetry(bucket, payload) { - console.log("bucket", bucket); - console.log("payload", payload); - return true; + async _telemetry(schemaName, schemaVersion, payload) { + const pingId = await this.pioneerUtils.submitEncryptedPing( + schemaName, + schemaVersion, + payload, + ); + if (pingId) { + utilsLogger.log( + "Pioneer Telemetry sent (encrypted)", + JSON.stringify(payload), + ); + } else { + utilsLogger.log( + "Pioneer Telemetry not sent due to privacy preferences", + JSON.stringify(payload), + ); + } } } diff --git a/webExtensionApis/study/src/studyTypes/pioneer.public_keys.json b/webExtensionApis/study/src/studyTypes/pioneer.public_keys.json new file mode 100644 index 0000000..07e8f55 --- /dev/null +++ b/webExtensionApis/study/src/studyTypes/pioneer.public_keys.json @@ -0,0 +1,20 @@ +{ + "stage": { + "id": "pioneer-20170905", + "key": { + "e": "AQAB", + "kty": "RSA", + "n": + "3nI-DQ7NoUZCvT348Vi4JfGC1h6R3Qf_yXR0dKM5DmwsuQMxguce6sZ28GWQHJjgbdcs8nTuNQihyVtr9vLsoKUVSmPs_a3QEGXEhTpuTtm7cCb_7HyAlwGtysn2AsdElG8HsDFWlZmiDaHTrTmdLnuk-Z3GRg4nnA4xs4vvUuh0fCVIKoSMFyt3Tkc6IBWJ9X3XrDEbSPrghXV7Cu8LMK3Y4avy6rjEGjWXL-WqIPhiYJcBiFnCcqUCMPvdW7Fs9B36asc_2EQAM5d7BAiBwMjoosSyU6b4JGpI530c3xhqLbX00q1ePCG732cIwp0-bGWV_q0FpQX2M9cNv2Ax4Q" + } + }, + "prod": { + "id": "pioneer-20170905", + "key": { + "e": "AQAB", + "kty": "RSA", + "n": + "_uqWswIJpR-cFdwwtNdAI_B_0sPIyQyBy6hiiQ0GKLF2k1PkN6RaxtbZK8v1_BriYtEgWn3hNzJNbKBWBMFtF5-8OfvxH-hgIIeDmRmeHmynLBBCDVf2HAZYaDXJiM7s6LBubDuoPDc3Ovoj287W7E4LgzsBS0wo3ARIwlKn6x0Dj5tu6CQ5r3t0GKZoSFkiVZA7nke-VC55nlDacIIYAqkMX0dzsBaCRmf2C5JJTP-K14iRLB5VFGZ_vnoZ-Wi1BGRV2TNRl3xl0lFJIcPklFpU3hsnRPiF4y7kenU6OIhJVQMqX1CtCF698k7SFCYJt7r1ymWJE-tv0ZwF9b1MFw" + } + } +} diff --git a/webExtensionApis/study/src/studyTypes/shield.js b/webExtensionApis/study/src/studyTypes/shield.js index 902694a..6c44874 100644 --- a/webExtensionApis/study/src/studyTypes/shield.js +++ b/webExtensionApis/study/src/studyTypes/shield.js @@ -35,7 +35,7 @@ class ShieldStudyType { * @param payload * @returns {*|Promise} */ - sendTelemetry(bucket, payload) { + async sendTelemetry(bucket, payload) { const telOptions = { addClientId: true, addEnvironment: true }; return TelemetryController.submitExternalPing(bucket, payload, telOptions); } diff --git a/webExtensionApis/study/src/studyUtils.js b/webExtensionApis/study/src/studyUtils.js index 3979938..713050f 100644 --- a/webExtensionApis/study/src/studyUtils.js +++ b/webExtensionApis/study/src/studyUtils.js @@ -265,6 +265,7 @@ class StudyUtils { throw new ExtensionError("StudyUtils is already setup"); } guard.it("studySetup", studySetup, "(in studySetup)"); + this._internals.studySetup = studySetup; // Different study types treat data and configuration differently if (studySetup.studyType === "shield") { @@ -299,7 +300,6 @@ class StudyUtils { utilsLogger.debug(`setting up: variation ${variation.name}`); this._internals.variation = variation; - this._internals.studySetup = studySetup; this._internals.isSetup = true; // isFirstRun? ever seen before? From 99697517771e4d706736477d8bbd07ac65b4ec14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Fri, 31 Aug 2018 23:48:53 +0300 Subject: [PATCH 05/28] Added browser.study.getDataPermissions() and using it to check eligibility in getStudySetup --- test-addon/src/studySetup.js | 21 +++++++++----- webExtensionApis/study/api.md | 35 +++++++++++++++++++---- webExtensionApis/study/schema.json | 33 +++++++++++++++++++++- webExtensionApis/study/schema.yaml | 44 ++++++++++++++++------------- webExtensionApis/study/src/index.js | 6 ++++ 5 files changed, 106 insertions(+), 33 deletions(-) diff --git a/test-addon/src/studySetup.js b/test-addon/src/studySetup.js index 2dd2f3d..5e0c58f 100644 --- a/test-addon/src/studySetup.js +++ b/test-addon/src/studySetup.js @@ -99,10 +99,11 @@ const baseStudySetup = { * * This implementation caches in local storage to speed up second run. * + * @param {object} studySetup A complete study setup object * @returns {Promise} answer An boolean answer about whether the user should be * allowed to enroll in the study */ -async function cachingFirstRunShouldAllowEnroll() { +async function cachingFirstRunShouldAllowEnroll(studySetup) { // Cached answer. Used on 2nd run let allowed = await browser.storage.local.get("allowedEnrollOnFirstRun"); if (allowed.allowedEnrollOnFirstRun === true) return true; @@ -113,7 +114,13 @@ async function cachingFirstRunShouldAllowEnroll() { */ // could have other reasons to be eligible, such add-ons, prefs - allowed = true; + const dataPermissions = await browser.study.getDataPermissions(); + if (studySetup.studyType === "shield") { + allowed = dataPermissions.shield; + } + if (studySetup.studyType === "pioneer") { + allowed = dataPermissions.pioneer; + } // cache the answer await browser.storage.local.set({ allowedEnrollOnFirstRun: allowed }); @@ -129,7 +136,11 @@ async function getStudySetup() { // shallow copy const studySetup = Object.assign({}, baseStudySetup); - studySetup.allowEnroll = await cachingFirstRunShouldAllowEnroll(); + // internal testing override necessary to be able to test all study types + const internalTestingOverrides = await browser.studyDebug.getInternalTestingOverrides(); + studySetup.studyType = internalTestingOverrides.studyType; + + studySetup.allowEnroll = await cachingFirstRunShouldAllowEnroll(studySetup); const testingOverrides = await browser.study.getTestingOverrides(); studySetup.testing = { @@ -138,9 +149,5 @@ async function getStudySetup() { expired: testingOverrides.expired, }; - // internal testing override necessary to be able to test all study types - const internalTestingOverrides = await browser.studyDebug.getInternalTestingOverrides(); - studySetup.studyType = internalTestingOverrides.studyType; - return studySetup; } diff --git a/webExtensionApis/study/api.md b/webExtensionApis/study/api.md index 922b945..9ee24f1 100644 --- a/webExtensionApis/study/api.md +++ b/webExtensionApis/study/api.md @@ -11,7 +11,7 @@ Attempt an setup/enrollment, with these effects: * sets 'studyType' as Shield or Pioneer * affects telemetry - * (5.1 TODO) watches for dataPermission changes that should _always_ + * (5.2+ TODO) watches for dataPermission changes that should _always_ stop that kind of study * Use or choose variation @@ -139,6 +139,12 @@ Throws Error if called before `browser.study.setup` **Parameters** +### `browser.study.getDataPermissions( )` + +Object of current dataPermissions (shield enabled true/false, pioneer enabled true/false) + +**Parameters** + ### `browser.study.sendTelemetry( payload )` Send Telemetry using appropriate shield or pioneer methods. @@ -575,7 +581,26 @@ Act on it by } ``` -### [9] studySetup +### [9] dataPermissionsObject + +```json +{ + "id": "dataPermissionsObject", + "type": "object", + "additionalProperties": false, + "properties": { + "shield": { + "type": "boolean" + }, + "pioneer": { + "type": "boolean" + } + }, + "required": ["shield", "pioneer"] +} +``` + +### [10] studySetup ```json { @@ -787,7 +812,7 @@ Act on it by } ``` -### [10] telemetryPayload +### [11] telemetryPayload ```json { @@ -801,7 +826,7 @@ Act on it by } ``` -### [11] searchTelemetryQuery +### [12] searchTelemetryQuery ```json { @@ -839,7 +864,7 @@ Act on it by } ``` -### [12] anEndingAnswer +### [13] anEndingAnswer ```json { diff --git a/webExtensionApis/study/schema.json b/webExtensionApis/study/schema.json index 8363798..a536cc0 100644 --- a/webExtensionApis/study/schema.json +++ b/webExtensionApis/study/schema.json @@ -276,6 +276,20 @@ "isFirstRun" ] }, + { + "id": "dataPermissionsObject", + "type": "object", + "additionalProperties": false, + "properties": { + "shield": { + "type": "boolean" + }, + "pioneer": { + "type": "boolean" + } + }, + "required": ["shield", "pioneer"] + }, { "id": "studySetup", "$schema": "http://json-schema.org/draft-04/schema", @@ -539,7 +553,7 @@ "type": "function", "async": true, "description": - "Attempt an setup/enrollment, with these effects:\n\n- sets 'studyType' as Shield or Pioneer\n - affects telemetry\n - (5.1 TODO) watches for dataPermission changes that should *always*\n stop that kind of study\n\n- Use or choose variation\n - `testing.variation` if present\n - OR (internal) deterministicVariation\n from `weightedVariations`\n based on hash of\n\n - activeExperimentName\n - clientId\n\n- During firstRun[1] only:\n - set firstRunTimestamp pref value\n - send 'enter' ping\n - if `allowEnroll`, send 'install' ping\n - else endStudy(\"ineligible\") and return\n\n- Every Run\n - setActiveExperiment(studySetup)\n - monitor shield | pioneer permission endings\n - suggests alarming if `expire` is set.\n\nReturns:\n- studyInfo object (see `getStudyInfo`)\n\nTelemetry Sent (First run only)\n\n - enter\n - install\n\nFires Events\n\n(At most one of)\n- study:onReady OR\n- study:onEndStudy\n\nPreferences set\n- `shield.${runtime.id}.firstRunTimestamp`\n\nNote:\n1. allowEnroll is ONLY used during first run (install)\n", + "Attempt an setup/enrollment, with these effects:\n\n- sets 'studyType' as Shield or Pioneer\n - affects telemetry\n - (5.2+ TODO) watches for dataPermission changes that should *always*\n stop that kind of study\n\n- Use or choose variation\n - `testing.variation` if present\n - OR (internal) deterministicVariation\n from `weightedVariations`\n based on hash of\n\n - activeExperimentName\n - clientId\n\n- During firstRun[1] only:\n - set firstRunTimestamp pref value\n - send 'enter' ping\n - if `allowEnroll`, send 'install' ping\n - else endStudy(\"ineligible\") and return\n\n- Every Run\n - setActiveExperiment(studySetup)\n - monitor shield | pioneer permission endings\n - suggests alarming if `expire` is set.\n\nReturns:\n- studyInfo object (see `getStudyInfo`)\n\nTelemetry Sent (First run only)\n\n - enter\n - install\n\nFires Events\n\n(At most one of)\n- study:onReady OR\n- study:onEndStudy\n\nPreferences set\n- `shield.${runtime.id}.firstRunTimestamp`\n\nNote:\n1. allowEnroll is ONLY used during first run (install)\n", "parameters": [ { "name": "studySetup", @@ -593,6 +607,23 @@ } ] }, + { + "name": "getDataPermissions", + "type": "function", + "async": true, + "description": + "Object of current dataPermissions (shield enabled true/false, pioneer enabled true/false)", + "defaultReturn": { + "shield": true, + "pioneer": false + }, + "parameters": [], + "returns": [ + { + "$ref": "dataPermissionsObject" + } + ] + }, { "name": "sendTelemetry", "type": "function", diff --git a/webExtensionApis/study/schema.yaml b/webExtensionApis/study/schema.yaml index 7c4f80d..5691473 100644 --- a/webExtensionApis/study/schema.yaml +++ b/webExtensionApis/study/schema.yaml @@ -169,16 +169,20 @@ - activeExperimentName - isFirstRun - #- id: dataPermissionsObject - # type: object - # additionalProperties: true - # properties: - # shield: - # type: - # boolean - # - # required: - # - shield + - id: dataPermissionsObject + type: object + additionalProperties: false + properties: + shield: + type: + boolean + pioneer: + type: + boolean + + required: + - shield + - pioneer - id: studySetup $schema: "http://json-schema.org/draft-04/schema" @@ -299,7 +303,7 @@ - sets 'studyType' as Shield or Pioneer - affects telemetry - - (5.1 TODO) watches for dataPermission changes that should *always* + - (5.2+ TODO) watches for dataPermission changes that should *always* stop that kind of study - Use or choose variation @@ -423,14 +427,14 @@ returns: - $ref: studyInfoObject - # - name: getDataPermissions - # type: function - # async: true - # description: object of current dataPermissions with keys shield, pioneer, telemetry, 'ok' - # defaultReturn: {shield: true, pioneer: false, telemetry: true, alwaysPrivateBrowsing: false} - # parameters: [] - # returns: - # - $ref: dataPermissionsObject + - name: getDataPermissions + type: function + async: true + description: Object of current dataPermissions (shield enabled true/false, pioneer enabled true/false) + defaultReturn: {shield: true, pioneer: false} + parameters: [] + returns: + - $ref: dataPermissionsObject # telemetry related things - name: sendTelemetry @@ -557,7 +561,7 @@ - name: ending type: object - # TODO 5.1 + # TODO 5.2+ # - name: onDataPermissionsChange # type: function # defaultReturn: {shield: true, pioneer: false} diff --git a/webExtensionApis/study/src/index.js b/webExtensionApis/study/src/index.js index ea2abf3..6568a5d 100644 --- a/webExtensionApis/study/src/index.js +++ b/webExtensionApis/study/src/index.js @@ -10,6 +10,7 @@ import { utilsLogger, createLogger } from "./logger"; import makeWidgetId from "./makeWidgetId"; import * as testingOverrides from "./testingOverrides"; +import * as dataPermissions from "./dataPermissions"; ChromeUtils.import("resource://gre/modules/ExtensionCommon.jsm"); ChromeUtils.import("resource://gre/modules/ExtensionUtils.jsm"); @@ -271,6 +272,11 @@ this.study = class extends ExtensionAPI { return studyUtils.info(); }, + /* Object of current dataPermissions (shield enabled true/false, pioneer enabled true/false) */ + getDataPermissions: async function getDataPermissions() { + return dataPermissions.getDataPermissions(); + }, + /** Send Telemetry using appropriate shield or pioneer methods. * * shield: From 5d1dad6a7e0078f72d1526a5841db1251a590b77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Fri, 31 Aug 2018 23:49:48 +0300 Subject: [PATCH 06/28] Synced small-study and test-addon boilerplate code --- examples/small-study/src/.eslintrc.js | 3 +++ examples/small-study/src/studySetup.js | 14 +++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/examples/small-study/src/.eslintrc.js b/examples/small-study/src/.eslintrc.js index 7e9f4bb..8ef4351 100644 --- a/examples/small-study/src/.eslintrc.js +++ b/examples/small-study/src/.eslintrc.js @@ -7,4 +7,7 @@ module.exports = { es6: true, webextensions: true, }, + rules: { + "no-console": "off", + }, }; diff --git a/examples/small-study/src/studySetup.js b/examples/small-study/src/studySetup.js index b853a0b..eb9c99a 100644 --- a/examples/small-study/src/studySetup.js +++ b/examples/small-study/src/studySetup.js @@ -99,10 +99,11 @@ const baseStudySetup = { * * This implementation caches in local storage to speed up second run. * + * @param {object} studySetup A complete study setup object * @returns {Promise} answer An boolean answer about whether the user should be * allowed to enroll in the study */ -async function cachingFirstRunShouldAllowEnroll() { +async function cachingFirstRunShouldAllowEnroll(studySetup) { // Cached answer. Used on 2nd run let allowed = await browser.storage.local.get("allowedEnrollOnFirstRun"); if (allowed.allowedEnrollOnFirstRun === true) return true; @@ -113,7 +114,13 @@ async function cachingFirstRunShouldAllowEnroll() { */ // could have other reasons to be eligible, such add-ons, prefs - allowed = true; + const dataPermissions = await browser.study.getDataPermissions(); + if (studySetup.studyType === "shield") { + allowed = dataPermissions.shield; + } + if (studySetup.studyType === "pioneer") { + allowed = dataPermissions.pioneer; + } // cache the answer await browser.storage.local.set({ allowedEnrollOnFirstRun: allowed }); @@ -129,7 +136,7 @@ async function getStudySetup() { // shallow copy const studySetup = Object.assign({}, baseStudySetup); - studySetup.allowEnroll = await cachingFirstRunShouldAllowEnroll(); + studySetup.allowEnroll = await cachingFirstRunShouldAllowEnroll(studySetup); const testingOverrides = await browser.study.getTestingOverrides(); studySetup.testing = { @@ -137,5 +144,6 @@ async function getStudySetup() { firstRunTimestamp: testingOverrides.firstRunTimestamp, expired: testingOverrides.expired, }; + return studySetup; } From 8ec4325b2d9fb1ecd8c062427a3d730de0e7f3ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Sat, 1 Sep 2018 00:23:26 +0300 Subject: [PATCH 07/28] Scripts to import/build and selenium instructions to install the pioneer opt-in add-on (necessary for the test add-on to pass eligibility checks when tested as a pioneer study) --- bin/import-pioneer-opt-in.sh | 20 ++++++++++++++++++++ package.json | 1 + test/functional/browser.study.api.js | 3 +++ test/functional/test-addon.js | 3 +++ testUtils/setupWebdriver.js | 14 ++++++++++++++ 5 files changed, 41 insertions(+) create mode 100755 bin/import-pioneer-opt-in.sh diff --git a/bin/import-pioneer-opt-in.sh b/bin/import-pioneer-opt-in.sh new file mode 100755 index 0000000..cdf3e8b --- /dev/null +++ b/bin/import-pioneer-opt-in.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash + +echo "$@" + +set -eu +#set -o xtrace + +BASE_DIR="$(dirname "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)")" + +# download and build xpi for https://github.com/mozilla/pioneer-opt-in.git +if [ ! -d "pioneer-opt-in" ]; then + git clone https://github.com/mozilla/pioneer-opt-in.git +fi +cd pioneer-opt-in +bin/make-xpi.sh . +cd - + +echo +echo "SUCCESS: pioneer-opt-in xpi available at pioneer-opt-in/pioneer-opt-in.xpi" +echo diff --git a/package.json b/package.json index 9460cce..81449b0 100644 --- a/package.json +++ b/package.json @@ -86,6 +86,7 @@ "lint:fixpack": "fixpack # cleans up package.json", "postbuild": "if [ -z ${SKIPLINT} ]; then npm run format; fi", "postformat": "run-p lint:fixpack eslint-fix", + "postinstall": "bin/import-pioneer-opt-in.sh", "prebuild": "if [ -z ${SKIPLINT} ]; then npm run lint; fi", "prepare": "export SKIPLINT=1 && fixpack && npm run build", "pretest": "npm run build && npm run test-addon:bundle-utils && npm run test-addon:build", diff --git a/test/functional/browser.study.api.js b/test/functional/browser.study.api.js index d2ddbde..96ac4ca 100644 --- a/test/functional/browser.study.api.js +++ b/test/functional/browser.study.api.js @@ -127,6 +127,9 @@ const publicApiTests = function(studyType) { if (addonId) { await utils.setupWebdriver.uninstallAddon(driver, addonId); } + if (studyType === "pioneer") { + await utils.setupWebdriver.installPioneerOptInAddon(driver); + } addonId = await utils.setupWebdriver.installAddon(driver); } diff --git a/test/functional/test-addon.js b/test/functional/test-addon.js index 87c5e7c..23e7ed4 100644 --- a/test/functional/test-addon.js +++ b/test/functional/test-addon.js @@ -30,6 +30,9 @@ const testAddonTests = function(studyType) { `extensions.${widgetId}.test.studyType`, studyType, ); + if (studyType === "pioneer") { + await utils.setupWebdriver.installPioneerOptInAddon(driver); + } await utils.setupWebdriver.installAddon(driver); await utils.ui.openBrowserConsole(driver); }); diff --git a/testUtils/setupWebdriver.js b/testUtils/setupWebdriver.js index a7ad728..b22e6a0 100644 --- a/testUtils/setupWebdriver.js +++ b/testUtils/setupWebdriver.js @@ -107,6 +107,20 @@ module.exports.setupWebdriver = { return addonId; }, + /** Install pioneer opt-in add-on from where it is expected to be if its + * repo is cloned in the current working directory and the xpi then built within + * + * @param {object} driver Configured Firefox webdriver + * @param {string} fileLocation location for add-on xpi/zip + * @returns {Promise} returns add-on id) + */ + installPioneerOptInAddon: async (driver, fileLocation) => { + fileLocation = + fileLocation || + path.join(process.cwd(), "pioneer-opt-in/pioneer-opt-in.xpi"); + return module.exports.setupWebdriver.installAddon(driver, fileLocation); + }, + uninstallAddon: async (driver, addonId) => { const executor = driver.getExecutor(); executor.defineCommand( From 69a221cff1877c62831855151a56bc162b3b22f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Sat, 1 Sep 2018 02:39:17 +0300 Subject: [PATCH 08/28] Added studySetup.telemetry.internalTelemetryArchive flag for internally tracking all pings sent using study utils --- examples/small-study/src/studySetup.js | 6 ++- test-addon/src/studySetup.js | 6 ++- test/functional/browser.study.api.js | 62 ++++++++++++++---------- testUtils/telemetry.js | 16 ++++++ webExtensionApis/study/api.md | 12 +++-- webExtensionApis/study/schema.json | 12 +++-- webExtensionApis/study/schema.yaml | 9 ++-- webExtensionApis/study/src/studyUtils.js | 18 +++---- webExtensionApis/study/src/telemetry.js | 2 - 9 files changed, 94 insertions(+), 49 deletions(-) diff --git a/examples/small-study/src/studySetup.js b/examples/small-study/src/studySetup.js index eb9c99a..9b6385f 100644 --- a/examples/small-study/src/studySetup.js +++ b/examples/small-study/src/studySetup.js @@ -27,10 +27,12 @@ const baseStudySetup = { // telemetry telemetry: { - // default false. Actually send pings. + // Actually submit the pings to Telemetry. [default if omitted: false] send: true, - // Marks pings with testing=true. Set flag to `true` before final release + // Marks pings with testing=true. Set flag to `true` for pings are meant to be seen by analysts [default if omitted: false] removeTestingFlag: false, + // Keep an internal telemetry archive. Useful for verifying payloads of Pioneer studies without risking actually sending any unencrypted payloads [default if omitted: false] + internalTelemetryArchive: false, }, // endings with urls diff --git a/test-addon/src/studySetup.js b/test-addon/src/studySetup.js index 5e0c58f..3a6ba9b 100644 --- a/test-addon/src/studySetup.js +++ b/test-addon/src/studySetup.js @@ -27,10 +27,12 @@ const baseStudySetup = { // telemetry telemetry: { - // default false. Actually send pings. + // Actually submit the pings to Telemetry. [default if omitted: false] send: true, - // Marks pings with testing=true. Set flag to `true` before final release + // Marks pings with testing=true. Set flag to `true` for pings are meant to be seen by analysts [default if omitted: false] removeTestingFlag: false, + // Keep an internal telemetry archive. Useful for verifying payloads of Pioneer studies without risking actually sending any unencrypted payloads [default if omitted: false] + internalTelemetryArchive: true, }, // endings with urls diff --git a/test/functional/browser.study.api.js b/test/functional/browser.study.api.js index 96ac4ca..60ba6ed 100644 --- a/test/functional/browser.study.api.js +++ b/test/functional/browser.study.api.js @@ -43,6 +43,7 @@ const MINUTES_PER_DAY = 60 * 24; // node's util, for printing a deeply nested object to node console const { inspect } = require("util"); + // eslint-disable-next-line no-unused-vars function full(myObject) { return inspect(myObject, { showHidden: false, depth: null }); @@ -79,8 +80,9 @@ const publicApiTests = function(studyType) { }, }, telemetry: { - send: false, // assumed false. Actually send pings if true - removeTestingFlag: false, // Marks pings to be discarded, set true for to have the pings processed in the pipeline + send: false, + removeTestingFlag: false, + internalTelemetryArchive: true, }, logLevel: 10, weightedVariations: [ @@ -321,9 +323,9 @@ const publicApiTests = function(studyType) { // tests const now = Number(Date.now()); - const seenTelemetryStates = internals.seenTelemetry["shield-study"].map( - x => x.data.study_state, - ); + const seenTelemetryStates = internals.seenTelemetry + .filter(ping => ping.type === "shield-study") + .map(x => x.data.study_state); assert(internals.isSetup, "should be isSetup"); assert(!internals.isEnded, "should not be ended"); assert(!internals.isEnding, "should not be ending"); @@ -372,9 +374,9 @@ const publicApiTests = function(studyType) { const { info, internals } = data; // tests - const seenTelemetryStates = internals.seenTelemetry["shield-study"].map( - x => x.data.study_state, - ); + const seenTelemetryStates = internals.seenTelemetry + .filter(ping => ping.type === "shield-study") + .map(x => x.data.study_state); assert(internals.isSetup, "should be isSetup"); assert(!internals.isEnded, "should not be ended"); @@ -425,9 +427,9 @@ const publicApiTests = function(studyType) { const { info, internals } = data; // tests - const seenTelemetryStates = internals.seenTelemetry["shield-study"].map( - x => x.data.study_state, - ); + const seenTelemetryStates = internals.seenTelemetry + .filter(ping => ping.type === "shield-study") + .map(x => x.data.study_state); assert(internals.isSetup, "should be isSetup"); assert(internals.isEnded, "should be ended"); @@ -584,6 +586,7 @@ const publicApiTests = function(studyType) { telemetry: { send: true, removeTestingFlag: false, + internalTelemetryArchive: true, }, endings: { customEnding: { @@ -621,13 +624,18 @@ const publicApiTests = function(studyType) { before(async () => { studyPings = await addonExec(async callback => { const _studyPings = await browser.study.searchSentTelemetry({ - type: ["shield-study", "shield-study-addon"], + type: [ + "shield-study", + "shield-study-addon", + "shield-study-error", + "pioneer-study", + ], }); callback(_studyPings); }); - // console.debug(full(studyPings.map(x => x.payload))); // For debugging tests // console.debug("Pings report: ", utils.telemetry.pingsReport(studyPings)); + // console.debug("Pings with id and payload: ", utils.telemetry.pingsDebug(studyPings)); }); it("should have set the experiment to active in Telemetry", async () => { @@ -641,14 +649,15 @@ const publicApiTests = function(studyType) { }); it("shield-study-addon telemetry should be working (as seen by telemetry)", async () => { - const shieldTelemetryPings = await addonExec(async callback => { - const _studyPings = await browser.study.searchSentTelemetry({ - type: ["shield-study-addon"], - }); - callback(_studyPings.map(x => x.payload)); - }); - // console.debug("pings", full(shieldTelemetryPings)); - assert(shieldTelemetryPings[0].data.attributes.foo === "bar"); + const filteredPings = studyPings.filter( + ping => ping.type === "shield-study-addon", + ); + assert( + filteredPings.length > 0, + "at least one shield-study-addon telemetry ping", + ); + // console.debug("shield-study-addon pings", full(filteredPings)); + assert(filteredPings[0].payload.data.attributes.foo === "bar"); }); it("should have sent at least one shield telemetry ping", async () => { @@ -752,8 +761,8 @@ const publicApiTests = function(studyType) { type: ["shield-study", "shield-study-addon"], }); // For debugging tests - // console.debug(full(studyPings.map(x => [x.type, x.payload]))); // console.debug("Final pings report: ", utils.telemetry.pingsReport(studyPings)); + // console.debug("Final pings with id and payload: ", utils.telemetry.pingsDebug(studyPings)); }); it("one shield-study telemetry ping with study_state=exit", async () => { @@ -791,6 +800,7 @@ const publicApiTests = function(studyType) { telemetry: { send: true, removeTestingFlag: false, + internalTelemetryArchive: true, }, endings: { ineligible: { @@ -860,8 +870,8 @@ const publicApiTests = function(studyType) { type: ["shield-study", "shield-study-addon"], }); // For debugging tests - // console.debug(full(studyPings.map(x => [x.type, x.payload]))); // console.debug("Final pings report: ", utils.telemetry.pingsReport(studyPings)); + // console.debug("Final pings with id and payload: ", utils.telemetry.pingsDebug(studyPings)); }); it("one shield-study telemetry ping with study_state=exit", async () => { @@ -898,6 +908,7 @@ const publicApiTests = function(studyType) { telemetry: { send: true, removeTestingFlag: false, + internalTelemetryArchive: true, }, endings: { expired: { @@ -970,8 +981,8 @@ const publicApiTests = function(studyType) { type: ["shield-study", "shield-study-addon"], }); // For debugging tests - // console.debug(full(studyPings.map(x => [x.type, x.payload]))); // console.debug("Final pings report: ", utils.telemetry.pingsReport(studyPings)); + // console.debug("Final pings with id and payload: ", utils.telemetry.pingsDebug(studyPings)); }); it("one shield-study telemetry ping with study_state=exit", async () => { @@ -1010,6 +1021,7 @@ const publicApiTests = function(studyType) { telemetry: { send: true, removeTestingFlag: false, + internalTelemetryArchive: true, }, expire: { days: 1, @@ -1105,8 +1117,8 @@ const publicApiTests = function(studyType) { type: ["shield-study", "shield-study-addon"], }); // For debugging tests - // console.debug(full(studyPings.map(x => [x.type, x.payload]))); // console.debug("Final pings report: ", utils.telemetry.pingsReport(studyPings)); + // console.debug("Final pings with id and payload: ", utils.telemetry.pingsDebug(studyPings)); }); it("one shield-study telemetry ping with study_state=exit", async () => { diff --git a/testUtils/telemetry.js b/testUtils/telemetry.js index 974b6a7..7bb0eaa 100644 --- a/testUtils/telemetry.js +++ b/testUtils/telemetry.js @@ -7,6 +7,14 @@ const { const firefox = require("selenium-webdriver/firefox"); const Context = firefox.Context; +// node's util, for printing a deeply nested object to node console +const { inspect } = require("util"); + +// eslint-disable-next-line no-unused-vars +function full(myObject) { + return inspect(myObject, { showHidden: false, depth: null }); +} + module.exports.telemetry = { getActiveExperiments: async driver => { driver.setContext(Context.CHROME); @@ -52,6 +60,14 @@ module.exports.telemetry = { return pings.map(p => [p.payload.type, p.payload.data]); }, + pingsDebug: pings => { + return full( + pings.map(x => { + return { id: x.id, payload: x.payload }; + }), + ); + }, + pingsReport: pings => { if (pings.length === 0) { return { report: "No pings found" }; diff --git a/webExtensionApis/study/api.md b/webExtensionApis/study/api.md index 9ee24f1..c8d7d04 100644 --- a/webExtensionApis/study/api.md +++ b/webExtensionApis/study/api.md @@ -641,6 +641,10 @@ Act on it by }, "removeTestingFlag": { "type": "boolean" + }, + "internalTelemetryArchive": { + "optional": true, + "type": "boolean" } } }, @@ -729,7 +733,8 @@ Act on it by ], "telemetry": { "send": false, - "removeTestingFlag": false + "removeTestingFlag": false, + "internalTelemetryArchive": false }, "testing": { "variationName": "something", @@ -753,7 +758,8 @@ Act on it by ], "telemetry": { "send": false, - "removeTestingFlag": true + "removeTestingFlag": true, + "internalTelemetryArchive": true }, "testing": { "variationName": "something", @@ -1031,7 +1037,7 @@ About `this._internals`: * isSetup: bool `setup` * isFirstRun: bool `setup`, based on pref * studySetup: bool `setup` the config -* seenTelemetry: object of lists of seen telemetry by bucket +* seenTelemetry: array of seen telemetry. Fully populated only if studySetup.telemetry.internalTelemetryArchive is true * prefs: object of all created prefs and their names **Parameters** diff --git a/webExtensionApis/study/schema.json b/webExtensionApis/study/schema.json index a536cc0..94bcf04 100644 --- a/webExtensionApis/study/schema.json +++ b/webExtensionApis/study/schema.json @@ -328,6 +328,10 @@ }, "removeTestingFlag": { "type": "boolean" + }, + "internalTelemetryArchive": { + "optional": true, + "type": "boolean" } } }, @@ -416,7 +420,8 @@ ], "telemetry": { "send": false, - "removeTestingFlag": false + "removeTestingFlag": false, + "internalTelemetryArchive": false }, "testing": { "variationName": "something", @@ -440,7 +445,8 @@ ], "telemetry": { "send": false, - "removeTestingFlag": true + "removeTestingFlag": true, + "internalTelemetryArchive": true }, "testing": { "variationName": "something", @@ -895,7 +901,7 @@ "type": "function", "async": true, "description": - "Return `_internals` of the studyUtils object.\n\nUse this for debugging state.\n\nAbout `this._internals`:\n- variation: (chosen variation, `setup` )\n- isEnding: bool `endStudy`\n- isSetup: bool `setup`\n- isFirstRun: bool `setup`, based on pref\n- studySetup: bool `setup` the config\n- seenTelemetry: object of lists of seen telemetry by bucket\n- prefs: object of all created prefs and their names\n", + "Return `_internals` of the studyUtils object.\n\nUse this for debugging state.\n\nAbout `this._internals`:\n- variation: (chosen variation, `setup` )\n- isEnding: bool `endStudy`\n- isSetup: bool `setup`\n- isFirstRun: bool `setup`, based on pref\n- studySetup: bool `setup` the config\n- seenTelemetry: array of seen telemetry. Fully populated only if studySetup.telemetry.internalTelemetryArchive is true\n- prefs: object of all created prefs and their names\n", "parameters": [] }, { diff --git a/webExtensionApis/study/schema.yaml b/webExtensionApis/study/schema.yaml index 5691473..b9f4fd1 100644 --- a/webExtensionApis/study/schema.yaml +++ b/webExtensionApis/study/schema.yaml @@ -212,6 +212,9 @@ type: boolean removeTestingFlag: type: boolean + internalTelemetryArchive: + optional: true + type: boolean testing: type: object properties: @@ -250,7 +253,7 @@ expire: { "days": 10}, endings: {anEnding: {baseUrls: ['some.url']}}, weightedVariations: [{name: feature-active, weight: 1.5}], - telemetry: { send: false, removeTestingFlag: false}, + telemetry: { send: false, removeTestingFlag: false, internalTelemetryArchive: false}, testing: { variationName: "something", firstRunTimestamp: 1234567890, @@ -262,7 +265,7 @@ studyType: 'pioneer', endings: {anEnding: {baseUrls: ['some.url']}}, weightedVariations: [{name: feature-active, weight: 1.5}], - telemetry: { send: false, removeTestingFlag: true}, + telemetry: { send: false, removeTestingFlag: true, internalTelemetryArchive: true}, testing: { variationName: "something", firstRunTimestamp: 1234567890, @@ -698,7 +701,7 @@ - isSetup: bool `setup` - isFirstRun: bool `setup`, based on pref - studySetup: bool `setup` the config - - seenTelemetry: object of lists of seen telemetry by bucket + - seenTelemetry: array of seen telemetry. Fully populated only if studySetup.telemetry.internalTelemetryArchive is true - prefs: object of all created prefs and their names parameters: [] diff --git a/webExtensionApis/study/src/studyUtils.js b/webExtensionApis/study/src/studyUtils.js index 713050f..5bd2b6d 100644 --- a/webExtensionApis/study/src/studyUtils.js +++ b/webExtensionApis/study/src/studyUtils.js @@ -175,7 +175,7 @@ class StudyUtils { * - isSetup: bool `setup` * - isFirstRun: bool `setup`, based on pref * - studySetup: bool `setup` the config - * - seenTelemetry: object of lists of seen telemetry by bucket + * - seenTelemetry: array of seen telemetry. Fully populated only if studySetup.telemetry.internalTelemetryArchive is true * - prefs: object of all created prefs and their names * - endingRequested: string of ending name * - endingReturns: object with useful ending instructions @@ -222,11 +222,7 @@ class StudyUtils { isSetup: false, isEnding: false, isEnded: false, - seenTelemetry: { - "shield-study": [], - "shield-study-addon": [], - "shield-study-error": [], - }, + seenTelemetry: [], prefs: { firstRunTimestamp: `shield.${widgetId}.firstRunTimestamp`, }, @@ -733,9 +729,13 @@ class StudyUtils { } utilsLogger.debug(`telemetry: ${JSON.stringify(payload)}`); - // IF it's a shield-study or error ping, which are few in number - if (bucket === "shield-study" || bucket === "shield-study-error") { - this._internals.seenTelemetry[bucket].push(payload); + // Store a copy of the ping if it's a shield-study or error ping, which are few in number, or if we have activated the internal telemetry archive configuration + if ( + bucket === "shield-study" || + bucket === "shield-study-error" || + this.telemetryConfig.internalTelemetryArchive + ) { + this._internals.seenTelemetry.push(payload); } // during development, don't actually send diff --git a/webExtensionApis/study/src/telemetry.js b/webExtensionApis/study/src/telemetry.js index 6acceb3..5cef114 100644 --- a/webExtensionApis/study/src/telemetry.js +++ b/webExtensionApis/study/src/telemetry.js @@ -53,8 +53,6 @@ async function searchTelemetryArchive(TelemetryArchive, searchTelemetryQuery) { return Promise.all(pingData); } -// TODO pings report, from the utility add-on - module.exports = { searchTelemetryArchive, }; From fc5d83094a635a9d407e97086538491a8a96d810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Sat, 1 Sep 2018 02:43:47 +0300 Subject: [PATCH 09/28] Removed empty placeholders for tests that are already implemented --- test/functional/browser.study.api.js | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/test/functional/browser.study.api.js b/test/functional/browser.study.api.js index 60ba6ed..e6ed500 100644 --- a/test/functional/browser.study.api.js +++ b/test/functional/browser.study.api.js @@ -1186,24 +1186,6 @@ const publicApiTests = function(studyType) { }); }); - describe("endStudy", function() { - it("see the browser.study.endStudy() side effects above", () => - assert(true)); - }); - - describe("getStudyInfo", function() { - describe("correctness: see browser.study.setup() tests", function() { - // tests - it("during first run, isFirstRun is true", function() {}); - it("during second run, isFirstRun is false", function() {}); - it("if duration.days in studySetup(), have a delayInMinutes in studyInfo", async function() {}); - }); - }); - - describe("searchSentTelemetry (light testing)", function() { - it("searches get results, see the endStudy() and other test", function() {}); - }); - describe("uninstall by users?", function() {}); // TODO 5.1 From 1828973e8305b980d26239c385e0739567b119ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Sat, 1 Sep 2018 02:53:20 +0300 Subject: [PATCH 10/28] Implemented tests for browser.study.getDataPermissions() --- test/functional/browser.study.api.js | 30 +++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/test/functional/browser.study.api.js b/test/functional/browser.study.api.js index e6ed500..46f396b 100644 --- a/test/functional/browser.study.api.js +++ b/test/functional/browser.study.api.js @@ -248,6 +248,33 @@ const publicApiTests = function(studyType) { }); }); + describe("getDataPermissions", function() { + it("returns correct and current list of permissions", async () => { + const thisSetup = studySetupForTests(); + const dataPermissions = await addonExec(async (setup, cb) => { + // this is what runs in the webExtension scope. + const $dataPermissions = await browser.study.getDataPermissions(); + // call back with all the data we care about to Mocha / node + cb($dataPermissions); + }, thisSetup); + // console.debug(full(dataPermissions)); + + // tests + assert(dataPermissions.shield, "shield should be enabled"); + if (studyType === "pioneer") { + assert( + dataPermissions.pioneer, + "user should have opted in for pioneer", + ); + } else { + assert( + !dataPermissions.pioneer, + "user should not have opted in for pioneer", + ); + } + }); + }); + describe("test the setup requirement", function() { it("should not be able to send telemetry before setup", async () => { const caughtError = await utils.executeJs.executeAsyncScriptInExtensionPageForTests( @@ -1190,9 +1217,6 @@ const publicApiTests = function(studyType) { // TODO 5.1 describe.skip("possible 5.1 future tests.", function() { - describe("getDataPermissions", function() { - it("returns correct and current list of permissions"); - }); describe("surveyUrl", function() { describe("needs setup", function() { From ae2082129df5ea7220342cc86c211e7b8aee686e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Sat, 1 Sep 2018 03:19:06 +0300 Subject: [PATCH 11/28] Internal telemetry archive stores ping id and is exposed to the tests --- test/functional/browser.study.api.js | 111 ++++++++++++------ .../study/src/studyTypes/pioneer.js | 1 + .../study/src/studyTypes/shield.js | 2 +- webExtensionApis/study/src/studyUtils.js | 20 ++-- 4 files changed, 91 insertions(+), 43 deletions(-) diff --git a/test/functional/browser.study.api.js b/test/functional/browser.study.api.js index 46f396b..f133b8a 100644 --- a/test/functional/browser.study.api.js +++ b/test/functional/browser.study.api.js @@ -351,8 +351,8 @@ const publicApiTests = function(studyType) { // tests const now = Number(Date.now()); const seenTelemetryStates = internals.seenTelemetry - .filter(ping => ping.type === "shield-study") - .map(x => x.data.study_state); + .filter(ping => ping.payload.type === "shield-study") + .map(ping => ping.payload.data.study_state); assert(internals.isSetup, "should be isSetup"); assert(!internals.isEnded, "should not be ended"); assert(!internals.isEnding, "should not be ending"); @@ -402,8 +402,8 @@ const publicApiTests = function(studyType) { // tests const seenTelemetryStates = internals.seenTelemetry - .filter(ping => ping.type === "shield-study") - .map(x => x.data.study_state); + .filter(ping => ping.payload.type === "shield-study") + .map(ping => ping.payload.data.study_state); assert(internals.isSetup, "should be isSetup"); assert(!internals.isEnded, "should not be ended"); @@ -455,8 +455,8 @@ const publicApiTests = function(studyType) { // tests const seenTelemetryStates = internals.seenTelemetry - .filter(ping => ping.type === "shield-study") - .map(x => x.data.study_state); + .filter(ping => ping.payload.type === "shield-study") + .map(ping => ping.payload.data.study_state); assert(internals.isSetup, "should be isSetup"); assert(internals.isEnded, "should be ended"); @@ -658,11 +658,15 @@ const publicApiTests = function(studyType) { "pioneer-study", ], }); - callback(_studyPings); + const internals = await browser.studyDebug.getInternals(); + callback({ + sent: _studyPings, + seen: internals.seenTelemetry.reverse(), + }); // Using reverse() to mimic the default sorting of telemetry archive results }); // For debugging tests - // console.debug("Pings report: ", utils.telemetry.pingsReport(studyPings)); - // console.debug("Pings with id and payload: ", utils.telemetry.pingsDebug(studyPings)); + // console.debug("Pings report: ", utils.telemetry.pingsReport(studyPings.sent)); + // console.debug("Pings with id and payload: ", utils.telemetry.pingsDebug(studyPings.sent)); }); it("should have set the experiment to active in Telemetry", async () => { @@ -676,7 +680,7 @@ const publicApiTests = function(studyType) { }); it("shield-study-addon telemetry should be working (as seen by telemetry)", async () => { - const filteredPings = studyPings.filter( + const filteredPings = studyPings.sent.filter( ping => ping.type === "shield-study-addon", ); assert( @@ -688,11 +692,14 @@ const publicApiTests = function(studyType) { }); it("should have sent at least one shield telemetry ping", async () => { - assert(studyPings.length > 0, "at least one shield telemetry ping"); + assert( + studyPings.sent.length > 0, + "at least one shield telemetry ping", + ); }); it("should have sent one shield-study telemetry ping with study_state=enter", async () => { - const filteredPings = studyPings.filter( + const filteredPings = studyPings.sent.filter( ping => ping.type === "shield-study" && ping.payload.data.study_state === "enter", @@ -704,7 +711,7 @@ const publicApiTests = function(studyType) { }); it("should have sent one shield-study telemetry ping with study_state=installed", async () => { - const filteredPings = studyPings.filter( + const filteredPings = studyPings.sent.filter( ping => ping.type === "shield-study" && ping.payload.data.study_state === "installed", @@ -716,7 +723,7 @@ const publicApiTests = function(studyType) { }); it("should have sent one shield-study-addon telemetry ping with payload.data.attributes.foo=bar", async () => { - const filteredPings = studyPings.filter( + const filteredPings = studyPings.sent.filter( ping => ping.type === "shield-study-addon" && ping.payload.data.attributes.foo === "bar", @@ -784,16 +791,25 @@ const publicApiTests = function(studyType) { let studyPings; before(async () => { - studyPings = await utils.telemetry.searchSentTelemetry(driver, { - type: ["shield-study", "shield-study-addon"], - }); + studyPings = {}; + studyPings.sent = await utils.telemetry.searchSentTelemetry( + driver, + { + type: [ + "shield-study", + "shield-study-addon", + "shield-study-error", + "pioneer-study", + ], + }, + ); // For debugging tests // console.debug("Final pings report: ", utils.telemetry.pingsReport(studyPings)); // console.debug("Final pings with id and payload: ", utils.telemetry.pingsDebug(studyPings)); }); it("one shield-study telemetry ping with study_state=exit", async () => { - const filteredPings = studyPings.filter( + const filteredPings = studyPings.sent.filter( ping => ping.type === "shield-study" && ping.payload.data.study_state === "exit", @@ -805,7 +821,7 @@ const publicApiTests = function(studyType) { }); it("one shield-study telemetry ping with study_state_fullname=customEnding", async () => { - const filteredPings = studyPings.filter( + const filteredPings = studyPings.sent.filter( ping => ping.type === "shield-study" && ping.payload.data.study_state === "ended-positive" && @@ -893,16 +909,25 @@ const publicApiTests = function(studyType) { let studyPings; before(async () => { - studyPings = await utils.telemetry.searchSentTelemetry(driver, { - type: ["shield-study", "shield-study-addon"], - }); + studyPings = {}; + studyPings.sent = await utils.telemetry.searchSentTelemetry( + driver, + { + type: [ + "shield-study", + "shield-study-addon", + "shield-study-error", + "pioneer-study", + ], + }, + ); // For debugging tests // console.debug("Final pings report: ", utils.telemetry.pingsReport(studyPings)); // console.debug("Final pings with id and payload: ", utils.telemetry.pingsDebug(studyPings)); }); it("one shield-study telemetry ping with study_state=exit", async () => { - const filteredPings = studyPings.filter( + const filteredPings = studyPings.sent.filter( ping => ping.type === "shield-study" && ping.payload.data.study_state === "exit", @@ -914,7 +939,7 @@ const publicApiTests = function(studyType) { }); it("one shield-study telemetry ping with study_state=ineligible", async () => { - const filteredPings = studyPings.filter( + const filteredPings = studyPings.sent.filter( ping => ping.type === "shield-study" && ping.payload.data.study_state === "ineligible", @@ -1004,16 +1029,25 @@ const publicApiTests = function(studyType) { let studyPings; before(async () => { - studyPings = await utils.telemetry.searchSentTelemetry(driver, { - type: ["shield-study", "shield-study-addon"], - }); + studyPings = {}; + studyPings.sent = await utils.telemetry.searchSentTelemetry( + driver, + { + type: [ + "shield-study", + "shield-study-addon", + "shield-study-error", + "pioneer-study", + ], + }, + ); // For debugging tests // console.debug("Final pings report: ", utils.telemetry.pingsReport(studyPings)); // console.debug("Final pings with id and payload: ", utils.telemetry.pingsDebug(studyPings)); }); it("one shield-study telemetry ping with study_state=exit", async () => { - const filteredPings = studyPings.filter( + const filteredPings = studyPings.sent.filter( ping => ping.type === "shield-study" && ping.payload.data.study_state === "exit", @@ -1025,7 +1059,7 @@ const publicApiTests = function(studyType) { }); it("one shield-study telemetry ping with study_state=expired", async () => { - const filteredPings = studyPings.filter( + const filteredPings = studyPings.sent.filter( ping => ping.type === "shield-study" && ping.payload.data.study_state === "expired", @@ -1140,16 +1174,25 @@ const publicApiTests = function(studyType) { let studyPings; before(async () => { - studyPings = await utils.telemetry.searchSentTelemetry(driver, { - type: ["shield-study", "shield-study-addon"], - }); + studyPings = {}; + studyPings.sent = await utils.telemetry.searchSentTelemetry( + driver, + { + type: [ + "shield-study", + "shield-study-addon", + "shield-study-error", + "pioneer-study", + ], + }, + ); // For debugging tests // console.debug("Final pings report: ", utils.telemetry.pingsReport(studyPings)); // console.debug("Final pings with id and payload: ", utils.telemetry.pingsDebug(studyPings)); }); it("one shield-study telemetry ping with study_state=exit", async () => { - const filteredPings = studyPings.filter( + const filteredPings = studyPings.sent.filter( ping => ping.type === "shield-study" && ping.payload.data.study_state === "exit", @@ -1161,7 +1204,7 @@ const publicApiTests = function(studyType) { }); it("one shield-study telemetry ping with study_state=expired", async () => { - const filteredPings = studyPings.filter( + const filteredPings = studyPings.sent.filter( ping => ping.type === "shield-study" && ping.payload.data.study_state === "expired", diff --git a/webExtensionApis/study/src/studyTypes/pioneer.js b/webExtensionApis/study/src/studyTypes/pioneer.js index 08bbfeb..02d979a 100644 --- a/webExtensionApis/study/src/studyTypes/pioneer.js +++ b/webExtensionApis/study/src/studyTypes/pioneer.js @@ -331,6 +331,7 @@ class PioneerStudyType { JSON.stringify(payload), ); } + return pingId; } } diff --git a/webExtensionApis/study/src/studyTypes/shield.js b/webExtensionApis/study/src/studyTypes/shield.js index 6c44874..24f37b0 100644 --- a/webExtensionApis/study/src/studyTypes/shield.js +++ b/webExtensionApis/study/src/studyTypes/shield.js @@ -33,7 +33,7 @@ class ShieldStudyType { /** * @param bucket * @param payload - * @returns {*|Promise} + * @returns {Promise} The ID of the ping that was submitted */ async sendTelemetry(bucket, payload) { const telOptions = { addClientId: true, addEnvironment: true }; diff --git a/webExtensionApis/study/src/studyUtils.js b/webExtensionApis/study/src/studyUtils.js index 5bd2b6d..2ec348f 100644 --- a/webExtensionApis/study/src/studyUtils.js +++ b/webExtensionApis/study/src/studyUtils.js @@ -729,22 +729,26 @@ class StudyUtils { } utilsLogger.debug(`telemetry: ${JSON.stringify(payload)}`); + let pingId; + + // during development, don't actually send + if (!this.telemetryConfig.send) { + utilsLogger.debug("NOT sending. `telemetryConfig.send` is false"); + pingId = false; + } else { + pingId = await this.studyType.sendTelemetry(bucket, payload); + } + // Store a copy of the ping if it's a shield-study or error ping, which are few in number, or if we have activated the internal telemetry archive configuration if ( bucket === "shield-study" || bucket === "shield-study-error" || this.telemetryConfig.internalTelemetryArchive ) { - this._internals.seenTelemetry.push(payload); - } - - // during development, don't actually send - if (!this.telemetryConfig.send) { - utilsLogger.debug("NOT sending. `telemetryConfig.send` is false"); - return false; + this._internals.seenTelemetry.push({ id: pingId, payload }); } - return this.studyType.sendTelemetry(bucket, payload); + return pingId; } /** From dd7190ea399a973a1e30e73efa93c0a01ab64d1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Sat, 1 Sep 2018 04:31:17 +0300 Subject: [PATCH 12/28] Telemetry-related tests simplified and restored --- test/functional/browser.study.api.js | 498 +++++++++++++++------------ 1 file changed, 286 insertions(+), 212 deletions(-) diff --git a/test/functional/browser.study.api.js b/test/functional/browser.study.api.js index f133b8a..047518b 100644 --- a/test/functional/browser.study.api.js +++ b/test/functional/browser.study.api.js @@ -665,8 +665,8 @@ const publicApiTests = function(studyType) { }); // Using reverse() to mimic the default sorting of telemetry archive results }); // For debugging tests - // console.debug("Pings report: ", utils.telemetry.pingsReport(studyPings.sent)); - // console.debug("Pings with id and payload: ", utils.telemetry.pingsDebug(studyPings.sent)); + // console.debug("Pings report: ", utils.telemetry.pingsReport(studyPings.seen)); + // console.debug("Pings with id and payload: ", utils.telemetry.pingsDebug(studyPings.seen)); }); it("should have set the experiment to active in Telemetry", async () => { @@ -679,18 +679,6 @@ const publicApiTests = function(studyType) { ); }); - it("shield-study-addon telemetry should be working (as seen by telemetry)", async () => { - const filteredPings = studyPings.sent.filter( - ping => ping.type === "shield-study-addon", - ); - assert( - filteredPings.length > 0, - "at least one shield-study-addon telemetry ping", - ); - // console.debug("shield-study-addon pings", full(filteredPings)); - assert(filteredPings[0].payload.data.attributes.foo === "bar"); - }); - it("should have sent at least one shield telemetry ping", async () => { assert( studyPings.sent.length > 0, @@ -698,52 +686,55 @@ const publicApiTests = function(studyType) { ); }); - it("should have sent one shield-study telemetry ping with study_state=enter", async () => { - const filteredPings = studyPings.sent.filter( - ping => - ping.type === "shield-study" && - ping.payload.data.study_state === "enter", - ); - assert( - filteredPings.length > 0, - "at least one shield-study telemetry ping with study_state=enter", - ); - }); - - it("should have sent one shield-study telemetry ping with study_state=installed", async () => { - const filteredPings = studyPings.sent.filter( - ping => - ping.type === "shield-study" && - ping.payload.data.study_state === "installed", - ); - assert( - filteredPings.length > 0, - "at least one shield-study telemetry ping with study_state=installed", - ); - }); - - it("should have sent one shield-study-addon telemetry ping with payload.data.attributes.foo=bar", async () => { - const filteredPings = studyPings.sent.filter( - ping => - ping.type === "shield-study-addon" && - ping.payload.data.attributes.foo === "bar", + it("should have sent expected telemetry", async () => { + const observed = utils.telemetry.summarizePings( + studyType === "shield" ? studyPings.sent : studyPings.seen, ); - assert( - filteredPings.length > 0, - "at least one shield-study-addon telemetry ping with payload.data.attributes.foo=bar", + const expected = [ + [ + "shield-study-addon", + { + attributes: { + foo: "bar", + }, + }, + ], + [ + "shield-study", + { + study_state: "installed", + }, + ], + [ + "shield-study", + { + study_state: "enter", + }, + ], + ]; + assert.deepStrictEqual( + expected, + observed, + "telemetry pings as as expected", ); }); }); describe("browser.study.endStudy() side effects for first time called", function() { - let endingResult; + let endingResult, endingInternals; before(async () => { - endingResult = await addonExec(async callback => { + const _ = await addonExec(async callback => { browser.study.onEndStudy.addListener(async _endingResult => { - callback(_endingResult); + const internals = await browser.studyDebug.getInternals(); + callback({ + endingResult: _endingResult, + endingInternals: internals, + }); }); await browser.study.endStudy("customEnding"); }); + endingResult = _.endingResult; + endingInternals = _.endingInternals; // let telemetry and disk/files sync up await delay(1000); }); @@ -792,6 +783,7 @@ const publicApiTests = function(studyType) { before(async () => { studyPings = {}; + studyPings.seen = endingInternals.seenTelemetry.reverse(); studyPings.sent = await utils.telemetry.searchSentTelemetry( driver, { @@ -804,32 +796,60 @@ const publicApiTests = function(studyType) { }, ); // For debugging tests - // console.debug("Final pings report: ", utils.telemetry.pingsReport(studyPings)); - // console.debug("Final pings with id and payload: ", utils.telemetry.pingsDebug(studyPings)); + // console.debug("Final pings report: ", utils.telemetry.pingsReport(studyPings.seen)); + // console.debug("Final pings with id and payload: ", utils.telemetry.pingsDebug(studyPings.seen)); }); - it("one shield-study telemetry ping with study_state=exit", async () => { - const filteredPings = studyPings.sent.filter( - ping => - ping.type === "shield-study" && - ping.payload.data.study_state === "exit", - ); + it("should have sent at least one shield telemetry ping", async () => { assert( - filteredPings.length > 0, - "at least one shield-study telemetry ping with study_state=exit", + studyPings.sent.length > 0, + "at least one shield telemetry ping", ); }); - it("one shield-study telemetry ping with study_state_fullname=customEnding", async () => { - const filteredPings = studyPings.sent.filter( - ping => - ping.type === "shield-study" && - ping.payload.data.study_state === "ended-positive" && - ping.payload.data.study_state_fullname === "customEnding", + it("should have sent expected telemetry", async () => { + const observed = utils.telemetry.summarizePings( + studyType === "shield" ? studyPings.sent : studyPings.seen, ); - assert( - filteredPings.length > 0, - "at least one shield-study telemetry ping with study_state_fullname=customEnding", + const expected = [ + [ + "shield-study", + { + study_state: "exit", + }, + ], + [ + "shield-study", + { + study_state: "ended-positive", + study_state_fullname: "customEnding", + }, + ], + [ + "shield-study-addon", + { + attributes: { + foo: "bar", + }, + }, + ], + [ + "shield-study", + { + study_state: "installed", + }, + ], + [ + "shield-study", + { + study_state: "enter", + }, + ], + ]; + assert.deepStrictEqual( + expected, + observed, + "telemetry pings as as expected", ); }); }); @@ -837,7 +857,7 @@ const publicApiTests = function(studyType) { }); describe("setup of an ineligible study should result in endStudy('ineligible') without even emitting onReady", function() { - let endingResult; + let endingResult, endingInternals; const overrides = { activeExperimentName: "test:browser.study.api", telemetry: { @@ -856,30 +876,30 @@ const publicApiTests = function(studyType) { before(async function reinstallSetupAndAwaitEndStudy() { await installAddon(); - endingResult = await addonExec( - async (_studySetupForTests, callback) => { - // Ensure we have a configured study and are supposed to run our feature - browser.study.onEndStudy.addListener(async _endingResult => { - console.log( - "In resetSetupAndAwaitEndStudy - onEndStudy listener", - _endingResult, - ); - callback(_endingResult); - }); - browser.study.onReady.addListener(async _studyInfo => { - console.log( - "In resetSetupAndAwaitEndStudy - onReady listener", - _studyInfo, - ); - throw new Error( - "onReady should not have been emitted", - _studyInfo, - ); + const _ = await addonExec(async (_studySetupForTests, callback) => { + // Ensure we have a configured study and are supposed to run our feature + browser.study.onEndStudy.addListener(async _endingResult => { + console.log( + "In resetSetupAndAwaitEndStudy - onEndStudy listener", + _endingResult, + ); + const internals = await browser.studyDebug.getInternals(); + callback({ + endingResult: _endingResult, + endingInternals: internals, }); - await browser.study.setup(_studySetupForTests); - }, - studySetupForTests(overrides), - ); + }); + browser.study.onReady.addListener(async _studyInfo => { + console.log( + "In resetSetupAndAwaitEndStudy - onReady listener", + _studyInfo, + ); + throw new Error("onReady should not have been emitted", _studyInfo); + }); + await browser.study.setup(_studySetupForTests); + }, studySetupForTests(overrides)); + endingResult = _.endingResult; + endingInternals = _.endingInternals; }); describe("browser.study.endStudy() side effects", function() { @@ -910,6 +930,7 @@ const publicApiTests = function(studyType) { before(async () => { studyPings = {}; + studyPings.seen = endingInternals.seenTelemetry.reverse(); studyPings.sent = await utils.telemetry.searchSentTelemetry( driver, { @@ -922,31 +943,46 @@ const publicApiTests = function(studyType) { }, ); // For debugging tests - // console.debug("Final pings report: ", utils.telemetry.pingsReport(studyPings)); - // console.debug("Final pings with id and payload: ", utils.telemetry.pingsDebug(studyPings)); + // console.debug("Final pings report: ", utils.telemetry.pingsReport(studyPings.seen)); + // console.debug("Final pings with id and payload: ", utils.telemetry.pingsDebug(studyPings.seen)); }); - it("one shield-study telemetry ping with study_state=exit", async () => { - const filteredPings = studyPings.sent.filter( - ping => - ping.type === "shield-study" && - ping.payload.data.study_state === "exit", - ); + it("should have sent at least one shield telemetry ping", async () => { assert( - filteredPings.length > 0, - "at least one shield-study telemetry ping with study_state=exit", + studyPings.sent.length > 0, + "at least one shield telemetry ping", ); }); - it("one shield-study telemetry ping with study_state=ineligible", async () => { - const filteredPings = studyPings.sent.filter( - ping => - ping.type === "shield-study" && - ping.payload.data.study_state === "ineligible", + it("should have sent expected telemetry", async () => { + const observed = utils.telemetry.summarizePings( + studyType === "shield" ? studyPings.sent : studyPings.seen, ); - assert( - filteredPings.length > 0, - "at least one shield-study telemetry ping with study_state=ineligible", + const expected = [ + [ + "shield-study", + { + study_state: "exit", + }, + ], + [ + "shield-study", + { + study_state: "ineligible", + study_state_fullname: "ineligible", + }, + ], + [ + "shield-study", + { + study_state: "enter", + }, + ], + ]; + assert.deepStrictEqual( + expected, + observed, + "telemetry pings as as expected", ); }); }); @@ -954,7 +990,7 @@ const publicApiTests = function(studyType) { }); describe("setup of an already expired study should result in endStudy('expired') without even emitting onReady", function() { - let endingResult; + let endingResult, endingInternals; const overrides = { activeExperimentName: "test:browser.study.api", telemetry: { @@ -976,30 +1012,30 @@ const publicApiTests = function(studyType) { before(async function reinstallSetupAndAwaitEndStudy() { await installAddon(); - endingResult = await addonExec( - async (_studySetupForTests, callback) => { - // Ensure we have a configured study and are supposed to run our feature - browser.study.onEndStudy.addListener(async _endingResult => { - console.log( - "In resetSetupAndAwaitEndStudy - onEndStudy listener", - _endingResult, - ); - callback(_endingResult); - }); - browser.study.onReady.addListener(async _studyInfo => { - console.log( - "In resetSetupAndAwaitEndStudy - onReady listener", - _studyInfo, - ); - throw new Error( - "onReady should not have been emitted", - _studyInfo, - ); + const _ = await addonExec(async (_studySetupForTests, callback) => { + // Ensure we have a configured study and are supposed to run our feature + browser.study.onEndStudy.addListener(async _endingResult => { + console.log( + "In resetSetupAndAwaitEndStudy - onEndStudy listener", + _endingResult, + ); + const internals = await browser.studyDebug.getInternals(); + callback({ + endingResult: _endingResult, + endingInternals: internals, }); - await browser.study.setup(_studySetupForTests); - }, - studySetupForTests(overrides), - ); + }); + browser.study.onReady.addListener(async _studyInfo => { + console.log( + "In resetSetupAndAwaitEndStudy - onReady listener", + _studyInfo, + ); + throw new Error("onReady should not have been emitted", _studyInfo); + }); + await browser.study.setup(_studySetupForTests); + }, studySetupForTests(overrides)); + endingResult = _.endingResult; + endingInternals = _.endingInternals; }); describe("browser.study.endStudy() side effects", function() { @@ -1030,6 +1066,7 @@ const publicApiTests = function(studyType) { before(async () => { studyPings = {}; + studyPings.seen = endingInternals.seenTelemetry.reverse(); studyPings.sent = await utils.telemetry.searchSentTelemetry( driver, { @@ -1042,31 +1079,46 @@ const publicApiTests = function(studyType) { }, ); // For debugging tests - // console.debug("Final pings report: ", utils.telemetry.pingsReport(studyPings)); - // console.debug("Final pings with id and payload: ", utils.telemetry.pingsDebug(studyPings)); + // console.debug("Final pings report: ", utils.telemetry.pingsReport(studyPings.seen)); + // console.debug("Final pings with id and payload: ", utils.telemetry.pingsDebug(studyPings.seen)); }); - it("one shield-study telemetry ping with study_state=exit", async () => { - const filteredPings = studyPings.sent.filter( - ping => - ping.type === "shield-study" && - ping.payload.data.study_state === "exit", - ); + it("should have sent at least one shield telemetry ping", async () => { assert( - filteredPings.length > 0, - "at least one shield-study telemetry ping with study_state=exit", + studyPings.sent.length > 0, + "at least one shield telemetry ping", ); }); - it("one shield-study telemetry ping with study_state=expired", async () => { - const filteredPings = studyPings.sent.filter( - ping => - ping.type === "shield-study" && - ping.payload.data.study_state === "expired", + it("should have sent expected telemetry", async () => { + const observed = utils.telemetry.summarizePings( + studyType === "shield" ? studyPings.sent : studyPings.seen, ); - assert( - filteredPings.length > 0, - "at least one shield-study telemetry ping with study_state=expired", + const expected = [ + [ + "shield-study", + { + study_state: "exit", + }, + ], + [ + "shield-study", + { + study_state: "expired", + study_state_fullname: "expired", + }, + ], + [ + "shield-study", + { + study_state: "enter", + }, + ], + ]; + assert.deepStrictEqual( + expected, + observed, + "telemetry pings as as expected", ); }); }); @@ -1074,7 +1126,7 @@ const publicApiTests = function(studyType) { }); describe("setup of a study that expires within a few seconds should result in endStudy('expired') after a few seconds", function() { - let endingResult; + let endingResult, endingInternals; const now = Number(Date.now()); const msInOneDay = 60 * 60 * 24 * 1000; const overrides = { @@ -1101,50 +1153,54 @@ const publicApiTests = function(studyType) { before(async function reinstallSetupAndConfigureAlarm() { await installAddon(); - endingResult = await addonExec( - async (_studySetupForTests, callback) => { + const _ = await addonExec(async (_studySetupForTests, callback) => { + console.log( + "In resetSetupAndConfigureAlarm - addonExec", + _studySetupForTests, + ); + // Ensure we have a configured study and are supposed to run our feature + browser.study.onEndStudy.addListener(async _endingResult => { console.log( - "In resetSetupAndConfigureAlarm - addonExec", - _studySetupForTests, + "In resetSetupAndConfigureAlarm - onEndStudy listener", + _endingResult, ); - // Ensure we have a configured study and are supposed to run our feature - browser.study.onEndStudy.addListener(async _endingResult => { - console.log( - "In resetSetupAndConfigureAlarm - onEndStudy listener", - _endingResult, - ); - callback(_endingResult); - }); - browser.study.onReady.addListener(async _studyInfo => { - console.log( - "In resetSetupAndConfigureAlarm - onReady listener", - _studyInfo, - ); - const { delayInMinutes } = _studyInfo; - if (delayInMinutes !== undefined) { - const alarmName = `${browser.runtime.id}:studyExpiration`; - const alarmListener = async alarm => { - console.log( - "In resetSetupAndConfigureAlarm - alarmListener", - alarm, - ); - if (alarm.name === alarmName) { - browser.alarms.onAlarm.removeListener(alarmListener); - await browser.study.endStudy("expired"); - } - }; - console.log("Setting up alarm listener", alarmListener); - browser.alarms.onAlarm.addListener(alarmListener); - console.log("Creating alarm", alarmName, delayInMinutes); - browser.alarms.create(alarmName, { - delayInMinutes, - }); - } + const internals = await browser.studyDebug.getInternals(); + callback({ + endingResult: _endingResult, + endingInternals: internals, }); - await browser.study.setup(_studySetupForTests); - }, - studySetupForTests(overrides), - ); + }); + browser.study.onReady.addListener(async _studyInfo => { + console.log( + "In resetSetupAndConfigureAlarm - onReady listener", + _studyInfo, + ); + await browser.study.sendTelemetry({ foo: "bar" }); + const { delayInMinutes } = _studyInfo; + if (delayInMinutes !== undefined) { + const alarmName = `${browser.runtime.id}:studyExpiration`; + const alarmListener = async alarm => { + console.log( + "In resetSetupAndConfigureAlarm - alarmListener", + alarm, + ); + if (alarm.name === alarmName) { + browser.alarms.onAlarm.removeListener(alarmListener); + await browser.study.endStudy("expired"); + } + }; + console.log("Setting up alarm listener", alarmListener); + browser.alarms.onAlarm.addListener(alarmListener); + console.log("Creating alarm", alarmName, delayInMinutes); + browser.alarms.create(alarmName, { + delayInMinutes, + }); + } + }); + await browser.study.setup(_studySetupForTests); + }, studySetupForTests(overrides)); + endingResult = _.endingResult; + endingInternals = _.endingInternals; }); describe("browser.study.endStudy() side effects", function() { @@ -1175,6 +1231,7 @@ const publicApiTests = function(studyType) { before(async () => { studyPings = {}; + studyPings.seen = endingInternals.seenTelemetry.reverse(); studyPings.sent = await utils.telemetry.searchSentTelemetry( driver, { @@ -1187,31 +1244,48 @@ const publicApiTests = function(studyType) { }, ); // For debugging tests - // console.debug("Final pings report: ", utils.telemetry.pingsReport(studyPings)); - // console.debug("Final pings with id and payload: ", utils.telemetry.pingsDebug(studyPings)); + // console.debug("Final pings report: ", utils.telemetry.pingsReport(studyPings.seen)); + // console.debug("Final pings with id and payload: ", utils.telemetry.pingsDebug(studyPings.seen)); }); - it("one shield-study telemetry ping with study_state=exit", async () => { - const filteredPings = studyPings.sent.filter( - ping => - ping.type === "shield-study" && - ping.payload.data.study_state === "exit", - ); + it("should have sent at least one shield telemetry ping", async () => { assert( - filteredPings.length > 0, - "at least one shield-study telemetry ping with study_state=exit", + studyPings.sent.length > 0, + "at least one shield telemetry ping", ); }); - it("one shield-study telemetry ping with study_state=expired", async () => { - const filteredPings = studyPings.sent.filter( - ping => - ping.type === "shield-study" && - ping.payload.data.study_state === "expired", + it("should have sent expected telemetry", async () => { + const observed = utils.telemetry.summarizePings( + studyType === "shield" ? studyPings.sent : studyPings.seen, ); - assert( - filteredPings.length > 0, - "at least one shield-study telemetry ping with study_state=expired", + const expected = [ + [ + "shield-study", + { + study_state: "exit", + }, + ], + [ + "shield-study", + { + study_state: "expired", + study_state_fullname: "expired", + }, + ], + [ + "shield-study-addon", + { + attributes: { + foo: "bar", + }, + }, + ], + ]; + assert.deepStrictEqual( + expected, + observed, + "telemetry pings as as expected", ); }); }); From ee4434087caa903a8ca58cdd08a9096eb4c57061 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Sat, 1 Sep 2018 04:32:03 +0300 Subject: [PATCH 13/28] Increased wait time for expiration test due to intermittent failures --- test/functional/browser.study.api.js | 4 ++-- webExtensionApis/study/src/studyUtils.js | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/test/functional/browser.study.api.js b/test/functional/browser.study.api.js index 047518b..51322fa 100644 --- a/test/functional/browser.study.api.js +++ b/test/functional/browser.study.api.js @@ -103,7 +103,7 @@ const publicApiTests = function(studyType) { } // This gives Firefox time to start, and us a bit longer during some of the tests. - this.timeout(15000 + KEEPOPEN * 1000 * 2); + this.timeout(30000 + KEEPOPEN * 1000 * 2); let driver; let addonId; @@ -1147,7 +1147,7 @@ const publicApiTests = function(studyType) { }, }, testing: { - firstRunTimestamp: now - msInOneDay + 2000, + firstRunTimestamp: now - msInOneDay + 15000, }, }; diff --git a/webExtensionApis/study/src/studyUtils.js b/webExtensionApis/study/src/studyUtils.js index 2ec348f..bc84f96 100644 --- a/webExtensionApis/study/src/studyUtils.js +++ b/webExtensionApis/study/src/studyUtils.js @@ -429,6 +429,14 @@ class StudyUtils { shieldId: this.getShieldId(), delayInMinutes: this.getDelayInMinutes(), }; + const now = new Date(); + const diff = Number(now) - studyInfo.firstRunTimestamp; + utilsLogger.debug( + "Study info date information: now, firstRunTimestamp and diff in minutes", + now, + new Date(studyInfo.firstRunTimestamp), + diff / 1000 / 60, + ); guard.it("studyInfoObject", studyInfo, "(in studyInfo)"); return studyInfo; } From 79910c66809d7ffd5d5b2dc3cc5f7edae1d99394 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Sat, 1 Sep 2018 04:34:13 +0300 Subject: [PATCH 14/28] Minor cleanup --- test/functional/browser.study.api.js | 9 ++++----- test/functional/test-addon.js | 4 ++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/test/functional/browser.study.api.js b/test/functional/browser.study.api.js index 51322fa..63c0b42 100644 --- a/test/functional/browser.study.api.js +++ b/test/functional/browser.study.api.js @@ -1330,14 +1330,13 @@ const publicApiTests = function(studyType) { }); }); - describe("uninstall by users?", function() {}); - - // TODO 5.1 - describe.skip("possible 5.1 future tests.", function() { + // TODO 5.2+ + describe.skip("possible 5.2+ future tests.", function() { + describe("uninstall by users?", function() {}); describe("surveyUrl", function() { describe("needs setup", function() { - it("throws StudyNotsSetupError if not setup"); + it("throws StudyNotSetupError if not setup"); }); describe("correctly constructs urls queryArgs from profile info", function() { it("an example url is correct"); diff --git a/test/functional/test-addon.js b/test/functional/test-addon.js index 23e7ed4..020cd1c 100644 --- a/test/functional/test-addon.js +++ b/test/functional/test-addon.js @@ -160,10 +160,10 @@ const testAddonTests = function(studyType) { }); }; -describe("Tests verifying that the test add-on works as expected (studyType=shield)", function() { +describe("Tests verifying that the test add-on works as expected (studyType: shield)", function() { testAddonTests.bind(this)("shield"); }); -describe("Tests verifying that the test add-on works as expected (studyType=pioneer)", function() { +describe("Tests verifying that the test add-on works as expected (studyType: pioneer)", function() { testAddonTests.bind(this)("pioneer"); }); From f9046d56de8e3e7b0f798e53f2739b910b2a3828 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Fri, 7 Sep 2018 17:57:19 +0300 Subject: [PATCH 15/28] Moved bin/import-pioneer-opt-in.sh to pretest where it belongs --- package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package.json b/package.json index 81449b0..0b57004 100644 --- a/package.json +++ b/package.json @@ -86,10 +86,9 @@ "lint:fixpack": "fixpack # cleans up package.json", "postbuild": "if [ -z ${SKIPLINT} ]; then npm run format; fi", "postformat": "run-p lint:fixpack eslint-fix", - "postinstall": "bin/import-pioneer-opt-in.sh", "prebuild": "if [ -z ${SKIPLINT} ]; then npm run lint; fi", "prepare": "export SKIPLINT=1 && fixpack && npm run build", - "pretest": "npm run build && npm run test-addon:bundle-utils && npm run test-addon:build", + "pretest": "npm run build && npm run test-addon:bundle-utils && npm run test-addon:build && bin/import-pioneer-opt-in.sh", "pretest-addon": "npm run pretest", "small-study": "cd examples/small-study && npm run rebuild && npm start", "test": "npm run test:func", From 434307540d24a76cc8649efa05d40388fb278403 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Fri, 7 Sep 2018 19:38:37 +0300 Subject: [PATCH 16/28] Restored tests --- test/functional/browser.study.api.js | 9 ++++++++- webExtensionApis/study/src/studyUtils.js | 4 +++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/test/functional/browser.study.api.js b/test/functional/browser.study.api.js index 63c0b42..f07a199 100644 --- a/test/functional/browser.study.api.js +++ b/test/functional/browser.study.api.js @@ -106,6 +106,7 @@ const publicApiTests = function(studyType) { this.timeout(30000 + KEEPOPEN * 1000 * 2); let driver; + let beginTime; let addonId; // run in the extension page @@ -115,7 +116,7 @@ const publicApiTests = function(studyType) { driver = await utils.setupWebdriver.promiseSetupDriver( utils.FIREFOX_PREFERENCES, ); - installAddon(); + await installAddon(); await utils.ui.openBrowserConsole(driver); // make a shorter alias @@ -126,8 +127,10 @@ const publicApiTests = function(studyType) { } async function installAddon() { + beginTime = Date.now(); if (addonId) { await utils.setupWebdriver.uninstallAddon(driver, addonId); + addonId = null; } if (studyType === "pioneer") { await utils.setupWebdriver.installPioneerOptInAddon(driver); @@ -793,6 +796,7 @@ const publicApiTests = function(studyType) { "shield-study-error", "pioneer-study", ], + timestamp: beginTime, }, ); // For debugging tests @@ -940,6 +944,7 @@ const publicApiTests = function(studyType) { "shield-study-error", "pioneer-study", ], + timestamp: beginTime, }, ); // For debugging tests @@ -1076,6 +1081,7 @@ const publicApiTests = function(studyType) { "shield-study-error", "pioneer-study", ], + timestamp: beginTime, }, ); // For debugging tests @@ -1241,6 +1247,7 @@ const publicApiTests = function(studyType) { "shield-study-error", "pioneer-study", ], + timestamp: beginTime, }, ); // For debugging tests diff --git a/webExtensionApis/study/src/studyUtils.js b/webExtensionApis/study/src/studyUtils.js index bc84f96..e9c7358 100644 --- a/webExtensionApis/study/src/studyUtils.js +++ b/webExtensionApis/study/src/studyUtils.js @@ -432,10 +432,12 @@ class StudyUtils { const now = new Date(); const diff = Number(now) - studyInfo.firstRunTimestamp; utilsLogger.debug( - "Study info date information: now, firstRunTimestamp and diff in minutes", + "Study info date information: now, new Date(firstRunTimestamp), firstRunTimestamp, diff (in minutes), delayInMinutes", now, new Date(studyInfo.firstRunTimestamp), + studyInfo.firstRunTimestamp, diff / 1000 / 60, + studyInfo.delayInMinutes, ); guard.it("studyInfoObject", studyInfo, "(in studyInfo)"); return studyInfo; From 798790ed8934ae403a41c37de36606a3c9779510 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Fri, 7 Sep 2018 20:09:50 +0300 Subject: [PATCH 17/28] Fixed a race condition with the expiration-related tests --- test/functional/browser.study.api.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/functional/browser.study.api.js b/test/functional/browser.study.api.js index f07a199..74cc0e9 100644 --- a/test/functional/browser.study.api.js +++ b/test/functional/browser.study.api.js @@ -1133,8 +1133,6 @@ const publicApiTests = function(studyType) { describe("setup of a study that expires within a few seconds should result in endStudy('expired') after a few seconds", function() { let endingResult, endingInternals; - const now = Number(Date.now()); - const msInOneDay = 60 * 60 * 24 * 1000; const overrides = { activeExperimentName: "test:browser.study.api", telemetry: { @@ -1153,12 +1151,17 @@ const publicApiTests = function(studyType) { }, }, testing: { - firstRunTimestamp: now - msInOneDay + 15000, + firstRunTimestamp: null, // needs to be set in the before-hook below in order to be executed just before the setup of the study }, }; before(async function reinstallSetupAndConfigureAlarm() { await installAddon(); + // Set the study to expire after a few seconds + const now = Number(Date.now()); + const msInOneDay = 60 * 60 * 24 * 1000; + overrides.testing.firstRunTimestamp = now - msInOneDay + 5000; + // console.log("Expiration debug: now, firstRunTimestamp, new Date(), new Date(now), new Date(firstRunTimestamp)", now, overrides.testing.firstRunTimestamp, new Date(), new Date(now), new Date(overrides.testing.firstRunTimestamp)); const _ = await addonExec(async (_studySetupForTests, callback) => { console.log( "In resetSetupAndConfigureAlarm - addonExec", From 693db4e9c7d20724969160e0d2046a43f4814692 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Fri, 7 Sep 2018 20:17:48 +0300 Subject: [PATCH 18/28] Made internalTelemetryArchive nullable so that it is truly optional also in the eyes of the schema guard --- webExtensionApis/study/api.md | 55 ++++++++++++++++++++++-------- webExtensionApis/study/schema.json | 24 ++++++++++++- webExtensionApis/study/schema.yaml | 21 +++++++++--- 3 files changed, 80 insertions(+), 20 deletions(-) diff --git a/webExtensionApis/study/api.md b/webExtensionApis/study/api.md index c8d7d04..510dad2 100644 --- a/webExtensionApis/study/api.md +++ b/webExtensionApis/study/api.md @@ -290,7 +290,34 @@ Act on it by } ``` -### [1] NullableInteger +### [1] NullableBoolean + +```json +{ + "id": "NullableBoolean", + "$schema": "http://json-schema.org/draft-04/schema", + "oneOf": [ + { + "type": "null" + }, + { + "type": "boolean" + } + ], + "choices": [ + { + "type": "null" + }, + { + "type": "boolean" + } + ], + "testcases": [null, true, false], + "failcases": ["1234567890", "foo", []] +} +``` + +### [2] NullableInteger ```json { @@ -317,7 +344,7 @@ Act on it by } ``` -### [2] NullableNumber +### [3] NullableNumber ```json { @@ -344,7 +371,7 @@ Act on it by } ``` -### [3] studyTypesEnum +### [4] studyTypesEnum ```json { @@ -357,7 +384,7 @@ Act on it by } ``` -### [4] weightedVariationObject +### [5] weightedVariationObject ```json { @@ -381,7 +408,7 @@ Act on it by } ``` -### [5] weightedVariationsArray +### [6] weightedVariationsArray ```json { @@ -414,7 +441,7 @@ Act on it by } ``` -### [6] anEndingRequest +### [7] anEndingRequest ```json { @@ -525,7 +552,7 @@ Act on it by } ``` -### [7] onEndStudyResponse +### [8] onEndStudyResponse ```json { @@ -547,7 +574,7 @@ Act on it by } ``` -### [8] studyInfoObject +### [9] studyInfoObject ```json { @@ -581,7 +608,7 @@ Act on it by } ``` -### [9] dataPermissionsObject +### [10] dataPermissionsObject ```json { @@ -600,7 +627,7 @@ Act on it by } ``` -### [10] studySetup +### [11] studySetup ```json { @@ -644,7 +671,7 @@ Act on it by }, "internalTelemetryArchive": { "optional": true, - "type": "boolean" + "$ref": "NullableBoolean" } } }, @@ -818,7 +845,7 @@ Act on it by } ``` -### [11] telemetryPayload +### [12] telemetryPayload ```json { @@ -832,7 +859,7 @@ Act on it by } ``` -### [12] searchTelemetryQuery +### [13] searchTelemetryQuery ```json { @@ -870,7 +897,7 @@ Act on it by } ``` -### [13] anEndingAnswer +### [14] anEndingAnswer ```json { diff --git a/webExtensionApis/study/schema.json b/webExtensionApis/study/schema.json index 94bcf04..263a89f 100644 --- a/webExtensionApis/study/schema.json +++ b/webExtensionApis/study/schema.json @@ -25,6 +25,28 @@ ], "testcases": [null, "a string"] }, + { + "id": "NullableBoolean", + "$schema": "http://json-schema.org/draft-04/schema", + "oneOf": [ + { + "type": "null" + }, + { + "type": "boolean" + } + ], + "choices": [ + { + "type": "null" + }, + { + "type": "boolean" + } + ], + "testcases": [null, true, false], + "failcases": ["1234567890", "foo", []] + }, { "id": "NullableInteger", "$schema": "http://json-schema.org/draft-04/schema", @@ -331,7 +353,7 @@ }, "internalTelemetryArchive": { "optional": true, - "type": "boolean" + "$ref": "NullableBoolean" } } }, diff --git a/webExtensionApis/study/schema.yaml b/webExtensionApis/study/schema.yaml index b9f4fd1..a23bea9 100644 --- a/webExtensionApis/study/schema.yaml +++ b/webExtensionApis/study/schema.yaml @@ -34,14 +34,25 @@ {type: 'string'}] testcases: [null, 'a string'] + - id: NullableBoolean + $schema: "http://json-schema.org/draft-04/schema" + oneOf: [ + {type: 'null'}, + {type: 'boolean'}] + choices: [ + {type: 'null'}, + {type: 'boolean'}] + testcases: [null, true, false] + failcases: ['1234567890', 'foo', []] + - id: NullableInteger $schema: "http://json-schema.org/draft-04/schema" oneOf: [ - {type: 'null'}, - {type: 'integer'}] + {type: 'null'}, + {type: 'integer'}] choices: [ - {type: 'null'}, - {type: 'integer'}] + {type: 'null'}, + {type: 'integer'}] testcases: [null, 1234567890] failcases: ['1234567890', []] @@ -214,7 +225,7 @@ type: boolean internalTelemetryArchive: optional: true - type: boolean + $ref: NullableBoolean testing: type: object properties: From 8d2c880b565a5ef0dbacd7ae015f69dedc0d38a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Mon, 10 Sep 2018 20:43:23 +0300 Subject: [PATCH 19/28] Importing and building the Pioneer opt-in add-on in circle --- .circleci/config.yml | 4 ++++ package.json | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 00c1edf..e4296e1 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -64,6 +64,10 @@ jobs: path: "test-addon/dist" destination: "test-addon/dist" + - run: + name: Import and build the Pioneer opt-in add-on + command: npm run import-pioneer-opt-in + # Needs signed add-on to work on branded releases #- run: # name: Test with Firefox Release diff --git a/package.json b/package.json index 0b57004..5e5eb70 100644 --- a/package.json +++ b/package.json @@ -81,6 +81,7 @@ "generate:generateSchema:study": "cd webExtensionApis/study && yaml2json schema.yaml -p > schema.json", "generate:generateStubApi:study": "cd webExtensionApis/study && generateStubApi ./schema.json > stubApi.js", "generate:verifyWeeSchema:study": "cd webExtensionApis/study && verifyWeeSchema schema.json", + "import-pioneer-opt-in": "bin/import-pioneer-opt-in.sh", "lint": "npm-run-all lint:*", "lint:eslint": "npm run eslint", "lint:fixpack": "fixpack # cleans up package.json", @@ -88,7 +89,7 @@ "postformat": "run-p lint:fixpack eslint-fix", "prebuild": "if [ -z ${SKIPLINT} ]; then npm run lint; fi", "prepare": "export SKIPLINT=1 && fixpack && npm run build", - "pretest": "npm run build && npm run test-addon:bundle-utils && npm run test-addon:build && bin/import-pioneer-opt-in.sh", + "pretest": "npm run build && npm run test-addon:bundle-utils && npm run test-addon:build && npm run import-pioneer-opt-in", "pretest-addon": "npm run pretest", "small-study": "cd examples/small-study && npm run rebuild && npm start", "test": "npm run test:func", From aa1fded276507355d508b062fcf5a6b303301876 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Tue, 11 Sep 2018 14:21:43 +0300 Subject: [PATCH 20/28] Nits code style --- test/functional/browser.study.api.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/functional/browser.study.api.js b/test/functional/browser.study.api.js index 74cc0e9..55297c5 100644 --- a/test/functional/browser.study.api.js +++ b/test/functional/browser.study.api.js @@ -57,7 +57,7 @@ function merge(...sources) { return Object.assign({}, ...sources); } -const publicApiTests = function(studyType) { +function publicApiTests(studyType) { /** return a studySetup, shallow merged from overrides * * @return {object} mergedStudySetup @@ -1356,12 +1356,12 @@ const publicApiTests = function(studyType) { it("log level works?"); }); }); -}; +} describe("PUBLIC API `browser.study` (studyType: shield)", function() { - publicApiTests.bind(this)("shield"); + publicApiTests.call(this, "shield"); }); describe("PUBLIC API `browser.study` (studyType: pioneer)", function() { - publicApiTests.bind(this)("pioneer"); + publicApiTests.call(this, "pioneer"); }); From 0d5ee6a395252df276be70a2536ce7c9bd59b6d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Tue, 11 Sep 2018 14:22:39 +0300 Subject: [PATCH 21/28] Correct schema versions specified when sending pioneer telemetry --- webExtensionApis/study/src/studyTypes/pioneer.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/webExtensionApis/study/src/studyTypes/pioneer.js b/webExtensionApis/study/src/studyTypes/pioneer.js index 02d979a..e1c1a06 100644 --- a/webExtensionApis/study/src/studyTypes/pioneer.js +++ b/webExtensionApis/study/src/studyTypes/pioneer.js @@ -295,7 +295,9 @@ class PioneerStudyType { * @returns {Promise<*>} */ async sendTelemetry(bucket, payload) { - return this._telemetry(bucket, 1, payload); + const schemaName = bucket; + const schemaVersion = 3; // Corresponds to the schema versions used in https://github.com/mozilla-services/mozilla-pipeline-schemas/tree/dev/templates/telemetry/shield-study (and the shield-study-addon, shield-study-error equivalents) + return this._telemetry(schemaName, schemaVersion, payload); } /** From ee9bddba0f66cfb4d288e19d1b0d4246fdb16c75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Sun, 28 Oct 2018 00:13:19 +0300 Subject: [PATCH 22/28] Set import pioneer opt in script as binary in npm package --- package.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 5e5eb70..00f2f57 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,8 @@ "version": "5.1.1", "author": "Mozilla", "bin": { - "copyStudyUtils": "bin/copyStudyUtils.js" + "copyStudyUtils": "bin/copyStudyUtils.js", + "importPioneerOptIn": "bin/import-pioneer-opt-in.sh" }, "bugs": { "url": "https://github.com/mozilla/shield-studies-addon-utils/issues" @@ -42,6 +43,7 @@ }, "files": [ "bin/copyStudyUtils.js", + "bin/import-pioneer-opt-in.sh", "testUtils", "webExtensionApis/study/api.js", "webExtensionApis/study/schema.json", From 32229ab078fd0b8950238592bb59d3035c0e3ca1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Sun, 28 Oct 2018 13:24:32 +0200 Subject: [PATCH 23/28] Pioneer telemetry logged at debug level like other telemetry log events --- webExtensionApis/study/src/studyTypes/pioneer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webExtensionApis/study/src/studyTypes/pioneer.js b/webExtensionApis/study/src/studyTypes/pioneer.js index e1c1a06..8cabcac 100644 --- a/webExtensionApis/study/src/studyTypes/pioneer.js +++ b/webExtensionApis/study/src/studyTypes/pioneer.js @@ -323,12 +323,12 @@ class PioneerStudyType { payload, ); if (pingId) { - utilsLogger.log( + utilsLogger.debug( "Pioneer Telemetry sent (encrypted)", JSON.stringify(payload), ); } else { - utilsLogger.log( + utilsLogger.debug( "Pioneer Telemetry not sent due to privacy preferences", JSON.stringify(payload), ); From 11b28e3ab7b022a83c3631108ed014db71ace2f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Sun, 28 Oct 2018 13:24:57 +0200 Subject: [PATCH 24/28] Commented out a rogue console.log statement --- webExtensionApis/study/src/studyTypes/shield.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webExtensionApis/study/src/studyTypes/shield.js b/webExtensionApis/study/src/studyTypes/shield.js index 24f37b0..5825502 100644 --- a/webExtensionApis/study/src/studyTypes/shield.js +++ b/webExtensionApis/study/src/studyTypes/shield.js @@ -15,7 +15,7 @@ class ShieldStudyType { * @param {object} studyUtils The studyUtils instance from where this class was instantiated */ constructor(studyUtils) { - console.log("studyUtils", studyUtils); + // console.log("studyUtils", studyUtils); } /** From 162d06c8764512b0e24705b2a13ef0bb350a0992 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Sun, 28 Oct 2018 13:33:30 +0200 Subject: [PATCH 25/28] Make eslint happy --- webExtensionApis/study/src/studyTypes/pioneer.js | 14 ++++++++------ webExtensionApis/study/src/studyTypes/shield.js | 6 +++--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/webExtensionApis/study/src/studyTypes/pioneer.js b/webExtensionApis/study/src/studyTypes/pioneer.js index 8cabcac..4d02bf5 100644 --- a/webExtensionApis/study/src/studyTypes/pioneer.js +++ b/webExtensionApis/study/src/studyTypes/pioneer.js @@ -56,7 +56,7 @@ joseSetCrypto(crypto); */ class PioneerUtils { /** - * @param {Config} config + * @param {Config} config Object with Pioneer-related configuration as specified above */ constructor(config) { this.config = config; @@ -72,7 +72,9 @@ class PioneerUtils { return PUBLIC_KEYS[env]; } - /** */ + /** + * @returns {void} + */ setupEncrypter() { if (this.encrypter === null) { const pk = this.getPublicKey(); @@ -121,7 +123,7 @@ class PioneerUtils { /** * @private * @param {String} data The data to encrypt - * @returns {String} + * @returns {String} The encrypted data */ async encryptData(data) { this.setupEncrypter(); @@ -290,9 +292,9 @@ class PioneerStudyType { } /** - * @param bucket - * @param payload - * @returns {Promise<*>} + * @param {String} bucket The type of telemetry payload + * @param {Object} payload The telemetry payload + * @returns {Promise} The ID of the ping that was submitted */ async sendTelemetry(bucket, payload) { const schemaName = bucket; diff --git a/webExtensionApis/study/src/studyTypes/shield.js b/webExtensionApis/study/src/studyTypes/shield.js index 5825502..1ab1084 100644 --- a/webExtensionApis/study/src/studyTypes/shield.js +++ b/webExtensionApis/study/src/studyTypes/shield.js @@ -19,7 +19,7 @@ class ShieldStudyType { } /** - * @returns {Promise<*>} + * @returns {Promise} The telemetry client id */ async getTelemetryId() { const id = TelemetryController.clientID; @@ -31,8 +31,8 @@ class ShieldStudyType { } /** - * @param bucket - * @param payload + * @param {String} bucket The type of telemetry payload + * @param {Object} payload The telemetry payload * @returns {Promise} The ID of the ping that was submitted */ async sendTelemetry(bucket, payload) { From f28bf0564dcf6d3148338972fea59a1a11cf49b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Sun, 28 Oct 2018 23:57:37 +0200 Subject: [PATCH 26/28] Add browser.study.calculateTelemetryPingSize --- webExtensionApis/study/api.md | 26 ++++-- webExtensionApis/study/schema.json | 21 ++++- webExtensionApis/study/schema.yaml | 26 ++++-- webExtensionApis/study/src/getPingSize.js | 24 ++++++ webExtensionApis/study/src/index.js | 17 ++++ .../study/src/studyTypes/pioneer.js | 86 ++++++++++--------- .../study/src/studyTypes/shield.js | 17 ++++ webExtensionApis/study/src/studyUtils.js | 27 +++++- 8 files changed, 186 insertions(+), 58 deletions(-) create mode 100644 webExtensionApis/study/src/getPingSize.js diff --git a/webExtensionApis/study/api.md b/webExtensionApis/study/api.md index 510dad2..f00a366 100644 --- a/webExtensionApis/study/api.md +++ b/webExtensionApis/study/api.md @@ -149,14 +149,10 @@ Object of current dataPermissions (shield enabled true/false, pioneer enabled tr Send Telemetry using appropriate shield or pioneer methods. -shield: +shield & pioneer sends using the following schema: * `shield-study-addon` ping, requires object string keys and string values -pioneer: - -* TBD - Note: * no conversions / coercion of data happens. @@ -164,12 +160,30 @@ Note: Note: * undefined what happens if validation fails -* undefined what happens when you try to send 'shield' from 'pioneer' TBD fix the parameters here. **Parameters** +* `payload` + * type: payload + * $ref: + * optional: false + +### `browser.study.calculateTelemetryPingSize( payload )` + +Calculate Telemetry using appropriate shield or pioneer methods. + +shield: + +* Calculate the size of a ping + +pioneer: + +* Calculate the size of a ping that has Pioneer encrypted data + +**Parameters** + * `payload` * type: payload * $ref: diff --git a/webExtensionApis/study/schema.json b/webExtensionApis/study/schema.json index 263a89f..282e2ab 100644 --- a/webExtensionApis/study/schema.json +++ b/webExtensionApis/study/schema.json @@ -656,7 +656,7 @@ "name": "sendTelemetry", "type": "function", "description": - "Send Telemetry using appropriate shield or pioneer methods.\n\nshield:\n- `shield-study-addon` ping, requires object string keys and string values\n\npioneer:\n- TBD\n\nNote:\n- no conversions / coercion of data happens.\n\nNote:\n- undefined what happens if validation fails\n- undefined what happens when you try to send 'shield' from 'pioneer'\n\nTBD fix the parameters here.\n", + "Send Telemetry using appropriate shield or pioneer methods.\n\nshield & pioneer sends using the following schema:\n- `shield-study-addon` ping, requires object string keys and string values\n\nNote:\n- no conversions / coercion of data happens.\n\nNote:\n- undefined what happens if validation fails\n\nTBD fix the parameters here.\n", "async": true, "parameters": [ { @@ -667,6 +667,25 @@ "defaultReturn": "undefined", "returns": null }, + { + "name": "calculateTelemetryPingSize", + "type": "function", + "description": + "Calculate Telemetry using appropriate shield or pioneer methods.\n\nshield:\n- Calculate the size of a ping\n\npioneer:\n- Calculate the size of a ping that has Pioneer encrypted data\n", + "async": true, + "parameters": [ + { + "name": "payload", + "$ref": "telemetryPayload" + } + ], + "defaultReturn": "undefined", + "returns": [ + { + "type": "number" + } + ] + }, { "name": "searchSentTelemetry", "type": "function", diff --git a/webExtensionApis/study/schema.yaml b/webExtensionApis/study/schema.yaml index a23bea9..213dc74 100644 --- a/webExtensionApis/study/schema.yaml +++ b/webExtensionApis/study/schema.yaml @@ -456,18 +456,14 @@ description: | Send Telemetry using appropriate shield or pioneer methods. - shield: + shield & pioneer sends using the following schema: - `shield-study-addon` ping, requires object string keys and string values - pioneer: - - TBD - Note: - no conversions / coercion of data happens. Note: - undefined what happens if validation fails - - undefined what happens when you try to send 'shield' from 'pioneer' TBD fix the parameters here. @@ -479,6 +475,26 @@ defaultReturn: undefined # exception if out of policy based on config returns: + - name: calculateTelemetryPingSize + type: function + description: | + Calculate Telemetry using appropriate shield or pioneer methods. + + shield: + - Calculate the size of a ping + + pioneer: + - Calculate the size of a ping that has Pioneer encrypted data + + async: true + parameters: + - name: payload + $ref: telemetryPayload + + defaultReturn: undefined + returns: + - type: number + - name: searchSentTelemetry type: function async: true diff --git a/webExtensionApis/study/src/getPingSize.js b/webExtensionApis/study/src/getPingSize.js new file mode 100644 index 0000000..8926783 --- /dev/null +++ b/webExtensionApis/study/src/getPingSize.js @@ -0,0 +1,24 @@ +/* eslint-env commonjs */ + +/** + * Calculate the size of a ping. + * + * @param {Object} payload + * The data payload of the ping. + * + * @returns {Number} + * The total size of the ping. + */ +function getPingSize(payload) { + const converter = Cc[ + "@mozilla.org/intl/scriptableunicodeconverter" + ].createInstance(Ci.nsIScriptableUnicodeConverter); + converter.charset = "UTF-8"; + let utf8Payload = converter.ConvertFromUnicode(JSON.stringify(payload)); + utf8Payload += converter.Finish(); + return utf8Payload.length; +} + +module.exports = { + getPingSize, +}; diff --git a/webExtensionApis/study/src/index.js b/webExtensionApis/study/src/index.js index 6568a5d..2ee9629 100644 --- a/webExtensionApis/study/src/index.js +++ b/webExtensionApis/study/src/index.js @@ -315,6 +315,23 @@ this.study = class extends ExtensionAPI { return studyUtils.telemetry(payload); }, + /** Calculate Telemetry using appropriate shield or pioneer methods. + * + * shield: + * - Calculate the size of a ping + * + * pioneer: + * - Calculate the size of a ping that has Pioneer encrypted data + * + * @param {Object} payload Non-nested object with key strings, and key values + * @returns {Promise} The total size of the ping. + */ + calculateTelemetryPingSize: async function calculateTelemetryPingSize( + payload, + ) { + return studyUtils.calculateTelemetryPingSize(payload); + }, + /** Search locally stored telemetry pings using these fields (if set) * * n: diff --git a/webExtensionApis/study/src/studyTypes/pioneer.js b/webExtensionApis/study/src/studyTypes/pioneer.js index 4d02bf5..7096dfe 100644 --- a/webExtensionApis/study/src/studyTypes/pioneer.js +++ b/webExtensionApis/study/src/studyTypes/pioneer.js @@ -3,6 +3,7 @@ import { utilsLogger } from "../logger"; import * as dataPermissions from "../dataPermissions"; +import { getPingSize } from "../getPingSize"; const { Services } = ChromeUtils.import( "resource://gre/modules/Services.jsm", @@ -101,25 +102,6 @@ class PioneerUtils { return id; } - /** - * Calculate the size of a ping. - * - * @param {Object} payload - * The data payload of the ping. - * - * @returns {Number} - * The total size of the ping. - */ - getPingSize(payload) { - const converter = Cc[ - "@mozilla.org/intl/scriptableunicodeconverter" - ].createInstance(Ci.nsIScriptableUnicodeConverter); - converter.charset = "UTF-8"; - let utf8Payload = converter.ConvertFromUnicode(JSON.stringify(payload)); - utf8Payload += converter.Finish(); - return utf8Payload.length; - } - /** * @private * @param {String} data The data to encrypt @@ -158,27 +140,6 @@ class PioneerUtils { }; } - /** - * Calculate the size of a ping that has Pioneer encrypted data. - * - * @param {String} schemaName - * The name of the schema to be used for validation. - * - * @param {int} schemaVersion - * The version of the schema to be used for validation. - * - * @param {Object} data - * An object containing data to be encrypted and submitted. - * - * @returns {Number} - * The total size of the ping. - */ - async getEncryptedPingSize(schemaName, schemaVersion, data) { - return this.getPingSize( - await this.buildEncryptedPayload(schemaName, schemaVersion, data), - ); - } - /** * Encrypts the given data and submits a properly formatted * Pioneer ping to Telemetry. @@ -267,6 +228,7 @@ class PioneerStudyType { telemetryEnv: studySetup.telemetry.removeTestingFlag ? "prod" : "stage", }; this.pioneerUtils = new PioneerUtils(Config); + this.schemaVersion = 3; // Corresponds to the schema versions used in https://github.com/mozilla-services/mozilla-pipeline-schemas/tree/dev/templates/telemetry/shield-study (and the shield-study-addon, shield-study-error equivalents) } /** @@ -298,8 +260,7 @@ class PioneerStudyType { */ async sendTelemetry(bucket, payload) { const schemaName = bucket; - const schemaVersion = 3; // Corresponds to the schema versions used in https://github.com/mozilla-services/mozilla-pipeline-schemas/tree/dev/templates/telemetry/shield-study (and the shield-study-addon, shield-study-error equivalents) - return this._telemetry(schemaName, schemaVersion, payload); + return this._telemetry(schemaName, this.schemaVersion, payload); } /** @@ -337,6 +298,47 @@ class PioneerStudyType { } return pingId; } + + /** + * Calculate the size of a ping. + * + * @param {String} bucket The type of telemetry payload + * + * @param {Object} payload + * The data payload of the ping. + * + * @returns {Promise} + * The total size of the ping. + */ + async getPingSize(bucket, payload) { + const schemaName = bucket; + return this.getEncryptedPingSize(schemaName, this.schemaVersion, payload); + } + + /** + * Calculate the size of a ping that has Pioneer encrypted data. + * + * @param {String} schemaName + * The name of the schema to be used for validation. + * + * @param {int} schemaVersion + * The version of the schema to be used for validation. + * + * @param {Object} data + * An object containing data to be encrypted and submitted. + * + * @returns {Promise} + * The total size of the ping. + */ + async getEncryptedPingSize(schemaName, schemaVersion, data) { + return getPingSize( + await this.pioneerUtils.buildEncryptedPayload( + schemaName, + schemaVersion, + data, + ), + ); + } } export default PioneerStudyType; diff --git a/webExtensionApis/study/src/studyTypes/shield.js b/webExtensionApis/study/src/studyTypes/shield.js index 1ab1084..8a4abb2 100644 --- a/webExtensionApis/study/src/studyTypes/shield.js +++ b/webExtensionApis/study/src/studyTypes/shield.js @@ -1,5 +1,7 @@ /* eslint-env commonjs */ +import { getPingSize } from "../getPingSize"; + const { TelemetryController } = ChromeUtils.import( "resource://gre/modules/TelemetryController.jsm", null, @@ -39,6 +41,21 @@ class ShieldStudyType { const telOptions = { addClientId: true, addEnvironment: true }; return TelemetryController.submitExternalPing(bucket, payload, telOptions); } + + /** + * Calculate the size of a ping. + * + * @param {String} bucket The type of telemetry payload + * + * @param {Object} payload + * The data payload of the ping. + * + * @returns {Promise} + * The total size of the ping. + */ + async getPingSize(bucket, payload) { + return getPingSize(payload); + } } export default ShieldStudyType; diff --git a/webExtensionApis/study/src/studyUtils.js b/webExtensionApis/study/src/studyUtils.js index e9c7358..08b2c25 100644 --- a/webExtensionApis/study/src/studyUtils.js +++ b/webExtensionApis/study/src/studyUtils.js @@ -764,14 +764,14 @@ class StudyUtils { /** * Validates and submits telemetry pings from the add-on; mostly from * webExtension messages. - * @param {Object} data - the data to send as part of the telemetry packet + * @param {Object} payload - the data to send as part of the telemetry packet * @returns {Promise|boolean} - see StudyUtils._telemetry */ - async telemetry(data) { + async telemetry(payload) { this.throwIfNotSetup("telemetry"); - utilsLogger.debug(`telemetry ${JSON.stringify(data)}`); + utilsLogger.debug(`telemetry ${JSON.stringify(payload)}`); const toSubmit = { - attributes: data, + attributes: payload, }; return this._telemetry(toSubmit, "shield-study-addon"); } @@ -784,6 +784,25 @@ class StudyUtils { telemetryError(errorReport) { return this._telemetry(errorReport, "shield-study-error"); } + + /** Calculate Telemetry using appropriate shield or pioneer methods. + * + * shield: + * - Calculate the size of a ping + * + * pioneer: + * - Calculate the size of a ping that has Pioneer encrypted data + * + * @param {Object} payload Non-nested object with key strings, and key values + * @returns {Promise} The total size of the ping. + */ + async calculateTelemetryPingSize(payload) { + this.throwIfNotSetup("calculateTelemetryPingSize"); + const toSubmit = { + attributes: payload, + }; + return this.studyType.getPingSize(toSubmit, "shield-study-addon"); + } } // TODO, use the usual es6 exports From d91775cb1359eb57e5bc13781b28d950ec777785 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Mon, 29 Oct 2018 00:13:51 +0200 Subject: [PATCH 27/28] Test for browser.study.calculateTelemetryPingSize --- test/functional/browser.study.api.js | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/test/functional/browser.study.api.js b/test/functional/browser.study.api.js index 55297c5..b8246ec 100644 --- a/test/functional/browser.study.api.js +++ b/test/functional/browser.study.api.js @@ -610,7 +610,7 @@ function publicApiTests(studyType) { describe("life-cycle tests", function() { describe("setup, sendTelemetry, manually invoked endStudy", function() { - let studyInfo; + let studyInfo, calculatedPingSize; const overrides = { activeExperimentName: "test:browser.study.api", telemetry: { @@ -630,14 +630,23 @@ function publicApiTests(studyType) { before(async function reinstallSetupDoTelemetryAndWait() { await installAddon(); - studyInfo = await addonExec(async (_studySetupForTests, callback) => { + const _ = await addonExec(async (_studySetupForTests, callback) => { // Ensure we have a configured study and are supposed to run our feature browser.study.onReady.addListener(async _studyInfo => { - await browser.study.sendTelemetry({ foo: "bar" }); - callback(_studyInfo); + const samplePing = { foo: "bar" }; + await browser.study.sendTelemetry(samplePing); + const _calculatedPingSize = await browser.study.calculateTelemetryPingSize( + samplePing, + ); + callback({ + studyInfo: _studyInfo, + calculatedPingSize: _calculatedPingSize, + }); }); await browser.study.setup(_studySetupForTests); }, studySetupForTests(overrides)); + studyInfo = _.studyInfo; + calculatedPingSize = _.calculatedPingSize; await delay(1000); // wait a second to telemetry to settle on disk. }); @@ -649,6 +658,14 @@ function publicApiTests(studyType) { ); }); + it("calculated ping size is as expected", async () => { + const expectedPingSizes = { + shield: 20, + pioneer: 662, + }; + assert.strictEqual(calculatedPingSize, expectedPingSizes[studyType]); + }); + describe("telemetry archive / controller effects", function() { let studyPings; before(async () => { From e3671733444c55e1d9e6f938a92066ee319b476f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Wollse=CC=81n?= Date: Fri, 16 Nov 2018 00:14:13 +0200 Subject: [PATCH 28/28] Clarified the requirements of the telemetry payload argument --- webExtensionApis/study/schema.yaml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/webExtensionApis/study/schema.yaml b/webExtensionApis/study/schema.yaml index 213dc74..66b59e0 100644 --- a/webExtensionApis/study/schema.yaml +++ b/webExtensionApis/study/schema.yaml @@ -456,13 +456,10 @@ description: | Send Telemetry using appropriate shield or pioneer methods. - shield & pioneer sends using the following schema: - - `shield-study-addon` ping, requires object string keys and string values + Note: The payload must adhere to the `data.attributes` property in the [`shield-study-addon`](https://github.com/mozilla-services/mozilla-pipeline-schemas/blob/dev/templates/include/telemetry/shieldStudyAddonPayload.3.schema.json) schema. That is, it must be a flat object with string keys and string values. Note: - no conversions / coercion of data happens. - - Note: - undefined what happens if validation fails TBD fix the parameters here.