From 35989a78ba503c12f9989410696f42a12fb9a0d9 Mon Sep 17 00:00:00 2001 From: Maja Frydrychowicz Date: Wed, 15 Apr 2020 07:30:42 -0400 Subject: [PATCH] fix: set revision based on PUPPETEER_PRODUCT (#5643) --- .travis.yml | 10 +++++----- index.js | 8 ++++++-- src/Launcher.js | 19 +++++++++++-------- src/Puppeteer.js | 14 ++++++++++---- test/launcher.spec.js | 29 +++++++++++++++++++++++++++++ test/mocha-utils.js | 18 +++++++++++------- 6 files changed, 72 insertions(+), 26 deletions(-) diff --git a/.travis.yml b/.travis.yml index 286b8348e1288..0de02d4b2747d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -20,17 +20,17 @@ jobs: before_install: - "export DISPLAY=:99.0" - "sh -e /etc/init.d/xvfb start" + # populate .local-firefox for launcher tests + - PUPPETEER_PRODUCT=firefox npm install script: ./travis/chromium.sh - node_js: "10.19.0" dist: trusty env: - FIREFOX=true - - FIREFOX_HOME=$TRAVIS_HOME/firefox-latest before_install: - "export DISPLAY=:99.0" - "sh -e /etc/init.d/xvfb start" - - "pyenv shell 3.6 && pip3 install --user mozdownload" - - "rm -rf $FIREFOX_HOME && mozdownload -t daily -d $FIREFOX_HOME/latest.tar.bz --log-level DEBUG" - - "cd $FIREFOX_HOME; tar -xvf latest.tar.bz; cd -; ls $FIREFOX_HOME/firefox/firefox" + # install step will add .local-chromium for launcher tests + - PUPPETEER_PRODUCT=firefox npm install script: - - "BINARY=$FIREFOX_HOME/firefox/firefox npm run funit" + - "npm run funit" diff --git a/index.js b/index.js index 9b09868c3c790..3c687c095d98a 100644 --- a/index.js +++ b/index.js @@ -29,10 +29,14 @@ Page.prototype.emulateMedia = Page.prototype.emulateMediaType; // If node does not support async await, use the compiled version. const Puppeteer = require('./lib/Puppeteer'); const packageJson = require('./package.json'); -const preferredRevision = packageJson.puppeteer.chromium_revision; +let preferredRevision = packageJson.puppeteer.chromium_revision; const isPuppeteerCore = packageJson.name === 'puppeteer-core'; +// puppeteer-core ignores environment variables +const product = isPuppeteerCore ? undefined : process.env.PUPPETEER_PRODUCT || process.env.npm_config_puppeteer_product || process.env.npm_package_config_puppeteer_product; +if (!isPuppeteerCore && product === 'firefox') + preferredRevision = packageJson.puppeteer.firefox_revision; -const puppeteer = new Puppeteer(__dirname, preferredRevision, isPuppeteerCore); +const puppeteer = new Puppeteer(__dirname, preferredRevision, isPuppeteerCore, product); // The introspection in `Helper.installAsyncStackHooks` references `Puppeteer._launcher` // before the Puppeteer ctor is called, such that an invalid Launcher is selected at import, diff --git a/src/Launcher.js b/src/Launcher.js index 5d5651b72aaf5..b046f8c33b55c 100644 --- a/src/Launcher.js +++ b/src/Launcher.js @@ -411,14 +411,7 @@ class FirefoxLauncher { firefoxArguments.push(temporaryUserDataDir); } - // replace 'latest' placeholder with actual downloaded revision - if (this._preferredRevision === 'latest') { - const browserFetcher = new BrowserFetcher(this._projectRoot, {product: this.product}); - const localRevisions = await browserFetcher.localRevisions(); - if (localRevisions[0]) - this._preferredRevision = localRevisions[0]; - } - + await this._updateRevision(); let firefoxExecutable = executablePath; if (!executablePath) { const {missingText, executablePath} = resolveExecutablePath(this); @@ -480,6 +473,16 @@ class FirefoxLauncher { return resolveExecutablePath(this).executablePath; } + async _updateRevision() { + // replace 'latest' placeholder with actual downloaded revision + if (this._preferredRevision === 'latest') { + const browserFetcher = new BrowserFetcher(this._projectRoot, { product: this.product }); + const localRevisions = await browserFetcher.localRevisions(); + if (localRevisions[0]) + this._preferredRevision = localRevisions[0]; + } + } + /** * @return {string} */ diff --git a/src/Puppeteer.js b/src/Puppeteer.js index e477a05d21056..1a8e652f5a861 100644 --- a/src/Puppeteer.js +++ b/src/Puppeteer.js @@ -23,11 +23,14 @@ module.exports = class { * @param {string} projectRoot * @param {string} preferredRevision * @param {boolean} isPuppeteerCore + * @param {string} productName */ - constructor(projectRoot, preferredRevision, isPuppeteerCore) { + constructor(projectRoot, preferredRevision, isPuppeteerCore, productName) { this._projectRoot = projectRoot; this._preferredRevision = preferredRevision; this._isPuppeteerCore = isPuppeteerCore; + // track changes to Launcher configuration via options or environment variables + this.__productName = productName; } /** @@ -35,7 +38,8 @@ module.exports = class { * @return {!Promise} */ launch(options = {}) { - this._productName = options.product; + if (options.product) + this._productName = options.product; return this._launcher.launch(options); } @@ -44,7 +48,8 @@ module.exports = class { * @return {!Promise} */ connect(options) { - this._productName = options.product; + if (options.product) + this._productName = options.product; return this._launcher.connect(options); } @@ -52,7 +57,8 @@ module.exports = class { * @param {string} name */ set _productName(name) { - this._changedProduct = this.__productName !== name; + if (this.__productName !== name) + this._changedProduct = true; this.__productName = name; } diff --git a/test/launcher.spec.js b/test/launcher.spec.js index 1a32fe182cd93..8fc93f01056b0 100644 --- a/test/launcher.spec.js +++ b/test/launcher.spec.js @@ -353,6 +353,35 @@ describe('Launcher specs', function() { await browser.close(); }); }); + describe('Puppeteer.launch', function() { + let productName; + + before(async() => { + const {puppeteer} = getTestState(); + productName = puppeteer._productName; + }); + + after(async() => { + const {puppeteer} = getTestState(); + puppeteer._lazyLauncher = undefined; + puppeteer._productName = productName; + }); + + it('should be able to launch different products', async() => { + const {puppeteer} = getTestState(); + + const products = ['firefox', 'chrome']; + for (const product of products) { + const browser = await puppeteer.launch({product}); + const userAgent = await browser.userAgent(); + await browser.close(); + if (product === 'chrome') + expect(userAgent).toContain('Chrome'); + else + expect(userAgent).toContain('Firefox'); + } + }); + }); describe('Puppeteer.connect', function() { it('should be able to connect multiple times to the same browser', async() => { const { puppeteer, defaultBrowserOptions} = getTestState(); diff --git a/test/mocha-utils.js b/test/mocha-utils.js index 6e9dadbb3845f..48d0edce5c2b7 100644 --- a/test/mocha-utils.js +++ b/test/mocha-utils.js @@ -59,14 +59,18 @@ const defaultBrowserOptions = { dumpio: !!process.env.DUMPIO, }; +(async() => { + if (defaultBrowserOptions.executablePath) { + console.warn(`WARN: running ${product} tests with ${defaultBrowserOptions.executablePath}`); + } else { + if (product === 'firefox') + await puppeteer._launcher._updateRevision(); + const executablePath = puppeteer.executablePath(); + if (!fs.existsSync(executablePath)) + throw new Error(`Browser is not downloaded at ${executablePath}. Run 'npm install' and try to re-run tests`); + } +})(); -if (defaultBrowserOptions.executablePath) { - console.warn(`WARN: running ${product} tests with ${defaultBrowserOptions.executablePath}`); -} else { - const executablePath = puppeteer.executablePath(); - if (!fs.existsSync(executablePath)) - throw new Error(`Browser is not downloaded at ${executablePath}. Run 'npm install' and try to re-run tests`); -} const setupGoldenAssertions = () => { const suffix = product.toLowerCase();