From fd6b9c9aad2757ab47363bf31ca8bbf359069aa0 Mon Sep 17 00:00:00 2001 From: Dan Grebb Date: Sat, 23 Dec 2023 23:12:38 -0500 Subject: [PATCH 1/6] feat: defaults puppeteer to `new` headless mode (garris/BackstopJS/#1525) --- core/util/runPuppet.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/util/runPuppet.js b/core/util/runPuppet.js index 38bc4dcad..2a8e93bef 100644 --- a/core/util/runPuppet.js +++ b/core/util/runPuppet.js @@ -71,7 +71,7 @@ async function processScenarioView (scenario, variantOrScenarioLabelSafe, scenar {}, { ignoreHTTPSErrors: true, - headless: config.debugWindow ? false : config?.engineOptions?.headless || true + headless: config.debugWindow ? false : config?.engineOptions?.headless || 'new' }, config.engineOptions ); From 28df639acccd7d3d07c2dd7d34309ba138074de8 Mon Sep 17 00:00:00 2001 From: Dan Grebb Date: Sat, 23 Dec 2023 23:15:46 -0500 Subject: [PATCH 2/6] fix: corrects playwr typo in `test/configs/playwright.json` --- test/configs/playwright.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/configs/playwright.json b/test/configs/playwright.json index 33f4acd79..7e8e56d33 100644 --- a/test/configs/playwright.json +++ b/test/configs/playwright.json @@ -42,7 +42,7 @@ "ci_report": "backstop_data/ci_report" }, "report": ["browser"], - "engine": "playwr", + "engine": "playwright", "engineOptions": { "args": ["--no-sandbox"] }, From 096b587e9ca73ccdb61e89b66f79e29aba8ce4cc Mon Sep 17 00:00:00 2001 From: Dan Grebb Date: Sat, 23 Dec 2023 23:33:07 -0500 Subject: [PATCH 3/6] fix: supports string in `headless: "new"` for Playwright (#1534) --- core/util/runPlaywright.js | 76 ++++++++++++++++++++++++++++++++++---- 1 file changed, 69 insertions(+), 7 deletions(-) diff --git a/core/util/runPlaywright.js b/core/util/runPlaywright.js index 9153e19a1..46d7f591b 100644 --- a/core/util/runPlaywright.js +++ b/core/util/runPlaywright.js @@ -20,25 +20,86 @@ const DOCUMENT_SELECTOR = 'document'; const NOCLIP_SELECTOR = 'body:noclip'; const VIEWPORT_SELECTOR = 'viewport'; +/** + * @method createPlaywrightBrowser + * @function createPlaywrightBrowser + * @description Take configuration arguments, sanitize, and create a Playwright browser. + * @date 12/23/2023 - 10:13:35 PM + * + * @async + * @param {Object} config + * @returns {import('playwright').Browser} + */ module.exports.createPlaywrightBrowser = async function (config) { console.log('Creating Browser'); - let browserChoice = config.engineOptions.browser; + + // Copy and destructure engineOptions for headless mode sanitization + let { engineOptions: sanitizedEngineOptions } = JSON.parse(JSON.stringify(config)); + + // Destructure other properties to reduce repetition + let { browser: browserChoice, headless } = sanitizedEngineOptions; + + // Use Chrommium if no browser set in `engineOptions` if (!browserChoice) { console.warn(chalk.yellow('No Playwright browser specified, assuming Chromium.')); browserChoice = 'chromium'; } + // Warn when using an unrecognized variant of `headless` mode + if (typeof headless === 'string' && headless !== 'new') { + console.warn(chalk.yellow(`The headless mode, "${headless}", may not be supported by Playwright.`)); + } + + // Error when using unknown `browserChoice` if (!playwright[browserChoice]) { - console.error(chalk.red(`Unsupported playwright browser "${browserChoice}"`)); + console.error(chalk.red(`Unsupported Playwright browser "${browserChoice}"`)); return; } + /** + * If headless isn't defined, or it's not a boolean, proceed with sanitization + * of `engineOptions`, setting Playwright to ignore its built in + * `--headless` flag, and pass the custom `--headless='string'` flag. + * NOTE: This is will fail if user defined `headless` mode + * is an invalid option for Playwright, but is future-proof if they add something + * like 'old' headless mode when 'new' mode is default. + */ + if (typeof headless !== 'undefined' && typeof headless !== 'boolean') { + sanitizedEngineOptions = { + ...sanitizedEngineOptions, + ignoreDefaultArgs: sanitizedEngineOptions.ignoreDefaultArgs ? [...sanitizedEngineOptions.ignoredDefaultArgs, '--headless'] : ['--headless'] + }; + sanitizedEngineOptions.args.push(`--headless=${headless}`); + } + + /** + * @constant playwrightArgs + * @type {Object} + * @description The arguments to pass Playwright. Sanitizes for `new` headless + * mode with Playwright until it is fully supported. `ignoreDefaultArgs: + * ['--headless']` silences Playwright's non-boolean warning when passing 'new'. + * + * @see https://playwright.dev/docs/api/class-browsertype#browser-type-launch-option-headless + * @see https://github.com/microsoft/playwright/issues/21194#issuecomment-1444276676 + * + * @example + * + * ```javascript + * { + * args: [ '--no-sandbox', '--headless=new' ], + * headless: true, + * ignoreDefaultArgs: [ '--headless' ] + * } + * ``` + */ const playwrightArgs = Object.assign( {}, + sanitizedEngineOptions, { - headless: config.debugWindow ? false : config?.engineOptions?.headless || true - }, - config.engineOptions + headless: config.debugWindow + ? false + : typeof headless === 'boolean' ? headless : typeof headless === 'string' ? headless === 'new' ? true : headless : true + } ); return await playwright[browserChoice].launch(playwrightArgs); }; @@ -66,6 +127,7 @@ module.exports.disposePlaywrightBrowser = async function (browser) { }; async function processScenarioView (scenario, variantOrScenarioLabelSafe, scenarioLabelSafe, viewport, config, browser) { + const { engineOptions } = config; if (!config.paths) { config.paths = {}; } @@ -80,8 +142,8 @@ async function processScenarioView (scenario, variantOrScenarioLabelSafe, scenar const VP_W = viewport.width || viewport.viewport.width; const VP_H = viewport.height || viewport.viewport.height; - const ignoreHTTPSErrors = config.engineOptions.ignoreHTTPSErrors ? config.engineOptions.ignoreHTTPSErrors : true; - const storageState = config.engineOptions.storageState ? config.engineOptions.storageState : {}; + const ignoreHTTPSErrors = engineOptions.ignoreHTTPSErrors ? engineOptions.ignoreHTTPSErrors : true; + const storageState = engineOptions.storageState ? engineOptions.storageState : {}; const browserContext = await browser.newContext({ ignoreHTTPSErrors, storageState }); const page = await browserContext.newPage(); From 46067b1efdb61174570b7453749257cdb0dd3edf Mon Sep 17 00:00:00 2001 From: Dan Grebb Date: Sun, 24 Dec 2023 00:42:26 -0500 Subject: [PATCH 4/6] fix: corrects path for integration test pass/fail message scripts --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 2ad75753a..2d41e5dc8 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "precommit": "lint-staged", "build-compare": "cp ./node_modules/diverged/src/diverged.js ./compare/output/ && cp ./node_modules/diff/dist/diff.js ./compare/output/ && webpack --config ./compare/webpack.config.js && npm run -s lint", "dev-compare": "webpack-dev-server --content-base ./compare/output --config ./compare/webpack.config.js", - "integration-test": "rm -rf integrationTestDir && mkdir integrationTestDir && cd integrationTestDir && node ../cli/index.js init && node ../cli/index.js reference && node ../cli/index.js test && node -e \"require('../')('test')\" && npm --prefix ../../ run -s success-message || npm --prefix ../../ run -s fail-message", + "integration-test": "rm -rf integrationTestDir && mkdir integrationTestDir && cd integrationTestDir && node ../cli/index.js init && node ../cli/index.js reference && node ../cli/index.js test && node -e \"require('../')('test')\" && npm --prefix ../ run -s success-message || npm --prefix ../ run -s fail-message", "smoke-test": "cd test/configs/ && node ../../cli/index.js test --config=backstop_features && npm --prefix ../../ run -s success-message || npm --prefix ../../ run -s caution-message", "smoke-test-playwright": "cd test/configs/ && node ../../cli/index.js test --config=backstop_features_pw && npm --prefix ../../ run -s fail-message || npm --prefix ../../ run -s caution-message", "smoke-test-docker": "cd test/configs/ && node ../../cli/index.js test --config=backstop_features --docker && npm --prefix ../../ run -s fail-message || npm --prefix ../../ run -s caution-message", From 0aec8f34191b7e19bd4c0226375747a9145a3c62 Mon Sep 17 00:00:00 2001 From: Dan Grebb Date: Sun, 24 Dec 2023 02:25:11 -0500 Subject: [PATCH 5/6] documentation: update README with `new` headless mode information --- README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 8b63f5a67..1e23f3caf 100644 --- a/README.md +++ b/README.md @@ -669,7 +669,10 @@ ignoreHTTPSErrors: true, headless: ``` -You can add more settings (or override the defaults) with the engineOptions property. (properties are merged). This is where headless mode can also be set to 'new', until "new headless mode" is the default in Puppet/Playwright. +You can add more settings (or override the defaults) with the `engineOptions` property. (properties are merged). This is where headless mode can also be set to 'new', until "new headless mode" is less hacky and more supported by Playwright. + +> [!INFORMATION] +> Puppeteer now runs in `new` headless mode by default, but can be set to `old` headless by passing `"headless": true` in the configuration file. ```json "engineOptions": { From 5aef5eaf668b5eef8f7447f2cf34c4ba0f0b1020 Mon Sep 17 00:00:00 2001 From: Dan Grebb Date: Sun, 24 Dec 2023 02:37:22 -0500 Subject: [PATCH 6/6] documentation: adjust comment language in runPlaywright --- core/util/runPlaywright.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/util/runPlaywright.js b/core/util/runPlaywright.js index 46d7f591b..ed9320aa2 100644 --- a/core/util/runPlaywright.js +++ b/core/util/runPlaywright.js @@ -57,12 +57,12 @@ module.exports.createPlaywrightBrowser = async function (config) { } /** - * If headless isn't defined, or it's not a boolean, proceed with sanitization + * If headless is defined, and it's not a boolean, proceed with sanitization * of `engineOptions`, setting Playwright to ignore its built in - * `--headless` flag, and pass the custom `--headless='string'` flag. + * `--headless` flag. Then, pass the custom `--headless='string'` flag. * NOTE: This is will fail if user defined `headless` mode * is an invalid option for Playwright, but is future-proof if they add something - * like 'old' headless mode when 'new' mode is default. + * like 'old' headless mode when 'new' mode is default. A warning is included for this case. */ if (typeof headless !== 'undefined' && typeof headless !== 'boolean') { sanitizedEngineOptions = {