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

chore(browser): remove timing issues with restart and fork #5085

Merged
merged 8 commits into from
Dec 18, 2018
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 6 additions & 21 deletions lib/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
* Resolved when the browser is ready for use. Resolves to the browser, so
* you can do:
*
* forkedBrowser = await browser.forkNewDriverInstance().ready;
* forkedBrowser = await browser.forkNewDriverInstance();
*
* Set by the runner.
*
Expand Down Expand Up @@ -359,22 +359,6 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
ng12Hybrid_ = ng12Hybrid;
}
});
this.ready = this.angularAppRoot(opt_rootElement || '')
.then(() => {
return this.driver.getSession();
})
.then((session: Session) => {
// Internet Explorer does not accept data URLs, which are the default
// reset URL for Protractor.
// Safari accepts data urls, but SafariDriver fails after one is used.
// PhantomJS produces a "Detected a page unload event" if we use data urls
let browserName = session.getCapabilities().get('browserName');
if (browserName === 'internet explorer' || browserName === 'safari' ||
browserName === 'phantomjs' || browserName === 'MicrosoftEdge') {
this.resetUrl = 'about:blank';
}
return this;
});

this.trackOutstandingTimeouts_ = !opt_untrackOutstandingTimeouts;
this.mockModules_ = [];
Expand Down Expand Up @@ -423,7 +407,7 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
* Fork another instance of browser for use in interactive tests.
*
* @example
* var forked = await browser.forkNewDriverInstance().ready;
* var forked = await browser.forkNewDriverInstance();
* await forked.get('page1'); // 'page1' gotten by forked browser
*
* @param {boolean=} useSameUrl Whether to navigate to current url on creation
Expand All @@ -433,8 +417,9 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
*
* @returns {ProtractorBrowser} A browser instance.
*/
forkNewDriverInstance(useSameUrl?: boolean, copyMockModules?: boolean, copyConfigUpdates = true):
ProtractorBrowser {
async forkNewDriverInstance(
useSameUrl?: boolean, copyMockModules?: boolean,
copyConfigUpdates = true): Promise<ProtractorBrowser> {
return null;
}

Expand All @@ -456,7 +441,7 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
* await browser.get('page2'); // 'page2' gotten by restarted browser
*
* // Running against forked browsers
* var forked = await browser.forkNewDriverInstance().ready;
* var forked = await browser.forkNewDriverInstance();
* await fork.get('page1');
* fork = await fork.restart();
* await fork.get('page2'); // 'page2' gotten by restarted fork
Expand Down
5 changes: 3 additions & 2 deletions lib/driverProviders/attachSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ export class AttachSession extends DriverProvider {
* @public
* @return {WebDriver} webdriver instance
*/
getNewDriver(): WebDriver {
async getNewDriver(): Promise<WebDriver> {
const httpClient = new http.HttpClient(this.config_.seleniumAddress);
const executor = new http.Executor(httpClient);
const newDriver = WebDriver.attachToSession(executor, this.config_.seleniumSessionId, null);
const newDriver =
await WebDriver.attachToSession(executor, this.config_.seleniumSessionId, null);
this.drivers_.push(newDriver);
return newDriver;
}
Expand Down
6 changes: 3 additions & 3 deletions lib/driverProviders/direct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export class Direct extends DriverProvider {
* @override
* @return webdriver instance
*/
getNewDriver(): WebDriver {
async getNewDriver(): Promise<WebDriver> {
let driver: WebDriver;

switch (this.config_.capabilities.browserName) {
Expand All @@ -74,7 +74,7 @@ export class Direct extends DriverProvider {
}

let chromeService = new ChromeServiceBuilder(chromeDriverFile).build();
driver = DriverForChrome.createSession(
driver = await DriverForChrome.createSession(
new Capabilities(this.config_.capabilities), chromeService);
break;
case 'firefox':
Expand All @@ -98,7 +98,7 @@ export class Direct extends DriverProvider {
}

let firefoxService = new FirefoxServiceBuilder(geckoDriverFile).build();
driver = DriverForFirefox.createSession(
driver = await DriverForFirefox.createSession(
new Capabilities(this.config_.capabilities), firefoxService);
break;
default:
Expand Down
11 changes: 9 additions & 2 deletions lib/driverProviders/driverProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export abstract class DriverProvider {
* @public
* @return webdriver instance
*/
getNewDriver() {
async getNewDriver() {
let builder: Builder;
if (this.config_.useBlockingProxy) {
builder =
Expand All @@ -56,7 +56,13 @@ export abstract class DriverProvider {
if (this.config_.disableEnvironmentOverrides === true) {
builder.disableEnvironmentOverrides();
}
let newDriver = builder.build() as WebDriver;
let newDriver: WebDriver;
try {
newDriver = await builder.build();
} catch (e) {
e.code = 135;
cnishina marked this conversation as resolved.
Show resolved Hide resolved
throw e;
}
this.drivers_.push(newDriver);
return newDriver;
}
Expand All @@ -72,6 +78,7 @@ export abstract class DriverProvider {
if (driverIndex >= 0) {
this.drivers_.splice(driverIndex, 1);
try {
await driver.close();
await driver.quit();
} catch (err) {
// This happens when Protractor keeps track of all the webdrivers
Expand Down
2 changes: 1 addition & 1 deletion lib/driverProviders/mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class Mock extends DriverProvider {
* @override
* @return webdriver instance
*/
getNewDriver(): WebDriver {
async getNewDriver(): Promise<WebDriver> {
let mockSession = new Session('test_session_id', {});
let newDriver = new WebDriver(mockSession, new MockExecutor());
this.drivers_.push(newDriver);
Expand Down
80 changes: 28 additions & 52 deletions lib/runner.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {EventEmitter} from 'events';
// TODO(cnishina): remove when selenium webdriver is upgraded.
import {promise as wdpromise} from 'selenium-webdriver';
import {promise as wdpromise, WebDriver} from 'selenium-webdriver';
import * as util from 'util';

import {ProtractorBrowser} from './browser';
Expand Down Expand Up @@ -54,9 +54,6 @@ export class Runner extends EventEmitter {
nodedebug.on('exit', () => {
process.exit(1);
});
this.ready_ = new Promise(resolve => {
setTimeout(resolve, 1000);
});
}

if (config.capabilities && config.capabilities.seleniumAddress) {
Expand Down Expand Up @@ -206,9 +203,9 @@ export class Runner extends EventEmitter {
* @return {Protractor} a protractor instance.
* @public
*/
createBrowser(plugins: any, parentBrowser?: ProtractorBrowser): any {
async createBrowser(plugins: any, parentBrowser?: ProtractorBrowser): Promise<any> {
let config = this.config_;
let driver = this.driverprovider_.getNewDriver();
let driver = await this.driverprovider_.getNewDriver();

let blockingProxyUrl: string;
if (config.useBlockingProxy) {
Expand Down Expand Up @@ -258,58 +255,40 @@ export class Runner extends EventEmitter {
browser_.ng12Hybrid = initProperties.ng12Hybrid;
}

browser_.ready =
browser_.ready
.then(() => {
return browser_.waitForAngularEnabled(initProperties.waitForAngularEnabled);
})
.then(() => {
return driver.manage().timeouts().setScriptTimeout(
initProperties.allScriptsTimeout || 0);
})
.then(() => {
return browser_;
});
await browser_.waitForAngularEnabled(initProperties.waitForAngularEnabled);
await driver.manage().timeouts().setScriptTimeout(initProperties.allScriptsTimeout || 0);

browser_.getProcessedConfig = () => {
return Promise.resolve(config);
};

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);
browser_.forkNewDriverInstance = async(
useSameUrl: boolean, copyMockModules: boolean, copyConfigUpdates = true): Promise<any> => {
let newBrowser = await this.createBrowser(plugins);
if (copyMockModules) {
newBrowser.mockModules_ = browser_.mockModules_;
}
if (useSameUrl) {
const currentUrl = await browser_.driver.getCurrentUrl();
await newBrowser.get(currentUrl);
}
return newBrowser;
};

let replaceBrowser = async () => {
let newBrowser = await browser_.forkNewDriverInstance(false, true);
if (browser_ === protractor.browser) {
this.setupGlobals_(newBrowser);
}
return newBrowser;
};

browser_.restart = () => {
browser_.restart = async () => {
// Note: because tests are not paused at this point, any async
// calls here are not guaranteed to complete before the tests resume.
return this.driverprovider_.quitDriver(browser_.driver)
.then(replaceBrowser)
.then(newBrowser => newBrowser.ready);
const restartedBrowser = await replaceBrowser();
await this.driverprovider_.quitDriver(browser_.driver);
return restartedBrowser;
};

return browser_;
Expand Down Expand Up @@ -352,18 +331,15 @@ export class Runner extends EventEmitter {
this.config_.useBlockingProxy = true;
}

// 0) Wait for debugger
await Promise.resolve(this.ready_);

// 1) Setup environment
// noinspection JSValidateTypes
await this.driverprovider_.setupEnv();

// 2) Create a browser and setup globals
browser_ = this.createBrowser(plugins);
browser_ = await this.createBrowser(plugins);
this.setupGlobals_(browser_);
try {
const session = await browser_.ready.then(browser_.getSession);
const session = await browser_.getSession();
logger.debug(
'WebDriver session successfully started with capabilities ' +
util.inspect(session.getCapabilities()));
Expand Down Expand Up @@ -403,9 +379,9 @@ export class Runner extends EventEmitter {

if (this.config_.restartBrowserBetweenTests) {
// TODO(sjelin): replace with warnings once `afterEach` support is required
let restartDriver = () => {
let restartDriver = async () => {
if (!this.frameworkUsesAfterEach) {
this.restartPromise = Promise.resolve(browser_.restart());
this.restartPromise = await browser_.restart();
}
};
this.on('testPass', restartDriver);
Expand Down
Loading