Skip to content
This repository was archived by the owner on Jul 29, 2024. It is now read-only.

Commit d729bb7

Browse files
cnishinacnishina
committed
address comments
- In lieu of Promise.resolve(...) for async await, just return the item - Clean up JSDocs for q.promise - Clean up interactive tests to close the browser window and let Protractor quit the driver session
1 parent 052b591 commit d729bb7

File tree

12 files changed

+93
-142
lines changed

12 files changed

+93
-142
lines changed

‎lib/browser.ts

Lines changed: 73 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
198198
*/
199199
async angularAppRoot(valuePromise: string|wdpromise.Promise<string> = null): Promise<string> {
200200
if (valuePromise != null) {
201-
const value = await Promise.resolve(valuePromise);
201+
const value = await valuePromise;
202202
this.internalRootEl = value;
203203
if (this.bpClient) {
204204
await this.bpClient.setWaitParams(value);
@@ -414,7 +414,7 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
414414
async waitForAngularEnabled(enabledPromise: boolean|wdpromise.Promise<boolean> = null):
415415
Promise<boolean> {
416416
if (enabledPromise != null) {
417-
const enabled = await Promise.resolve(enabledPromise);
417+
const enabled = await enabledPromise;
418418
if (this.bpClient) {
419419
logger.debug('Setting waitForAngular' + !enabled);
420420
await this.bpClient.setWaitEnabled(enabled);
@@ -615,99 +615,88 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
615615
async waitForAngular(opt_description?: string): Promise<any> {
616616
let description = opt_description ? ' - ' + opt_description : '';
617617
if (this.ignoreSynchronization) {
618-
return Promise.resolve(true);
618+
return true;
619619
}
620620

621621
let runWaitForAngularScript = async(): Promise<any> => {
622622
if (this.plugins_.skipAngularStability() || this.bpClient) {
623-
return Promise.resolve(null);
623+
return null;
624624
} else {
625-
// Need to wrap this so that we read rootEl in the control flow, not synchronously.
626625
let rootEl = await this.angularAppRoot();
627626
return this.executeAsyncScript_(
628-
clientSideScripts.waitForAngular, 'Protractor.waitForAngular()' + description, rootEl);
627+
clientSideScripts.waitForAngular, `Protractor.waitForAngular() ${description}`, rootEl);
629628
}
630629
};
631630

632-
return runWaitForAngularScript()
633-
.then((browserErr: Function) => {
634-
if (browserErr) {
635-
throw new Error(
636-
'Error while waiting for Protractor to ' +
637-
'sync with the page: ' + JSON.stringify(browserErr));
631+
let browserErr = await runWaitForAngularScript();
632+
if (browserErr) {
633+
throw new Error(
634+
'Error while waiting for Protractor to ' +
635+
'sync with the page: ' + JSON.stringify(browserErr));
636+
}
637+
await this.plugins_.waitForPromise(this);
638+
try {
639+
await this.driver.wait(async () => {
640+
let results = await this.plugins_.waitForCondition(this);
641+
return results.reduce((x, y) => x && y, true);
642+
}, this.allScriptsTimeout, 'Plugins.waitForCondition()');
643+
} catch (err) {
644+
let timeout: RegExpExecArray;
645+
if (/asynchronous script timeout/.test(err.message)) {
646+
// Timeout on Chrome
647+
timeout = /-?[\d\.]*\ seconds/.exec(err.message);
648+
} else if (/Timed out waiting for async script/.test(err.message)) {
649+
// Timeout on Firefox
650+
timeout = /-?[\d\.]*ms/.exec(err.message);
651+
} else if (/Timed out waiting for an asynchronous script/.test(err.message)) {
652+
// Timeout on Safari
653+
timeout = /-?[\d\.]*\ ms/.exec(err.message);
654+
}
655+
if (timeout) {
656+
let errMsg = `Timed out waiting for asynchronous Angular tasks to finish after ` +
657+
`${timeout}. This may be because the current page is not an Angular ` +
658+
`application. Please see the FAQ for more details: ` +
659+
`https://github.com/angular/protractor/blob/master/docs/timeouts.md#waiting-for-angular`;
660+
if (description.indexOf(' - Locator: ') == 0) {
661+
errMsg += '\nWhile waiting for element with locator' + description;
662+
}
663+
let pendingTimeoutsPromise: wdpromise.Promise<any>;
664+
if (this.trackOutstandingTimeouts_) {
665+
pendingTimeoutsPromise = this.executeScriptWithDescription(
666+
'return window.NG_PENDING_TIMEOUTS',
667+
'Protractor.waitForAngular() - getting pending timeouts' + description);
668+
} else {
669+
pendingTimeoutsPromise = wdpromise.when({});
670+
}
671+
let pendingHttpsPromise = this.executeScriptWithDescription(
672+
clientSideScripts.getPendingHttpRequests,
673+
'Protractor.waitForAngular() - getting pending https' + description,
674+
this.internalRootEl);
675+
676+
let arr = await Promise.all([pendingTimeoutsPromise, pendingHttpsPromise]);
677+
678+
let pendingTimeouts = arr[0] || [];
679+
let pendingHttps = arr[1] || [];
680+
681+
let key: string, pendingTasks: string[] = [];
682+
for (key in pendingTimeouts) {
683+
if (pendingTimeouts.hasOwnProperty(key)) {
684+
pendingTasks.push(' - $timeout: ' + pendingTimeouts[key]);
638685
}
639-
})
640-
.then(
641-
async () => {
642-
await this.plugins_.waitForPromise(this);
643-
return this.driver.wait(async () => {
644-
let results = await this.plugins_.waitForCondition(this);
645-
return results.reduce((x, y) => x && y, true);
646-
}, this.allScriptsTimeout, 'Plugins.waitForCondition()');
647-
},
648-
(err: Error) => {
649-
let timeout: RegExpExecArray;
650-
if (/asynchronous script timeout/.test(err.message)) {
651-
// Timeout on Chrome
652-
timeout = /-?[\d\.]*\ seconds/.exec(err.message);
653-
} else if (/Timed out waiting for async script/.test(err.message)) {
654-
// Timeout on Firefox
655-
timeout = /-?[\d\.]*ms/.exec(err.message);
656-
} else if (/Timed out waiting for an asynchronous script/.test(err.message)) {
657-
// Timeout on Safari
658-
timeout = /-?[\d\.]*\ ms/.exec(err.message);
659-
}
660-
if (timeout) {
661-
let errMsg = `Timed out waiting for asynchronous Angular tasks to finish after ` +
662-
`${timeout}. This may be because the current page is not an Angular ` +
663-
`application. Please see the FAQ for more details: ` +
664-
`https://github.com/angular/protractor/blob/master/docs/timeouts.md#waiting-for-angular`;
665-
if (description.indexOf(' - Locator: ') == 0) {
666-
errMsg += '\nWhile waiting for element with locator' + description;
667-
}
668-
let pendingTimeoutsPromise: wdpromise.Promise<any>;
669-
if (this.trackOutstandingTimeouts_) {
670-
pendingTimeoutsPromise = this.executeScriptWithDescription(
671-
'return window.NG_PENDING_TIMEOUTS',
672-
'Protractor.waitForAngular() - getting pending timeouts' + description);
673-
} else {
674-
pendingTimeoutsPromise = wdpromise.when({});
675-
}
676-
let pendingHttpsPromise = this.executeScriptWithDescription(
677-
clientSideScripts.getPendingHttpRequests,
678-
'Protractor.waitForAngular() - getting pending https' + description,
679-
this.internalRootEl);
680-
681-
return wdpromise.all([pendingTimeoutsPromise, pendingHttpsPromise])
682-
.then(
683-
(arr: any[]) => {
684-
let pendingTimeouts = arr[0] || [];
685-
let pendingHttps = arr[1] || [];
686-
687-
let key: string, pendingTasks: string[] = [];
688-
for (key in pendingTimeouts) {
689-
if (pendingTimeouts.hasOwnProperty(key)) {
690-
pendingTasks.push(' - $timeout: ' + pendingTimeouts[key]);
691-
}
692-
}
693-
for (key in pendingHttps) {
694-
pendingTasks.push(' - $http: ' + pendingHttps[key].url);
695-
}
696-
if (pendingTasks.length) {
697-
errMsg += '. \nThe following tasks were pending:\n';
698-
errMsg += pendingTasks.join('\n');
699-
}
700-
err.message = errMsg;
701-
throw err;
702-
},
703-
() => {
704-
err.message = errMsg;
705-
throw err;
706-
});
707-
} else {
708-
throw err;
709-
}
710-
});
686+
}
687+
for (key in pendingHttps) {
688+
pendingTasks.push(' - $http: ' + pendingHttps[key].url);
689+
}
690+
if (pendingTasks.length) {
691+
errMsg += '. \nThe following tasks were pending:\n';
692+
errMsg += pendingTasks.join('\n');
693+
}
694+
err.message = errMsg;
695+
throw err;
696+
} else {
697+
throw err;
698+
}
699+
}
711700
}
712701

713702
/**

‎lib/driverProviders/attachSession.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,12 @@ export class AttachSession extends DriverProvider {
2121

2222
/**
2323
* Configure and launch (if applicable) the object's environment.
24-
* @return {q.promise} A promise which will resolve when the environment is
24+
* @return {Promise} A promise which will resolve when the environment is
2525
* ready to test.
2626
*/
2727
protected async setupDriverEnv(): Promise<any> {
2828
logger.info('Using the selenium server at ' + this.config_.seleniumAddress);
2929
logger.info('Using session id - ' + this.config_.seleniumSessionId);
30-
return Promise.resolve();
3130
}
3231

3332
/**
@@ -49,7 +48,5 @@ export class AttachSession extends DriverProvider {
4948
*
5049
* @public
5150
*/
52-
quitDriver(): Promise<void> {
53-
return Promise.resolve();
54-
}
51+
async quitDriver(): Promise<void> {}
5552
}

‎lib/driverProviders/browserStack.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,5 @@ export class BrowserStack extends DriverProvider {
9393
}
9494

9595
logger.info(`Using BrowserStack selenium server at ${this.config_.seleniumAddress}`);
96-
return Promise.resolve();
9796
}
9897
}

‎lib/driverProviders/direct.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export class Direct extends DriverProvider {
2525

2626
/**
2727
* Configure and launch (if applicable) the object's environment.
28-
* @return {q.promise} A promise which will resolve when the environment is
28+
* @return {Promise} A promise which will resolve when the environment is
2929
* ready to test.
3030
*/
3131
protected async setupDriverEnv(): Promise<any> {
@@ -42,7 +42,6 @@ export class Direct extends DriverProvider {
4242
'browserName ' + this.config_.capabilities.browserName +
4343
' is not supported with directConnect.');
4444
}
45-
return Promise.resolve(function() {});
4645
}
4746

4847
/**

‎lib/driverProviders/driverProvider.ts

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -71,38 +71,32 @@ export abstract class DriverProvider {
7171
let driverIndex = this.drivers_.indexOf(driver);
7272
if (driverIndex >= 0) {
7373
this.drivers_.splice(driverIndex, 1);
74-
}
75-
76-
try {
77-
if (driver.getSession() !== undefined) {
78-
const session = await driver.getSession();
79-
if (session) {
80-
return await driver.quit();
81-
}
74+
try {
75+
await driver.quit();
76+
} catch (err) {
77+
// This happens when Protractor keeps track of all the webdrivers
78+
// created and calls quit. If a user calls driver.quit, then this will
79+
// throw an error. This catch will swallow the error.
8280
}
83-
} catch (_) {
8481
}
8582
}
8683

87-
8884
/**
8985
* Quits an array of drivers and returns a q promise instead of a webdriver one
9086
*
9187
* @param drivers {webdriver.WebDriver[]} The webdriver instances
9288
*/
9389
static async quitDrivers(provider: DriverProvider, drivers: WebDriver[]): Promise<void> {
94-
await Promise.all(drivers.map(async (driver: WebDriver) => {
95-
await provider.quitDriver(driver);
90+
await Promise.all(drivers.map((driver: WebDriver) => {
91+
return provider.quitDriver(driver);
9692
}));
9793
}
9894

9995
/**
10096
* Default update job method.
10197
* @return a promise
10298
*/
103-
updateJob(update: any): Promise<any> {
104-
return Promise.resolve();
105-
};
99+
async updateJob(update: any): Promise<any>{};
106100

107101
/**
108102
* Default setup environment method, common to all driver providers.

‎lib/driverProviders/hosted.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ export class Hosted extends DriverProvider {
2020
* @return {Promise} A promise which will resolve when the environment is
2121
* ready to test.
2222
*/
23-
protected setupDriverEnv(): Promise<any> {
23+
protected async setupDriverEnv(): Promise<any> {
2424
logger.info('Using the selenium server at ' + this.config_.seleniumAddress);
25-
return Promise.resolve();
2625
}
2726
}

‎lib/driverProviders/kobiton.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,12 @@ export class Kobiton extends DriverProvider {
1919
* @return {Promise} A promise which will resolve when the environment is
2020
* ready to test.
2121
*/
22-
protected setupDriverEnv(): Promise<any> {
22+
protected async setupDriverEnv(): Promise<any> {
2323
this.config_.capabilities['kobitonUser'] = this.config_.kobitonUser;
2424
this.config_.capabilities['kobitonKey'] = this.config_.kobitonKey;
2525
this.config_.seleniumAddress = 'https://' + this.config_.kobitonUser + ':' +
2626
this.config_.kobitonKey + '@api.kobiton.com/wd/hub';
2727

2828
logger.info('Using Kobiton selenium server at ' + this.config_.seleniumAddress);
29-
return Promise.resolve();
3029
}
3130
}

‎lib/driverProviders/local.ts

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -161,22 +161,4 @@ export class Local extends DriverProvider {
161161
const address = await this.server_.address();
162162
this.config_.seleniumAddress = address;
163163
}
164-
165-
/**
166-
* Teardown and destroy the environment and do any associated cleanup.
167-
* Shuts down the drivers and server.
168-
*
169-
* @public
170-
* @override
171-
* @return {q.promise} A promise which will resolve when the environment
172-
* is down.
173-
*/
174-
async teardownEnv(): Promise<any> {
175-
try {
176-
await super.teardownEnv();
177-
logger.info('Shutting down selenium standalone server.');
178-
await this.server_.stop();
179-
} catch (_) {
180-
}
181-
}
182164
}

‎lib/driverProviders/mock.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,16 @@ export class Mock extends DriverProvider {
2020
/**
2121
* An execute function that returns a promise with a test value.
2222
*/
23-
execute(): Promise<any> {
24-
return Promise.resolve({value: 'test_response'});
23+
async execute(): Promise<any> {
24+
return {value: 'test_response'};
2525
}
2626

2727
/**
2828
* Configure and launch (if applicable) the object's environment.
2929
* @public
30-
* @return {q.promise} A promise which will resolve immediately.
30+
* @return {Promise} A promise which will resolve immediately.
3131
*/
32-
protected setupDriverEnv(): Promise<any> {
33-
return Promise.resolve();
34-
}
32+
protected async setupDriverEnv(): Promise<any> {}
3533

3634
/**
3735
* Create a new driver.

‎lib/driverProviders/sauce.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export class Sauce extends DriverProvider {
4949
* @return {Promise} A promise which will resolve when the environment is
5050
* ready to test.
5151
*/
52-
protected setupDriverEnv(): Promise<any> {
52+
protected async setupDriverEnv(): Promise<any> {
5353
this.sauceServer_ = new SauceLabs({
5454
username: this.config_.sauceUser,
5555
password: this.config_.sauceKey,
@@ -75,6 +75,5 @@ export class Sauce extends DriverProvider {
7575
logger.info(
7676
'Using SauceLabs selenium server at ' +
7777
this.config_.seleniumAddress.replace(/\/\/.+@/, '//'));
78-
return Promise.resolve();
7978
}
8079
}

‎lib/driverProviders/testObject.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,11 @@ export class TestObject extends DriverProvider {
1919
* @return {Promise} A promise which will resolve when the environment is
2020
* ready to test.
2121
*/
22-
protected setupDriverEnv(): Promise<any> {
22+
protected async setupDriverEnv(): Promise<any> {
2323
this.config_.capabilities['testobject.user'] = this.config_.testobjectUser;
2424
this.config_.capabilities['testobject_api_key'] = this.config_.testobjectKey;
2525
this.config_.seleniumAddress = 'https://us1.appium.testobject.com/wd/hub';
2626

2727
logger.info('Using TestObject selenium server at ' + this.config_.seleniumAddress);
28-
return Promise.resolve();
2928
}
3029
}

‎spec/interaction/interaction_spec.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,6 @@ describe('Browser', () => {
7575

7676

7777
describe('Multiple browsers', () => {
78-
79-
80-
8178
let p0, p1;
8279

8380
beforeEach(async() => {

0 commit comments

Comments
 (0)