From 36dfe0a41a21ed74f940a32d32454f0d66a3e5d8 Mon Sep 17 00:00:00 2001 From: ExE Boss <3889017+ExE-Boss@users.noreply.github.com> Date: Fri, 8 Feb 2019 23:45:00 +0100 Subject: [PATCH 1/6] =?UTF-8?q?test(devtools):=20Fix=C2=A0intermittent=20D?= =?UTF-8?q?evTools=C2=A0API=20test=C2=A0timeout?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # External links: - https://travis-ci.org/mozilla/webextension-polyfill/builds/487787360#L950 --- test/fixtures/devtools-extension/content.js | 8 ++++++-- test/fixtures/tape-standalone.js | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/test/fixtures/devtools-extension/content.js b/test/fixtures/devtools-extension/content.js index 5af9ae96..01f6f901 100644 --- a/test/fixtures/devtools-extension/content.js +++ b/test/fixtures/devtools-extension/content.js @@ -1,4 +1,6 @@ -test("devtools.inspectedWindow.eval resolved with an error result", async (t) => { +test("devtools.inspectedWindow.eval resolved with an error result", { + timeout: 5000, +}, async (t) => { const {evalResult} = await browser.runtime.sendMessage({ apiMethod: "devtools.inspectedWindow.eval", params: ["throw new Error('fake error');"], @@ -14,7 +16,9 @@ test("devtools.inspectedWindow.eval resolved with an error result", async (t) => "the second element value property should include the expected error message"); }); -test("devtools.inspectedWindow.eval resolved without an error result", async (t) => { +test("devtools.inspectedWindow.eval resolved without an error result", { + timeout: 5000, +}, async (t) => { const {evalResult} = await browser.runtime.sendMessage({ apiMethod: "devtools.inspectedWindow.eval", params: ["[document.documentElement.localName]"], diff --git a/test/fixtures/tape-standalone.js b/test/fixtures/tape-standalone.js index 1face237..0e40ecc7 100644 --- a/test/fixtures/tape-standalone.js +++ b/test/fixtures/tape-standalone.js @@ -10,8 +10,12 @@ if (navigator.userAgent.includes("Chrome/")) { } // Export as a global a wrapped test function which enforces a timeout by default. -window.test = (desc, fn) => { - tape(`${desc} (${browser})`, async (t) => { +window.test = (desc, options, fn) => { + if (!fn && typeof options === "function") { + fn = options; + options = undefined; + } + tape(`${desc} (${browser})`, options, async (t) => { t.timeoutAfter(DEFAULT_TIMEOUT); await fn(t); }); From 82e287c0af29ad86821c56e219986b51a0f01127 Mon Sep 17 00:00:00 2001 From: ExE Boss <3889017+ExE-Boss@users.noreply.github.com> Date: Thu, 28 Feb 2019 21:00:00 +0100 Subject: [PATCH 2/6] test: Fix test function timeout --- test/fixtures/tape-standalone.js | 46 ++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/test/fixtures/tape-standalone.js b/test/fixtures/tape-standalone.js index 0e40ecc7..ddff708c 100644 --- a/test/fixtures/tape-standalone.js +++ b/test/fixtures/tape-standalone.js @@ -9,14 +9,50 @@ if (navigator.userAgent.includes("Chrome/")) { browser = "Firefox"; } +function getArgs(/* desc, options, fn */) { + let desc = "(anonymous)"; + // By setting `timeout` to `0` here, we are allowing V8 to consider + // it a to have a signature of `{timeout: number}` instead of `{}` + // with a `timeout` property, which is easier on the C++ portion. + let options = {timeout: 0}; + /** @type {function(any):void|Promise} */ + let fn; + + for (let i = 0, arg; i < arguments.length; i++) { + arg = arguments[i]; + switch (typeof arg) { + case "string": + desc = arg; + break; + case "function": + fn = arg; + break; + case "object": + Object.assign(options, arg); + break; + } + } + + if (!options.timeout || options.timeout < DEFAULT_TIMEOUT) { + options.timeout = DEFAULT_TIMEOUT; + } + + return { + desc, + options, + fn, + }; +} + // Export as a global a wrapped test function which enforces a timeout by default. +/** + * @param {string} [desc] + * @param {object} [options] + * @param {function(any):void|Promise} fn + */ window.test = (desc, options, fn) => { - if (!fn && typeof options === "function") { - fn = options; - options = undefined; - } + ({desc, options, fn} = getArgs(desc, options, fn)); tape(`${desc} (${browser})`, options, async (t) => { - t.timeoutAfter(DEFAULT_TIMEOUT); await fn(t); }); }; From 18f960f26a7fb8e9f19a12323966cbd2e52fb18f Mon Sep 17 00:00:00 2001 From: ExE Boss <3889017+ExE-Boss@users.noreply.github.com> Date: Wed, 6 Mar 2019 20:45:00 +0100 Subject: [PATCH 3/6] =?UTF-8?q?test:=20Allow=20shorter=20test=C2=A0timeout?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/fixtures/tape-standalone.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/fixtures/tape-standalone.js b/test/fixtures/tape-standalone.js index ddff708c..8ba23b8f 100644 --- a/test/fixtures/tape-standalone.js +++ b/test/fixtures/tape-standalone.js @@ -15,7 +15,7 @@ function getArgs(/* desc, options, fn */) { // it a to have a signature of `{timeout: number}` instead of `{}` // with a `timeout` property, which is easier on the C++ portion. let options = {timeout: 0}; - /** @type {function(any):void|Promise} */ + /** @type {function(import("tape").Test):void|Promise} */ let fn; for (let i = 0, arg; i < arguments.length; i++) { @@ -33,7 +33,7 @@ function getArgs(/* desc, options, fn */) { } } - if (!options.timeout || options.timeout < DEFAULT_TIMEOUT) { + if (!options.timeout) { options.timeout = DEFAULT_TIMEOUT; } @@ -45,10 +45,11 @@ function getArgs(/* desc, options, fn */) { } // Export as a global a wrapped test function which enforces a timeout by default. +// eslint-disable-next-line /** * @param {string} [desc] * @param {object} [options] - * @param {function(any):void|Promise} fn + * @param {(test: import("tape").Test) => void|Promise} fn */ window.test = (desc, options, fn) => { ({desc, options, fn} = getArgs(desc, options, fn)); From 58d85c762c77d07e9cc7bb0e3ac1698e5233837a Mon Sep 17 00:00:00 2001 From: ExE Boss <3889017+ExE-Boss@users.noreply.github.com> Date: Tue, 12 Mar 2019 02:20:00 +0100 Subject: [PATCH 4/6] =?UTF-8?q?test:=20Only=C2=A0allow=20swapping=20`fn`?= =?UTF-8?q?=C2=A0and=C2=A0`options`=20parameters?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/fixtures/tape-standalone.js | 57 +++++++++++--------------------- test/integration/setup.js | 5 +-- 2 files changed, 22 insertions(+), 40 deletions(-) diff --git a/test/fixtures/tape-standalone.js b/test/fixtures/tape-standalone.js index 8ba23b8f..1e01fe02 100644 --- a/test/fixtures/tape-standalone.js +++ b/test/fixtures/tape-standalone.js @@ -9,50 +9,31 @@ if (navigator.userAgent.includes("Chrome/")) { browser = "Firefox"; } -function getArgs(/* desc, options, fn */) { - let desc = "(anonymous)"; - // By setting `timeout` to `0` here, we are allowing V8 to consider - // it a to have a signature of `{timeout: number}` instead of `{}` - // with a `timeout` property, which is easier on the C++ portion. - let options = {timeout: 0}; - /** @type {function(import("tape").Test):void|Promise} */ - let fn; - - for (let i = 0, arg; i < arguments.length; i++) { - arg = arguments[i]; - switch (typeof arg) { - case "string": - desc = arg; - break; - case "function": - fn = arg; - break; - case "object": - Object.assign(options, arg); - break; - } - } - - if (!options.timeout) { - options.timeout = DEFAULT_TIMEOUT; - } - - return { - desc, - options, - fn, - }; -} - // Export as a global a wrapped test function which enforces a timeout by default. // eslint-disable-next-line /** - * @param {string} [desc] - * @param {object} [options] + * @param {string} desc + * @param {import("tape").TestOptions|null} [options] * @param {(test: import("tape").Test) => void|Promise} fn */ window.test = (desc, options, fn) => { - ({desc, options, fn} = getArgs(desc, options, fn)); + if (typeof options === "function") { + // Allow swapping options with fn + if (typeof fn === "object" && fn) { + let tmp = fn; + fn = options; + options = tmp; + } else { + fn = options; + options = undefined; + } + } + + options = { + timeout: DEFAULT_TIMEOUT, + ...options, + }; + tape(`${desc} (${browser})`, options, async (t) => { await fn(t); }); diff --git a/test/integration/setup.js b/test/integration/setup.js index 7a4665d8..40af6c12 100644 --- a/test/integration/setup.js +++ b/test/integration/setup.js @@ -165,6 +165,7 @@ test.onFailure(() => { * @param {string} parameters.description * @param {string[]} parameters.extensions * @param {boolean|string|string[]} [parameters.skip] + * @param {boolean} [parameters.openDevTools] */ const defineExtensionTests = ({description, extensions, skip, openDevTools}) => { for (const extensionDirName of extensions) { @@ -192,8 +193,8 @@ const defineExtensionTests = ({description, extensions, skip, openDevTools}) => path.join(__dirname, "..", "fixtures", extensionDirName)); const srcPolyfill = path.join(__dirname, "..", "..", "dist", "browser-polyfill.js"); - const tmpDir = tmp.dirSync({unsafeCleanup: true}); - const extensionPath = path.join(tmpDir.name, extensionDirName); + tempDir = tmp.dirSync({unsafeCleanup: true}); + const extensionPath = path.join(tempDir.name, extensionDirName); cp("-rf", srcExtensionPath, extensionPath); cp("-f", srcPolyfill, extensionPath); From 768e38d1b47f6c93e5ea450d9a28bba47b62ff7d Mon Sep 17 00:00:00 2001 From: ExE Boss <3889017+ExE-Boss@users.noreply.github.com> Date: Fri, 15 Mar 2019 02:00:00 +0100 Subject: [PATCH 5/6] test: Improve test function JSDoc --- test/fixtures/tape-standalone.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/test/fixtures/tape-standalone.js b/test/fixtures/tape-standalone.js index 1e01fe02..eab9bc2b 100644 --- a/test/fixtures/tape-standalone.js +++ b/test/fixtures/tape-standalone.js @@ -10,19 +10,23 @@ if (navigator.userAgent.includes("Chrome/")) { } // Export as a global a wrapped test function which enforces a timeout by default. -// eslint-disable-next-line /** * @param {string} desc - * @param {import("tape").TestOptions|null} [options] - * @param {(test: import("tape").Test) => void|Promise} fn + * The test description + * @param {object} [options] + * The test options, can be omitted. + * @param {number} [options.timeout=DEFAULT_TIMEOUT] + * The time after which the test fails automatically, unless it has already passed. + * @param {boolean} [options.skip] + * Whether the test case should be skipped. + * @param {function(tape.Test):void|Promise} fn + * The test case function, takes the test object as a callback. */ window.test = (desc, options, fn) => { if (typeof options === "function") { // Allow swapping options with fn if (typeof fn === "object" && fn) { - let tmp = fn; - fn = options; - options = tmp; + [fn, options] = [options, fn]; } else { fn = options; options = undefined; From 2ce77ca9c67705a5324cd9a8c00b0bfc31afa314 Mon Sep 17 00:00:00 2001 From: ExE Boss <3889017+ExE-Boss@users.noreply.github.com> Date: Sat, 16 Mar 2019 14:00:00 +0100 Subject: [PATCH 6/6] Address review comments --- test/fixtures/tape-standalone.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/test/fixtures/tape-standalone.js b/test/fixtures/tape-standalone.js index eab9bc2b..710bfd91 100644 --- a/test/fixtures/tape-standalone.js +++ b/test/fixtures/tape-standalone.js @@ -19,18 +19,13 @@ if (navigator.userAgent.includes("Chrome/")) { * The time after which the test fails automatically, unless it has already passed. * @param {boolean} [options.skip] * Whether the test case should be skipped. - * @param {function(tape.Test):void|Promise} fn + * @param {function(tape.Test):(void|Promise)} fn * The test case function, takes the test object as a callback. */ window.test = (desc, options, fn) => { if (typeof options === "function") { // Allow swapping options with fn - if (typeof fn === "object" && fn) { - [fn, options] = [options, fn]; - } else { - fn = options; - options = undefined; - } + [fn, options] = [options, fn]; } options = {