diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 003f761127..2853721358 100644 --- a/.github/workflows/tests.yml +++ b/.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 diff --git a/src/rpc/client.ts b/src/rpc/client.ts index 6c5ecc7b73..fc8096937d 100644 --- a/src/rpc/client.ts +++ b/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 }); diff --git a/src/rpc/client/connection.ts b/src/rpc/client/connection.ts index b503995e85..9cc43e66fc 100644 --- a/src/rpc/client/connection.ts +++ b/src/rpc/client/connection.ts @@ -43,7 +43,7 @@ class Root extends ChannelOwner { export class Connection { readonly _objects = new Map(); private _waitingForObject = new Map(); - onmessage = (message: string): void => {}; + onmessage = (message: object): void => {}; private _lastId = 0; private _callbacks = new Map void, reject: (a: Error) => void }>(); private _rootObject: ChannelOwner; @@ -62,7 +62,7 @@ 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 })); } @@ -70,11 +70,10 @@ export class Connection { 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; diff --git a/src/rpc/server.ts b/src/rpc/server.ts index 68695ec961..e6fbe3b925 100644 --- a/src/rpc/server.ts +++ b/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); diff --git a/src/rpc/server/dispatcher.ts b/src/rpc/server/dispatcher.ts index 6c78f16155..1ffb71373e 100644 --- a/src/rpc/server/dispatcher.ts +++ b/src/rpc/server/dispatcher.ts @@ -113,10 +113,10 @@ class Root extends Dispatcher<{}, {}> { export class DispatcherConnection { readonly _dispatchers = new Map>(); private _rootDispatcher: Root; - onmessage = (message: string) => {}; + onmessage = (message: object) => {}; async sendMessageToClient(guid: string, method: string, params: any): Promise { - 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) }); } } diff --git a/src/rpc/transport.ts b/src/rpc/transport.ts index cf1ec92682..39e887b51d 100644 --- a/src/rpc/transport.ts +++ b/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) { diff --git a/test/defaultbrowsercontext.spec.js b/test/defaultbrowsercontext.spec.js index 01d62bed08..e120ce8195 100644 --- a/test/defaultbrowsercontext.spec.js +++ b/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); - }); + }); }); diff --git a/test/environments.js b/test/environments.js index cda1ede946..8b2b2f2e2d 100644 --- a/test/environments.js +++ b/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; diff --git a/test/evaluation.spec.js b/test/evaluation.spec.js index 60c584f230..366966789d 100644 --- a/test/evaluation.spec.js +++ b/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); diff --git a/test/launcher.spec.js b/test/launcher.spec.js index 671ade4fa1..5ba1448e23 100644 --- a/test/launcher.spec.js +++ b/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'); diff --git a/test/queryselector.spec.js b/test/queryselector.spec.js index 559db88c0e..d7bbd8c6b9 100644 --- a/test/queryselector.spec.js +++ b/test/queryselector.spec.js @@ -17,7 +17,7 @@ const path = require('path'); const utils = require('./utils'); -const {FFOX, CHROMIUM, WEBKIT, USES_HOOKS} = utils.testOptions(browserType); +const {FFOX, CHROMIUM, WEBKIT, CHANNEL} = utils.testOptions(browserType); describe('Page.$eval', function() { it('should work with css selector', async({page, server}) => { @@ -515,7 +515,7 @@ describe('text selector', () => { expect(await page.$$eval(`text=lowo`, els => els.map(e => e.outerHTML).join(''))).toBe('
helloworld
helloworld'); }); - it.skip(USES_HOOKS)('create', async ({page}) => { + it.skip(CHANNEL)('create', async ({page}) => { await page.setContent(`
yo
"ya
ye ye
`); expect(await playwright.selectors._createSelector('text', await page.$('div'))).toBe('yo'); expect(await playwright.selectors._createSelector('text', await page.$('div:nth-child(2)'))).toBe('"\\"ya"'); @@ -744,7 +744,7 @@ describe('attribute selector', () => { }); describe('selectors.register', () => { - it.skip(USES_HOOKS)('should work', async ({page}) => { + it.skip(CHANNEL)('should work', async ({page}) => { const createTagSelector = () => ({ create(root, target) { return target.nodeName; diff --git a/test/recorder.spec.js b/test/recorder.spec.js index 3b8da261ae..5b1a55a924 100644 --- a/test/recorder.spec.js +++ b/test/recorder.spec.js @@ -14,7 +14,7 @@ * limitations under the License. */ -const {FFOX, CHROMIUM, WEBKIT, USES_HOOKS} = require('./utils').testOptions(browserType); +const {FFOX, CHROMIUM, WEBKIT, CHANNEL} = require('./utils').testOptions(browserType); class WritableBuffer { constructor() { @@ -51,7 +51,7 @@ class WritableBuffer { } } -describe.skip(USES_HOOKS)('Recorder', function() { +describe.skip(CHANNEL)('Recorder', function() { beforeEach(async state => { state.context = await state.browser.newContext(); state.output = new WritableBuffer(); diff --git a/test/utils.js b/test/utils.js index 5683b36ab9..5519d86099 100644 --- a/test/utils.js +++ b/test/utils.js @@ -202,7 +202,7 @@ const utils = module.exports = { GOLDEN_DIR, OUTPUT_DIR, ASSETS_DIR, - USES_HOOKS: !!process.env.PWCHANNEL, + USES_HOOKS: process.env.PWCHANNELTRANSPORT === 'json', CHANNEL: !!process.env.PWCHANNEL, HEADLESS: !!valueFromEnv('HEADLESS', true), };