From c08b8af9822af3f26c679243284cd1cea01948d5 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Mon, 10 Oct 2016 02:48:35 +0200 Subject: [PATCH 01/23] test: introduced a test suite for unit testing. --- .gitignore | 4 ++ .travis.yml | 18 ++++++ Gruntfile.js | 8 +++ package.json | 19 +++++- test/.eslintrc | 11 ++++ test/setup.js | 75 ++++++++++++++++++++++ test/test-async-functions.js | 31 +++++++++ test/test-browser-global.js | 14 +++++ test/test-proxied-properties.js | 48 ++++++++++++++ test/test-runtime-onMessage.js | 108 ++++++++++++++++++++++++++++++++ 10 files changed, 335 insertions(+), 1 deletion(-) create mode 100644 .travis.yml create mode 100644 test/.eslintrc create mode 100644 test/setup.js create mode 100644 test/test-async-functions.js create mode 100644 test/test-browser-global.js create mode 100644 test/test-proxied-properties.js create mode 100644 test/test-runtime-onMessage.js diff --git a/.gitignore b/.gitignore index 3659f1a..cb52d64 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,6 @@ node_modules/* dist/* + +## code coverage +coverage/ +.nyc_output/ diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 0000000..8fe055f --- /dev/null +++ b/.travis.yml @@ -0,0 +1,18 @@ +language: node_js +sudo: false +node_js: +## Some of the ES6 syntax used in the browser-polyfill sources is only supported on nodejs >= 6 +- '6' + +script: +- npm run build +- COVERAGE=y npm run test-coverage + +after_script: npm run publish-coverage + +notifications: + irc: + channels: + - irc.mozilla.org#amo-bots + on_success: change + on_failure: always diff --git a/Gruntfile.js b/Gruntfile.js index 2b73734..4a2d074 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -12,8 +12,15 @@ module.exports = function(grunt) { grunt.initConfig({ pkg: grunt.file.readJSON("package.json"), + coveralls: { + all: { + src: "coverage/lcov.info", + }, + }, + eslint: { src: ["browser-polyfill.in.js", "Gruntfile.js"], + tests: ["tests/**/*.js"], }, replace: { @@ -81,6 +88,7 @@ module.exports = function(grunt) { grunt.loadNpmTasks("gruntify-eslint"); grunt.loadNpmTasks("grunt-replace"); + grunt.loadNpmTasks("grunt-coveralls"); require("google-closure-compiler").grunt(grunt); grunt.registerTask("default", ["eslint", "replace", "closure-compiler"]); diff --git a/package.json b/package.json index bf58da6..6809d80 100644 --- a/package.json +++ b/package.json @@ -13,9 +13,26 @@ }, "homepage": "https://github.com/mozilla/webextension-polyfill", "devDependencies": { + "chai": "^3.5.0", "google-closure-compiler": "^20160911.0.0", "grunt": "^1.0.1", + "grunt-coveralls": "^1.0.1", "grunt-replace": "*", - "gruntify-eslint": "*" + "gruntify-eslint": "*", + "istanbul-lib-instrument": "^1.1.3", + "jsdom": "^9.6.0", + "mocha": "^3.1.0", + "nyc": "^8.3.1", + "sinon": "^1.17.6" + }, + "nyc": { + "reporter": ["lcov", "text", "html"], + "instrument": false + }, + "scripts": { + "build": "grunt", + "test": "mocha", + "test-coverage": "COVERAGE=y nyc mocha", + "publish-coverage": "grunt coveralls" } } diff --git a/test/.eslintrc b/test/.eslintrc new file mode 100644 index 0000000..e275d67 --- /dev/null +++ b/test/.eslintrc @@ -0,0 +1,11 @@ +{ + "extends": "../.eslintrc", + "env": { + "mocha": true, + "node": true, + "browser": true, + "webextensions": true + }, + "globals": { + } +} \ No newline at end of file diff --git a/test/setup.js b/test/setup.js new file mode 100644 index 0000000..8f4b4c8 --- /dev/null +++ b/test/setup.js @@ -0,0 +1,75 @@ +"use strict"; + +const {createInstrumenter} = require("istanbul-lib-instrument"); +const fs = require("fs"); +const {jsdom, createVirtualConsole} = require("jsdom"); + +var virtualConsole = createVirtualConsole().sendTo(console); + +// Path to the browser-polyfill script, relative to the current work dir +// where mocha is executed. +const BROWSER_POLYFILL_PATH = "./dist/browser-polyfill.js"; + +function setupTestDOMWindow(chromeObject) { + return new Promise((resolve, reject) => { + let window; + + // If a jsdom window has already been created, reuse it so that + // we can retrieve the final code coverage data, which has been + // collected in the jsdom window (where the instrumented browser-polyfill + // is running). + if (global.window) { + window = global.window; + window.browser = undefined; + } else { + window = jsdom({virtualConsole}).defaultView; + global.window = window; + } + + // Inject the fake chrome object used as a fixture for the particular + // browser-polyfill test scenario. + window.chrome = chromeObject; + + const scriptEl = window.document.createElement("script"); + + if (process.env.COVERAGE == "y") { + // If the code coverage is enabled, instrument the code on the fly + // before executing it in the jsdom window. + const inst = createInstrumenter({ + compact: false, esModules: false, produceSourceMap: false, + }); + const scriptContent = fs.readFileSync(BROWSER_POLYFILL_PATH, "utf-8"); + scriptEl.textContent = inst.instrumentSync(scriptContent, BROWSER_POLYFILL_PATH); + } else { + scriptEl.src = BROWSER_POLYFILL_PATH; + } + + // Prepare to listen for script loading errors (which results in a rejection), + // and to detect when the browser-polyfill has been executed (which resolves + // to the jsdom window where the loading has been completed). + window.__browserPolyfillLoaded__ = {resolve, reject}; + window.addEventListener("error", (evt) => reject(evt)); + const loadedScriptEl = window.document.createElement("script"); + loadedScriptEl.textContent = ` + window.removeEventListener("error", window.__browserPolyfillLoaded__.reject); + window.__browserPolyfillLoaded__.resolve(window); + `; + + window.document.body.appendChild(scriptEl); + window.document.body.appendChild(loadedScriptEl); + }); +} + +// Copy the code coverage of the browser-polyfill script from the jsdom window +// to the nodejs global, where nyc expects to find the code coverage data to +// render in the reports. +after(() => { + if (global.window && process.env.COVERAGE == "y") { + global.__coverage__ = global.window.__coverage__; + } +}); + +module.exports = { + BROWSER_POLYFILL_PATH, + setupTestDOMWindow, +}; diff --git a/test/test-async-functions.js b/test/test-async-functions.js new file mode 100644 index 0000000..cf47816 --- /dev/null +++ b/test/test-async-functions.js @@ -0,0 +1,31 @@ +"use strict"; + +const {assert} = require("chai"); +const sinon = require("sinon"); + +const {setupTestDOMWindow} = require("./setup"); + +describe("browser-polyfill", () => { + describe("wrapped async functions", () => { + it("returns a promise which resolves to the callback parameters", () => { + const fakeChrome = { + alarms: {clear: sinon.stub()}, + runtime: {lastError: undefined}, + }; + return setupTestDOMWindow(fakeChrome).then(window => { + fakeChrome.alarms.clear + .onFirstCall().callsArgWith(1, "res1") + .onSecondCall().callsArgWith(1, "res1", "res2", "res3"); + + return Promise.all([ + window.browser.alarms.clear("test1"), + window.browser.alarms.clear("test2"), + ]); + }).then(results => { + assert.equal(results[0], "res1", "The first call resolved to a single value"); + assert.deepEqual(results[1], ["res1", "res2", "res3"], + "The second call resolved to an array of the expected values"); + }); + }); + }); +}); diff --git a/test/test-browser-global.js b/test/test-browser-global.js new file mode 100644 index 0000000..97ca4e3 --- /dev/null +++ b/test/test-browser-global.js @@ -0,0 +1,14 @@ +"use strict"; + +const {assert} = require("chai"); + +const {setupTestDOMWindow} = require("./setup"); + +describe("browser-polyfill", () => { + it("automatically wrapps chrome into a browser object", () => { + const fakeChrome = {}; + return setupTestDOMWindow(fakeChrome).then(window => { + assert.equal(typeof window.browser, "object", "Got the window.browser object"); + }); + }); +}); diff --git a/test/test-proxied-properties.js b/test/test-proxied-properties.js new file mode 100644 index 0000000..98f0f6a --- /dev/null +++ b/test/test-proxied-properties.js @@ -0,0 +1,48 @@ +"use strict"; + +const {assert} = require("chai"); +const sinon = require("sinon"); + +const {setupTestDOMWindow} = require("./setup"); + +describe("browser-polyfill", () => { + describe("proxies non-wrapped functions", () => { + it("should proxy getters and setters", () => { + const fakeChrome = { + runtime: {myprop: "previous-value"}, + nowrapns: {nowrapkey: "previous-value"}, + }; + return setupTestDOMWindow(fakeChrome).then(window => { + const setResult = window.browser.runtime.myprop = "new-value"; + const setResult2 = window.browser.nowrapns.nowrapkey = "new-value"; + + assert.equal(setResult, "new-value", + "Got the expected result from setting a wrapped property name"); + assert.equal(setResult2, "new-value", + "Got the expected result from setting a wrapped property name"); + }); + }); + + it("delete proxy getter/setter that are not wrapped", () => { + const fakeChrome = {}; + return setupTestDOMWindow(fakeChrome).then(window => { + window.browser.newns = {newkey: "test-value"}; + assert.equal(window.browser.newns.newkey, "test-value", + "Got the expected result from setting a wrapped property name"); + + const setRes = window.browser.newns = {newkey2: "new-value"}; + assert.equal(window.browser.newns.newkey2, "new-value", + "The new non-wrapped getter is cached"); + assert.deepEqual(setRes, {newkey2: "new-value"}, + "Got the expected result from setting a new wrapped property name"); + assert.deepEqual(window.browser.newns, window.chrome.newns, + "chrome.newns and browser.newns are the same"); + + + delete window.browser.newns.newkey2; + assert.equal(window.browser.newns.newkey2, undefined, + "Got the expected result from setting a wrapped property name"); + }); + }); + }); +}); diff --git a/test/test-runtime-onMessage.js b/test/test-runtime-onMessage.js new file mode 100644 index 0000000..08f9a03 --- /dev/null +++ b/test/test-runtime-onMessage.js @@ -0,0 +1,108 @@ +"use strict"; + +const {assert} = require("chai"); +const sinon = require("sinon"); + +const {setupTestDOMWindow} = require("./setup"); + +describe("browser-polyfill", () => { + describe("wrapped runtime.onMessage listener", () => { + it("keep track of the listeners added", () => { + const messageListener = sinon.spy(); + + const fakeChrome = { + runtime: { + lastError: undefined, + onMessage: { + addListener: sinon.spy(), + hasListener: sinon.stub(), + removeListener: sinon.spy(), + }, + }, + }; + + return setupTestDOMWindow(fakeChrome).then(window => { + fakeChrome.runtime.onMessage.hasListener + .onFirstCall().returns(false) + .onSecondCall().returns(true); + + assert.equal(window.browser.runtime.onMessage.hasListener(messageListener), + false, "Got hasListener==false before the listener has been added"); + window.browser.runtime.onMessage.addListener(messageListener); + assert.equal(window.browser.runtime.onMessage.hasListener(messageListener), + true, "Got hasListener=true once the listener has been added"); + window.browser.runtime.onMessage.addListener(messageListener); + + assert.ok(fakeChrome.runtime.onMessage.addListener.calledTwice, + "addListener has been called twice"); + assert.equal(fakeChrome.runtime.onMessage.addListener.secondCall.args[0], + fakeChrome.runtime.onMessage.addListener.firstCall.args[0], + "both the addListener calls received the same wrapped listener"); + + const wrappedListener = fakeChrome.runtime.onMessage.addListener.firstCall.args[0]; + wrappedListener("msg", {name: "sender"}, function sendResponse() {}); + assert.ok(messageListener.calledOnce, "The listener has been called once"); + + window.browser.runtime.onMessage.removeListener(messageListener); + assert.ok(fakeChrome.runtime.onMessage.removeListener.calledOnce, + "removeListener has been called once"); + assert.equal(fakeChrome.runtime.onMessage.addListener.secondCall.args[0], + fakeChrome.runtime.onMessage.removeListener.firstCall.args[0], + "both the addListener and removeListenercalls received the same wrapped listener"); + }); + }); + + it("sends the returned value as a message response", () => { + const fakeChrome = { + runtime: { + lastError: undefined, + onMessage: { + addListener: sinon.spy(), + }, + }, + }; + + const messageListener = sinon.stub(); + const firstResponse = "fake reply"; + const secondResponse = Promise.resolve("fake reply2"); + const sendResponseSpy = sinon.spy(); + + messageListener.onFirstCall().returns(firstResponse) + .onSecondCall().returns(secondResponse); + + return setupTestDOMWindow(fakeChrome).then(window => { + window.browser.runtime.onMessage.addListener(messageListener); + + assert.ok(fakeChrome.runtime.onMessage.addListener.calledOnce); + + const wrappedListener = fakeChrome.runtime.onMessage.addListener.firstCall.args[0]; + + wrappedListener("fake message", {name: "fake sender"}, sendResponseSpy); + + assert.ok(messageListener.calledOnce, "The unwrapped message listener has been called"); + assert.deepEqual(messageListener.firstCall.args, + ["fake message", {name: "fake sender"}], + "The unwrapped message listener has received the expected parameters"); + + assert.ok(sendResponseSpy.calledOnce, "The sendResponse function has been called"); + assert.equal(sendResponseSpy.firstCall.args[0], "fake reply", + "sendResponse callback has been called with the expected parameters"); + + wrappedListener("fake message2", {name: "fake sender2"}, sendResponseSpy); + + // Wait the second response promise to be resolved. + return secondResponse; + }).then(() => { + assert.ok(messageListener.calledTwice, + "The unwrapped message listener has been called"); + assert.deepEqual(messageListener.secondCall.args, + ["fake message2", {name: "fake sender2"}], + "The unwrapped message listener has received the expected parameters"); + + assert.ok(sendResponseSpy.calledTwice, "The sendResponse function has been called"); + assert.equal(sendResponseSpy.secondCall.args[0], "fake reply2", + "sendResponse callback has been called with the expected parameters"); + }); + }); + }); +}); From 9cb05a1190e1247b4935e4409c797a0d98a58037 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Tue, 11 Oct 2016 13:52:44 +0200 Subject: [PATCH 02/23] fix: fixed typo in Gruntfile.js. --- Gruntfile.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gruntfile.js b/Gruntfile.js index 4a2d074..2d860e3 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -20,7 +20,7 @@ module.exports = function(grunt) { eslint: { src: ["browser-polyfill.in.js", "Gruntfile.js"], - tests: ["tests/**/*.js"], + test: ["test/**/*.js"], }, replace: { From f2c82fcbeab89c7db3b4579c090386619dd93a0a Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Tue, 11 Oct 2016 13:53:20 +0200 Subject: [PATCH 03/23] fix: relax max-nested-callback eslint rules for tests. --- test/.eslintrc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/.eslintrc b/test/.eslintrc index e275d67..70b013e 100644 --- a/test/.eslintrc +++ b/test/.eslintrc @@ -1,11 +1,12 @@ { - "extends": "../.eslintrc", "env": { "mocha": true, "node": true, "browser": true, "webextensions": true }, - "globals": { + "globals": {}, + "rules": { + "max-nested-callbacks": ["warn", 6] } -} \ No newline at end of file +} From aaca860c2d944ee36866255408aac9cc97823102 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Tue, 11 Oct 2016 13:54:17 +0200 Subject: [PATCH 04/23] fix: cleanup tests setup function. --- test/setup.js | 58 +++++++++++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/test/setup.js b/test/setup.js index 8f4b4c8..1ee8e94 100644 --- a/test/setup.js +++ b/test/setup.js @@ -1,7 +1,7 @@ "use strict"; -const {createInstrumenter} = require("istanbul-lib-instrument"); const fs = require("fs"); +const {createInstrumenter} = require("istanbul-lib-instrument"); const {jsdom, createVirtualConsole} = require("jsdom"); var virtualConsole = createVirtualConsole().sendTo(console); @@ -10,26 +10,22 @@ var virtualConsole = createVirtualConsole().sendTo(console); // where mocha is executed. const BROWSER_POLYFILL_PATH = "./dist/browser-polyfill.js"; +// Create the jsdom window used to run the tests +const testDOMWindow = jsdom({virtualConsole}).defaultView; +global.window = testDOMWindow; + function setupTestDOMWindow(chromeObject) { return new Promise((resolve, reject) => { - let window; - - // If a jsdom window has already been created, reuse it so that - // we can retrieve the final code coverage data, which has been - // collected in the jsdom window (where the instrumented browser-polyfill - // is running). - if (global.window) { - window = global.window; - window.browser = undefined; - } else { - window = jsdom({virtualConsole}).defaultView; - global.window = window; - } + const window = testDOMWindow; // Inject the fake chrome object used as a fixture for the particular // browser-polyfill test scenario. window.chrome = chromeObject; + // Set the browser property to undefined. + // TODO: change into `delete window.browser` once tmpvar/jsdom#1622 has been fixed. + window.browser = undefined; + const scriptEl = window.document.createElement("script"); if (process.env.COVERAGE == "y") { @@ -44,19 +40,31 @@ function setupTestDOMWindow(chromeObject) { scriptEl.src = BROWSER_POLYFILL_PATH; } - // Prepare to listen for script loading errors (which results in a rejection), - // and to detect when the browser-polyfill has been executed (which resolves - // to the jsdom window where the loading has been completed). - window.__browserPolyfillLoaded__ = {resolve, reject}; - window.addEventListener("error", (evt) => reject(evt)); - const loadedScriptEl = window.document.createElement("script"); - loadedScriptEl.textContent = ` - window.removeEventListener("error", window.__browserPolyfillLoaded__.reject); - window.__browserPolyfillLoaded__.resolve(window); - `; + let onLoad; + let onLoadError; + let onError; + + let cleanLoadListeners = () => { + scriptEl.removeEventListener("load", onLoad); + scriptEl.removeEventListener("error", onLoadError); + + window.removeEventListener("error", onError); + }; + + onLoad = () => { cleanLoadListeners(); resolve(window); }; + onLoadError = () => { + cleanLoadListeners(); + reject(new Error(`Error loading script: ${BROWSER_POLYFILL_PATH}`)); + }; + onError = (err) => { cleanLoadListeners(); reject(err); }; + + // Listen to any uncaught errors. + window.addEventListener("error", onError); + scriptEl.addEventListener("error", onLoadError); + + scriptEl.addEventListener("load", onLoad); window.document.body.appendChild(scriptEl); - window.document.body.appendChild(loadedScriptEl); }); } From 88e3728c4604ebe766ce979d106de9ae5a026e83 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Tue, 11 Oct 2016 13:54:42 +0200 Subject: [PATCH 05/23] fix: removed unused import in test file. --- test/test-proxied-properties.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/test-proxied-properties.js b/test/test-proxied-properties.js index 98f0f6a..7fe199e 100644 --- a/test/test-proxied-properties.js +++ b/test/test-proxied-properties.js @@ -1,7 +1,6 @@ "use strict"; const {assert} = require("chai"); -const sinon = require("sinon"); const {setupTestDOMWindow} = require("./setup"); From b0a111cdc77102cd93c7a45861fc658c22ded4d6 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Tue, 11 Oct 2016 13:56:00 +0200 Subject: [PATCH 06/23] fix: init fake runtime.lastError to null. --- test/test-async-functions.js | 2 +- test/test-runtime-onMessage.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/test-async-functions.js b/test/test-async-functions.js index cf47816..2076e41 100644 --- a/test/test-async-functions.js +++ b/test/test-async-functions.js @@ -10,7 +10,7 @@ describe("browser-polyfill", () => { it("returns a promise which resolves to the callback parameters", () => { const fakeChrome = { alarms: {clear: sinon.stub()}, - runtime: {lastError: undefined}, + runtime: {lastError: null}, }; return setupTestDOMWindow(fakeChrome).then(window => { fakeChrome.alarms.clear diff --git a/test/test-runtime-onMessage.js b/test/test-runtime-onMessage.js index 08f9a03..ca919f2 100644 --- a/test/test-runtime-onMessage.js +++ b/test/test-runtime-onMessage.js @@ -12,7 +12,7 @@ describe("browser-polyfill", () => { const fakeChrome = { runtime: { - lastError: undefined, + lastError: null, onMessage: { addListener: sinon.spy(), hasListener: sinon.stub(), @@ -55,7 +55,7 @@ describe("browser-polyfill", () => { it("sends the returned value as a message response", () => { const fakeChrome = { runtime: { - lastError: undefined, + lastError: null, onMessage: { addListener: sinon.spy(), }, From f28858961fa765fbfdd0f5ce335eaadc7721e607 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Tue, 11 Oct 2016 14:27:26 +0200 Subject: [PATCH 07/23] fix: cleanup async function tests and other minor tweaks on tests. --- test/test-async-functions.js | 31 ++++++++++++++++++++++++------- test/test-browser-global.js | 2 +- test/test-proxied-properties.js | 4 +++- test/test-runtime-onMessage.js | 9 ++++++--- 4 files changed, 34 insertions(+), 12 deletions(-) diff --git a/test/test-async-functions.js b/test/test-async-functions.js index 2076e41..04f2270 100644 --- a/test/test-async-functions.js +++ b/test/test-async-functions.js @@ -10,21 +10,38 @@ describe("browser-polyfill", () => { it("returns a promise which resolves to the callback parameters", () => { const fakeChrome = { alarms: {clear: sinon.stub()}, - runtime: {lastError: null}, + runtime: { + lastError: null, + requestUpdateCheck: sinon.stub(), + }, + tabs: { + query: sinon.stub(), + }, }; return setupTestDOMWindow(fakeChrome).then(window => { + // Test for single callback argument. fakeChrome.alarms.clear - .onFirstCall().callsArgWith(1, "res1") - .onSecondCall().callsArgWith(1, "res1", "res2", "res3"); + .onFirstCall().callsArgWith(1, "res1"); + + // Test for single array callback argument. + fakeChrome.tabs.query + .onFirstCall().callsArgWith(1, ["res1", "res2"]); + + // Test for multiple callback arguments. + fakeChrome.runtime.requestUpdateCheck + .onFirstCall().callsArgWith(0, "res1", "res2"); return Promise.all([ window.browser.alarms.clear("test1"), - window.browser.alarms.clear("test2"), + window.browser.tabs.query({active: true}), + window.browser.runtime.requestUpdateCheck(), ]); }).then(results => { - assert.equal(results[0], "res1", "The first call resolved to a single value"); - assert.deepEqual(results[1], ["res1", "res2", "res3"], - "The second call resolved to an array of the expected values"); + assert.equal(results[0], "res1", "Fake alarms.clear call resolved to a single value"); + assert.deepEqual(results[1], ["res1", "res2"], + "Fake tabs.query resolved to an array of values"); + assert.deepEqual(results[2], ["res1", "res2"], + "Fake runtime.requestUpdateCheck resolved to an array of values"); }); }); }); diff --git a/test/test-browser-global.js b/test/test-browser-global.js index 97ca4e3..4bebb17 100644 --- a/test/test-browser-global.js +++ b/test/test-browser-global.js @@ -5,7 +5,7 @@ const {assert} = require("chai"); const {setupTestDOMWindow} = require("./setup"); describe("browser-polyfill", () => { - it("automatically wrapps chrome into a browser object", () => { + it("wraps the global chrome namespace with a global browser namespace", () => { const fakeChrome = {}; return setupTestDOMWindow(fakeChrome).then(window => { assert.equal(typeof window.browser, "object", "Got the window.browser object"); diff --git a/test/test-proxied-properties.js b/test/test-proxied-properties.js index 7fe199e..b7f62c2 100644 --- a/test/test-proxied-properties.js +++ b/test/test-proxied-properties.js @@ -22,7 +22,7 @@ describe("browser-polyfill", () => { }); }); - it("delete proxy getter/setter that are not wrapped", () => { + it("deletes proxy getter/setter that are not wrapped", () => { const fakeChrome = {}; return setupTestDOMWindow(fakeChrome).then(window => { window.browser.newns = {newkey: "test-value"}; @@ -41,6 +41,8 @@ describe("browser-polyfill", () => { delete window.browser.newns.newkey2; assert.equal(window.browser.newns.newkey2, undefined, "Got the expected result from setting a wrapped property name"); + assert.ok(!("newkey2" in window.browser.newns), + "The deleted property is not listed anymore"); }); }); }); diff --git a/test/test-runtime-onMessage.js b/test/test-runtime-onMessage.js index ca919f2..26d4640 100644 --- a/test/test-runtime-onMessage.js +++ b/test/test-runtime-onMessage.js @@ -7,7 +7,7 @@ const {setupTestDOMWindow} = require("./setup"); describe("browser-polyfill", () => { describe("wrapped runtime.onMessage listener", () => { - it("keep track of the listeners added", () => { + it("keeps track of the listeners added", () => { const messageListener = sinon.spy(); const fakeChrome = { @@ -24,13 +24,14 @@ describe("browser-polyfill", () => { return setupTestDOMWindow(fakeChrome).then(window => { fakeChrome.runtime.onMessage.hasListener .onFirstCall().returns(false) - .onSecondCall().returns(true); + .onSecondCall().returns(true) + .onThirdCall().returns(false); assert.equal(window.browser.runtime.onMessage.hasListener(messageListener), false, "Got hasListener==false before the listener has been added"); window.browser.runtime.onMessage.addListener(messageListener); assert.equal(window.browser.runtime.onMessage.hasListener(messageListener), - true, "Got hasListener=true once the listener has been added"); + true, "Got hasListener==true once the listener has been added"); window.browser.runtime.onMessage.addListener(messageListener); assert.ok(fakeChrome.runtime.onMessage.addListener.calledTwice, @@ -49,6 +50,8 @@ describe("browser-polyfill", () => { assert.equal(fakeChrome.runtime.onMessage.addListener.secondCall.args[0], fakeChrome.runtime.onMessage.removeListener.firstCall.args[0], "both the addListener and removeListenercalls received the same wrapped listener"); + assert.equal(fakeChrome.runtime.onMessage.hasListener(messageListener), false, + "Got hasListener==false once the listener has been removed"); }); }); From a1469d6f17baaf21093940a486e37976e5141694 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Tue, 11 Oct 2016 14:34:39 +0200 Subject: [PATCH 08/23] fix: added inline comments to special runtime.onMessage wrapper tests. --- test/test-runtime-onMessage.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/test-runtime-onMessage.js b/test/test-runtime-onMessage.js index 26d4640..048dad7 100644 --- a/test/test-runtime-onMessage.js +++ b/test/test-runtime-onMessage.js @@ -23,15 +23,23 @@ describe("browser-polyfill", () => { return setupTestDOMWindow(fakeChrome).then(window => { fakeChrome.runtime.onMessage.hasListener + // Fake the hasListener result for "listener not yet registered". .onFirstCall().returns(false) + // Fake the hasListener result for "listener registered". .onSecondCall().returns(true) + // Fake the hasListener result for "listener unregistered". .onThirdCall().returns(false); assert.equal(window.browser.runtime.onMessage.hasListener(messageListener), false, "Got hasListener==false before the listener has been added"); + window.browser.runtime.onMessage.addListener(messageListener); + assert.equal(window.browser.runtime.onMessage.hasListener(messageListener), true, "Got hasListener==true once the listener has been added"); + + // Add the same listener again to test that it will be called with the + // same wrapped listener. window.browser.runtime.onMessage.addListener(messageListener); assert.ok(fakeChrome.runtime.onMessage.addListener.calledTwice, @@ -40,10 +48,12 @@ describe("browser-polyfill", () => { fakeChrome.runtime.onMessage.addListener.firstCall.args[0], "both the addListener calls received the same wrapped listener"); + // Retrieve the wrapped listener and execute it to fake a received message. const wrappedListener = fakeChrome.runtime.onMessage.addListener.firstCall.args[0]; wrappedListener("msg", {name: "sender"}, function sendResponse() {}); assert.ok(messageListener.calledOnce, "The listener has been called once"); + // Remove the listener. window.browser.runtime.onMessage.removeListener(messageListener); assert.ok(fakeChrome.runtime.onMessage.removeListener.calledOnce, "removeListener has been called once"); From 7a247f56f2af1cec23f4098d8e3391af532ea310 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Tue, 11 Oct 2016 14:47:38 +0200 Subject: [PATCH 09/23] test: tweak the proxied properties tests. --- test/test-proxied-properties.js | 34 ++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/test/test-proxied-properties.js b/test/test-proxied-properties.js index b7f62c2..f969d69 100644 --- a/test/test-proxied-properties.js +++ b/test/test-proxied-properties.js @@ -9,16 +9,48 @@ describe("browser-polyfill", () => { it("should proxy getters and setters", () => { const fakeChrome = { runtime: {myprop: "previous-value"}, - nowrapns: {nowrapkey: "previous-value"}, + nowrapns: { + nowrapkey: "previous-value", + nowrapkey2: "previous-value", + }, }; return setupTestDOMWindow(fakeChrome).then(window => { + // Check that the property values on the generated wrapper. + assert.equal(window.browser.runtime.myprop, "previous-value", + "Got the expected result from setting a wrapped property name"); + assert.equal(window.browser.nowrapns.nowrapkey, "previous-value", + "Got the expected result from setting a wrapped property name"); + + // Update the properties on the generated wrapper. const setResult = window.browser.runtime.myprop = "new-value"; const setResult2 = window.browser.nowrapns.nowrapkey = "new-value"; + // Check the results of setting the new value of the wrapped properties. assert.equal(setResult, "new-value", "Got the expected result from setting a wrapped property name"); assert.equal(setResult2, "new-value", "Got the expected result from setting a wrapped property name"); + + // Verify that the wrapped properties has been updated. + assert.equal(window.browser.runtime.myprop, "new-value", + "Got the expected updated value from the browser property"); + assert.equal(window.browser.nowrapns.nowrapkey, "new-value", + "Got the expected updated value from the browser property"); + + // Verify that the target properties has been updated. + assert.equal(window.chrome.runtime.myprop, "new-value", + "Got the expected updated value on the related chrome property"); + assert.equal(window.chrome.nowrapns.nowrapkey, "new-value", + "Got the expected updated value on the related chrome property"); + + // Set a property multiple times before read. + window.browser.nowrapns.nowrapkey2 = "new-value2"; + window.browser.nowrapns.nowrapkey2 = "new-value3"; + + assert.equal(window.chrome.nowrapns.nowrapkey2, "new-value3", + "Got the expected updated value on the related chrome property"); + assert.equal(window.browser.nowrapns.nowrapkey2, "new-value3", + "Got the expected updated value on the wrapped property"); }); }); From a585b4b07a7ff71edcfd88190be9e4a8e926dbb7 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Tue, 11 Oct 2016 15:08:24 +0200 Subject: [PATCH 10/23] test: added test case for async function rejection, and arguments length validation. --- test/test-async-functions.js | 42 ++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/test/test-async-functions.js b/test/test-async-functions.js index 04f2270..4e5cfb9 100644 --- a/test/test-async-functions.js +++ b/test/test-async-functions.js @@ -44,5 +44,47 @@ describe("browser-polyfill", () => { "Fake runtime.requestUpdateCheck resolved to an array of values"); }); }); + + it("rejects the returned promise if chrome.runtime.lastError is not null", () => { + const fakeChrome = { + runtime: { + lastError: new Error("fake lastError"), + }, + tabs: { + query: sinon.stub(), + }, + }; + + return setupTestDOMWindow(fakeChrome).then(window => { + // Test for single array callback argument. + fakeChrome.tabs.query + .onFirstCall().callsArgWith(1, ["res1", "res2"]); + + return window.browser.tabs.query({active: true}).then( + () => assert.fail("Expected a rejected promise"), + (err) => assert.equal(err, fakeChrome.runtime.lastError, + "Got the expected error in the rejected promise") + ); + }); + }); + + it("throws if the number of arguments are not in the range defined in the metadata", () => { + const fakeChrome = { + runtime: { + lastError: null, + sendMessage: sinon.spy(), + }, + }; + + return setupTestDOMWindow(fakeChrome).then(window => { + assert.throws(() => { + window.browser.runtime.sendMessage(); + }, "Expected at least 1 arguments for sendMessage(), got 0"); + + assert.throws(() => { + window.browser.runtime.sendMessage("0", "1", "2", "3"); + }, "Expected at most 3 arguments for sendMessage(), got 4"); + }); + }); }); }); From 92ccea2e155dbbffb3b20bbe0dbe9f21e26295da Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Tue, 11 Oct 2016 16:35:50 +0200 Subject: [PATCH 11/23] test: tweaks on test helpers (support for 'existent browser global' test). --- test/setup.js | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/test/setup.js b/test/setup.js index 1ee8e94..e17c9db 100644 --- a/test/setup.js +++ b/test/setup.js @@ -4,17 +4,31 @@ const fs = require("fs"); const {createInstrumenter} = require("istanbul-lib-instrument"); const {jsdom, createVirtualConsole} = require("jsdom"); -var virtualConsole = createVirtualConsole().sendTo(console); +var virtualConsole = createVirtualConsole(); + +// Optionally print console logs from the jsdom window. +if (process.env.ENABLE_JSDOM_CONSOLE == "y") { + virtualConsole.sendTo(console); +} // Path to the browser-polyfill script, relative to the current work dir // where mocha is executed. const BROWSER_POLYFILL_PATH = "./dist/browser-polyfill.js"; // Create the jsdom window used to run the tests -const testDOMWindow = jsdom({virtualConsole}).defaultView; +const testDOMWindow = jsdom("", {virtualConsole}).defaultView; global.window = testDOMWindow; -function setupTestDOMWindow(chromeObject) { +// Copy the code coverage of the browser-polyfill script from the jsdom window +// to the nodejs global, where nyc expects to find the code coverage data to +// render in the reports. +after(() => { + if (global.window && process.env.COVERAGE == "y") { + global.__coverage__ = global.window.__coverage__; + } +}); + +function setupTestDOMWindow(chromeObject, browserObject = undefined) { return new Promise((resolve, reject) => { const window = testDOMWindow; @@ -22,9 +36,13 @@ function setupTestDOMWindow(chromeObject) { // browser-polyfill test scenario. window.chrome = chromeObject; - // Set the browser property to undefined. - // TODO: change into `delete window.browser` once tmpvar/jsdom#1622 has been fixed. - window.browser = undefined; + // Set (or reset) the browser property. + if (browserObject) { + window.browser = browserObject; + } else { + // TODO: change into `delete window.browser` once tmpvar/jsdom#1622 has been fixed. + window.browser = undefined; + } const scriptEl = window.document.createElement("script"); @@ -68,15 +86,6 @@ function setupTestDOMWindow(chromeObject) { }); } -// Copy the code coverage of the browser-polyfill script from the jsdom window -// to the nodejs global, where nyc expects to find the code coverage data to -// render in the reports. -after(() => { - if (global.window && process.env.COVERAGE == "y") { - global.__coverage__ = global.window.__coverage__; - } -}); - module.exports = { BROWSER_POLYFILL_PATH, setupTestDOMWindow, From 6cca044a5c931ad66032b834008f42041f12e0e1 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Tue, 11 Oct 2016 16:39:13 +0200 Subject: [PATCH 12/23] test: add more test cases to reach a full code coverage. - if the a browser global already exists, it should not be overridden - use Object.defineProperty on the wrapped browser global (and test "has" for cached properties) - delete a property defined with Object.defineProperty - methods that are not wrapped are proxied correctly - the special onMessage wrapper should not wrap a listener that is not a function - test that a returned rejected Promise on the onMessage listener is turned in the parameter of the sendResponse callback --- test/test-browser-global.js | 51 ++++++++++++++++++++++++++++ test/test-proxied-properties.js | 25 +++++++++++++- test/test-runtime-onMessage.js | 60 +++++++++++++++++++++++++++++---- 3 files changed, 129 insertions(+), 7 deletions(-) diff --git a/test/test-browser-global.js b/test/test-browser-global.js index 4bebb17..cc9f56e 100644 --- a/test/test-browser-global.js +++ b/test/test-browser-global.js @@ -11,4 +11,55 @@ describe("browser-polyfill", () => { assert.equal(typeof window.browser, "object", "Got the window.browser object"); }); }); + + it("do not override the global browser namespace if it already exists", () => { + const fakeChrome = { + runtime: {lastError: null}, + }; + const fakeBrowser = { + mycustomns: {mykey: true}, + }; + + return setupTestDOMWindow(fakeChrome, fakeBrowser).then(window => { + assert.deepEqual(window.browser, fakeBrowser, + "The existent browser has not been wrapped"); + }); + }); + + describe("browser wrapper", () => { + it("supports custom properties defined using Object.defineProperty", () => { + const fakeChrome = {}; + return setupTestDOMWindow(fakeChrome).then(window => { + Object.defineProperty(window.browser, "myns", { + enumerable: true, + configurable: true, + value: {mykey: true}, + }); + + assert.ok("myns" in window.browser, "The custom property exists"); + assert.ok("mykey" in window.browser.myns, + "The content of the custom property exists"); + + assert.deepEqual(window.browser.myns, {mykey: true}, + "The custom property has the expected content"); + + delete window.browser.myns; + + assert.ok(!("myns" in window.browser), + "The deleted custom defined property has been removed"); + }); + }); + + it("returns undefined for property undefined in the target", () => { + const fakeChrome = {myns: {mykey: true}}; + return setupTestDOMWindow(fakeChrome).then(window => { + assert.equal(window.browser.myns.mykey, true, + "Got the expected result from a wrapped property"); + assert.equal(window.browser.myns.nonexistent, undefined, + "Got undefined for non existent property"); + assert.equal(window.browser.nonexistent, undefined, + "Got undefined for non existent namespaces"); + }); + }); + }); }); diff --git a/test/test-proxied-properties.js b/test/test-proxied-properties.js index f969d69..867ddf6 100644 --- a/test/test-proxied-properties.js +++ b/test/test-proxied-properties.js @@ -1,11 +1,31 @@ "use strict"; const {assert} = require("chai"); +const sinon = require("sinon"); const {setupTestDOMWindow} = require("./setup"); describe("browser-polyfill", () => { describe("proxies non-wrapped functions", () => { + it("should proxy non-wrapped methods", () => { + const fakeChrome = { + runtime: { + nonwrappedmethod: sinon.spy(), + }, + }; + return setupTestDOMWindow(fakeChrome).then(window => { + assert.ok(window.browser.runtime.nonwrappedmethod); + + const fakeCallback = () => {}; + window.browser.runtime.nonwrappedmethod(fakeCallback); + + const receivedCallback = fakeChrome.runtime.nonwrappedmethod.firstCall.args[0]; + + assert.equal(fakeCallback, receivedCallback, + "The callback has not been wrapped for the nonwrappedmethod"); + }); + }); + it("should proxy getters and setters", () => { const fakeChrome = { runtime: {myprop: "previous-value"}, @@ -58,6 +78,10 @@ describe("browser-polyfill", () => { const fakeChrome = {}; return setupTestDOMWindow(fakeChrome).then(window => { window.browser.newns = {newkey: "test-value"}; + + assert.ok("newns" in window.browser, "The custom namespace is in the wrapper"); + assert.ok("newns" in window.chrome, "The custom namespace is in the target"); + assert.equal(window.browser.newns.newkey, "test-value", "Got the expected result from setting a wrapped property name"); @@ -69,7 +93,6 @@ describe("browser-polyfill", () => { assert.deepEqual(window.browser.newns, window.chrome.newns, "chrome.newns and browser.newns are the same"); - delete window.browser.newns.newkey2; assert.equal(window.browser.newns.newkey2, undefined, "Got the expected result from setting a wrapped property name"); diff --git a/test/test-runtime-onMessage.js b/test/test-runtime-onMessage.js index 048dad7..54085d1 100644 --- a/test/test-runtime-onMessage.js +++ b/test/test-runtime-onMessage.js @@ -7,6 +7,29 @@ const {setupTestDOMWindow} = require("./setup"); describe("browser-polyfill", () => { describe("wrapped runtime.onMessage listener", () => { + it("do not wrap the listener if it is not a function", () => { + const fakeChrome = { + runtime: { + lastError: null, + onMessage: { + addListener: sinon.spy(), + hasListener: sinon.stub(), + removeListener: sinon.spy(), + }, + }, + }; + + return setupTestDOMWindow(fakeChrome).then(window => { + const fakeNonFunctionListener = {fake: "non function listener"}; + + window.browser.runtime.onMessage.addListener(fakeNonFunctionListener); + + assert.deepEqual(fakeChrome.runtime.onMessage.addListener.firstCall.args[0], + fakeNonFunctionListener, + "The non-function listener has not been wrapped"); + }); + }); + it("keeps track of the listeners added", () => { const messageListener = sinon.spy(); @@ -75,20 +98,29 @@ describe("browser-polyfill", () => { }, }; + // Plain value returned. const messageListener = sinon.stub(); const firstResponse = "fake reply"; - const secondResponse = Promise.resolve("fake reply2"); + // Resolved Promise returned. + const secondResponse = Promise.resolve("fake reply 2"); + // Rejected Promise returned. + const thirdResponse = Promise.reject("fake error 3"); + const sendResponseSpy = sinon.spy(); - messageListener.onFirstCall().returns(firstResponse) - .onSecondCall().returns(secondResponse); + messageListener + .onFirstCall().returns(firstResponse) + .onSecondCall().returns(secondResponse) + .onThirdCall().returns(thirdResponse); + + let wrappedListener; return setupTestDOMWindow(fakeChrome).then(window => { window.browser.runtime.onMessage.addListener(messageListener); assert.ok(fakeChrome.runtime.onMessage.addListener.calledOnce); - const wrappedListener = fakeChrome.runtime.onMessage.addListener.firstCall.args[0]; + wrappedListener = fakeChrome.runtime.onMessage.addListener.firstCall.args[0]; wrappedListener("fake message", {name: "fake sender"}, sendResponseSpy); @@ -110,11 +142,27 @@ describe("browser-polyfill", () => { "The unwrapped message listener has been called"); assert.deepEqual(messageListener.secondCall.args, ["fake message2", {name: "fake sender2"}], - "The unwrapped message listener has received the expected parameters"); + "The unwrapped listener has received the expected parameters"); assert.ok(sendResponseSpy.calledTwice, "The sendResponse function has been called"); - assert.equal(sendResponseSpy.secondCall.args[0], "fake reply2", + assert.equal(sendResponseSpy.secondCall.args[0], "fake reply 2", "sendResponse callback has been called with the expected parameters"); + }).then(() => { + wrappedListener("fake message3", {name: "fake sender3"}, sendResponseSpy); + + // Wait the third response promise to be rejected. + return thirdResponse.catch(err => { + assert.equal(messageListener.callCount, 3, + "The unwrapped message listener has been called"); + assert.deepEqual(messageListener.thirdCall.args, + ["fake message3", {name: "fake sender3"}], + "The unwrapped listener has received the expected parameters"); + + assert.equal(sendResponseSpy.callCount, 3, + "The sendResponse function has been called"); + assert.equal(sendResponseSpy.thirdCall.args[0], err, + "sendResponse callback has been called with the expected parameters"); + }); }); }); }); From 2853e98546568a17530e12abe460daeaab96e8bb Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Wed, 12 Oct 2016 02:03:25 +0200 Subject: [PATCH 13/23] fix: the test jsdom window doesn't need to be in the shared global. --- test/setup.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/setup.js b/test/setup.js index e17c9db..de6055c 100644 --- a/test/setup.js +++ b/test/setup.js @@ -17,14 +17,13 @@ const BROWSER_POLYFILL_PATH = "./dist/browser-polyfill.js"; // Create the jsdom window used to run the tests const testDOMWindow = jsdom("", {virtualConsole}).defaultView; -global.window = testDOMWindow; // Copy the code coverage of the browser-polyfill script from the jsdom window // to the nodejs global, where nyc expects to find the code coverage data to // render in the reports. after(() => { - if (global.window && process.env.COVERAGE == "y") { - global.__coverage__ = global.window.__coverage__; + if (testDOMWindow && process.env.COVERAGE == "y") { + global.__coverage__ = testDOMWindow.__coverage__; } }); From 3842bd16936878697809e7c6dfdbb273667497d5 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Wed, 12 Oct 2016 02:08:52 +0200 Subject: [PATCH 14/23] style: destructure the assert methods to globals --- test/test-async-functions.js | 16 ++++++------ test/test-browser-global.js | 20 +++++++------- test/test-proxied-properties.js | 42 +++++++++++++++--------------- test/test-runtime-onMessage.js | 46 ++++++++++++++++----------------- 4 files changed, 62 insertions(+), 62 deletions(-) diff --git a/test/test-async-functions.js b/test/test-async-functions.js index 4e5cfb9..7f14fa4 100644 --- a/test/test-async-functions.js +++ b/test/test-async-functions.js @@ -1,6 +1,6 @@ "use strict"; -const {assert} = require("chai"); +const {deepEqual, equal, fail, throws} = require("chai").assert; const sinon = require("sinon"); const {setupTestDOMWindow} = require("./setup"); @@ -37,10 +37,10 @@ describe("browser-polyfill", () => { window.browser.runtime.requestUpdateCheck(), ]); }).then(results => { - assert.equal(results[0], "res1", "Fake alarms.clear call resolved to a single value"); - assert.deepEqual(results[1], ["res1", "res2"], + equal(results[0], "res1", "Fake alarms.clear call resolved to a single value"); + deepEqual(results[1], ["res1", "res2"], "Fake tabs.query resolved to an array of values"); - assert.deepEqual(results[2], ["res1", "res2"], + deepEqual(results[2], ["res1", "res2"], "Fake runtime.requestUpdateCheck resolved to an array of values"); }); }); @@ -61,8 +61,8 @@ describe("browser-polyfill", () => { .onFirstCall().callsArgWith(1, ["res1", "res2"]); return window.browser.tabs.query({active: true}).then( - () => assert.fail("Expected a rejected promise"), - (err) => assert.equal(err, fakeChrome.runtime.lastError, + () => fail("Expected a rejected promise"), + (err) => equal(err, fakeChrome.runtime.lastError, "Got the expected error in the rejected promise") ); }); @@ -77,11 +77,11 @@ describe("browser-polyfill", () => { }; return setupTestDOMWindow(fakeChrome).then(window => { - assert.throws(() => { + throws(() => { window.browser.runtime.sendMessage(); }, "Expected at least 1 arguments for sendMessage(), got 0"); - assert.throws(() => { + throws(() => { window.browser.runtime.sendMessage("0", "1", "2", "3"); }, "Expected at most 3 arguments for sendMessage(), got 4"); }); diff --git a/test/test-browser-global.js b/test/test-browser-global.js index cc9f56e..184ff65 100644 --- a/test/test-browser-global.js +++ b/test/test-browser-global.js @@ -1,6 +1,6 @@ "use strict"; -const {assert} = require("chai"); +const {deepEqual, equal, ok} = require("chai").assert; const {setupTestDOMWindow} = require("./setup"); @@ -8,7 +8,7 @@ describe("browser-polyfill", () => { it("wraps the global chrome namespace with a global browser namespace", () => { const fakeChrome = {}; return setupTestDOMWindow(fakeChrome).then(window => { - assert.equal(typeof window.browser, "object", "Got the window.browser object"); + equal(typeof window.browser, "object", "Got the window.browser object"); }); }); @@ -21,7 +21,7 @@ describe("browser-polyfill", () => { }; return setupTestDOMWindow(fakeChrome, fakeBrowser).then(window => { - assert.deepEqual(window.browser, fakeBrowser, + deepEqual(window.browser, fakeBrowser, "The existent browser has not been wrapped"); }); }); @@ -36,16 +36,16 @@ describe("browser-polyfill", () => { value: {mykey: true}, }); - assert.ok("myns" in window.browser, "The custom property exists"); - assert.ok("mykey" in window.browser.myns, + ok("myns" in window.browser, "The custom property exists"); + ok("mykey" in window.browser.myns, "The content of the custom property exists"); - assert.deepEqual(window.browser.myns, {mykey: true}, + deepEqual(window.browser.myns, {mykey: true}, "The custom property has the expected content"); delete window.browser.myns; - assert.ok(!("myns" in window.browser), + ok(!("myns" in window.browser), "The deleted custom defined property has been removed"); }); }); @@ -53,11 +53,11 @@ describe("browser-polyfill", () => { it("returns undefined for property undefined in the target", () => { const fakeChrome = {myns: {mykey: true}}; return setupTestDOMWindow(fakeChrome).then(window => { - assert.equal(window.browser.myns.mykey, true, + equal(window.browser.myns.mykey, true, "Got the expected result from a wrapped property"); - assert.equal(window.browser.myns.nonexistent, undefined, + equal(window.browser.myns.nonexistent, undefined, "Got undefined for non existent property"); - assert.equal(window.browser.nonexistent, undefined, + equal(window.browser.nonexistent, undefined, "Got undefined for non existent namespaces"); }); }); diff --git a/test/test-proxied-properties.js b/test/test-proxied-properties.js index 867ddf6..b033883 100644 --- a/test/test-proxied-properties.js +++ b/test/test-proxied-properties.js @@ -1,6 +1,6 @@ "use strict"; -const {assert} = require("chai"); +const {deepEqual, equal, ok} = require("chai").assert; const sinon = require("sinon"); const {setupTestDOMWindow} = require("./setup"); @@ -14,14 +14,14 @@ describe("browser-polyfill", () => { }, }; return setupTestDOMWindow(fakeChrome).then(window => { - assert.ok(window.browser.runtime.nonwrappedmethod); + ok(window.browser.runtime.nonwrappedmethod); const fakeCallback = () => {}; window.browser.runtime.nonwrappedmethod(fakeCallback); const receivedCallback = fakeChrome.runtime.nonwrappedmethod.firstCall.args[0]; - assert.equal(fakeCallback, receivedCallback, + equal(fakeCallback, receivedCallback, "The callback has not been wrapped for the nonwrappedmethod"); }); }); @@ -36,9 +36,9 @@ describe("browser-polyfill", () => { }; return setupTestDOMWindow(fakeChrome).then(window => { // Check that the property values on the generated wrapper. - assert.equal(window.browser.runtime.myprop, "previous-value", + equal(window.browser.runtime.myprop, "previous-value", "Got the expected result from setting a wrapped property name"); - assert.equal(window.browser.nowrapns.nowrapkey, "previous-value", + equal(window.browser.nowrapns.nowrapkey, "previous-value", "Got the expected result from setting a wrapped property name"); // Update the properties on the generated wrapper. @@ -46,30 +46,30 @@ describe("browser-polyfill", () => { const setResult2 = window.browser.nowrapns.nowrapkey = "new-value"; // Check the results of setting the new value of the wrapped properties. - assert.equal(setResult, "new-value", + equal(setResult, "new-value", "Got the expected result from setting a wrapped property name"); - assert.equal(setResult2, "new-value", + equal(setResult2, "new-value", "Got the expected result from setting a wrapped property name"); // Verify that the wrapped properties has been updated. - assert.equal(window.browser.runtime.myprop, "new-value", + equal(window.browser.runtime.myprop, "new-value", "Got the expected updated value from the browser property"); - assert.equal(window.browser.nowrapns.nowrapkey, "new-value", + equal(window.browser.nowrapns.nowrapkey, "new-value", "Got the expected updated value from the browser property"); // Verify that the target properties has been updated. - assert.equal(window.chrome.runtime.myprop, "new-value", + equal(window.chrome.runtime.myprop, "new-value", "Got the expected updated value on the related chrome property"); - assert.equal(window.chrome.nowrapns.nowrapkey, "new-value", + equal(window.chrome.nowrapns.nowrapkey, "new-value", "Got the expected updated value on the related chrome property"); // Set a property multiple times before read. window.browser.nowrapns.nowrapkey2 = "new-value2"; window.browser.nowrapns.nowrapkey2 = "new-value3"; - assert.equal(window.chrome.nowrapns.nowrapkey2, "new-value3", + equal(window.chrome.nowrapns.nowrapkey2, "new-value3", "Got the expected updated value on the related chrome property"); - assert.equal(window.browser.nowrapns.nowrapkey2, "new-value3", + equal(window.browser.nowrapns.nowrapkey2, "new-value3", "Got the expected updated value on the wrapped property"); }); }); @@ -79,24 +79,24 @@ describe("browser-polyfill", () => { return setupTestDOMWindow(fakeChrome).then(window => { window.browser.newns = {newkey: "test-value"}; - assert.ok("newns" in window.browser, "The custom namespace is in the wrapper"); - assert.ok("newns" in window.chrome, "The custom namespace is in the target"); + ok("newns" in window.browser, "The custom namespace is in the wrapper"); + ok("newns" in window.chrome, "The custom namespace is in the target"); - assert.equal(window.browser.newns.newkey, "test-value", + equal(window.browser.newns.newkey, "test-value", "Got the expected result from setting a wrapped property name"); const setRes = window.browser.newns = {newkey2: "new-value"}; - assert.equal(window.browser.newns.newkey2, "new-value", + equal(window.browser.newns.newkey2, "new-value", "The new non-wrapped getter is cached"); - assert.deepEqual(setRes, {newkey2: "new-value"}, + deepEqual(setRes, {newkey2: "new-value"}, "Got the expected result from setting a new wrapped property name"); - assert.deepEqual(window.browser.newns, window.chrome.newns, + deepEqual(window.browser.newns, window.chrome.newns, "chrome.newns and browser.newns are the same"); delete window.browser.newns.newkey2; - assert.equal(window.browser.newns.newkey2, undefined, + equal(window.browser.newns.newkey2, undefined, "Got the expected result from setting a wrapped property name"); - assert.ok(!("newkey2" in window.browser.newns), + ok(!("newkey2" in window.browser.newns), "The deleted property is not listed anymore"); }); }); diff --git a/test/test-runtime-onMessage.js b/test/test-runtime-onMessage.js index 54085d1..8fca603 100644 --- a/test/test-runtime-onMessage.js +++ b/test/test-runtime-onMessage.js @@ -1,6 +1,6 @@ "use strict"; -const {assert} = require("chai"); +const {deepEqual, equal, ok} = require("chai").assert; const sinon = require("sinon"); const {setupTestDOMWindow} = require("./setup"); @@ -24,7 +24,7 @@ describe("browser-polyfill", () => { window.browser.runtime.onMessage.addListener(fakeNonFunctionListener); - assert.deepEqual(fakeChrome.runtime.onMessage.addListener.firstCall.args[0], + deepEqual(fakeChrome.runtime.onMessage.addListener.firstCall.args[0], fakeNonFunctionListener, "The non-function listener has not been wrapped"); }); @@ -53,37 +53,37 @@ describe("browser-polyfill", () => { // Fake the hasListener result for "listener unregistered". .onThirdCall().returns(false); - assert.equal(window.browser.runtime.onMessage.hasListener(messageListener), + equal(window.browser.runtime.onMessage.hasListener(messageListener), false, "Got hasListener==false before the listener has been added"); window.browser.runtime.onMessage.addListener(messageListener); - assert.equal(window.browser.runtime.onMessage.hasListener(messageListener), + equal(window.browser.runtime.onMessage.hasListener(messageListener), true, "Got hasListener==true once the listener has been added"); // Add the same listener again to test that it will be called with the // same wrapped listener. window.browser.runtime.onMessage.addListener(messageListener); - assert.ok(fakeChrome.runtime.onMessage.addListener.calledTwice, + ok(fakeChrome.runtime.onMessage.addListener.calledTwice, "addListener has been called twice"); - assert.equal(fakeChrome.runtime.onMessage.addListener.secondCall.args[0], + equal(fakeChrome.runtime.onMessage.addListener.secondCall.args[0], fakeChrome.runtime.onMessage.addListener.firstCall.args[0], "both the addListener calls received the same wrapped listener"); // Retrieve the wrapped listener and execute it to fake a received message. const wrappedListener = fakeChrome.runtime.onMessage.addListener.firstCall.args[0]; wrappedListener("msg", {name: "sender"}, function sendResponse() {}); - assert.ok(messageListener.calledOnce, "The listener has been called once"); + ok(messageListener.calledOnce, "The listener has been called once"); // Remove the listener. window.browser.runtime.onMessage.removeListener(messageListener); - assert.ok(fakeChrome.runtime.onMessage.removeListener.calledOnce, + ok(fakeChrome.runtime.onMessage.removeListener.calledOnce, "removeListener has been called once"); - assert.equal(fakeChrome.runtime.onMessage.addListener.secondCall.args[0], + equal(fakeChrome.runtime.onMessage.addListener.secondCall.args[0], fakeChrome.runtime.onMessage.removeListener.firstCall.args[0], "both the addListener and removeListenercalls received the same wrapped listener"); - assert.equal(fakeChrome.runtime.onMessage.hasListener(messageListener), false, + equal(fakeChrome.runtime.onMessage.hasListener(messageListener), false, "Got hasListener==false once the listener has been removed"); }); }); @@ -118,19 +118,19 @@ describe("browser-polyfill", () => { return setupTestDOMWindow(fakeChrome).then(window => { window.browser.runtime.onMessage.addListener(messageListener); - assert.ok(fakeChrome.runtime.onMessage.addListener.calledOnce); + ok(fakeChrome.runtime.onMessage.addListener.calledOnce); wrappedListener = fakeChrome.runtime.onMessage.addListener.firstCall.args[0]; wrappedListener("fake message", {name: "fake sender"}, sendResponseSpy); - assert.ok(messageListener.calledOnce, "The unwrapped message listener has been called"); - assert.deepEqual(messageListener.firstCall.args, + ok(messageListener.calledOnce, "The unwrapped message listener has been called"); + deepEqual(messageListener.firstCall.args, ["fake message", {name: "fake sender"}], "The unwrapped message listener has received the expected parameters"); - assert.ok(sendResponseSpy.calledOnce, "The sendResponse function has been called"); - assert.equal(sendResponseSpy.firstCall.args[0], "fake reply", + ok(sendResponseSpy.calledOnce, "The sendResponse function has been called"); + equal(sendResponseSpy.firstCall.args[0], "fake reply", "sendResponse callback has been called with the expected parameters"); wrappedListener("fake message2", {name: "fake sender2"}, sendResponseSpy); @@ -138,29 +138,29 @@ describe("browser-polyfill", () => { // Wait the second response promise to be resolved. return secondResponse; }).then(() => { - assert.ok(messageListener.calledTwice, + ok(messageListener.calledTwice, "The unwrapped message listener has been called"); - assert.deepEqual(messageListener.secondCall.args, + deepEqual(messageListener.secondCall.args, ["fake message2", {name: "fake sender2"}], "The unwrapped listener has received the expected parameters"); - assert.ok(sendResponseSpy.calledTwice, "The sendResponse function has been called"); - assert.equal(sendResponseSpy.secondCall.args[0], "fake reply 2", + ok(sendResponseSpy.calledTwice, "The sendResponse function has been called"); + equal(sendResponseSpy.secondCall.args[0], "fake reply 2", "sendResponse callback has been called with the expected parameters"); }).then(() => { wrappedListener("fake message3", {name: "fake sender3"}, sendResponseSpy); // Wait the third response promise to be rejected. return thirdResponse.catch(err => { - assert.equal(messageListener.callCount, 3, + equal(messageListener.callCount, 3, "The unwrapped message listener has been called"); - assert.deepEqual(messageListener.thirdCall.args, + deepEqual(messageListener.thirdCall.args, ["fake message3", {name: "fake sender3"}], "The unwrapped listener has received the expected parameters"); - assert.equal(sendResponseSpy.callCount, 3, + equal(sendResponseSpy.callCount, 3, "The sendResponse function has been called"); - assert.equal(sendResponseSpy.thirdCall.args[0], err, + equal(sendResponseSpy.thirdCall.args[0], err, "sendResponse callback has been called with the expected parameters"); }); }); From 0517edaa67669aed1aa038295d8455f1d29dab40 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Wed, 12 Oct 2016 02:17:55 +0200 Subject: [PATCH 15/23] fix: refactor runtime.onMessage tests. --- test/test-runtime-onMessage.js | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/test/test-runtime-onMessage.js b/test/test-runtime-onMessage.js index 8fca603..2f9146b 100644 --- a/test/test-runtime-onMessage.js +++ b/test/test-runtime-onMessage.js @@ -32,27 +32,23 @@ describe("browser-polyfill", () => { it("keeps track of the listeners added", () => { const messageListener = sinon.spy(); - + const fakeChromeListeners = new Set(); const fakeChrome = { runtime: { lastError: null, onMessage: { - addListener: sinon.spy(), - hasListener: sinon.stub(), - removeListener: sinon.spy(), + addListener: sinon.spy((listener, ...args) => { + fakeChromeListeners.add(listener); + }), + hasListener: sinon.spy(listener => fakeChromeListeners.has(listener)), + removeListener: sinon.spy(listener => { + fakeChromeListeners.delete(listener); + }), }, }, }; return setupTestDOMWindow(fakeChrome).then(window => { - fakeChrome.runtime.onMessage.hasListener - // Fake the hasListener result for "listener not yet registered". - .onFirstCall().returns(false) - // Fake the hasListener result for "listener registered". - .onSecondCall().returns(true) - // Fake the hasListener result for "listener unregistered". - .onThirdCall().returns(false); - equal(window.browser.runtime.onMessage.hasListener(messageListener), false, "Got hasListener==false before the listener has been added"); From de50f2047dc51e2c57d923290ae088cb80b1acbb Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Wed, 2 Nov 2016 20:38:14 +0100 Subject: [PATCH 16/23] test: update eslint to version 3.9.1 and add enforced indentation on call expression params --- .eslintrc | 6 +++++- package.json | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.eslintrc b/.eslintrc index 239f70e..f4e2e2d 100644 --- a/.eslintrc +++ b/.eslintrc @@ -69,7 +69,11 @@ "generator-star-spacing": [2, {"before": false, "after": true}], // Two space indent - "indent": [2, 2, {"SwitchCase": 1}], + "indent": [2, 2, { + "SwitchCase": 1, + "CallExpression": {"arguments": "first"} + } + ], // Space after colon not before in property declarations "key-spacing": [2, {"beforeColon": false, "afterColon": true, "mode": "minimum"}], diff --git a/package.json b/package.json index 6809d80..a3debed 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,7 @@ "grunt-coveralls": "^1.0.1", "grunt-replace": "*", "gruntify-eslint": "*", + "eslint": "3.9.1", "istanbul-lib-instrument": "^1.1.3", "jsdom": "^9.6.0", "mocha": "^3.1.0", From 6486e551beed08f1367552aa491adbfc7a571084 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Wed, 2 Nov 2016 20:39:07 +0100 Subject: [PATCH 17/23] fix: bulk fix call expression params indentation using eslint --- test/test-async-functions.js | 6 ++--- test/test-browser-global.js | 14 ++++++------ test/test-proxied-properties.js | 34 ++++++++++++++-------------- test/test-runtime-onMessage.js | 40 ++++++++++++++++----------------- 4 files changed, 47 insertions(+), 47 deletions(-) diff --git a/test/test-async-functions.js b/test/test-async-functions.js index 7f14fa4..5c8e8de 100644 --- a/test/test-async-functions.js +++ b/test/test-async-functions.js @@ -39,9 +39,9 @@ describe("browser-polyfill", () => { }).then(results => { equal(results[0], "res1", "Fake alarms.clear call resolved to a single value"); deepEqual(results[1], ["res1", "res2"], - "Fake tabs.query resolved to an array of values"); + "Fake tabs.query resolved to an array of values"); deepEqual(results[2], ["res1", "res2"], - "Fake runtime.requestUpdateCheck resolved to an array of values"); + "Fake runtime.requestUpdateCheck resolved to an array of values"); }); }); @@ -63,7 +63,7 @@ describe("browser-polyfill", () => { return window.browser.tabs.query({active: true}).then( () => fail("Expected a rejected promise"), (err) => equal(err, fakeChrome.runtime.lastError, - "Got the expected error in the rejected promise") + "Got the expected error in the rejected promise") ); }); }); diff --git a/test/test-browser-global.js b/test/test-browser-global.js index 184ff65..b21495d 100644 --- a/test/test-browser-global.js +++ b/test/test-browser-global.js @@ -22,7 +22,7 @@ describe("browser-polyfill", () => { return setupTestDOMWindow(fakeChrome, fakeBrowser).then(window => { deepEqual(window.browser, fakeBrowser, - "The existent browser has not been wrapped"); + "The existent browser has not been wrapped"); }); }); @@ -38,15 +38,15 @@ describe("browser-polyfill", () => { ok("myns" in window.browser, "The custom property exists"); ok("mykey" in window.browser.myns, - "The content of the custom property exists"); + "The content of the custom property exists"); deepEqual(window.browser.myns, {mykey: true}, - "The custom property has the expected content"); + "The custom property has the expected content"); delete window.browser.myns; ok(!("myns" in window.browser), - "The deleted custom defined property has been removed"); + "The deleted custom defined property has been removed"); }); }); @@ -54,11 +54,11 @@ describe("browser-polyfill", () => { const fakeChrome = {myns: {mykey: true}}; return setupTestDOMWindow(fakeChrome).then(window => { equal(window.browser.myns.mykey, true, - "Got the expected result from a wrapped property"); + "Got the expected result from a wrapped property"); equal(window.browser.myns.nonexistent, undefined, - "Got undefined for non existent property"); + "Got undefined for non existent property"); equal(window.browser.nonexistent, undefined, - "Got undefined for non existent namespaces"); + "Got undefined for non existent namespaces"); }); }); }); diff --git a/test/test-proxied-properties.js b/test/test-proxied-properties.js index b033883..589d0f1 100644 --- a/test/test-proxied-properties.js +++ b/test/test-proxied-properties.js @@ -22,7 +22,7 @@ describe("browser-polyfill", () => { const receivedCallback = fakeChrome.runtime.nonwrappedmethod.firstCall.args[0]; equal(fakeCallback, receivedCallback, - "The callback has not been wrapped for the nonwrappedmethod"); + "The callback has not been wrapped for the nonwrappedmethod"); }); }); @@ -37,9 +37,9 @@ describe("browser-polyfill", () => { return setupTestDOMWindow(fakeChrome).then(window => { // Check that the property values on the generated wrapper. equal(window.browser.runtime.myprop, "previous-value", - "Got the expected result from setting a wrapped property name"); + "Got the expected result from setting a wrapped property name"); equal(window.browser.nowrapns.nowrapkey, "previous-value", - "Got the expected result from setting a wrapped property name"); + "Got the expected result from setting a wrapped property name"); // Update the properties on the generated wrapper. const setResult = window.browser.runtime.myprop = "new-value"; @@ -47,30 +47,30 @@ describe("browser-polyfill", () => { // Check the results of setting the new value of the wrapped properties. equal(setResult, "new-value", - "Got the expected result from setting a wrapped property name"); + "Got the expected result from setting a wrapped property name"); equal(setResult2, "new-value", - "Got the expected result from setting a wrapped property name"); + "Got the expected result from setting a wrapped property name"); // Verify that the wrapped properties has been updated. equal(window.browser.runtime.myprop, "new-value", - "Got the expected updated value from the browser property"); + "Got the expected updated value from the browser property"); equal(window.browser.nowrapns.nowrapkey, "new-value", - "Got the expected updated value from the browser property"); + "Got the expected updated value from the browser property"); // Verify that the target properties has been updated. equal(window.chrome.runtime.myprop, "new-value", - "Got the expected updated value on the related chrome property"); + "Got the expected updated value on the related chrome property"); equal(window.chrome.nowrapns.nowrapkey, "new-value", - "Got the expected updated value on the related chrome property"); + "Got the expected updated value on the related chrome property"); // Set a property multiple times before read. window.browser.nowrapns.nowrapkey2 = "new-value2"; window.browser.nowrapns.nowrapkey2 = "new-value3"; equal(window.chrome.nowrapns.nowrapkey2, "new-value3", - "Got the expected updated value on the related chrome property"); + "Got the expected updated value on the related chrome property"); equal(window.browser.nowrapns.nowrapkey2, "new-value3", - "Got the expected updated value on the wrapped property"); + "Got the expected updated value on the wrapped property"); }); }); @@ -83,21 +83,21 @@ describe("browser-polyfill", () => { ok("newns" in window.chrome, "The custom namespace is in the target"); equal(window.browser.newns.newkey, "test-value", - "Got the expected result from setting a wrapped property name"); + "Got the expected result from setting a wrapped property name"); const setRes = window.browser.newns = {newkey2: "new-value"}; equal(window.browser.newns.newkey2, "new-value", - "The new non-wrapped getter is cached"); + "The new non-wrapped getter is cached"); deepEqual(setRes, {newkey2: "new-value"}, - "Got the expected result from setting a new wrapped property name"); + "Got the expected result from setting a new wrapped property name"); deepEqual(window.browser.newns, window.chrome.newns, - "chrome.newns and browser.newns are the same"); + "chrome.newns and browser.newns are the same"); delete window.browser.newns.newkey2; equal(window.browser.newns.newkey2, undefined, - "Got the expected result from setting a wrapped property name"); + "Got the expected result from setting a wrapped property name"); ok(!("newkey2" in window.browser.newns), - "The deleted property is not listed anymore"); + "The deleted property is not listed anymore"); }); }); }); diff --git a/test/test-runtime-onMessage.js b/test/test-runtime-onMessage.js index 2f9146b..5f178fa 100644 --- a/test/test-runtime-onMessage.js +++ b/test/test-runtime-onMessage.js @@ -25,8 +25,8 @@ describe("browser-polyfill", () => { window.browser.runtime.onMessage.addListener(fakeNonFunctionListener); deepEqual(fakeChrome.runtime.onMessage.addListener.firstCall.args[0], - fakeNonFunctionListener, - "The non-function listener has not been wrapped"); + fakeNonFunctionListener, + "The non-function listener has not been wrapped"); }); }); @@ -50,22 +50,22 @@ describe("browser-polyfill", () => { return setupTestDOMWindow(fakeChrome).then(window => { equal(window.browser.runtime.onMessage.hasListener(messageListener), - false, "Got hasListener==false before the listener has been added"); + false, "Got hasListener==false before the listener has been added"); window.browser.runtime.onMessage.addListener(messageListener); equal(window.browser.runtime.onMessage.hasListener(messageListener), - true, "Got hasListener==true once the listener has been added"); + true, "Got hasListener==true once the listener has been added"); // Add the same listener again to test that it will be called with the // same wrapped listener. window.browser.runtime.onMessage.addListener(messageListener); ok(fakeChrome.runtime.onMessage.addListener.calledTwice, - "addListener has been called twice"); + "addListener has been called twice"); equal(fakeChrome.runtime.onMessage.addListener.secondCall.args[0], - fakeChrome.runtime.onMessage.addListener.firstCall.args[0], - "both the addListener calls received the same wrapped listener"); + fakeChrome.runtime.onMessage.addListener.firstCall.args[0], + "both the addListener calls received the same wrapped listener"); // Retrieve the wrapped listener and execute it to fake a received message. const wrappedListener = fakeChrome.runtime.onMessage.addListener.firstCall.args[0]; @@ -75,12 +75,12 @@ describe("browser-polyfill", () => { // Remove the listener. window.browser.runtime.onMessage.removeListener(messageListener); ok(fakeChrome.runtime.onMessage.removeListener.calledOnce, - "removeListener has been called once"); + "removeListener has been called once"); equal(fakeChrome.runtime.onMessage.addListener.secondCall.args[0], - fakeChrome.runtime.onMessage.removeListener.firstCall.args[0], - "both the addListener and removeListenercalls received the same wrapped listener"); + fakeChrome.runtime.onMessage.removeListener.firstCall.args[0], + "both the addListener and removeListenercalls received the same wrapped listener"); equal(fakeChrome.runtime.onMessage.hasListener(messageListener), false, - "Got hasListener==false once the listener has been removed"); + "Got hasListener==false once the listener has been removed"); }); }); @@ -123,11 +123,11 @@ describe("browser-polyfill", () => { ok(messageListener.calledOnce, "The unwrapped message listener has been called"); deepEqual(messageListener.firstCall.args, ["fake message", {name: "fake sender"}], - "The unwrapped message listener has received the expected parameters"); + "The unwrapped message listener has received the expected parameters"); ok(sendResponseSpy.calledOnce, "The sendResponse function has been called"); equal(sendResponseSpy.firstCall.args[0], "fake reply", - "sendResponse callback has been called with the expected parameters"); + "sendResponse callback has been called with the expected parameters"); wrappedListener("fake message2", {name: "fake sender2"}, sendResponseSpy); @@ -135,29 +135,29 @@ describe("browser-polyfill", () => { return secondResponse; }).then(() => { ok(messageListener.calledTwice, - "The unwrapped message listener has been called"); + "The unwrapped message listener has been called"); deepEqual(messageListener.secondCall.args, ["fake message2", {name: "fake sender2"}], - "The unwrapped listener has received the expected parameters"); + "The unwrapped listener has received the expected parameters"); ok(sendResponseSpy.calledTwice, "The sendResponse function has been called"); equal(sendResponseSpy.secondCall.args[0], "fake reply 2", - "sendResponse callback has been called with the expected parameters"); + "sendResponse callback has been called with the expected parameters"); }).then(() => { wrappedListener("fake message3", {name: "fake sender3"}, sendResponseSpy); // Wait the third response promise to be rejected. return thirdResponse.catch(err => { equal(messageListener.callCount, 3, - "The unwrapped message listener has been called"); + "The unwrapped message listener has been called"); deepEqual(messageListener.thirdCall.args, ["fake message3", {name: "fake sender3"}], - "The unwrapped listener has received the expected parameters"); + "The unwrapped listener has received the expected parameters"); equal(sendResponseSpy.callCount, 3, - "The sendResponse function has been called"); + "The sendResponse function has been called"); equal(sendResponseSpy.thirdCall.args[0], err, - "sendResponse callback has been called with the expected parameters"); + "sendResponse callback has been called with the expected parameters"); }); }); }); From b28d3d9d745e454e392a7528af81573e4deacaff Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Wed, 2 Nov 2016 21:31:56 +0100 Subject: [PATCH 18/23] fix: fixed typos and nits in tests. --- test/test-browser-global.js | 4 ++-- test/test-runtime-onMessage.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/test-browser-global.js b/test/test-browser-global.js index b21495d..cde9bf5 100644 --- a/test/test-browser-global.js +++ b/test/test-browser-global.js @@ -12,7 +12,7 @@ describe("browser-polyfill", () => { }); }); - it("do not override the global browser namespace if it already exists", () => { + it("does not override the global browser namespace if it already exists", () => { const fakeChrome = { runtime: {lastError: null}, }; @@ -22,7 +22,7 @@ describe("browser-polyfill", () => { return setupTestDOMWindow(fakeChrome, fakeBrowser).then(window => { deepEqual(window.browser, fakeBrowser, - "The existent browser has not been wrapped"); + "The existing browser has not been wrapped"); }); }); diff --git a/test/test-runtime-onMessage.js b/test/test-runtime-onMessage.js index 5f178fa..e587c83 100644 --- a/test/test-runtime-onMessage.js +++ b/test/test-runtime-onMessage.js @@ -7,7 +7,7 @@ const {setupTestDOMWindow} = require("./setup"); describe("browser-polyfill", () => { describe("wrapped runtime.onMessage listener", () => { - it("do not wrap the listener if it is not a function", () => { + it("does not wrap the listener if it is not a function", () => { const fakeChrome = { runtime: { lastError: null, From 705e93bc2f200c60a72c15a1ddbe9af64f199180 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Wed, 2 Nov 2016 21:32:32 +0100 Subject: [PATCH 19/23] test: added test case to check that different listeners are wrapped by different wrappers --- test/test-runtime-onMessage.js | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/test-runtime-onMessage.js b/test/test-runtime-onMessage.js index e587c83..c321714 100644 --- a/test/test-runtime-onMessage.js +++ b/test/test-runtime-onMessage.js @@ -84,6 +84,40 @@ describe("browser-polyfill", () => { }); }); + it("generates different wrappers for different listeners", () => { + const fakeChromeListeners = []; + const fakeChrome = { + runtime: { + lastError: null, + onMessage: { + addListener: sinon.spy((listener, ...args) => { + fakeChromeListeners.push(listener); + }), + hasListener: sinon.spy(), + removeListener: sinon.spy(), + }, + }, + }; + + return setupTestDOMWindow(fakeChrome).then(window => { + const firstMessageListener = sinon.spy(); + const secondMessageListener = sinon.spy(); + + window.browser.runtime.onMessage.addListener(firstMessageListener); + window.browser.runtime.onMessage.addListener(secondMessageListener); + + equal(fakeChromeListeners.length, 2, "Got two wrapped listeners"); + + fakeChromeListeners[0]("call first wrapper"); + ok(firstMessageListener.calledOnce); + equal(firstMessageListener.firstCall.args[0], "call first wrapper"); + + fakeChromeListeners[1]("call second wrapper"); + ok(secondMessageListener.calledOnce); + equal(secondMessageListener.firstCall.args[0], "call second wrapper"); + }); + }); + it("sends the returned value as a message response", () => { const fakeChrome = { runtime: { From 9a04fc387679f34ef45ae29575ec8779684bb8d5 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Wed, 2 Nov 2016 21:45:02 +0100 Subject: [PATCH 20/23] fix: pluralize argument in raised error message --- browser-polyfill.in.js | 6 ++++-- test/test-async-functions.js | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/browser-polyfill.in.js b/browser-polyfill.in.js index df26346..b7041e7 100644 --- a/browser-polyfill.in.js +++ b/browser-polyfill.in.js @@ -105,13 +105,15 @@ if (typeof browser === "undefined") { * The generated wrapper function. */ const wrapAsyncFunction = (name, metadata) => { + const pluralizeArguments = (numArgs) => numArgs == 1 ? "argument" : "arguments"; + return function asyncFunctionWrapper(target, ...args) { if (args.length < metadata.minArgs) { - throw new Error(`Expected at least ${metadata.minArgs} arguments for ${name}(), got ${args.length}`); + throw new Error(`Expected at least ${metadata.minArgs} ${pluralizeArguments(metadata.minArgs)} for ${name}(), got ${args.length}`); } if (args.length > metadata.maxArgs) { - throw new Error(`Expected at most ${metadata.maxArgs} arguments for ${name}(), got ${args.length}`); + throw new Error(`Expected at most ${metadata.maxArgs} ${pluralizeArguments(metadata.maxArgs)} for ${name}(), got ${args.length}`); } return new Promise((resolve, reject) => { diff --git a/test/test-async-functions.js b/test/test-async-functions.js index 5c8e8de..836163a 100644 --- a/test/test-async-functions.js +++ b/test/test-async-functions.js @@ -79,7 +79,7 @@ describe("browser-polyfill", () => { return setupTestDOMWindow(fakeChrome).then(window => { throws(() => { window.browser.runtime.sendMessage(); - }, "Expected at least 1 arguments for sendMessage(), got 0"); + }, "Expected at least 1 argument for sendMessage(), got 0"); throws(() => { window.browser.runtime.sendMessage("0", "1", "2", "3"); From 573c438c2384e7f1e7ecf073b2d38d90c428d072 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Wed, 2 Nov 2016 21:51:12 +0100 Subject: [PATCH 21/23] fix: change notified irc channel --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 8fe055f..1f1b2ea 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,6 +13,6 @@ after_script: npm run publish-coverage notifications: irc: channels: - - irc.mozilla.org#amo-bots + - irc.mozilla.org#webextensions on_success: change on_failure: always From e19cf823326717ec6addab2a448c249bd26ec776 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Mon, 7 Nov 2016 16:30:26 +0100 Subject: [PATCH 22/23] fix: indentation missed by eslint --- test/test-runtime-onMessage.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/test-runtime-onMessage.js b/test/test-runtime-onMessage.js index c321714..0e42899 100644 --- a/test/test-runtime-onMessage.js +++ b/test/test-runtime-onMessage.js @@ -156,7 +156,7 @@ describe("browser-polyfill", () => { ok(messageListener.calledOnce, "The unwrapped message listener has been called"); deepEqual(messageListener.firstCall.args, - ["fake message", {name: "fake sender"}], + ["fake message", {name: "fake sender"}], "The unwrapped message listener has received the expected parameters"); ok(sendResponseSpy.calledOnce, "The sendResponse function has been called"); @@ -171,7 +171,7 @@ describe("browser-polyfill", () => { ok(messageListener.calledTwice, "The unwrapped message listener has been called"); deepEqual(messageListener.secondCall.args, - ["fake message2", {name: "fake sender2"}], + ["fake message2", {name: "fake sender2"}], "The unwrapped listener has received the expected parameters"); ok(sendResponseSpy.calledTwice, "The sendResponse function has been called"); @@ -185,7 +185,7 @@ describe("browser-polyfill", () => { equal(messageListener.callCount, 3, "The unwrapped message listener has been called"); deepEqual(messageListener.thirdCall.args, - ["fake message3", {name: "fake sender3"}], + ["fake message3", {name: "fake sender3"}], "The unwrapped listener has received the expected parameters"); equal(sendResponseSpy.callCount, 3, From d1b0ff929a248b892e36a01f5f45f128620c8b04 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Mon, 7 Nov 2016 16:56:48 +0100 Subject: [PATCH 23/23] fix: rename polyfill source into src/browser-polyfill.js --- Gruntfile.js | 4 ++-- browser-polyfill.in.js => src/browser-polyfill.js | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename browser-polyfill.in.js => src/browser-polyfill.js (100%) diff --git a/Gruntfile.js b/Gruntfile.js index 2d860e3..7f6e503 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -19,7 +19,7 @@ module.exports = function(grunt) { }, eslint: { - src: ["browser-polyfill.in.js", "Gruntfile.js"], + src: ["src/browser-polyfill.js", "Gruntfile.js"], test: ["test/**/*.js"], }, @@ -48,7 +48,7 @@ module.exports = function(grunt) { { expand: true, flatten: true, - src: ["browser-polyfill.in.js"], + src: ["src/browser-polyfill.js"], dest: "dist/", }, ], diff --git a/browser-polyfill.in.js b/src/browser-polyfill.js similarity index 100% rename from browser-polyfill.in.js rename to src/browser-polyfill.js