From 7a55d349fbc670430062c012e9ccb40f5e1235a1 Mon Sep 17 00:00:00 2001 From: Jason Cheatham Date: Wed, 22 Jan 2020 13:16:02 -0500 Subject: [PATCH] refactor(core): resolve environments in resolveConfig Resolve the remote environments (resolving aliases, normalizing environment descriptors) in resolveConfig. Defer parameter aliasing (e.g., ensuring both browser and browserName are present) until after environment resolution so we don't accidentally create too many environment permutations. resolves #1065 resolves #1063 --- packages/core/src/lib/common/util.ts | 10 +- packages/core/src/lib/executors/Node.ts | 303 ++++++++++--------- packages/core/src/lib/resolveEnvironments.ts | 57 ++-- 3 files changed, 191 insertions(+), 179 deletions(-) diff --git a/packages/core/src/lib/common/util.ts b/packages/core/src/lib/common/util.ts index 13c0fb813..c2508e95c 100644 --- a/packages/core/src/lib/common/util.ts +++ b/packages/core/src/lib/common/util.ts @@ -551,15 +551,11 @@ export function processOption( } _value = _value.map((val: any) => { if (typeof val === 'object') { - if (val.browserName == null) { + // Use browserName instead of browser + if (val.browserName == null && typeof val.browser !== 'undefined') { val.browserName = val.browser; } - if (val.browserVersion == null) { - val.browserVersion = val.version; - } - } - if (typeof val === 'object' && val.version == null) { - val.version = val.browserVersion; + delete val.browser; } return val; }); diff --git a/packages/core/src/lib/executors/Node.ts b/packages/core/src/lib/executors/Node.ts index 78bec23c8..4d0e42504 100644 --- a/packages/core/src/lib/executors/Node.ts +++ b/packages/core/src/lib/executors/Node.ts @@ -391,46 +391,19 @@ export default class Node extends Executor { return serverTask .then(() => { - const tunnelOptions = config.tunnelOptions; - if (config.tunnel === 'browserstack') { - const options = tunnelOptions; - options.servers = options.servers || []; - options.servers.push(config.serverUrl); - } - - if ('proxy' in config && !('proxy' in tunnelOptions)) { - tunnelOptions.proxy = config.proxy; - } - - let TunnelConstructor = this.getTunnel(config.tunnel); - const tunnel = (this.tunnel = new TunnelConstructor( - this.config.tunnelOptions - )); - - tunnel.on('downloadprogress', progress => { - this.emit('tunnelDownloadProgress', { - tunnel, - progress - }); - }); - - tunnel.on('status', status => { - this.emit('tunnelStatus', { - tunnel, - status: status.status - }); - }); + // Tunnel will have been created in resolveConfig + const tunnel = this.tunnel!; config.capabilities = deepMixin( tunnel.extraCapabilities, config.capabilities ); - return this._createSessionSuites().then(() => { - return tunnel - .start() - .then(() => this.emit('tunnelStart', { tunnel })); - }); + this._createSessionSuites(); + + return tunnel + .start() + .then(() => this.emit('tunnelStart', { tunnel })); }) .then(() => { return false; @@ -441,13 +414,52 @@ export default class Node extends Executor { }); } + protected _createTunnel() { + const config = this.config; + const tunnelOptions = config.tunnelOptions; + if (config.tunnel === 'browserstack') { + const options = tunnelOptions; + options.servers = options.servers || []; + options.servers.push(config.serverUrl); + } + + if ('proxy' in config && !('proxy' in tunnelOptions)) { + tunnelOptions.proxy = config.proxy; + } + + const TunnelConstructor = this.getTunnel(config.tunnel); + const tunnel = new TunnelConstructor(this.config.tunnelOptions); + this.tunnel = tunnel; + + tunnel.on('downloadprogress', progress => { + this.emit('tunnelDownloadProgress', { + tunnel, + progress + }); + }); + + tunnel.on('status', status => { + this.emit('tunnelStatus', { + tunnel, + status: status.status + }); + }); + + return tunnel; + } + /** * Creates suites for each environment in which tests will be executed. This * method will only be called if there are both environments and suites to * run. */ - protected _createSessionSuites() { - const tunnel = this.tunnel!; + protected _createSessionSuites(): void { + if (!this.tunnel) { + this.log('No tunnel - Not creating session suites'); + return; + } + + const tunnel = this.tunnel; const config = this.config; const leadfootServer = new LeadfootServer(tunnel.clientUrl, { @@ -466,118 +478,110 @@ export default class Node extends Executor { leadfootServer.sessionConstructor = InitializedProxiedSession; - return tunnel.getEnvironments().then(tunnelEnvironments => { - this._sessionSuites = resolveEnvironments( - config.capabilities, - config.environments.filter(isRemoteEnvironment), - tunnelEnvironments - ).map(environmentType => { - let session: ProxiedSession; - - // Create a new root suite for each environment - const suite = new Suite({ - name: String(environmentType), - publishAfterSetup: true, - grep: config.grep, - bail: config.bail, - tests: [], - timeout: config.defaultTimeout, - executor: this, - - before() { - executor.log('Creating session for', environmentType); - return leadfootServer - .createSession(environmentType) - .then(_session => { - session = _session; - this.executor.log('Created session:', session.capabilities); - - let remote: Remote = new Command(session); - remote.environmentType = new Environment(session.capabilities); - remote.requestedEnvironment = environmentType; - this.remote = remote; - this.sessionId = remote.session.sessionId; - - // Update the name with details from the remote - // environment - this.name = remote.environmentType.toString(); - - const timeouts = config.functionalTimeouts; - let promise = Promise.resolve(); - if (timeouts.executeAsync != null) { - promise = promise.then(() => - remote.setExecuteAsyncTimeout(timeouts.executeAsync!) - ); - this.executor.log( - 'Set remote executeAsync timeout to ', - timeouts.executeAsync - ); - } - if (timeouts.find != null) { - promise = promise.then(() => - remote.setFindTimeout(timeouts.find!) - ); - this.executor.log( - 'Set remote find timeout to ', - timeouts.find - ); - } - if (timeouts.pageLoad != null) { - promise = promise.then(() => - remote.setPageLoadTimeout(timeouts.pageLoad!) - ); - this.executor.log( - 'Set remote pageLoad timeout to ', - timeouts.pageLoad - ); - } - - return promise; - }); - }, + // config.environments was resolved in resolveConfig + this._sessionSuites = this.config.environments.map(environmentType => { + let session: ProxiedSession; + + // Create a new root suite for each environment + const suite = new Suite({ + name: String(environmentType), + publishAfterSetup: true, + grep: config.grep, + bail: config.bail, + tests: [], + timeout: config.defaultTimeout, + executor: this, + + before() { + executor.log('Creating session for', environmentType); + return leadfootServer + .createSession(environmentType) + .then(_session => { + session = _session; + this.executor.log('Created session:', session.capabilities); + + const remote: Remote = new Command(session); + remote.environmentType = new Environment(session.capabilities); + remote.requestedEnvironment = environmentType; + this.remote = remote; + this.sessionId = remote.session.sessionId; + + // Update the name with details from the remote + // environment + this.name = remote.environmentType.toString(); + + const timeouts = config.functionalTimeouts; + let promise = Promise.resolve(); + if (timeouts.executeAsync != null) { + promise = promise.then(() => + remote.setExecuteAsyncTimeout(timeouts.executeAsync!) + ); + this.executor.log( + 'Set remote executeAsync timeout to ', + timeouts.executeAsync + ); + } + if (timeouts.find != null) { + promise = promise.then(() => + remote.setFindTimeout(timeouts.find!) + ); + this.executor.log('Set remote find timeout to ', timeouts.find); + } + if (timeouts.pageLoad != null) { + promise = promise.then(() => + remote.setPageLoadTimeout(timeouts.pageLoad!) + ); + this.executor.log( + 'Set remote pageLoad timeout to ', + timeouts.pageLoad + ); + } - after() { - const remote = this.remote; - - if (remote) { - const endSession = () => { - // Check for an error in this suite or a - // sub-suite. This check is a bit more involved - // than just checking for a local suite error or - // failed tests since sub-suites may have - // failures that don't result in failed tests. - function hasError(suite: Suite): boolean { - if (suite.error != null || suite.numFailedTests > 0) { - return true; - } - return suite.tests.filter(isSuite).some(hasError); + return promise; + }); + }, + + after() { + const remote = this.remote; + + if (remote != null) { + const endSession = () => { + // Check for an error in this suite or a + // sub-suite. This check is a bit more involved + // than just checking for a local suite error or + // failed tests since sub-suites may have + // failures that don't result in failed tests. + function hasError(suite: Suite): boolean { + if (suite.error != null || suite.numFailedTests > 0) { + return true; } - return tunnel.sendJobState(remote.session.sessionId, { - success: !hasError(this) - }); - }; - - if ( - config.leaveRemoteOpen === true || - (config.leaveRemoteOpen === 'fail' && this.numFailedTests > 0) - ) { - return endSession(); + return suite.tests.filter(isSuite).some(hasError); } + return tunnel.sendJobState(remote.session.sessionId, { + success: !hasError(this) + }); + }; - return remote.quit().finally(endSession); + if ( + config.leaveRemoteOpen === true || + (config.leaveRemoteOpen === 'fail' && this.numFailedTests > 0) + ) { + return endSession(); } - } - }); - // If browser-compatible unit tests were added to this executor, - // add a RemoteSuite to the session suite. The RemoteSuite will - // run the suites listed in config.browser.suites. - if (config.browser.suites.length > 0) { - suite.add(new RemoteSuite()); + return remote.quit().finally(endSession); + } } - - return suite; }); + + // If browser-compatible unit tests were added to this executor, + // add a RemoteSuite to the session suite. The RemoteSuite will + // run the suites listed in config.browser.suites. + if (config.browser.suites.length > 0) { + suite.add(new RemoteSuite()); + } + + return suite; }); } @@ -636,16 +640,6 @@ export default class Node extends Executor { config.environments.push({ browserName: 'node' }); } - // Normalize browser names - config.environments.forEach(env => { - const { browserName } = env; - const newName = getNormalizedBrowserName(browserName)!; - env.browserName = newName; - if (env.browser) { - env.browser = newName; - } - }); - // Normalize tunnel driver names if (config.tunnelOptions.drivers) { config.tunnelOptions.drivers = config.tunnelOptions.drivers.map( @@ -838,6 +832,15 @@ export default class Node extends Executor { tunnelOptions.drivers = driverNames.map(name => ({ name })); } } + + const tunnel = this._createTunnel(); + return tunnel.getEnvironments().then(environments => { + config.environments = resolveEnvironments( + config.capabilities, + config.environments.filter(isRemoteEnvironment), + environments + ); + }); }); } diff --git a/packages/core/src/lib/resolveEnvironments.ts b/packages/core/src/lib/resolveEnvironments.ts index 8d1c832e9..bb71e14d0 100644 --- a/packages/core/src/lib/resolveEnvironments.ts +++ b/packages/core/src/lib/resolveEnvironments.ts @@ -1,6 +1,7 @@ import { NormalizedEnvironment } from '@theintern/digdug/dist/Tunnel'; import { normalize } from 'path'; +import { EnvironmentSpec } from './common/config'; import process from './node/process'; import Environment from './Environment'; @@ -17,18 +18,17 @@ export default function resolveEnvironments( capabilities: { [key: string]: any }, environments: EnvironmentOptions[], available?: NormalizedEnvironment[] -) { +): EnvironmentSpec[] { // Pre-process the environments list to resolve any uses of {pwd} and do any // top-level substitutions environments = environments.map(expandPwd); + // Update the browserName to match the target environment (only relevant for + // edge / MicrosoftEdge) if (available) { environments = normalizeBrowserNames(environments, available); } - // Update the browserName to match the target environment (only relevant for - // edge / MicrosoftEdge) - // flatEnviroments will have non-array versions const flatEnvironments = createPermutations(capabilities, environments); @@ -44,7 +44,7 @@ export default function resolveEnvironments( // Perform a second round of permuting to handle any expanded version ranges return createPermutations({}, expandedEnvironments) .map(environment => new Environment(environment)) - .map(normalizeVersion); + .map(normalizeEnvironment); } export interface EnvironmentOptions { @@ -91,18 +91,22 @@ function expandPwd(value: T): T { /** * Ensure environment has both `version` and `browserVersion` properties with - * the same value + * the same value, and `browser` and `browserName`. */ -function normalizeVersion(env: Environment) { +function normalizeEnvironment(env: Environment): EnvironmentSpec { + const normEnv = { ...env }; + const browserVersion = getVersion(env); - if (browserVersion == null) { - return env; + if (browserVersion != null) { + normEnv.version = browserVersion; + normEnv.browserVersion = browserVersion; } - return { - ...env, - browserVersion, - version: browserVersion - }; + + const browserName = env.browserName || env.browser; + normEnv.browser = browserName; + normEnv.browserName = browserName; + + return normEnv as EnvironmentSpec; } /** @@ -229,7 +233,10 @@ function getVersions( // have 'browserName'. .filter( key => - key !== 'browserVersion' && key !== 'version' && key !== 'browser' + key === 'browserName' || + key === 'platformName' || + key === 'platform' || + key === 'platformVersion' ) .some(envKey => { const key = envKey; @@ -237,18 +244,25 @@ function getVersions( return false; } - const value = environment[key]; + let value = environment[key]; + if (typeof value === 'string') { + value = value.toLowerCase(); + } + + let availableValue = availableEnvironment[key]; + if (typeof availableValue === 'string') { + availableValue = availableValue.toLowerCase(); + } // At least BrowserStack uses 'edge' for MicrosoftEdge, while everyone // else + the Edge webdrivers use 'MicrosoftEdge'. - if (key === 'browserName' && value === 'MicrosoftEdge') { + if (key === 'browserName' && value === 'microsoftedge') { return ( - availableEnvironment[key] !== 'MicrosoftEdge' || - availableEnvironment[key] !== 'edge' + availableValue !== 'microsoftedge' && availableValue !== 'edge' ); } - return availableEnvironment[key] !== value; + return availableValue !== value; }); }) .forEach(function (availableEnvironment) { @@ -396,8 +410,7 @@ function normalizeBrowserNames( if (available.some(ae => ae.browserName === 'edge')) { return { ...env, - browserName: 'edge', - browser: 'edge' + browserName: 'edge' }; } }