feat(rpc): run rpc tests in-process and out-of-process (#2929)

.github/workflows/tests.yml

@@ -174,7 +174,7 @@ jobs:
       fail-fast: false
       matrix:
         browser: [chromium, firefox, webkit]
-        transport: [json, object]
+        transport: [wire, object]
     runs-on: ubuntu-18.04
     steps:
     - uses: actions/checkout@v2
@@ -194,8 +194,7 @@ jobs:
       env:
         BROWSER: ${{ matrix.browser }}
         DEBUG: "*,-pw:wrapped*"
-        PWCHANNEL: "1"
+        PWCHANNEL: ${{ matrix.transport }}
-        PWCHANNELTRANSPORT: ${{ matrix.transport }}
     - uses: actions/upload-artifact@v1
       if: failure()
       with:

src/rpc/client.ts

@@ -22,6 +22,7 @@ import { Transport } from './transport';
 (async () => {
   const spawnedProcess = childProcess.fork(path.join(__dirname, 'server'), [], { stdio: 'pipe' });
   const transport = new Transport(spawnedProcess.stdin, spawnedProcess.stdout);
+  transport.onclose = () => process.exit(0);
   const connection = new Connection();
   connection.onmessage = message => transport.send(JSON.stringify(message));
   transport.onmessage = message => connection.dispatch(JSON.parse(message));

src/rpc/server.ts

@@ -21,6 +21,7 @@ import { PlaywrightDispatcher } from './server/playwrightDispatcher';
 
 const dispatcherConnection = new DispatcherConnection();
 const transport = new Transport(process.stdout, process.stdin);
+transport.onclose = () => process.exit(0);
 transport.onmessage = message => dispatcherConnection.dispatch(JSON.parse(message));
 dispatcherConnection.onmessage = message => transport.send(JSON.stringify(message));
 

src/rpc/transport.ts

@@ -29,7 +29,7 @@ export class Transport {
   constructor(pipeWrite: NodeJS.WritableStream, pipeRead: NodeJS.ReadableStream) {
     this._pipeWrite = pipeWrite;
     pipeRead.on('data', buffer => this._dispatch(buffer));
-    pipeRead.on('close', () => process.exit(0));
+    pipeRead.on('close', () => this.onclose && this.onclose());
     this.onmessage = undefined;
     this.onclose = undefined;
   }

test/chromium/session.spec.js

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-const {FFOX, CHROMIUM, WEBKIT, CHANNEL} = require('../utils').testOptions(browserType);
+const {FFOX, CHROMIUM, WEBKIT, CHANNEL, USES_HOOKS} = require('../utils').testOptions(browserType);
 
 describe('ChromiumBrowserContext.createSession', function() {
   it('should work', async function({page, browser, server}) {
@@ -66,7 +66,7 @@ describe('ChromiumBrowserContext.createSession', function() {
     }
     expect(error.message).toContain(CHANNEL ? 'Target browser or context has been closed' : 'Session closed.');
   });
-  it('should throw nice errors', async function({page, browser}) {
+  it.skip(USES_HOOKS)('should throw nice errors', async function({page, browser}) {
     const client = await page.context().newCDPSession(page);
     const error = await theSourceOfTheProblems().catch(error => error);
     expect(error.stack).toContain('theSourceOfTheProblems');

test/dispatchevent.spec.js

@@ -15,7 +15,7 @@
  */
 
 const utils = require('./utils');
-const {FFOX, CHROMIUM, WEBKIT, WIN} = utils.testOptions(browserType);
+const {FFOX, CHROMIUM, WEBKIT, WIN, USES_HOOKS} = utils.testOptions(browserType);
 
 describe('Page.dispatchEvent(click)', function() {
   it('should dispatch click event', async({page, server}) => {
@@ -97,7 +97,7 @@ describe('Page.dispatchEvent(click)', function() {
     await watchdog;
     expect(await page.evaluate(() => window.clicked)).toBe(true);
   });
-  it('should be atomic', async({page}) => {
+  it.skip(USES_HOOKS)('should be atomic', async({page}) => {
     const createDummySelector = () => ({
       create(root, target) {},
       query(root, selector) {

test/elementhandle.spec.js

@@ -16,7 +16,7 @@
  */
 
 const utils = require('./utils');
-const {FFOX, CHROMIUM, WEBKIT} = require('./utils').testOptions(browserType);
+const {FFOX, CHROMIUM, WEBKIT, USES_HOOKS} = require('./utils').testOptions(browserType);
 
 describe('ElementHandle.boundingBox', function() {
   it.fail(FFOX && !HEADLESS)('should work', async({page, server}) => {
@@ -484,7 +484,7 @@ describe('ElementHandle convenience API', function() {
     expect(await handle.textContent()).toBe('Text,\nmore text');
     expect(await page.textContent('#inner')).toBe('Text,\nmore text');
   });
-  it('textContent should be atomic', async({page}) => {
+  it.skip(USES_HOOKS)('textContent should be atomic', async({page}) => {
     const createDummySelector = () => ({
       create(root, target) {},
       query(root, selector) {
@@ -506,7 +506,7 @@ describe('ElementHandle convenience API', function() {
     expect(tc).toBe('Hello');
     expect(await page.evaluate(() => document.querySelector('div').textContent)).toBe('modified');
   });
-  it('innerText should be atomic', async({page}) => {
+  it.skip(USES_HOOKS)('innerText should be atomic', async({page}) => {
     const createDummySelector = () => ({
       create(root, target) {},
       query(root, selector) {
@@ -528,7 +528,7 @@ describe('ElementHandle convenience API', function() {
     expect(tc).toBe('Hello');
     expect(await page.evaluate(() => document.querySelector('div').innerText)).toBe('modified');
   });
-  it('innerHTML should be atomic', async({page}) => {
+  it.skip(USES_HOOKS)('innerHTML should be atomic', async({page}) => {
     const createDummySelector = () => ({
       create(root, target) {},
       query(root, selector) {
@@ -550,7 +550,7 @@ describe('ElementHandle convenience API', function() {
     expect(tc).toBe('Hello<span>world</span>');
     expect(await page.evaluate(() => document.querySelector('div').innerHTML)).toBe('modified');
   });
-  it('getAttribute should be atomic', async({page}) => {
+  it.skip(USES_HOOKS)('getAttribute should be atomic', async({page}) => {
     const createDummySelector = () => ({
       create(root, target) {},
       query(root, selector) {

test/emulation.spec.js

@@ -17,8 +17,6 @@
 
 const utils = require('./utils');
 const {FFOX, CHROMIUM, WEBKIT, LINUX} = utils.testOptions(browserType);
-const iPhone = playwright.devices['iPhone 6'];
-const iPhoneLandscape = playwright.devices['iPhone 6 landscape'];
 
 describe('BrowserContext({viewport})', function() {
   it('should get the proper default viewport size', async({page, server}) => {
@@ -84,7 +82,8 @@ describe('BrowserContext({viewport})', function() {
 
 describe.skip(FFOX)('viewport.isMobile', () => {
   // Firefox does not support isMobile.
-  it('should support mobile emulation', async({browser, server}) => {
+  it('should support mobile emulation', async({playwright, browser, server}) => {
+    const iPhone = playwright.devices['iPhone 6'];
     const context = await browser.newContext({ ...iPhone });
     const page = await context.newPage();
     await page.goto(server.PREFIX + '/mobile.html');
@@ -93,7 +92,8 @@ describe.skip(FFOX)('viewport.isMobile', () => {
     expect(await page.evaluate(() => window.innerWidth)).toBe(400);
     await context.close();
   });
-  it('should support touch emulation', async({browser, server}) => {
+  it('should support touch emulation', async({playwright, browser, server}) => {
+    const iPhone = playwright.devices['iPhone 6'];
     const context = await browser.newContext({ ...iPhone });
     const page = await context.newPage();
     await page.goto(server.PREFIX + '/mobile.html');
@@ -114,7 +114,8 @@ describe.skip(FFOX)('viewport.isMobile', () => {
       return promise;
     }
   });
-  it('should be detectable by Modernizr', async({browser, server}) => {
+  it('should be detectable by Modernizr', async({playwright, browser, server}) => {
+    const iPhone = playwright.devices['iPhone 6'];
     const context = await browser.newContext({ ...iPhone });
     const page = await context.newPage();
     await page.goto(server.PREFIX + '/detect-touch.html');
@@ -129,7 +130,9 @@ describe.skip(FFOX)('viewport.isMobile', () => {
     expect(await page.evaluate(() => Modernizr.touchevents)).toBe(true);
     await context.close();
   });
-  it('should support landscape emulation', async({browser, server}) => {
+  it('should support landscape emulation', async({playwright, browser, server}) => {
+    const iPhone = playwright.devices['iPhone 6'];
+    const iPhoneLandscape = playwright.devices['iPhone 6 landscape'];
     const context1 = await browser.newContext({ ...iPhone });
     const page1 = await context1.newPage();
     await page1.goto(server.PREFIX + '/mobile.html');
@@ -184,7 +187,8 @@ describe.skip(FFOX)('viewport.isMobile', () => {
 });
 
 describe.skip(FFOX)('Page.emulate', function() {
-  it('should work', async({browser, server}) => {
+  it('should work', async({playwright, browser, server}) => {
+    const iPhone = playwright.devices['iPhone 6'];
     const context = await browser.newContext({ ...iPhone });
     const page = await context.newPage();
     await page.goto(server.PREFIX + '/mobile.html');
@@ -192,7 +196,8 @@ describe.skip(FFOX)('Page.emulate', function() {
     expect(await page.evaluate(() => navigator.userAgent)).toContain('iPhone');
     await context.close();
   });
-  it('should support clicking', async({browser, server}) => {
+  it('should support clicking', async({playwright, browser, server}) => {
+    const iPhone = playwright.devices['iPhone 6'];
     const context = await browser.newContext({ ...iPhone });
     const page = await context.newPage();
     await page.goto(server.PREFIX + '/input/button.html');

test/environments.js

@@ -19,9 +19,11 @@ const utils = require('./utils');
 const fs = require('fs');
 const path = require('path');
 const rm = require('rimraf').sync;
+const childProcess = require('child_process');
 const {TestServer} = require('../utils/testserver/');
 const { DispatcherConnection } = require('../lib/rpc/server/dispatcher');
 const { Connection } = require('../lib/rpc/client/connection');
+const { Transport } = require('../lib/rpc/transport');
 const { PlaywrightDispatcher } = require('../lib/rpc/server/playwrightDispatcher');
 
 class ServerEnvironment {
@@ -156,37 +158,72 @@ class TraceTestEnvironment {
 class PlaywrightEnvironment {
   constructor(playwright) {
     this._playwright = playwright;
+    this.spawnedProcess = undefined;
+    this.expectExit = false;
   }
 
   name() { return 'Playwright'; };
 
   async beforeAll(state) {
-    // Channel substitute
-    this.overriddenPlaywright = this._playwright;
     if (process.env.PWCHANNEL) {
-      const dispatcherConnection = new DispatcherConnection();
       const connection = new Connection();
-      dispatcherConnection.onmessage = async message => {
+      if (process.env.PWCHANNEL === 'wire') {
-        if (process.env.PWCHANNELJSON)
+        this.spawnedProcess = childProcess.fork(path.join(__dirname, '..', 'lib', 'rpc', 'server'), [], {
-          message = JSON.parse(JSON.stringify(message));
+          stdio: 'pipe',
-        setImmediate(() => connection.dispatch(message));
+          detached: process.platform !== 'win32',
-      };
+        });
-      connection.onmessage = async message => {
+        this.spawnedProcess.once('exit', (exitCode, signal) => {
-        if (process.env.PWCHANNELJSON)
+          this.spawnedProcess = undefined;
-          message = JSON.parse(JSON.stringify(message));
+          if (!this.expectExit)
-        const result = await dispatcherConnection.dispatch(message);
+            throw new Error(`Server closed with exitCode=${exitCode} signal=${signal}`);
-        await new Promise(f => setImmediate(f));
+        });
-        return result;
+        process.on('exit', () => this._killProcess());
-      };
+        const transport = new Transport(this.spawnedProcess.stdin, this.spawnedProcess.stdout);
-      new PlaywrightDispatcher(dispatcherConnection.rootDispatcher(), this._playwright);
+        connection.onmessage = message => transport.send(JSON.stringify(message));
-      this.overriddenPlaywright = await connection.waitForObjectWithKnownName('playwright');
+        transport.onmessage = message => connection.dispatch(JSON.parse(message));
+      } else {
+        const dispatcherConnection = new DispatcherConnection();
+        dispatcherConnection.onmessage = async message => {
+          setImmediate(() => connection.dispatch(message));
+        };
+        connection.onmessage = async message => {
+          const result = await dispatcherConnection.dispatch(message);
+          await new Promise(f => setImmediate(f));
+          return result;
+        };
+        new PlaywrightDispatcher(dispatcherConnection.rootDispatcher(), this._playwright);
+        state.toImpl = x => dispatcherConnection._dispatchers.get(x._guid)._object;
+      }
+      state.playwright = await connection.waitForObjectWithKnownName('playwright');
+    } else {
+      state.toImpl = x => x;
+      state.playwright = this._playwright;
     }
-    state.playwright = this.overriddenPlaywright;
   }
 
   async afterAll(state) {
+    if (this.spawnedProcess) {
+      const exited = new Promise(f => this.spawnedProcess.once('exit', f));
+      this.expectExit = true;
+      this.spawnedProcess.kill();
+      await exited;
+    }
     delete state.playwright;
   }
+
+  _killProcess() {
+    if (this.spawnedProcess && this.spawnedProcess.pid) {
+      this.expectExit = true;
+      try {
+        if (process.platform === 'win32')
+          childProcess.execSync(`taskkill /pid ${this.spawnedProcess.pid} /T /F`);
+        else
+          process.kill(-this.spawnedProcess.pid, 'SIGKILL');
+      } catch (e) {
+        // the process might have already stopped
+      }
+    }
+  }
 }
 
 class BrowserTypeEnvironment {

test/evaluation.spec.js

@@ -17,7 +17,7 @@
 
 const utils = require('./utils');
 const path = require('path');
-const {FFOX, CHROMIUM, WEBKIT, CHANNEL} = utils.testOptions(browserType);
+const {FFOX, CHROMIUM, WEBKIT, USES_HOOKS} = utils.testOptions(browserType);
 
 describe('Page.evaluate', function() {
   it('should work', async({page, server}) => {
@@ -373,7 +373,7 @@ describe('Page.evaluate', function() {
     });
     expect(result).toBe(undefined);
   });
-  it.slow()('should transfer 100Mb of data from page to node.js', async({page, server}) => {
+  it.slow().skip(USES_HOOKS)('should transfer 100Mb of data from page to node.js', async({page, server}) => {
     const a = await page.evaluate(() => Array(100 * 1024 * 1024 + 1).join('a'));
     expect(a.length).toBe(100 * 1024 * 1024);
   });
@@ -568,25 +568,25 @@ describe('Frame.evaluate', function() {
     expect(await page.frames()[1].evaluate(() => document.body.textContent.trim())).toBe(`Hi, I'm frame`);
   });
 
-  function expectContexts(page, count) {
+  function expectContexts(pageImpl, count) {
     if (CHROMIUM)
-      expect(page._delegate._mainFrameSession._contextIdToContext.size).toBe(count);
+      expect(pageImpl._delegate._mainFrameSession._contextIdToContext.size).toBe(count);
     else
-      expect(page._delegate._contextIdToContext.size).toBe(count);
+      expect(pageImpl._delegate._contextIdToContext.size).toBe(count);
   }
-  it.skip(CHANNEL)('should dispose context on navigation', async({page, server}) => {
+  it.skip(USES_HOOKS)('should dispose context on navigation', async({page, server, toImpl}) => {
     await page.goto(server.PREFIX + '/frames/one-frame.html');
     expect(page.frames().length).toBe(2);
-    expectContexts(page, 4);
+    expectContexts(toImpl(page), 4);
     await page.goto(server.EMPTY_PAGE);
-    expectContexts(page, 2);
+    expectContexts(toImpl(page), 2);
   });
-  it.skip(CHANNEL)('should dispose context on cross-origin navigation', async({page, server}) => {
+  it.skip(USES_HOOKS)('should dispose context on cross-origin navigation', async({page, server, toImpl}) => {
     await page.goto(server.PREFIX + '/frames/one-frame.html');
     expect(page.frames().length).toBe(2);
-    expectContexts(page, 4);
+    expectContexts(toImpl(page), 4);
     await page.goto(server.CROSS_PROCESS_PREFIX + '/empty.html');
-    expectContexts(page, 2);
+    expectContexts(toImpl(page), 2);
   });
 
   it('should execute after cross-site navigation', async({page, server}) => {

test/page.spec.js

@@ -18,7 +18,7 @@
 const path = require('path');
 const util = require('util');
 const vm = require('vm');
-const {FFOX, CHROMIUM, WEBKIT, WIN, CHANNEL} = require('./utils').testOptions(browserType);
+const {FFOX, CHROMIUM, WEBKIT, WIN, USES_HOOKS} = require('./utils').testOptions(browserType);
 
 describe('Page.close', function() {
   it('should reject all promises when page is closed', async({context}) => {
@@ -101,7 +101,7 @@ describe('Page.Events.Load', function() {
   });
 });
 
-describe('Async stacks', () => {
+describe.skip(USES_HOOKS)('Async stacks', () => {
   it('should work', async({page, server}) => {
     server.setRoute('/empty.html', (req, res) => {
       req.socket.end();
@@ -113,50 +113,50 @@ describe('Async stacks', () => {
   });
 });
 
-describe.fail(FFOX && WIN).skip(CHANNEL)('Page.Events.Crash', function() {
+describe.fail(FFOX && WIN).skip(USES_HOOKS)('Page.Events.Crash', function() {
   // Firefox Win: it just doesn't crash sometimes.
 
-  function crash(page) {
+  function crash(pageImpl) {
     if (CHROMIUM)
-      page.goto('chrome://crash').catch(e => {});
+      pageImpl.goto('chrome://crash').catch(e => {});
     else if (WEBKIT)
-      page._delegate._session.send('Page.crash', {}).catch(e => {});
+      pageImpl._delegate._session.send('Page.crash', {}).catch(e => {});
     else if (FFOX)
-      page._delegate._session.send('Page.crash', {}).catch(e => {});
+      pageImpl._delegate._session.send('Page.crash', {}).catch(e => {});
   }
 
-  it('should emit crash event when page crashes', async({page}) => {
+  it('should emit crash event when page crashes', async({page, toImpl}) => {
     await page.setContent(`<div>This page should crash</div>`);
-    crash(page);
+    crash(toImpl(page));
     await new Promise(f => page.on('crash', f));
   });
-  it('should throw on any action after page crashes', async({page}) => {
+  it('should throw on any action after page crashes', async({page, toImpl}) => {
     await page.setContent(`<div>This page should crash</div>`);
-    crash(page);
+    crash(toImpl(page));
     await page.waitForEvent('crash');
     const err = await page.evaluate(() => {}).then(() => null, e => e);
     expect(err).toBeTruthy();
     expect(err.message).toContain('crash');
   });
-  it('should cancel waitForEvent when page crashes', async({page}) => {
+  it('should cancel waitForEvent when page crashes', async({page, toImpl}) => {
     await page.setContent(`<div>This page should crash</div>`);
     const promise = page.waitForEvent('response').catch(e => e);
-    crash(page);
+    crash(toImpl(page));
     const error = await promise;
     expect(error.message).toContain('Page crashed');
   });
-  it('should cancel navigation when page crashes', async({page, server}) => {
+  it('should cancel navigation when page crashes', async({page, toImpl, server}) => {
     await page.setContent(`<div>This page should crash</div>`);
     server.setRoute('/one-style.css', () => {});
     const promise = page.goto(server.PREFIX + '/one-style.html').catch(e => e);
     await page.waitForNavigation({ waitUntil: 'domcontentloaded' });
-    crash(page);
+    crash(toImpl(page));
     const error = await promise;
     expect(error.message).toContain('Navigation failed because page crashed');
   });
-  it('should be able to close context when page crashes', async({page}) => {
+  it('should be able to close context when page crashes', async({page, toImpl}) => {
     await page.setContent(`<div>This page should crash</div>`);
-    crash(page);
+    crash(toImpl(page));
     await page.waitForEvent('crash');
     await page.context().close();
   });
@@ -1341,9 +1341,3 @@ describe('Page api coverage', function() {
     expect(await frame.evaluate(() => document.querySelector('textarea').value)).toBe('a');
   });
 });
-
-describe.skip(!CHANNEL)('Page channel', function() {
-  it('page should be client stub', async({page, server}) => {
-    expect(!!page._channel).toBeTruthy();
-  });
-});

test/queryselector.spec.js

@@ -17,7 +17,7 @@
 
 const path = require('path');
 const utils = require('./utils');
-const {FFOX, CHROMIUM, WEBKIT, CHANNEL} = utils.testOptions(browserType);
+const {FFOX, CHROMIUM, WEBKIT, CHANNEL, USES_HOOKS} = utils.testOptions(browserType);
 
 describe('Page.$eval', function() {
   it('should work with css selector', async({page, server}) => {
@@ -743,7 +743,7 @@ describe('attribute selector', () => {
   });
 });
 
-describe('selectors.register', () => {
+describe.skip(USES_HOOKS)('selectors.register', () => {
   it.skip(CHANNEL)('should work', async ({page}) => {
     const createTagSelector = () => ({
       create(root, target) {

test/recorder.spec.js

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-const {FFOX, CHROMIUM, WEBKIT, CHANNEL} = require('./utils').testOptions(browserType);
+const {FFOX, CHROMIUM, WEBKIT, USES_HOOKS} = require('./utils').testOptions(browserType);
 
 class WritableBuffer {
   constructor() {
@@ -51,15 +51,15 @@ class WritableBuffer {
   }
 }
 
-describe.skip(CHANNEL)('Recorder', function() {
+describe.skip(USES_HOOKS)('Recorder', function() {
   beforeEach(async state => {
     state.context = await state.browser.newContext();
     state.output = new WritableBuffer();
-    const debugController = state.context._initDebugModeForTest({ recorderOutput: state.output });
+    const debugController = state.toImpl(state.context)._initDebugModeForTest({ recorderOutput: state.output });
     state.page = await state.context.newPage();
     state.setContent = async (content) => {
       await state.page.setContent(content);
-      await debugController.ensureInstalledInFrameForTest(state.page.mainFrame());
+      await debugController.ensureInstalledInFrameForTest(state.toImpl(state.page.mainFrame()));
     };
   });
 

test/test.js

@@ -87,6 +87,7 @@ function collect(browserNames) {
   for (const e of config.globalEnvironments || [])
     testRunner.collector().useEnvironment(e);
 
+  // TODO(rpc): do not use global playwright and browserType, rely solely on environments.
   global.playwright = playwright;
 
   for (const browserName of browserNames) {

test/utils.js

@@ -202,7 +202,7 @@ const utils = module.exports = {
       GOLDEN_DIR,
       OUTPUT_DIR,
       ASSETS_DIR,
-      USES_HOOKS: process.env.PWCHANNELTRANSPORT === 'json',
+      USES_HOOKS: process.env.PWCHANNEL === 'wire',
       CHANNEL: !!process.env.PWCHANNEL,
       HEADLESS: !!valueFromEnv('HEADLESS', true),
     };

test/waittask.spec.js

@@ -16,7 +16,7 @@
  */
 
 const utils = require('./utils');
-const {FFOX, CHROMIUM, WEBKIT} = utils.testOptions(browserType);
+const {FFOX, CHROMIUM, WEBKIT, USES_HOOKS} = utils.testOptions(browserType);
 
 async function giveItTimeToLog(frame) {
   await frame.evaluate(() => new Promise(f => requestAnimationFrame(() => requestAnimationFrame(f))));
@@ -458,7 +458,7 @@ describe('Frame.waitForSelector', function() {
     await page.setContent(`<div class='zombo'>anything</div>`);
     expect(await page.evaluate(x => x.textContent, await waitForSelector)).toBe('anything');
   });
-  it('should have correct stack trace for timeout', async({page, server}) => {
+  it.skip(USES_HOOKS)('should have correct stack trace for timeout', async({page, server}) => {
     let error;
     await page.waitForSelector('.zombo', { timeout: 10 }).catch(e => error = e);
     expect(error.stack).toContain('waittask.spec.js');