From c64a0a8c4f4934acff1f8d3c78e3ca4222f7fd51 Mon Sep 17 00:00:00 2001 From: Daniel Rozenberg Date: Thu, 22 Feb 2024 11:15:09 -0500 Subject: [PATCH 1/3] Fix for newly broken Chromedriver issues --- .circleci/config.yml | 2 +- build-system/tasks/e2e/.npmrc | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 build-system/tasks/e2e/.npmrc diff --git a/.circleci/config.yml b/.circleci/config.yml index 197b5e8922c9..942f21c18eb6 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,7 +1,7 @@ version: 2.1 orbs: - browser-tools: circleci/browser-tools@1.4.7 + browser-tools: circleci/browser-tools@1.4.8 codecov: codecov/codecov@4.0.1 node: circleci/node@5.2.0 diff --git a/build-system/tasks/e2e/.npmrc b/build-system/tasks/e2e/.npmrc new file mode 100644 index 000000000000..dc7716326768 --- /dev/null +++ b/build-system/tasks/e2e/.npmrc @@ -0,0 +1 @@ +chromedriver_skip_download=true From c543fae609b24a2f962fcb432230a0cbb2cd76eb Mon Sep 17 00:00:00 2001 From: Daniel Rozenberg Date: Thu, 22 Feb 2024 11:15:09 -0500 Subject: [PATCH 2/3] Use the Chrome version installed by CircleCI in `amp visual-diff` --- .circleci/config.yml | 1 + build-system/tasks/visual-diff/.npmrc | 1 + build-system/tasks/visual-diff/browser.js | 111 ++++++---------------- build-system/tasks/visual-diff/index.js | 19 ++-- 4 files changed, 42 insertions(+), 90 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 942f21c18eb6..fcb06090d697 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -319,6 +319,7 @@ jobs: name: node-docker-large steps: - setup_vm + - install_chrome - run: name: '⭐ Visual Diff Tests ⭐' command: node build-system/pr-check/visual-diff-tests.js diff --git a/build-system/tasks/visual-diff/.npmrc b/build-system/tasks/visual-diff/.npmrc index 5c6ff9df9dd2..bb47df101475 100644 --- a/build-system/tasks/visual-diff/.npmrc +++ b/build-system/tasks/visual-diff/.npmrc @@ -1 +1,2 @@ +puppeteer_skip_chrome_download=true puppeteer_skip_chromium_download=true diff --git a/build-system/tasks/visual-diff/browser.js b/build-system/tasks/visual-diff/browser.js index 69b2130830b2..ea23b3f05282 100644 --- a/build-system/tasks/visual-diff/browser.js +++ b/build-system/tasks/visual-diff/browser.js @@ -3,44 +3,48 @@ const argv = require('minimist')(process.argv.slice(2)); const fs = require('fs'); const path = require('path'); -const { - Browser: BrowserEnum, - computeExecutablePath, - getInstalledBrowsers, - install, -} = require('@puppeteer/browsers'); const {cyan, yellow} = require('kleur/colors'); const {HOST} = require('./consts'); const {log} = require('./log'); const puppeteer = require('puppeteer-core'); +const {getStdout} = require('../../common/process'); -const cacheDir = path.join(__dirname, '.cache'); - -// REPEATING TODO(@ampproject/wg-infra): Update this whenever the Percy backend -// starts using a new version of Chrome to render DOM snapshots. -// -// Steps: -// 1. Open a recent Percy build, and click the “ⓘ” icon -// 2. Note the Chrome major version at the bottom -// 3. Open https://googlechromelabs.github.io/chrome-for-testing/known-good-versions.json -// 4. In this JSON file look for the latest full version number that begins -// with the same major version number you noted in step 2. The list is -// ordered from earliest to latest version numbers. -// 5. Update the value below: -const PUPPETEER_CHROME_VERSION = '115.0.5790.170'; +const CHROME_BASENAMES = [ + 'chrome', + 'google-chrome', + 'google-chrome-stable', + 'chromium', +]; const VIEWPORT_WIDTH = 1400; const VIEWPORT_HEIGHT = 100000; +/** + * Attempts to locale the full executable path for Chrome/Chromium. + * @return {string} + */ +function locateChromeExecutablePath() { + if (argv.executablePath) { + return argv.executablePath; + } + for (const executableBaseName of CHROME_BASENAMES) { + const executablePath = getStdout(`which ${executableBaseName}`).trim(); + if (executablePath) { + return executablePath; + } + } + throw new Error( + `Could not locate Chrome/Chromium executable. Make sure it is on your $PATH (looking for any of {${CHROME_BASENAMES.join(', ')}}) or pass --executablePath to amp visual-diff` + ); +} + /** * Launches a Puppeteer controlled browser. * - * @param {string} executablePath browser executable path. * @return {!Promise} a Puppeteer controlled browser. */ -async function launchBrowser(executablePath) { - /** @type {"new" | false} */ - const headless = !argv.dev && 'new'; +async function launchBrowser() { + /** @type {import('puppeteer-core').PuppeteerLaunchOptions} */ const browserOptions = { args: [ '--disable-background-media-suspend', @@ -53,8 +57,8 @@ async function launchBrowser(executablePath) { '--no-startup-window', ], dumpio: argv.chrome_debug, - headless, - executablePath, + headless: !argv.dev && 'new', + executablePath: locateChromeExecutablePath(), waitForInitialPage: false, }; return puppeteer.launch(browserOptions); @@ -142,62 +146,9 @@ async function resetPage(page, viewport = null) { await page.setViewport({width, height}); } -/** - * Returns the executable path of the browser, potentially installing and caching it. - * - * @return {Promise} - */ -async function fetchBrowserExecutablePath() { - const installedBrowsers = await getInstalledBrowsers({cacheDir}); - const installedBrowser = installedBrowsers.find( - ({browser, buildId}) => - browser == BrowserEnum.CHROME && buildId == PUPPETEER_CHROME_VERSION - ); - if (installedBrowser) { - log( - 'info', - 'Using cached Percy-compatible version of Chrome', - cyan(PUPPETEER_CHROME_VERSION) - ); - } else { - log( - 'info', - 'Percy-compatible version of Chrome', - cyan(PUPPETEER_CHROME_VERSION), - 'was not found in cache. Downloading...' - ); - let logThrottler = true; - await install({ - cacheDir, - browser: BrowserEnum.CHROME, - buildId: PUPPETEER_CHROME_VERSION, - downloadProgressCallback(downloadedBytes, totalBytes) { - if (logThrottler) { - log('info', downloadedBytes, '/', totalBytes, 'bytes'); - logThrottler = false; - setTimeout(() => { - logThrottler = true; - }, 1000); - } else if (downloadedBytes == totalBytes) { - log( - 'info', - 'Finished downloading Chrome version', - cyan(PUPPETEER_CHROME_VERSION) - ); - } - }, - }); - } - return computeExecutablePath({ - cacheDir, - browser: BrowserEnum.CHROME, - buildId: PUPPETEER_CHROME_VERSION, - }); -} - module.exports = { + locateChromeExecutablePath, launchBrowser, - fetchBrowserExecutablePath, newPage, resetPage, }; diff --git a/build-system/tasks/visual-diff/index.js b/build-system/tasks/visual-diff/index.js index 89e3ccd30b12..9a26c3d67f21 100644 --- a/build-system/tasks/visual-diff/index.js +++ b/build-system/tasks/visual-diff/index.js @@ -19,8 +19,8 @@ const { shortSha, } = require('../../common/git'); const { - fetchBrowserExecutablePath, launchBrowser, + locateChromeExecutablePath, newPage, resetPage, } = require('./browser'); @@ -143,10 +143,9 @@ function setPercyTargetCommit() { /** * Launches a Percy agent instance. * - * @param {string} executablePath browser executable path. * @return {!Promise} percy agent instance. */ -async function launchPercyAgent(executablePath) { +async function launchPercyAgent() { if (argv.percy_disabled) { return; } @@ -159,7 +158,7 @@ async function launchPercyAgent(executablePath) { config: path.join(__dirname, '.percy.yaml'), discovery: { launchOptions: { - executable: executablePath, + executable: locateChromeExecutablePath(), }, }, }); @@ -614,10 +613,9 @@ async function visualDiff() { log('fatal', 'Could not find', cyan('PERCY_TOKEN'), 'environment variable'); } - const executablePath = await fetchBrowserExecutablePath(); - const percy = await launchPercyAgent(executablePath); + const percy = await launchPercyAgent(); try { - await performVisualTests(executablePath); + await performVisualTests(); } finally { await percy?.stop(); } @@ -627,13 +625,12 @@ async function visualDiff() { /** * Runs the AMP visual diff tests. * - * @param {string} executablePath browser executable path. * @return {Promise} */ -async function performVisualTests(executablePath) { +async function performVisualTests() { setDebuggingLevel(); - const browser = await launchBrowser(executablePath); + const browser = await launchBrowser(); const handlerProcess = createCtrlcHandler( 'visual-diff:browser', browser.process()?.pid @@ -703,6 +700,8 @@ visualDiff.flags = { 'percy_agent_debug': 'Print debug info from the @percy/agent instance', 'debug': 'Set all debugging flags', 'verbose': 'Print verbose log statements', + 'executablePath': + 'Full path to the Chrome/Chromium executable (optional if Chrome is on $PATH)', 'grep': 'Run tests that match the pattern', 'minified': 'Serve minified JS', 'percy_token': 'Override the PERCY_TOKEN environment variable', From a487954e240e4aaec9366629be1b419e6fecb072 Mon Sep 17 00:00:00 2001 From: Daniel Rozenberg Date: Thu, 22 Feb 2024 13:27:22 -0500 Subject: [PATCH 3/3] Disable flaky tests --- test/integration/test-video-players.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/test-video-players.js b/test/integration/test-video-players.js index 6d56cf888771..af88ec44bbb4 100644 --- a/test/integration/test-video-players.js +++ b/test/integration/test-video-players.js @@ -60,7 +60,8 @@ describes.sandboxed('amp-dailymotion', {}, (env) => { ); }); -describes.sandboxed('amp-3q-player', {}, (env) => { +// TODO(#39857): Unskip when integration is fixed. +describes.sandboxed.skip('amp-3q-player', {}, (env) => { runVideoPlayerIntegrationTests( env, (fixture) => {