Skip to content

Commit

Permalink
🏗 Fix for recent changes to Chromedriver installation on CircleCI (#3…
Browse files Browse the repository at this point in the history
…9856)

* Fix for newly broken Chromedriver issues

* Use the Chrome version installed by CircleCI in `amp visual-diff`

* Disable flaky tests
  • Loading branch information
danielrozenberg authored Feb 22, 2024
1 parent 051cfaf commit 764f2f5
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 92 deletions.
3 changes: 2 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions build-system/tasks/e2e/.npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
chromedriver_skip_download=true
1 change: 1 addition & 0 deletions build-system/tasks/visual-diff/.npmrc
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
puppeteer_skip_chrome_download=true
puppeteer_skip_chromium_download=true
111 changes: 31 additions & 80 deletions build-system/tasks/visual-diff/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<!puppeteer.Browser>} 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',
Expand All @@ -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);
Expand Down Expand Up @@ -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<string>}
*/
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,
};
19 changes: 9 additions & 10 deletions build-system/tasks/visual-diff/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ const {
shortSha,
} = require('../../common/git');
const {
fetchBrowserExecutablePath,
launchBrowser,
locateChromeExecutablePath,
newPage,
resetPage,
} = require('./browser');
Expand Down Expand Up @@ -143,10 +143,9 @@ function setPercyTargetCommit() {
/**
* Launches a Percy agent instance.
*
* @param {string} executablePath browser executable path.
* @return {!Promise<Percy|undefined>} percy agent instance.
*/
async function launchPercyAgent(executablePath) {
async function launchPercyAgent() {
if (argv.percy_disabled) {
return;
}
Expand All @@ -159,7 +158,7 @@ async function launchPercyAgent(executablePath) {
config: path.join(__dirname, '.percy.yaml'),
discovery: {
launchOptions: {
executable: executablePath,
executable: locateChromeExecutablePath(),
},
},
});
Expand Down Expand Up @@ -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();
}
Expand All @@ -627,13 +625,12 @@ async function visualDiff() {
/**
* Runs the AMP visual diff tests.
*
* @param {string} executablePath browser executable path.
* @return {Promise<void>}
*/
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
Expand Down Expand Up @@ -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',
Expand Down
3 changes: 2 additions & 1 deletion test/integration/test-video-players.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down

0 comments on commit 764f2f5

Please sign in to comment.