From 9c2274d8f218cabc946dbc6a11d725458c1b4e3a Mon Sep 17 00:00:00 2001 From: Sammy Jelin Date: Tue, 31 Jan 2017 02:02:47 -0800 Subject: [PATCH] fix(restart): preserve properties like `browser.baseUrl` upon restart (#4037) I also fixed a minor issue where `internalRootEl` wasn't being set when blocking proxy was being used. I also just cleaned up our internal uses of `this.rootEl`. Closes https://github.com/angular/protractor/issues/4032 --- lib/browser.ts | 73 ++++++++++++++++++++++------------------ lib/runner.ts | 90 ++++++++++++++++++++++++++++++++------------------ 2 files changed, 98 insertions(+), 65 deletions(-) diff --git a/lib/browser.ts b/lib/browser.ts index 68dc551d3..65293e1eb 100644 --- a/lib/browser.ts +++ b/lib/browser.ts @@ -104,8 +104,8 @@ function buildElementHelper(browser: ProtractorBrowser): ElementHelper { * @extends {webdriver_extensions.ExtendedWebDriver} * @param {webdriver.WebDriver} webdriver * @param {string=} opt_baseUrl A base URL to run get requests against. - * @param {string=} opt_rootElement Selector element that has an ng-app in - * scope. + * @param {string|webdriver.promise.Promise=} opt_rootElement Selector element that has an + * ng-app in scope. * @param {boolean=} opt_untrackOutstandingTimeouts Whether Protractor should * stop tracking outstanding $timeouts. */ @@ -192,17 +192,19 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { * this method is called use the new app root. Pass nothing to get a promise that * resolves to the value of the selector. * - * @param {string} The new selector. + * @param {string|webdriver.promise.Promise} value The new selector. * @returns A promise that resolves with the value of the selector. */ - angularAppRoot(value: string = null): wdpromise.Promise { + angularAppRoot(value: string|wdpromise.Promise = null): wdpromise.Promise { return this.driver.controlFlow().execute(() => { if (value != null) { - if (this.bpClient) { - return this.bpClient.setWaitParams(value).then(() => this.internalRootEl); - } - this.internalRootEl = value; - return this.internalRootEl; + return wdpromise.when(value).then((value: string) => { + this.internalRootEl = value; + if (this.bpClient) { + return this.bpClient.setWaitParams(value).then(() => this.internalRootEl); + } + return this.internalRootEl; + }); } }, `Set angular root selector to ${value}`); } @@ -316,8 +318,9 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { [key: string]: any; constructor( - webdriverInstance: WebDriver, opt_baseUrl?: string, opt_rootElement?: string, - opt_untrackOutstandingTimeouts?: boolean, opt_blockingProxyUrl?: string) { + webdriverInstance: WebDriver, opt_baseUrl?: string, + opt_rootElement?: string|wdpromise.Promise, opt_untrackOutstandingTimeouts?: boolean, + opt_blockingProxyUrl?: string) { super(); // These functions should delegate to the webdriver instance, but should // wait for Angular to sync up before performing the action. This does not @@ -352,7 +355,7 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { this.$ = build$(this.element, By); this.$$ = build$$(this.element, By); this.baseUrl = opt_baseUrl || ''; - this.rootEl = opt_rootElement || ''; + this.angularAppRoot(opt_rootElement || ''); this.ignoreSynchronization = false; this.getPageTimeout = DEFAULT_GET_PAGE_TIMEOUT; this.params = {}; @@ -454,13 +457,14 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { * var forked = await browser.forkNewDriverInstance().ready; * await forked.get('page1'); // 'page1' gotten by forked browser * - * @param {boolean} opt_useSameUrl Whether to navigate to current url on - * creation - * @param {boolean} opt_copyMockModules Whether to apply same mock modules on - * creation - * @returns {Browser} A browser instance. + * @param {boolean=} useSameUrl Whether to navigate to current url on creation + * @param {boolean=} copyMockModules Whether to apply same mock modules on creation + * @param {boolean=} copyConfigUpdates Whether to copy over changes to `baseUrl` and similar + * properties initialized to values in the the config. Defaults to `true` + * + * @returns {ProtractorBrowser} A browser instance. */ - forkNewDriverInstance(opt_useSameUrl?: boolean, opt_copyMockModules?: boolean): + forkNewDriverInstance(useSameUrl?: boolean, copyMockModules?: boolean, copyConfigUpdates = true): ProtractorBrowser { return null; } @@ -556,7 +560,7 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { useAllAngular2AppRoots() { // The empty string is an invalid css selector, so we use it to easily // signal to scripts to not find a root element. - this.rootEl = ''; + this.angularAppRoot(''); } /** @@ -698,7 +702,7 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { let pendingHttpsPromise = this.executeScriptWithDescription( clientSideScripts.getPendingHttpRequests, 'Protractor.waitForAngular() - getting pending https' + description, - this.rootEl); + this.internalRootEl); return wdpromise.all([pendingTimeoutsPromise, pendingHttpsPromise]) .then( @@ -1035,15 +1039,18 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { * page has been changed. */ setLocation(url: string): wdpromise.Promise { - return this.waitForAngular().then( - () => this.executeScriptWithDescription( - clientSideScripts.setLocation, 'Protractor.setLocation()', this.rootEl, url) - .then((browserErr: Error) => { - if (browserErr) { - throw 'Error while navigating to \'' + url + '\' : ' + - JSON.stringify(browserErr); - } - })); + return this.waitForAngular() + .then(() => this.angularAppRoot()) + .then( + (rootEl) => + this.executeScriptWithDescription( + clientSideScripts.setLocation, 'Protractor.setLocation()', rootEl, url) + .then((browserErr: Error) => { + if (browserErr) { + throw 'Error while navigating to \'' + url + '\' : ' + + JSON.stringify(browserErr); + } + })); } /** @@ -1064,9 +1071,11 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { getLocationAbsUrl(): wdpromise.Promise { logger.warn( '`browser.getLocationAbsUrl()` is deprecated, please use `browser.getCurrentUrl` instead.'); - return this.waitForAngular().then( - () => this.executeScriptWithDescription( - clientSideScripts.getLocationAbsUrl, 'Protractor.getLocationAbsUrl()', this.rootEl)); + return this.waitForAngular() + .then(() => this.angularAppRoot()) + .then( + (rootEl) => this.executeScriptWithDescription( + clientSideScripts.getLocationAbsUrl, 'Protractor.getLocationAbsUrl()', rootEl)); } /** diff --git a/lib/runner.ts b/lib/runner.ts index 56055f57c..9c0caa1b0 100644 --- a/lib/runner.ts +++ b/lib/runner.ts @@ -214,12 +214,13 @@ export class Runner extends EventEmitter { * This is used to set up the initial protractor instances and any * future ones. * - * @param {?Plugin} The plugin functions + * @param {Plugin} plugins The plugin functions + * @param {ProtractorBrowser=} parentBrowser The browser which spawned this one * * @return {Protractor} a protractor instance. * @public */ - createBrowser(plugins: any): any { + createBrowser(plugins: any, parentBrowser?: ProtractorBrowser): any { let config = this.config_; let driver = this.driverprovider_.getNewDriver(); @@ -228,31 +229,53 @@ export class Runner extends EventEmitter { blockingProxyUrl = this.driverprovider_.getBPUrl(); } + let initProperties = { + baseUrl: config.baseUrl, + rootElement: config.rootElement as string | wdpromise.Promise, + untrackOutstandingTimeouts: config.untrackOutstandingTimeouts, + params: config.params, + getPageTimeout: config.getPageTimeout, + allScriptsTimeout: config.allScriptsTimeout, + debuggerServerPort: config.debuggerServerPort, + ng12Hybrid: config.ng12Hybrid + }; + + if (parentBrowser) { + initProperties.baseUrl = parentBrowser.baseUrl; + initProperties.rootElement = parentBrowser.angularAppRoot(); + initProperties.untrackOutstandingTimeouts = !parentBrowser.trackOutstandingTimeouts_; + initProperties.params = parentBrowser.params; + initProperties.getPageTimeout = parentBrowser.getPageTimeout; + initProperties.allScriptsTimeout = parentBrowser.allScriptsTimeout; + initProperties.debuggerServerPort = parentBrowser.debuggerServerPort; + initProperties.ng12Hybrid = parentBrowser.ng12Hybrid; + } + let browser_ = new ProtractorBrowser( - driver, config.baseUrl, config.rootElement, config.untrackOutstandingTimeouts, - blockingProxyUrl); + driver, initProperties.baseUrl, initProperties.rootElement, + initProperties.untrackOutstandingTimeouts, blockingProxyUrl); - browser_.params = config.params; + browser_.params = initProperties.params; if (plugins) { browser_.plugins_ = plugins; } - if (config.getPageTimeout) { - browser_.getPageTimeout = config.getPageTimeout; + if (initProperties.getPageTimeout) { + browser_.getPageTimeout = initProperties.getPageTimeout; } - if (config.allScriptsTimeout) { - browser_.allScriptsTimeout = config.allScriptsTimeout; + if (initProperties.allScriptsTimeout) { + browser_.allScriptsTimeout = initProperties.allScriptsTimeout; } - if (config.debuggerServerPort) { - browser_.debuggerServerPort = config.debuggerServerPort; + if (initProperties.debuggerServerPort) { + browser_.debuggerServerPort = initProperties.debuggerServerPort; } - if (config.ng12Hybrid) { - browser_.ng12Hybrid = config.ng12Hybrid; + if (initProperties.ng12Hybrid) { + browser_.ng12Hybrid = initProperties.ng12Hybrid; } browser_.ready = browser_.ready .then(() => { - return driver.manage().timeouts().setScriptTimeout(config.allScriptsTimeout); + return driver.manage().timeouts().setScriptTimeout(initProperties.allScriptsTimeout); }) .then(() => { return browser_; @@ -262,25 +285,26 @@ export class Runner extends EventEmitter { return wdpromise.when(config); }; - browser_.forkNewDriverInstance = (opt_useSameUrl: boolean, opt_copyMockModules: boolean) => { - let newBrowser = this.createBrowser(plugins); - if (opt_copyMockModules) { - newBrowser.mockModules_ = browser_.mockModules_; - } - if (opt_useSameUrl) { - newBrowser.ready = newBrowser.ready - .then(() => { - return browser_.driver.getCurrentUrl(); - }) - .then((url: string) => { - return newBrowser.get(url); - }) - .then(() => { - return newBrowser; - }); - } - return newBrowser; - }; + browser_.forkNewDriverInstance = + (useSameUrl: boolean, copyMockModules: boolean, copyConfigUpdates = true) => { + let newBrowser = this.createBrowser(plugins); + if (copyMockModules) { + newBrowser.mockModules_ = browser_.mockModules_; + } + if (useSameUrl) { + newBrowser.ready = newBrowser.ready + .then(() => { + return browser_.driver.getCurrentUrl(); + }) + .then((url: string) => { + return newBrowser.get(url); + }) + .then(() => { + return newBrowser; + }); + } + return newBrowser; + }; let replaceBrowser = () => { let newBrowser = browser_.forkNewDriverInstance(false, true);