From 9654e03b2881efd21b4e47a1ce2f665518e619ae Mon Sep 17 00:00:00 2001 From: Sammy Jelin Date: Wed, 25 Jan 2017 22:13:46 -0800 Subject: [PATCH] feat(browser): chain promises in `lib/browser.ts` and return promise from `waitForAngularEnabled` Minor breaking change since `waitForAngularEnabled` no longer returns a boolean Closes https://github.com/angular/protractor/issues/3904 Also fixed a minor bug in `lib/clientsidescripts.js` while debuging --- lib/browser.ts | 311 +++++++++++++++++++++------------------ lib/clientsidescripts.js | 4 +- 2 files changed, 167 insertions(+), 148 deletions(-) diff --git a/lib/browser.ts b/lib/browser.ts index 128c656a5..85f1f54d0 100644 --- a/lib/browser.ts +++ b/lib/browser.ts @@ -88,10 +88,16 @@ function ptorMixin(to: any, from: any, fnName: string, setupFn?: Function) { arguments[i] = arguments[i].getWebElement(); } } + const run = () => { + return from[fnName].apply(from, arguments); + }; if (setupFn) { - setupFn(); + const setupResult = setupFn(); + if (setupResult && (typeof setupResult.then === 'function')) { + return setupResult.then(run); + } } - return from[fnName].apply(from, arguments); + return run(); }; }; @@ -206,13 +212,7 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { * @type {boolean} */ set ignoreSynchronization(value) { - this.driver.controlFlow().execute(() => { - if (this.bpClient) { - logger.debug('Setting waitForAngular' + value); - this.bpClient.setSynchronization(!value); - } - }, `Set proxy synchronization to ${value}`); - this.internalIgnoreSynchronization = value; + this.waitForAngularEnabled(!value); } get ignoreSynchronization() { @@ -403,11 +403,20 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { * Call waitForAngularEnabled() without passing a value to read the current * state without changing it. */ - waitForAngularEnabled(enabled: boolean = null): boolean { + waitForAngularEnabled(enabled: boolean = null): wdpromise.Promise { if (enabled != null) { - this.ignoreSynchronization = !enabled; + const ret = this.driver.controlFlow().execute(() => { + if (this.bpClient) { + logger.debug('Setting waitForAngular' + !enabled); + return this.bpClient.setSynchronization(enabled).then(() => { + return enabled; + }); + } + }, `Set proxy synchronization enabled to ${enabled}`); + this.internalIgnoreSynchronization = !enabled; + return ret; } - return !this.ignoreSynchronization; + return wdpromise.when(!this.ignoreSynchronization); } /** @@ -613,7 +622,9 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { let runWaitForAngularScript: () => wdpromise.Promise = () => { if (this.plugins_.skipAngularStability() || this.bpClient) { - return wdpromise.fulfilled(); + return this.driver.controlFlow().execute(() => { + return true; + }, 'A plugin has set skipAngularStability'); } else { return this.executeAsyncScript_( clientSideScripts.waitForAngular, 'Protractor.waitForAngular()' + description, @@ -833,131 +844,139 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { get(destination: string, timeout = this.getPageTimeout) { destination = this.baseUrl.indexOf('file://') === 0 ? this.baseUrl + destination : url.resolve(this.baseUrl, destination); - let msg = (str: string) => { - return 'Protractor.get(' + destination + ') - ' + str; - }; - - if (this.bpClient) { - this.driver.controlFlow().execute(() => { - return this.bpClient.setSynchronization(false); - }); - } if (this.ignoreSynchronization) { - this.driver.get(destination); - return this.driver.controlFlow().execute(() => this.plugins_.onPageLoad()).then(() => {}); + return this.driver.get(destination) + .then(() => this.driver.controlFlow().execute(() => this.plugins_.onPageLoad())) + .then(() => null); } - let deferred = wdpromise.defer(); - - this.driver.get(this.resetUrl).then(null, deferred.reject); - this.executeScriptWithDescription( - 'window.name = "' + DEFER_LABEL + '" + window.name;' + - 'window.location.replace("' + destination + '");', - msg('reset url')) - .then(null, deferred.reject); - - // We need to make sure the new url has loaded before - // we try to execute any asynchronous scripts. - this.driver - .wait( - () => { - return this - .executeScriptWithDescription('return window.location.href;', msg('get url')) - .then( - (url: any) => { - return url !== this.resetUrl; - }, - (err: IError) => { - if (err.code == 13) { - // Ignore the error, and continue trying. This is - // because IE driver sometimes (~1%) will throw an - // unknown error from this execution. See - // https://github.com/angular/protractor/issues/841 - // This shouldn't mask errors because it will fail - // with the timeout anyway. - return false; - } else { - throw err; - } - }); - }, - timeout, 'waiting for page to load for ' + timeout + 'ms') - .then(null, deferred.reject); - - this.driver.controlFlow().execute(() => { - return this.plugins_.onPageLoad(); - }); + let msg = (str: string) => { + return 'Protractor.get(' + destination + ') - ' + str; + }; - // Make sure the page is an Angular page. - this.executeAsyncScript_( - clientSideScripts.testForAngular, msg('test for angular'), Math.floor(timeout / 1000), - this.ng12Hybrid) - .then( - (angularTestResult: {ver: number, message: string}) => { - let angularVersion = angularTestResult.ver; - if (!angularVersion) { - let message = angularTestResult.message; - logger.error(`Could not find Angular on page ${destination} : ${message}`); - throw new Error( - `Angular could not be found on the page ${destination}. If this is not an ` + - `Angular application, you may need to turn off waiting for Angular. Please ` + - `see https://github.com/angular/protractor/blob/master/docs/timeouts.md#waiting-for-angular-on-page-load`); - } - return angularVersion; - }, - (err: Error) => { - throw new Error('Error while running testForAngular: ' + err.message); - }) - .then(loadMocks, deferred.reject); - - let self = this; - function loadMocks(angularVersion: number) { - if (angularVersion === 1) { - // At this point, Angular will pause for us until angular.resumeBootstrap is called. - let moduleNames: string[] = []; - for (const {name, script, args} of self.mockModules_) { - moduleNames.push(name); - let executeScriptArgs = [script, msg('add mock module ' + name), ...args]; - self.executeScriptWithDescription.apply(self, executeScriptArgs) + return this.driver.controlFlow() + .execute(() => { + return wdpromise.when(null); + }) + .then(() => { + if (this.bpClient) { + return this.driver.controlFlow().execute(() => { + return this.bpClient.setSynchronization(false); + }); + } + }) + .then(() => { + // Go to reset url + return this.driver.get(this.resetUrl); + }) + .then(() => { + // Set defer label and navigate + return this.executeScriptWithDescription( + 'window.name = "' + DEFER_LABEL + '" + window.name;' + + 'window.location.replace("' + destination + '");', + msg('reset url')); + }) + .then(() => { + // We need to make sure the new url has loaded before + // we try to execute any asynchronous scripts. + return this.driver.wait(() => { + return this.executeScriptWithDescription('return window.location.href;', msg('get url')) + .then( + (url: any) => { + return url !== this.resetUrl; + }, + (err: IError) => { + if (err.code == 13) { + // Ignore the error, and continue trying. This is + // because IE driver sometimes (~1%) will throw an + // unknown error from this execution. See + // https://github.com/angular/protractor/issues/841 + // This shouldn't mask errors because it will fail + // with the timeout anyway. + return false; + } else { + throw err; + } + }); + }, timeout, 'waiting for page to load for ' + timeout + 'ms'); + }) + .then(() => { + // Run Plugins + return this.driver.controlFlow().execute(() => { + return this.plugins_.onPageLoad(); + }); + }) + .then(() => { + // Make sure the page is an Angular page. + return this + .executeAsyncScript_( + clientSideScripts.testForAngular, msg('test for angular'), + Math.floor(timeout / 1000), this.ng12Hybrid) .then( - null, + (angularTestResult: {ver: number, message: string}) => { + let angularVersion = angularTestResult.ver; + if (!angularVersion) { + let message = angularTestResult.message; + logger.error(`Could not find Angular on page ${destination} : ${message}`); + throw new Error( + 'Angular could not be found on the page ${destination}.' + + `If this is not an Angular application, you may need to turn off waiting for Angular. + Please see + https://github.com/angular/protractor/blob/master/docs/timeouts.md#waiting-for-angular-on-page-load`); + } + return angularVersion; + }, (err: Error) => { - throw new Error( - 'Error while running module script ' + name + ': ' + err.message); - }) - .then(null, deferred.reject); - } - - self.executeScriptWithDescription( - 'window.__TESTABILITY__NG1_APP_ROOT_INJECTOR__ = ' + - 'angular.resumeBootstrap(arguments[0]);', - msg('resume bootstrap'), moduleNames) - .then(null, deferred.reject); - } else { - // TODO: support mock modules in Angular2. For now, error if someone - // has tried to use one. - if (self.mockModules_.length > 1) { - deferred.reject( - 'Trying to load mock modules on an Angular2 app ' + - 'is not yet supported.'); - } - } - } - - if (this.bpClient) { - this.driver.controlFlow().execute(() => { - return this.bpClient.setSynchronization(!this.internalIgnoreSynchronization); - }); - } - - this.driver.controlFlow().execute(() => { - return this.plugins_.onPageStable().then(() => { - deferred.fulfill(); - }, deferred.reject); - }); + throw new Error('Error while running testForAngular: ' + err.message); + }); - return deferred.promise; + }) + .then((angularVersion) => { + // Load Angular Mocks + if (angularVersion === 1) { + // At this point, Angular will pause for us until angular.resumeBootstrap is called. + let moduleNames: string[] = []; + let modulePromise: wdpromise.Promise = wdpromise.when(null); + for (const {name, script, args} of this.mockModules_) { + moduleNames.push(name); + let executeScriptArgs = [script, msg('add mock module ' + name), ...args]; + modulePromise = modulePromise.then( + () => this.executeScriptWithDescription.apply(this, executeScriptArgs) + .then(null, (err: Error) => { + throw new Error( + 'Error while running module script ' + name + ': ' + err.message); + })); + } + + return modulePromise.then( + () => this.executeScriptWithDescription( + 'window.__TESTABILITY__NG1_APP_ROOT_INJECTOR__ = ' + + 'angular.resumeBootstrap(arguments[0]);', + msg('resume bootstrap'), moduleNames)); + } else { + // TODO: support mock modules in Angular2. For now, error if someone + // has tried to use one. + if (this.mockModules_.length > 1) { + throw 'Trying to load mock modules on an Angular2 app is not yet supported.'; + } + } + }) + .then(() => { + // Reset bpClient sync + if (this.bpClient) { + return this.driver.controlFlow().execute(() => { + return this.bpClient.setSynchronization(!this.internalIgnoreSynchronization); + }); + } + }) + .then(() => { + // Run Plugins + return this.driver.controlFlow().execute(() => { + return this.plugins_.onPageStable(); + }); + }) + .then(() => null); } /** @@ -1007,15 +1026,15 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { * page has been changed. */ setLocation(url: string): wdpromise.Promise { - this.waitForAngular(); - return 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.executeScriptWithDescription( + clientSideScripts.setLocation, 'Protractor.setLocation()', this.rootEl, url) + .then((browserErr: Error) => { + if (browserErr) { + throw 'Error while navigating to \'' + url + '\' : ' + + JSON.stringify(browserErr); + } + })); } /** @@ -1029,9 +1048,9 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { * AngularJS. */ getLocationAbsUrl(): wdpromise.Promise { - this.waitForAngular(); - return this.executeScriptWithDescription( - clientSideScripts.getLocationAbsUrl, 'Protractor.getLocationAbsUrl()', this.rootEl); + return this.waitForAngular().then( + () => this.executeScriptWithDescription( + clientSideScripts.getLocationAbsUrl, 'Protractor.getLocationAbsUrl()', this.rootEl)); } /** @@ -1056,10 +1075,10 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { */ debugger() { // jshint debug: true - this.driver.executeScript(clientSideScripts.installInBrowser); - wdpromise.controlFlow().execute(() => { - debugger; - }, 'add breakpoint to control flow'); + return this.driver.executeScript(clientSideScripts.installInBrowser) + .then(() => wdpromise.controlFlow().execute(() => { + debugger; + }, 'add breakpoint to control flow')); } /** diff --git a/lib/clientsidescripts.js b/lib/clientsidescripts.js index e86f776fa..c741c8d7e 100644 --- a/lib/clientsidescripts.js +++ b/lib/clientsidescripts.js @@ -109,8 +109,8 @@ function getNg1Hooks(selector, injectorPlease) { return {$injector: $injector, $$testability: $$testability}; } else { return tryEl(document.body) || - trySelector('[ng-app]') || trySelector('[ng:app]') || - trySelector('[ng-controller]') || trySelector('[ng:controller]'); + trySelector('[ng-app]') || trySelector('[ng\\:app]') || + trySelector('[ng-controller]') || trySelector('[ng\\:controller]'); } }