feat(rpc): support no-serialization mode, run hook tests (#2925)

Added rpc_json_linux bots to excercise serialization - these
do not run hook tests.

Author: dgozman

.github/workflows/tests.yml

@@ -174,6 +174,7 @@ jobs:
       fail-fast: false
       matrix:
         browser: [chromium, firefox, webkit]
+        transport: [json, object]
     runs-on: ubuntu-18.04
     steps:
     - uses: actions/checkout@v2
@@ -194,21 +195,19 @@ jobs:
         BROWSER: ${{ matrix.browser }}
         DEBUG: "*,-pw:wrapped*"
         PWCHANNEL: "1"
-    # Ensure output folder exists just in case it was not created by the test run.
-    - run: node -e "require('fs').mkdirSync(require('path').join('test', 'output-${{ matrix.browser }}'), {recursive:true})"
-      if: failure()
+        PWCHANNELTRANSPORT: ${{ matrix.transport }}
     - uses: actions/upload-artifact@v1
       if: failure()
       with:
-        name: rpc-${{ matrix.browser }}-linux-output
+        name: rpc-${{ matrix.transport }}-${{ matrix.browser }}-linux-output
         path: test/output-${{ matrix.browser }}
     - uses: actions/upload-artifact@v1
       if: failure()
       with:
-        name: rpc-${{ matrix.browser }}-linux-testrun.log
+        name: rpc-${{ matrix.transport }}-${{ matrix.browser }}-linux-testrun.log
         path: testrun.log
     - uses: actions/upload-artifact@v1
       if: failure()
       with:
-        name: rpc-${{ matrix.browser }}-linux-coredumps
+        name: rpc-${{ matrix.transport }}-${{ matrix.browser }}-linux-coredumps
         path: coredumps

src/rpc/client.ts

@@ -23,8 +23,8 @@ import { Transport } from './transport';
   const spawnedProcess = childProcess.fork(path.join(__dirname, 'server'), [], { stdio: 'pipe' });
   const transport = new Transport(spawnedProcess.stdin, spawnedProcess.stdout);
   const connection = new Connection();
-  connection.onmessage = message => transport.send(message);
-  transport.onmessage = message => connection.dispatch(message);
+  connection.onmessage = message => transport.send(JSON.stringify(message));
+  transport.onmessage = message => connection.dispatch(JSON.parse(message));
 
   const playwright = await connection.waitForObjectWithKnownName('playwright');
   const browser = await playwright.chromium.launch({ headless: false });

src/rpc/client/connection.ts

@@ -43,7 +43,7 @@ class Root extends ChannelOwner<Channel, {}> {
 export class Connection {
   readonly _objects = new Map<string, ChannelOwner>();
   private _waitingForObject = new Map<string, any>();
-  onmessage = (message: string): void => {};
+  onmessage = (message: object): void => {};
   private _lastId = 0;
   private _callbacks = new Map<number, { resolve: (a: any) => void, reject: (a: Error) => void }>();
   private _rootObject: ChannelOwner;
@@ -62,19 +62,18 @@ export class Connection {
     const id = ++this._lastId;
     const converted = { id, ...message, params: this._replaceChannelsWithGuids(message.params) };
     debug('pw:channel:command')(converted);
-    this.onmessage(JSON.stringify(converted));
+    this.onmessage(converted);
     return new Promise((resolve, reject) => this._callbacks.set(id, { resolve, reject }));
   }
 
   _debugScopeState(): any {
     return this._rootObject._debugScopeState();
   }
 
-  dispatch(message: string) {
-    const parsedMessage = JSON.parse(message);
-    const { id, guid, method, params, result, error } = parsedMessage;
+  dispatch(message: object) {
+    const { id, guid, method, params, result, error } = message as any;
     if (id) {
-      debug('pw:channel:response')(parsedMessage);
+      debug('pw:channel:response')(message);
       const callback = this._callbacks.get(id)!;
       this._callbacks.delete(id);
       if (error)
@@ -84,7 +83,7 @@ export class Connection {
       return;
     }
 
-    debug('pw:channel:event')(parsedMessage);
+    debug('pw:channel:event')(message);
     if (method === '__create__') {
       this._createRemoteObject(guid, params.type, params.guid, params.initializer);
       return;

src/rpc/server.ts

@@ -21,8 +21,8 @@ import { PlaywrightDispatcher } from './server/playwrightDispatcher';
 
 const dispatcherConnection = new DispatcherConnection();
 const transport = new Transport(process.stdout, process.stdin);
-transport.onmessage = message => dispatcherConnection.dispatch(message);
-dispatcherConnection.onmessage = message => transport.send(message);
+transport.onmessage = message => dispatcherConnection.dispatch(JSON.parse(message));
+dispatcherConnection.onmessage = message => transport.send(JSON.stringify(message));
 
 const playwright = new Playwright(__dirname, require('../../browsers.json')['browsers']);
 new PlaywrightDispatcher(dispatcherConnection.rootDispatcher(), playwright);

src/rpc/server/dispatcher.ts

@@ -113,10 +113,10 @@ class Root extends Dispatcher<{}, {}> {
 export class DispatcherConnection {
   readonly _dispatchers = new Map<string, Dispatcher<any, any>>();
   private _rootDispatcher: Root;
-  onmessage = (message: string) => {};
+  onmessage = (message: object) => {};
 
   async sendMessageToClient(guid: string, method: string, params: any): Promise<any> {
-    this.onmessage(JSON.stringify({ guid, method, params: this._replaceDispatchersWithGuids(params) }));
+    this.onmessage({ guid, method, params: this._replaceDispatchersWithGuids(params) });
   }
 
   constructor() {
@@ -127,23 +127,22 @@ export class DispatcherConnection {
     return this._rootDispatcher;
   }
 
-  async dispatch(message: string) {
-    const parsedMessage = JSON.parse(message);
-    const { id, guid, method, params } = parsedMessage;
+  async dispatch(message: object) {
+    const { id, guid, method, params } = message as any;
     const dispatcher = this._dispatchers.get(guid);
     if (!dispatcher) {
-      this.onmessage(JSON.stringify({ id, error: serializeError(new Error('Target browser or context has been closed')) }));
+      this.onmessage({ id, error: serializeError(new Error('Target browser or context has been closed')) });
       return;
     }
     if (method === 'debugScopeState') {
-      this.onmessage(JSON.stringify({ id, result: this._rootDispatcher._debugScopeState() }));
+      this.onmessage({ id, result: this._rootDispatcher._debugScopeState() });
       return;
     }
     try {
       const result = await (dispatcher as any)[method](this._replaceGuidsWithDispatchers(params));
-      this.onmessage(JSON.stringify({ id, result: this._replaceDispatchersWithGuids(result) }));
+      this.onmessage({ id, result: this._replaceDispatchersWithGuids(result) });
     } catch (e) {
-      this.onmessage(JSON.stringify({ id, error: serializeError(e) }));
+      this.onmessage({ id, error: serializeError(e) });
     }
   }
 

src/rpc/transport.ts

@@ -23,7 +23,7 @@ export class Transport {
   private _closed = false;
   private _bytesLeft = 0;
 
-  onmessage?: (message: any) => void;
+  onmessage?: (message: string) => void;
   onclose?: () => void;
 
   constructor(pipeWrite: NodeJS.WritableStream, pipeRead: NodeJS.ReadableStream) {

test/defaultbrowsercontext.spec.js

@@ -335,9 +335,9 @@ describe('launchPersistentContext()', function() {
     expect(error.message).toContain('can not specify page');
     await removeUserDataDir(userDataDir);
   });
-  it.skip(USES_HOOKS)('should have passed URL when launching with ignoreDefaultArgs: true', async ({browserType, defaultBrowserOptions, server}) => {
+  it('should have passed URL when launching with ignoreDefaultArgs: true', async ({playwrightPath, browserType, defaultBrowserOptions, server}) => {
     const userDataDir = await makeUserDataDir();
-    const args = browserType._defaultArgs(defaultBrowserOptions, 'persistent', userDataDir, 0).filter(a => a !== 'about:blank');
+    const args = require(playwrightPath)[browserType.name()]._defaultArgs(defaultBrowserOptions, 'persistent', userDataDir, 0).filter(a => a !== 'about:blank');
     const options = {
       ...defaultBrowserOptions,
       args: [...args, server.EMPTY_PAGE],
@@ -364,7 +364,7 @@ describe('launchPersistentContext()', function() {
     const e = new Error('Dummy');
     const options = { ...defaultBrowserOptions, __testHookBeforeCreateBrowser: () => { throw e; } };
     const error = await browserType.launchPersistentContext(userDataDir, options).catch(e => e);
-    expect(error).toBe(e);
+    expect(error.message).toContain('Dummy');
     await removeUserDataDir(userDataDir);
   });
   it('should fire close event for a persistent context', async(state) => {
@@ -373,5 +373,5 @@ describe('launchPersistentContext()', function() {
     context.on('close', () => closed = true);
     await close(state);
     expect(closed).toBe(true);
-  });  
+  });
 });

test/environments.js

@@ -167,9 +167,13 @@ class PlaywrightEnvironment {
       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;

test/evaluation.spec.js

@@ -17,7 +17,7 @@
 
 const utils = require('./utils');
 const path = require('path');
-const {FFOX, CHROMIUM, WEBKIT, USES_HOOKS} = utils.testOptions(browserType);
+const {FFOX, CHROMIUM, WEBKIT, CHANNEL} = utils.testOptions(browserType);
 
 describe('Page.evaluate', function() {
   it('should work', async({page, server}) => {
@@ -574,14 +574,14 @@ describe('Frame.evaluate', function() {
     else
       expect(page._delegate._contextIdToContext.size).toBe(count);
   }
-  it.skip(USES_HOOKS)('should dispose context on navigation', async({page, server}) => {
+  it.skip(CHANNEL)('should dispose context on navigation', async({page, server}) => {
     await page.goto(server.PREFIX + '/frames/one-frame.html');
     expect(page.frames().length).toBe(2);
     expectContexts(page, 4);
     await page.goto(server.EMPTY_PAGE);
     expectContexts(page, 2);
   });
-  it.skip(USES_HOOKS)('should dispose context on cross-origin navigation', async({page, server}) => {
+  it.skip(CHANNEL)('should dispose context on cross-origin navigation', async({page, server}) => {
     await page.goto(server.PREFIX + '/frames/one-frame.html');
     expect(page.frames().length).toBe(2);
     expectContexts(page, 4);

test/launcher.spec.js

@@ -67,7 +67,7 @@ describe('Playwright', function() {
       const e = new Error('Dummy');
       const options = { ...defaultBrowserOptions, __testHookBeforeCreateBrowser: () => { throw e; }, timeout: 9000 };
       const error = await browserType.launch(options).catch(e => e);
-      expect(error).toBe(e);
+      expect(error.message).toContain('Dummy');
     });
     it.skip(USES_HOOKS)('should report launch log', async({browserType, defaultBrowserOptions}) => {
       const e = new Error('Dummy');