From 3645df4a81ffd213114787e186c8bdbfd42f294a 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 | 110 ++++++++++++++++++++++++++++++++++++++++++++++--- lib/runner.ts | 32 ++++++++++++-- 2 files changed, 133 insertions(+), 9 deletions(-) diff --git a/lib/browser.ts b/lib/browser.ts index 174272ae2..cc4937636 100644 --- a/lib/browser.ts +++ b/lib/browser.ts @@ -236,11 +236,16 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { params: any; /** + * Resolved when the browser is ready for use. Resolves to the browser, so + * you can do: + * + * forkedBrowser = await browser.forkNewDriverInstance().ready; + * * Set by the runner. * - * @type {q.Promise} Done when the new browser is ready for use + * @type {webdriver.promise.Promise.} */ - ready: wdpromise.Promise; + ready: wdpromise.Promise; /* * Set by the runner. @@ -418,7 +423,15 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { /** * Fork another instance of browser for use in interactive tests. * - * Set by the runner. + * @example + * // Running with control flow enabled + * var fork = browser.forkNewDriverInstance(); + * fork.get('page1'); // 'page1' gotten by forked browser + * + * @example + * // Running with control flow disabled + * var forked = await browser.forkNewDriverInstance().ready; + * await fork.get('page1'); // 'page1' gotten by forked browser * * @param {boolean} opt_useSameUrl Whether to navigate to current url on * creation @@ -432,11 +445,84 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { } /** - * Restart the browser instance. + * Restart the browser. This is done by closing this browser instance and creating a new one. + * A promise resolving to the new instance is returned, and if this function was called on the + * global `browser` instance then Protractor will automatically overwrite the global `browser` + * variable. + * + * When restarting a forked browser, it is the caller's job to overwrite references to the old + * instance. + * + * This function behaves slightly differently depending on if the webdriver control flow is + * enabled. If the control flow is enabled, the global `browser` object is synchronously + * replaced. If the control flow is disabled, the global `browser` is replaced asynchronously + * after the old driver quits. * * Set by the runner. + * + * @example + * // Running against global browser, with control flow enabled + * browser.get('page1'); + * browser.restart(); + * browser.get('page2'); // 'page2' gotten by restarted browser + * + * @example + * // Running against global browser, with control flow disabled + * await browser.get('page1'); + * await browser.restart(); + * await browser.get('page2'); // 'page2' gotten by restarted browser + * + * @example + * // Running against forked browsers, with the control flow enabled + * // In this case, you may prefer `restartSync` (documented below) + * var forked = browser.forkNewDriverInstance(); + * fork.get('page1'); + * fork.restart().then(function(fork) { + * fork.get('page2'); // 'page2' gotten by restarted fork + * }); + * + * @example + * // Running against forked browsers, with the control flow disabled + * var forked = await browser.forkNewDriverInstance().ready; + * await fork.get('page1'); + * fork = await fork.restart(); + * await fork.get('page2'); // 'page2' gotten by restarted fork + * + * @example + * // Unexpected behavior can occur if you save references to the global `browser` + * var savedBrowser = browser; + * browser.restart().then(function() { + * console.log(browser === savedBrowser); // false + * }); + * + * @returns {webdriver.promise.Promise} A promise resolving to the restarted + * browser */ - restart() { + restart(): wdpromise.Promise { + return; + } + + /** + * Like `restart`, but instead of returning a promise resolving to the new browser instance, + * returns the new browser instance directly. Can only be used when the control flow is enabled. + * + * @example + * // Running against global browser + * browser.get('page1'); + * browser.restartSync(); + * browser.get('page2'); // 'page2' gotten by restarted browser + * + * @example + * // Running against forked browsers + * var forked = browser.forkNewDriverInstance(); + * fork.get('page1'); + * fork = fork.restartSync(); + * fork.get('page2'); // 'page2' gotten by restarted fork + * + * @throws {TypeError} Will throw an error if the control flow is not enabled + * @returns {ProtractorBrowser} The restarted browser + */ + restartSync(): ProtractorBrowser { return; } @@ -1054,4 +1140,18 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { }; this.debugHelper.init(debuggerClientPath, onStartFn, opt_debugPort); } + + /** + * Determine if the control flow is enabled. + * + * @returns true if the control flow is enabled, false otherwise. + */ + controlFlowIsEnabled() { + if ((wdpromise as any).USE_PROMISE_MANAGER !== undefined) { + return (wdpromise as any).USE_PROMISE_MANAGER; + } else { + // True for old versions of `selenium-webdriver`, probably false in >=5.0.0 + return !!wdpromise.ControlFlow; + } + } } diff --git a/lib/runner.ts b/lib/runner.ts index e4015b2eb..b3ea8012b 100644 --- a/lib/runner.ts +++ b/lib/runner.ts @@ -113,7 +113,7 @@ export class Runner extends EventEmitter { this.frameworkUsesAfterEach = true; if (this.config_.restartBrowserBetweenTests) { // TODO(sjelin): remove the `|| q()` once `restart()` returns a promise - this.restartPromise = this.restartPromise || protractor.browser.restart() || q(); + this.restartPromise = this.restartPromise || q(protractor.browser.restart()); ret = this.restartPromise; this.restartPromise = undefined; } @@ -246,7 +246,8 @@ export class Runner extends EventEmitter { browser_.ng12Hybrid = config.ng12Hybrid; } - browser_.ready = driver.manage().timeouts().setScriptTimeout(config.allScriptsTimeout); + browser_.ready = + driver.manage().timeouts().setScriptTimeout(config.allScriptsTimeout).then(() => browser_); browser_.getProcessedConfig = () => { return wdpromise.fulfilled(config); @@ -265,12 +266,35 @@ export class Runner extends EventEmitter { return newBrowser; }; + let replaceBrowser = () => { + let newBrowser = browser_.forkNewDriverInstance(false, true); + if (browser_ === protractor.browser) { + this.setupGlobals_(newBrowser); + } + return newBrowser; + }; + browser_.restart = () => { // Note: because tests are not paused at this point, any async // calls here are not guaranteed to complete before the tests resume. + + // Seperate solutions depending on if the control flow is enabled (see lib/browser.ts) + if (browser_.controlFlowIsEnabled()) { + return browser_.restartSync().ready; + } else { + return this.driverprovider_.quitDriver(browser_.driver) + .then(replaceBrowser) + .then(newBrowser => newBrowser.ready); + } + }; + + browser_.restartSync = () => { + if (!browser_.controlFlowIsEnabled()) { + throw TypeError('Unable to use `browser.restartSync()` when the control flow is disabled'); + } + this.driverprovider_.quitDriver(browser_.driver); - browser_ = browser_.forkNewDriverInstance(false, true); - this.setupGlobals_(browser_); + return replaceBrowser(); }; return browser_;