feat(launcher): throw on browser launch failure, default args (#119)

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<Browser>;
+    }
+
     if (dumpio) {
       chromeProcess.stderr.pipe(process.stderr);
       chromeProcess.stdout.pipe(process.stdout);

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<Browser>;
+    }
+
     if (dumpio) {
       firefoxProcess.stderr.pipe(process.stderr);
       firefoxProcess.stdout.pipe(process.stdout);

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<Browser> {
     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<Browser>;
+    }
+
     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,

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);
   }

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);