This repository was archived by the owner on Jul 29, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
chore(driverProviders): clean up driver provider q usage #5034
Merged
cnishina
merged 3 commits into
angular:selenium4
from
cnishina:driverProviderUseExisting
Nov 15, 2018
Merged
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -193,24 +193,18 @@ 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|webdriver.promise.Promise<string>} value The new selector. | ||
* @param {string|webdriver.promise.Promise<string>} valuePromise The new selector. | ||
* @returns A promise that resolves with the value of the selector. | ||
*/ | ||
angularAppRoot(value: string|wdpromise.Promise<string> = null): wdpromise.Promise<string> { | ||
return this.driver.controlFlow().execute(() => { | ||
if (value != null) { | ||
return wdpromise.when(value).then((value: string) => { | ||
this.internalRootEl = value; | ||
if (this.bpClient) { | ||
const bpCommandPromise = this.bpClient.setWaitParams(value); | ||
// Convert to webdriver promise as best as possible | ||
return wdpromise.when(bpCommandPromise as any).then(() => this.internalRootEl); | ||
} | ||
return this.internalRootEl; | ||
}); | ||
async angularAppRoot(valuePromise: string|wdpromise.Promise<string> = null): Promise<string> { | ||
if (valuePromise != null) { | ||
const value = await Promise.resolve(valuePromise); | ||
this.internalRootEl = value; | ||
if (this.bpClient) { | ||
await this.bpClient.setWaitParams(value); | ||
} | ||
return wdpromise.when(this.internalRootEl); | ||
}, `Set angular root selector to ${value}`); | ||
} | ||
return this.internalRootEl; | ||
} | ||
|
||
/** | ||
|
@@ -417,23 +411,17 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { | |
* Call waitForAngularEnabled() without passing a value to read the current | ||
* state without changing it. | ||
*/ | ||
waitForAngularEnabled(enabled: boolean|wdpromise.Promise<boolean> = null): | ||
wdpromise.Promise<boolean> { | ||
if (enabled != null) { | ||
const ret = this.driver.controlFlow().execute(() => { | ||
return wdpromise.when(enabled).then((enabled: boolean) => { | ||
if (this.bpClient) { | ||
logger.debug('Setting waitForAngular' + !enabled); | ||
const bpCommandPromise = this.bpClient.setWaitEnabled(enabled); | ||
// Convert to webdriver promise as best as possible | ||
return wdpromise.when(bpCommandPromise as any).then(() => enabled); | ||
} | ||
}); | ||
}, `Set proxy synchronization enabled to ${enabled}`); | ||
async waitForAngularEnabled(enabledPromise: boolean|wdpromise.Promise<boolean> = null): | ||
Promise<boolean> { | ||
if (enabledPromise != null) { | ||
const enabled = await Promise.resolve(enabledPromise); | ||
cnishina marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (this.bpClient) { | ||
logger.debug('Setting waitForAngular' + !enabled); | ||
await this.bpClient.setWaitEnabled(enabled); | ||
} | ||
this.internalIgnoreSynchronization = !enabled; | ||
return ret; | ||
} | ||
return wdpromise.when(!this.ignoreSynchronization); | ||
return !this.ignoreSynchronization; | ||
} | ||
|
||
/** | ||
|
@@ -602,15 +590,15 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { | |
* @template T | ||
*/ | ||
private executeAsyncScript_(script: string|Function, description: string, ...scriptArgs: any[]): | ||
wdpromise.Promise<any> { | ||
Promise<any> { | ||
if (typeof script === 'function') { | ||
script = 'return (' + script + ').apply(null, arguments);'; | ||
} | ||
return this.driver.schedule( | ||
new Command(CommandName.EXECUTE_ASYNC_SCRIPT) | ||
.setParameter('script', script) | ||
.setParameter('args', scriptArgs), | ||
description); | ||
new Command(CommandName.EXECUTE_ASYNC_SCRIPT) | ||
.setParameter('script', script) | ||
.setParameter('args', scriptArgs), | ||
description) as Promise<any>; | ||
} | ||
|
||
/** | ||
|
@@ -624,26 +612,20 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { | |
* @returns {!webdriver.promise.Promise} A promise that will resolve to the | ||
* scripts return value. | ||
*/ | ||
waitForAngular(opt_description?: string): wdpromise.Promise<any> { | ||
async waitForAngular(opt_description?: string): Promise<any> { | ||
let description = opt_description ? ' - ' + opt_description : ''; | ||
if (this.ignoreSynchronization) { | ||
return this.driver.controlFlow().execute(() => { | ||
return true; | ||
}, 'Ignore Synchronization Protractor.waitForAngular()'); | ||
return Promise.resolve(true); | ||
} | ||
|
||
let runWaitForAngularScript: () => wdpromise.Promise<any> = () => { | ||
let runWaitForAngularScript = async(): Promise<any> => { | ||
if (this.plugins_.skipAngularStability() || this.bpClient) { | ||
return this.driver.controlFlow().execute(() => { | ||
return wdpromise.when(null); | ||
}, 'bpClient or plugin stability override'); | ||
return Promise.resolve(null); | ||
cnishina marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} else { | ||
// Need to wrap this so that we read rootEl in the control flow, not synchronously. | ||
return this.angularAppRoot().then((rootEl: string) => { | ||
return this.executeAsyncScript_( | ||
clientSideScripts.waitForAngular, 'Protractor.waitForAngular()' + description, | ||
rootEl); | ||
}); | ||
let rootEl = await this.angularAppRoot(); | ||
return this.executeAsyncScript_( | ||
clientSideScripts.waitForAngular, 'Protractor.waitForAngular()' + description, rootEl); | ||
} | ||
}; | ||
|
||
|
@@ -656,20 +638,12 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { | |
} | ||
}) | ||
.then( | ||
() => { | ||
return this.driver.controlFlow() | ||
.execute( | ||
() => { | ||
return this.plugins_.waitForPromise(this); | ||
}, | ||
'Plugins.waitForPromise()') | ||
.then(() => { | ||
return this.driver.wait(() => { | ||
return this.plugins_.waitForCondition(this).then((results: boolean[]) => { | ||
return results.reduce((x, y) => x && y, true); | ||
}); | ||
}, this.allScriptsTimeout, 'Plugins.waitForCondition()'); | ||
}); | ||
async () => { | ||
await this.plugins_.waitForPromise(this); | ||
return this.driver.wait(async () => { | ||
let results = await this.plugins_.waitForCondition(this); | ||
return results.reduce((x, y) => x && y, true); | ||
}, this.allScriptsTimeout, 'Plugins.waitForCondition()'); | ||
}, | ||
(err: Error) => { | ||
let timeout: RegExpExecArray; | ||
|
@@ -978,16 +952,14 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver { | |
.then(() => { | ||
// Reset bpClient sync | ||
if (this.bpClient) { | ||
return this.driver.controlFlow().execute(() => { | ||
return this.bpClient.setWaitEnabled(!this.internalIgnoreSynchronization); | ||
}); | ||
return this.bpClient.setWaitEnabled(!this.internalIgnoreSynchronization); | ||
} | ||
}) | ||
.then(() => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This promise chain should eventually become async/await, but let's save that for another Pr. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, this promise chain might be better contained in a separate PR. |
||
// Run Plugins | ||
return this.driver.controlFlow().execute(() => { | ||
if (!this.ignoreSynchronization) { | ||
return this.plugins_.onPageStable(this); | ||
}); | ||
} | ||
}) | ||
.then(() => null); | ||
} | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.