Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🏗 Fix for recent changes to Chromedriver installation on CircleCI #39856

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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/[email protected].7
browser-tools: circleci/[email protected].8
codecov: codecov/[email protected]
node: circleci/[email protected]

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
Loading