From 53f410f5ad3267ca36b0f8750fa1cd1ceca661d1 Mon Sep 17 00:00:00 2001 From: Sammy Jelin Date: Mon, 23 Jan 2017 15:30:58 -0800 Subject: [PATCH] feat(restart): `browser.restart` should return a promise Also allows `browser.restart` to work when the control flow is disabled, and fixes it for forked browsers. Closes https://github.com/angular/protractor/issues/3899 and https://github.com/angular/protractor/issues/3896 --- lib/browser.ts | 82 +++++++++++++++++++++++++++++--------------- lib/runner.ts | 21 +++++++++--- lib/wdpromiseUtil.ts | 9 +++++ 3 files changed, 80 insertions(+), 32 deletions(-) create mode 100644 lib/wdpromiseUtil.ts diff --git a/lib/browser.ts b/lib/browser.ts index 174272ae2..f40f72d7b 100644 --- a/lib/browser.ts +++ b/lib/browser.ts @@ -76,10 +76,12 @@ export class AbstractExtendedWebDriver extends AbstractWebDriver { * `ElementFinder` will be unwrapped to their underlying `WebElement` instance * * @private - * @param {Object} to - * @param {Object} from - * @param {string} fnName - * @param {function=} setupFn + * @param {Object} to The object to copy a function to + * @param {string|Object} from Where to get the function from. If an object, this is the object + * containing the function to copy over. If a string, this is the name of a property on `to` + * which contains the object. + * @param {string} fnName The name of the function to copy + * @param {function=} setupFn An optional function to run before the copied function */ function ptorMixin(to: any, from: any, fnName: string, setupFn?: Function) { to[fnName] = function() { @@ -91,7 +93,8 @@ function ptorMixin(to: any, from: any, fnName: string, setupFn?: Function) { if (setupFn) { setupFn(); } - return from[fnName].apply(from, arguments); + let fromObj = typeof from === 'string' ? to[from] : from; + return fromObj[fnName].apply(fromObj, arguments); }; }; @@ -236,9 +239,11 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { params: any; /** - * Set by the runner. + * Resolved when the browser is ready for use + * + * Set partially by `setDriver_()`, partially by the runner. * - * @type {q.Promise} Done when the new browser is ready for use + * @type {webdriver.promise.Promise} */ ready: wdpromise.Promise; @@ -306,31 +311,24 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { webdriverInstance: WebDriver, opt_baseUrl?: string, opt_rootElement?: string, opt_untrackOutstandingTimeouts?: boolean, opt_blockingProxyUrl?: string) { super(); + this.setDriver_(webdriverInstance); + // These functions should delegate to the webdriver instance, but should // wait for Angular to sync up before performing the action. This does not // include functions which are overridden by protractor below. let methodsToSync = ['getCurrentUrl', 'getPageSource', 'getTitle']; - let extendWDInstance: ExtendedWebDriver; - try { - extendWDInstance = extendWD(webdriverInstance); - } catch (e) { - // Probably not a driver that can be extended (e.g. gotten using - // `directConnect: true` in the config) - extendWDInstance = webdriverInstance as ExtendedWebDriver; - } // Mix all other driver functionality into Protractor. Object.getOwnPropertyNames(WebDriver.prototype).forEach(method => { - if (!this[method] && typeof(extendWDInstance as any)[method] === 'function') { + if (!this[method] && typeof(this.driver as any)[method] === 'function') { if (methodsToSync.indexOf(method) !== -1) { - ptorMixin(this, extendWDInstance, method, this.waitForAngular.bind(this)); + ptorMixin(this, 'driver', method, this.waitForAngular.bind(this)); } else { - ptorMixin(this, extendWDInstance, method); + ptorMixin(this, 'driver', method); } } }); - this.driver = extendWDInstance; if (opt_blockingProxyUrl) { logger.info('Starting BP client for ' + opt_blockingProxyUrl); this.bpClient = new BPClient(opt_blockingProxyUrl); @@ -343,7 +341,6 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { this.ignoreSynchronization = false; this.getPageTimeout = DEFAULT_GET_PAGE_TIMEOUT; this.params = {}; - this.ready = null; this.plugins_ = new Plugins({}); this.resetUrl = DEFAULT_RESET_URL; this.debugHelper = new DebugHelper(this); @@ -365,7 +362,33 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { ng12Hybrid_ = ng12Hybrid; } }); - this.driver.getCapabilities().then((caps: Capabilities) => { + this.trackOutstandingTimeouts_ = !opt_untrackOutstandingTimeouts; + this.mockModules_ = []; + this.addBaseMockModules_(); + + // set up expected conditions + this.ExpectedConditions = new ProtractorExpectedConditions(this); + } + + /** + * Sets the `WebDriver` instance which `ProtractorBrowser` wraps around + * + * @param {WebDriver} webdriverInstance The WebDriver instance to wrap around + */ + setDriver_(webdriverInstance: WebDriver) { + // First extend the driver + let extendWDInstance: ExtendedWebDriver; + try { + extendWDInstance = extendWD(webdriverInstance); + } catch (e) { + // Probably not a driver that can be extended (e.g. gotten using + // `directConnect: true` in the config) + extendWDInstance = webdriverInstance as ExtendedWebDriver; + } + + this.driver = extendWDInstance; + + this.ready = this.driver.getCapabilities().then((caps) => { // Internet Explorer does not accept data URLs, which are the default // reset URL for Protractor. // Safari accepts data urls, but SafariDriver fails after one is used. @@ -376,13 +399,6 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { this.resetUrl = 'about:blank'; } }); - - this.trackOutstandingTimeouts_ = !opt_untrackOutstandingTimeouts; - this.mockModules_ = []; - this.addBaseMockModules_(); - - // set up expected conditions - this.ExpectedConditions = new ProtractorExpectedConditions(this); } /** @@ -434,7 +450,17 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { /** * Restart the browser instance. * + * Replaces a browser's `.driver` property, so if you've saved a reference directly to + * `browser.driver`, make sure to update it. + * + * Behaves slightly differently depending on if the webdriver control flow is enabled. If the + * control flow is enabled, `browser.driver` is synchronously replaced. If the control flow is + * disabled, `browser.driver` is replaced asynchronously after the old driver quits. + * * Set by the runner. + * + * @returns {webdriver.promise.Promise.} A promise which resolves once the browser has + * restarted. */ restart() { return; diff --git a/lib/runner.ts b/lib/runner.ts index 983e17338..92ad96804 100644 --- a/lib/runner.ts +++ b/lib/runner.ts @@ -10,6 +10,7 @@ import {Logger} from './logger'; import {Plugins} from './plugins'; import {protractor} from './ptor'; import * as helper from './util'; +import * as wdpromiseUtil from './wdpromiseUtil'; declare let global: any; declare let process: any; @@ -234,7 +235,9 @@ export class Runner extends EventEmitter { browser_.ng12Hybrid = config.ng12Hybrid; } - browser_.ready = driver.manage().timeouts().setScriptTimeout(config.allScriptsTimeout); + browser_.ready = browser_.ready.then(() => { + return driver.manage().timeouts().setScriptTimeout(config.allScriptsTimeout); + }); browser_.getProcessedConfig = () => { return wdpromise.fulfilled(config); @@ -256,9 +259,19 @@ export class Runner extends EventEmitter { browser_.restart = () => { // Note: because tests are not paused at this point, any async // calls here are not guaranteed to complete before the tests resume. - this.driverprovider_.quitDriver(browser_.driver); - browser_ = browser_.forkNewDriverInstance(false, true); - this.setupGlobals_(browser_); + + // Seperate solutions for if the control flow is on or off, see + // https://github.com/angular/protractor/issues/3899 + if (wdpromiseUtil.usePromiseManager()) { + this.driverprovider_.quitDriver(browser_.driver); + browser_.setDriver_(this.driverprovider_.getNewDriver()); + return browser_.ready; + } else { + return this.driverprovider_.quitDriver(browser_.driver).then(() => { + browser_.setDriver_(this.driverprovider_.getNewDriver()); + return browser_.ready; + }); + } }; return browser_; diff --git a/lib/wdpromiseUtil.ts b/lib/wdpromiseUtil.ts new file mode 100644 index 000000000..7d7e24635 --- /dev/null +++ b/lib/wdpromiseUtil.ts @@ -0,0 +1,9 @@ +import {promise as wdpromise} from 'selenium-webdriver'; + +export function usePromiseManager() { + if ((wdpromise as any).USE_PROMISE_MANAGER !== undefined) { + return (wdpromise as any).USE_PROMISE_MANAGER; + } else { + return !!wdpromise.ControlFlow; + } +}