PATCH: Feature/1525 headless mode deprecation

README.md

@@ -669,7 +669,10 @@ ignoreHTTPSErrors: true,
 headless: <!!!config.debugWindow>
 ```
 
-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": {

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();
 

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
   );

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",

test/configs/playwright.json

@@ -42,7 +42,7 @@
     "ci_report": "backstop_data/ci_report"
   },
   "report": ["browser"],
-  "engine": "playwr",
+  "engine": "playwright",
   "engineOptions": {
     "args": ["--no-sandbox"]
   },