From 2ca2a4cb1891b0fcc993702e98631e1efac0512e Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Mon, 2 Dec 2019 17:17:53 -0700 Subject: [PATCH] feat(launcher): throw on browser launch failure, default args (#119) --- src/chromium/Launcher.ts | 9 +++++++++ src/firefox/Launcher.ts | 9 +++++++++ src/webkit/Launcher.ts | 29 ++++++++++++++++++++++++++++- src/webkit/Playwright.ts | 4 ++++ test/launcher.spec.js | 22 ++++++++++++---------- 5 files changed, 62 insertions(+), 11 deletions(-) diff --git a/src/chromium/Launcher.ts b/src/chromium/Launcher.ts index 07f17da1b5..d2f7082b1c 100644 --- a/src/chromium/Launcher.ts +++ b/src/chromium/Launcher.ts @@ -137,6 +137,15 @@ export class Launcher { } ); + if (!chromeProcess.pid) { + let reject; + const result = new Promise((f, r) => reject = r); + chromeProcess.once('error', error => { + reject(new Error('Failed to launch browser: ' + error)); + }); + return result as Promise; + } + if (dumpio) { chromeProcess.stderr.pipe(process.stderr); chromeProcess.stdout.pipe(process.stdout); diff --git a/src/firefox/Launcher.ts b/src/firefox/Launcher.ts index 30cdcd6df2..c89454f626 100644 --- a/src/firefox/Launcher.ts +++ b/src/firefox/Launcher.ts @@ -121,6 +121,15 @@ export class Launcher { } ); + if (!firefoxProcess.pid) { + let reject; + const result = new Promise((f, r) => reject = r); + firefoxProcess.once('error', error => { + reject(new Error('Failed to launch browser: ' + error)); + }); + return result as Promise; + } + if (dumpio) { firefoxProcess.stderr.pipe(process.stderr); firefoxProcess.stdout.pipe(process.stdout); diff --git a/src/webkit/Launcher.ts b/src/webkit/Launcher.ts index 4ab5337982..bc7d396013 100644 --- a/src/webkit/Launcher.ts +++ b/src/webkit/Launcher.ts @@ -22,6 +22,8 @@ import { Connection } from './Connection'; import { Viewport } from './Page'; import { PipeTransport } from './PipeTransport'; +const DEFAULT_ARGS = [ +]; export class Launcher { private _projectRoot: string; @@ -32,8 +34,18 @@ export class Launcher { this._preferredRevision = preferredRevision; } + defaultArgs(options: any = {}) { + const { + args = [], + } = options; + const webkitArguments = [...DEFAULT_ARGS]; + webkitArguments.push(...args); + return webkitArguments; + } + async launch(options: LauncherLaunchOptions = {}): Promise { const { + ignoreDefaultArgs = false, args = [], dumpio = false, executablePath = null, @@ -45,7 +57,12 @@ export class Launcher { slowMo = 0 } = options; - const webkitArguments = args.slice(); + const webkitArguments = []; + if (!ignoreDefaultArgs) + webkitArguments.push(...this.defaultArgs(options)); + else + webkitArguments.push(...args); + let webkitExecutable = executablePath; if (!executablePath) { const {missingText, executablePath} = this._resolveExecutablePath(); @@ -73,6 +90,15 @@ export class Launcher { } ); + if (!webkitProcess.pid) { + let reject; + const result = new Promise((f, r) => reject = r); + webkitProcess.once('error', error => { + reject(new Error('Failed to launch browser: ' + error)); + }); + return result as Promise; + } + if (dumpio) { webkitProcess.stderr.pipe(process.stderr); webkitProcess.stdout.pipe(process.stdout); @@ -147,6 +173,7 @@ export class Launcher { } export type LauncherLaunchOptions = { + ignoreDefaultArgs?: boolean, args?: string[], executablePath?: string, handleSIGINT?: boolean, diff --git a/src/webkit/Playwright.ts b/src/webkit/Playwright.ts index 9710cbfa2b..6e8282243d 100644 --- a/src/webkit/Playwright.ts +++ b/src/webkit/Playwright.ts @@ -48,6 +48,10 @@ export class Playwright { return Errors; } + defaultArgs(options: any | undefined): string[] { + return this._launcher.defaultArgs(options); + } + createBrowserFetcher(options: BrowserFetcherOptions | undefined): BrowserFetcher { return new BrowserFetcher(this._projectRoot, options); } diff --git a/test/launcher.spec.js b/test/launcher.spec.js index e6e6dfa0bf..b8a3012a76 100644 --- a/test/launcher.spec.js +++ b/test/launcher.spec.js @@ -101,7 +101,7 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p } }); }); - describe.skip(WEBKIT)('Playwright.launch', function() { + describe('Playwright.launch', function() { it('should reject all promises when browser is closed', async() => { const browser = await playwright.launch(defaultBrowserOptions); const page = await browser.newPage(); @@ -117,7 +117,7 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p await playwright.launch(options).catch(e => waitError = e); expect(waitError.message).toContain('Failed to launch'); }); - it('userDataDir option', async({server}) => { + it.skip(WEBKIT)('userDataDir option', async({server}) => { const userDataDir = await mkdtempAsync(TMP_FOLDER); const options = Object.assign({userDataDir}, defaultBrowserOptions); const browser = await playwright.launch(options); @@ -129,7 +129,7 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p // This might throw. See https://github.com/GoogleChrome/puppeteer/issues/2778 await rmAsync(userDataDir).catch(e => {}); }); - it('userDataDir argument', async({server}) => { + it.skip(WEBKIT)('userDataDir argument', async({server}) => { const userDataDir = await mkdtempAsync(TMP_FOLDER); const options = Object.assign({}, defaultBrowserOptions); if (CHROME || WEBKIT) { @@ -151,7 +151,7 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p // This might throw. See https://github.com/GoogleChrome/puppeteer/issues/2778 await rmAsync(userDataDir).catch(e => {}); }); - it('userDataDir option should restore state', async({server}) => { + it.skip(WEBKIT)('userDataDir option should restore state', async({server}) => { const userDataDir = await mkdtempAsync(TMP_FOLDER); const options = Object.assign({userDataDir}, defaultBrowserOptions); const browser = await playwright.launch(options); @@ -187,11 +187,13 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p await rmAsync(userDataDir).catch(e => {}); }); it('should return the default arguments', async() => { - if (CHROME || WEBKIT) { + if (CHROME) { expect(playwright.defaultArgs()).toContain('--no-first-run'); expect(playwright.defaultArgs()).toContain('--headless'); expect(playwright.defaultArgs({headless: false})).not.toContain('--headless'); expect(playwright.defaultArgs({userDataDir: 'foo'})).toContain('--user-data-dir=foo'); + } else if (WEBKIT) { + expect(playwright.defaultArgs().length).toBe(0); } else { expect(playwright.defaultArgs({browser: 'firefox'})).toContain('-headless'); expect(playwright.defaultArgs({browser: 'firefox', headless: false})).not.toContain('-headless'); @@ -208,7 +210,7 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p await page.close(); await browser.close(); }); - it('should filter out ignored default arguments', async() => { + it.skip(WEBKIT)('should filter out ignored default arguments', async() => { // Make sure we launch with `--enable-automation` by default. const defaultArgs = playwright.defaultArgs(defaultBrowserOptions); const browser = await playwright.launch(Object.assign({}, defaultBrowserOptions, { @@ -221,13 +223,13 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p expect(spawnargs.indexOf(defaultArgs[2])).toBe(-1); await browser.close(); }); - it.skip(FFOX)('should have default URL when launching browser', async function() { + it.skip(FFOX || WEBKIT)('should have default URL when launching browser', async function() { const browser = await playwright.launch(defaultBrowserOptions); const pages = (await browser.pages()).map(page => page.url()); expect(pages).toEqual(['about:blank']); await browser.close(); }); - it.skip(FFOX)('should have custom URL when launching browser', async function({server}) { + it.skip(FFOX || WEBKIT)('should have custom URL when launching browser', async function({server}) { const options = Object.assign({}, defaultBrowserOptions); options.args = [server.EMPTY_PAGE].concat(options.args || []); const browser = await playwright.launch(options); @@ -239,7 +241,7 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p expect(page.url()).toBe(server.EMPTY_PAGE); await browser.close(); }); - it('should set the default viewport', async() => { + it.skip(WEBKIT)('should set the default viewport', async() => { const options = Object.assign({}, defaultBrowserOptions, { defaultViewport: { width: 456, @@ -360,7 +362,7 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p }); }); - describe.skip(WEBKIT)('Top-level requires', function() { + describe('Top-level requires', function() { it('should require top-level Errors', async() => { const Errors = require(path.join(utils.projectRoot(), '/Errors')); expect(Errors.TimeoutError).toBe(playwright.errors.TimeoutError);