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": { diff --git a/core/util/runPlaywright.js b/core/util/runPlaywright.js index 9153e19a1..ed9320aa2 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 is defined, and it's not a boolean, proceed with sanitization + * of `engineOptions`, setting Playwright to ignore its built in + * `--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. A warning is included for this case. + */ + 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(); 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 ); 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", 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"] },