From 4d707edd49e1461ee5ae98f4fb552f91b5f6d2e7 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 14 Sep 2020 10:53:39 -0700 Subject: [PATCH] fix(launchServer): do not throw when 'port' option is present We now use 'launch' under the hood, which erroneously throws when 'port' is present. Instead, moved validation to the client side where it belongs, added tests for validation errors. --- src/client/browserType.ts | 3 +-- src/server/browserType.ts | 5 +---- test/browsertype-launch-server.spec.ts | 6 ++++++ test/browsertype-launch.spec.ts | 14 +++++++++++++- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/client/browserType.ts b/src/client/browserType.ts index d0c3158896ec6..824a59804f4bd 100644 --- a/src/client/browserType.ts +++ b/src/client/browserType.ts @@ -63,7 +63,6 @@ export class BrowserType extends ChannelOwner { const logger = options.logger; - options = { ...options, logger: undefined }; return this._wrapApiCall('browserType.launch', async () => { assert(!(options as any).userDataDir, 'userDataDir option is not supported in `browserType.launch`. Use `browserType.launchPersistentContext` instead'); assert(!(options as any).port, 'Cannot specify a port without launching as a server.'); @@ -87,8 +86,8 @@ export class BrowserType extends ChannelOwner { const logger = options.logger; - options = { ...options, logger: undefined }; return this._wrapApiCall('browserType.launchPersistentContext', async () => { + assert(!(options as any).port, 'Cannot specify a port without launching as a server.'); if (options.extraHTTPHeaders) validateHeaders(options.extraHTTPHeaders); const persistentOptions: channels.BrowserTypeLaunchPersistentContextParams = { diff --git a/src/server/browserType.ts b/src/server/browserType.ts index 088fc23e8773b..b353cdb7da2b0 100644 --- a/src/server/browserType.ts +++ b/src/server/browserType.ts @@ -28,7 +28,7 @@ import { Progress, ProgressController } from './progress'; import * as types from './types'; import { TimeoutSettings } from '../utils/timeoutSettings'; import { validateHostRequirements } from './validateDependencies'; -import { assert, isDebugMode } from '../utils/utils'; +import { isDebugMode } from '../utils/utils'; const mkdirAsync = util.promisify(fs.mkdir); const mkdtempAsync = util.promisify(fs.mkdtemp); @@ -65,8 +65,6 @@ export abstract class BrowserType { } async launch(options: types.LaunchOptions = {}): Promise { - assert(!(options as any).userDataDir, 'userDataDir option is not supported in `browserType.launch`. Use `browserType.launchPersistentContext` instead'); - assert(!(options as any).port, 'Cannot specify a port without launching as a server.'); options = validateLaunchOptions(options); const controller = new ProgressController(TimeoutSettings.timeout(options)); controller.setLogName('browser'); @@ -75,7 +73,6 @@ export abstract class BrowserType { } async launchPersistentContext(userDataDir: string, options: types.LaunchPersistentOptions = {}): Promise { - assert(!(options as any).port, 'Cannot specify a port without launching as a server.'); options = validateLaunchOptions(options); const persistent: types.BrowserContextOptions = options; validateBrowserContextOptions(persistent); diff --git a/test/browsertype-launch-server.spec.ts b/test/browsertype-launch-server.spec.ts index 423213e55dd17..26c4b8ceb87ab 100644 --- a/test/browsertype-launch-server.spec.ts +++ b/test/browsertype-launch-server.spec.ts @@ -26,6 +26,12 @@ describe('lauch server', suite => { await browserServer.close(); }); + it('should work with port', async ({browserType, defaultBrowserOptions, parallelIndex}) => { + const browserServer = await browserType.launchServer({ ...defaultBrowserOptions, port: 8800 + parallelIndex }); + expect(browserServer.wsEndpoint()).toContain(String(8800 + parallelIndex)); + await browserServer.close(); + }); + it('should fire "close" event during kill', async ({browserType, defaultBrowserOptions}) => { const order = []; const browserServer = await browserType.launchServer(defaultBrowserOptions); diff --git a/test/browsertype-launch.spec.ts b/test/browsertype-launch.spec.ts index c02d54d9b4dbf..9178ffd24120c 100644 --- a/test/browsertype-launch.spec.ts +++ b/test/browsertype-launch.spec.ts @@ -33,7 +33,19 @@ it('should throw if userDataDir option is passed', async ({browserType, defaultB let waitError = null; const options = Object.assign({}, defaultBrowserOptions, {userDataDir: 'random-path'}); await browserType.launch(options).catch(e => waitError = e); - expect(waitError.message).toContain('launchPersistentContext'); + expect(waitError.message).toContain('userDataDir option is not supported in `browserType.launch`. Use `browserType.launchPersistentContext` instead'); +}); + +it('should throw if port option is passed', async ({browserType, defaultBrowserOptions}) => { + const options = Object.assign({}, defaultBrowserOptions, {port: 1234}); + const error = await browserType.launch(options).catch(e => e); + expect(error.message).toContain('Cannot specify a port without launching as a server.'); +}); + +it('should throw if port option is passed for persistent context', async ({browserType, defaultBrowserOptions}) => { + const options = Object.assign({}, defaultBrowserOptions, {port: 1234}); + const error = await browserType.launchPersistentContext('foo', options).catch(e => e); + expect(error.message).toContain('Cannot specify a port without launching as a server.'); }); it('should throw if page argument is passed', (test, parameters) => {