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

Author: dgozman

.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"
-        PWCHANNELTRANSPORT: ${{ matrix.transport }}
+        PWCHANNEL: ${{ 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,15 +187,17 @@ 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');
     expect(await page.evaluate(() => window.innerWidth)).toBe(375);
     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.PWCHANNELJSON)
-          message = JSON.parse(JSON.stringify(message));
-        setImmediate(() => connection.dispatch(message));
-      };
-      connection.onmessage = async message => {
-        if (process.env.PWCHANNELJSON)
-          message = JSON.parse(JSON.stringify(message));
-        const result = await dispatcherConnection.dispatch(message);
-        await new Promise(f => setImmediate(f));
-        return result;
-      };
-      new PlaywrightDispatcher(dispatcherConnection.rootDispatcher(), this._playwright);
-      this.overriddenPlaywright = await connection.waitForObjectWithKnownName('playwright');
+      if (process.env.PWCHANNEL === 'wire') {
+        this.spawnedProcess = childProcess.fork(path.join(__dirname, '..', 'lib', 'rpc', 'server'), [], {
+          stdio: 'pipe',
+          detached: process.platform !== 'win32',
+        });
+        this.spawnedProcess.once('exit', (exitCode, signal) => {
+          this.spawnedProcess = undefined;
+          if (!this.expectExit)
+            throw new Error(`Server closed with exitCode=${exitCode} signal=${signal}`);
+        });
+        process.on('exit', () => this._killProcess());
+        const transport = new Transport(this.spawnedProcess.stdin, this.spawnedProcess.stdout);
+        connection.onmessage = message => transport.send(JSON.stringify(message));
+        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}) => {