From 669f04a7a6e96cc8353a8cb152898edbc25e7c15 Mon Sep 17 00:00:00 2001 From: Henrik Skupin Date: Fri, 5 Mar 2021 10:00:56 +0100 Subject: [PATCH] chore: enable unit tests for Firefox on Windows (#6895) Co-authored-by: Jan Scheffler --- .github/workflows/main.yml | 7 +++++ package.json | 2 +- src/node/BrowserRunner.ts | 6 +++- src/node/Launcher.ts | 2 ++ test/launcher.spec.ts | 9 ++---- test/screenshot.spec.ts | 63 ++++++++++++++++++++------------------ 6 files changed, 50 insertions(+), 39 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 13d102b2..65edf9aa 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -155,3 +155,10 @@ jobs: CHROMIUM: true run: | npm run unit + + - name: Run unit tests on Firefox + env: + FIREFOX: true + MOZ_WEBRENDER: 0 + run: | + npm run funit diff --git a/package.json b/package.json index 4bb60b54..0c19f1cb 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "unit-debug": "npm run tsc-cjs && mocha --inspect-brk --config mocha-config/puppeteer-unit-tests.js", "unit-with-coverage": "cross-env COVERAGE=1 npm run unit", "assert-unit-coverage": "cross-env COVERAGE=1 mocha --config mocha-config/coverage-tests.js", - "funit": "PUPPETEER_PRODUCT=firefox npm run unit", + "funit": "cross-env PUPPETEER_PRODUCT=firefox npm run unit", "test": "npm run tsc && npm run lint --silent && npm run unit-with-coverage && npm run test-browser", "prepare": "node typescript-if-required.js", "prepublishOnly": "npm run build", diff --git a/src/node/BrowserRunner.ts b/src/node/BrowserRunner.ts index e7e11bb1..2a29aca6 100644 --- a/src/node/BrowserRunner.ts +++ b/src/node/BrowserRunner.ts @@ -24,6 +24,7 @@ import { LaunchOptions } from './LaunchOptions.js'; import { Connection } from '../common/Connection.js'; import { NodeWebSocketTransport as WebSocketTransport } from '../node/NodeWebSocketTransport.js'; import { PipeTransport } from './PipeTransport.js'; +import { Product } from '../common/Product.js'; import * as readline from 'readline'; import { TimeoutError } from '../common/Errors.js'; import { promisify } from 'util'; @@ -36,6 +37,7 @@ Please check your open processes and ensure that the browser processes that Pupp If you think this is a bug, please report it on the Puppeteer issue tracker.`; export class BrowserRunner { + private _product: Product; private _executablePath: string; private _processArguments: string[]; private _tempDirectory?: string; @@ -48,10 +50,12 @@ export class BrowserRunner { private _processClosing: Promise; constructor( + product: Product, executablePath: string, processArguments: string[], tempDirectory?: string ) { + this._product = product; this._executablePath = executablePath; this._processArguments = processArguments; this._tempDirectory = tempDirectory; @@ -128,7 +132,7 @@ export class BrowserRunner { close(): Promise { if (this._closed) return Promise.resolve(); - if (this._tempDirectory) { + if (this._tempDirectory && this._product !== 'firefox') { this.kill(); } else if (this.connection) { // Attempt to close the browser gracefully diff --git a/src/node/Launcher.ts b/src/node/Launcher.ts index 61ddbe74..828b18c0 100644 --- a/src/node/Launcher.ts +++ b/src/node/Launcher.ts @@ -116,6 +116,7 @@ class ChromeLauncher implements ProductLauncher { const usePipe = chromeArguments.includes('--remote-debugging-pipe'); const runner = new BrowserRunner( + this.product, chromeExecutable, chromeArguments, temporaryUserDataDir @@ -281,6 +282,7 @@ class FirefoxLauncher implements ProductLauncher { } const runner = new BrowserRunner( + this.product, firefoxExecutable, firefoxArguments, temporaryUserDataDir diff --git a/test/launcher.spec.ts b/test/launcher.spec.ts index 41b5cb14..2d7e3768 100644 --- a/test/launcher.spec.ts +++ b/test/launcher.spec.ts @@ -22,7 +22,6 @@ import { getTestState, itFailsFirefox, itOnlyRegularInstall, - itFailsWindows, } from './mocha-utils'; // eslint-disable-line import/extensions import utils from './utils.js'; import expect from 'expect'; @@ -339,7 +338,7 @@ describe('Launcher specs', function () { if (isChrome) expect(puppeteer.product).toBe('chrome'); else if (isFirefox) expect(puppeteer.product).toBe('firefox'); }); - it('should work with no default arguments', async () => { + itFailsFirefox('should work with no default arguments', async () => { const { defaultBrowserOptions, puppeteer } = getTestState(); const options = Object.assign({}, defaultBrowserOptions); options.ignoreDefaultArgs = true; @@ -470,11 +469,7 @@ describe('Launcher specs', function () { ]); }); - /* We think there's a bug in the FF Windows launcher, or some - * combo of that plus it running on CI, but it's hard to track down. - * See comment here: https://github.com/puppeteer/puppeteer/issues/5673#issuecomment-670141377. - */ - itFailsWindows('should be able to launch Firefox', async function () { + it('should be able to launch Firefox', async function () { this.timeout(FIREFOX_TIMEOUT); const { puppeteer } = getTestState(); const browser = await puppeteer.launch({ product: 'firefox' }); diff --git a/test/screenshot.spec.ts b/test/screenshot.spec.ts index 55b422e0..223d6c4a 100644 --- a/test/screenshot.spec.ts +++ b/test/screenshot.spec.ts @@ -203,39 +203,42 @@ describe('Screenshots', function () { const screenshot = await elementHandle.screenshot(); expect(screenshot).toBeGolden('screenshot-element-padding-border.png'); }); - it('should capture full element when larger than viewport', async () => { - const { page } = getTestState(); + itFailsFirefox( + 'should capture full element when larger than viewport', + async () => { + const { page } = getTestState(); - await page.setViewport({ width: 500, height: 500 }); + await page.setViewport({ width: 500, height: 500 }); - await page.setContent(` - something above - -
- `); - const elementHandle = await page.$('div.to-screenshot'); - const screenshot = await elementHandle.screenshot(); - expect(screenshot).toBeGolden( - 'screenshot-element-larger-than-viewport.png' - ); + await page.setContent(` + something above + +
+ `); + const elementHandle = await page.$('div.to-screenshot'); + const screenshot = await elementHandle.screenshot(); + expect(screenshot).toBeGolden( + 'screenshot-element-larger-than-viewport.png' + ); - expect( - await page.evaluate(() => ({ - w: window.innerWidth, - h: window.innerHeight, - })) - ).toEqual({ w: 500, h: 500 }); - }); + expect( + await page.evaluate(() => ({ + w: window.innerWidth, + h: window.innerHeight, + })) + ).toEqual({ w: 500, h: 500 }); + } + ); it('should scroll element into view', async () => { const { page } = getTestState();