From 0e26b218d5f385dd9871a40553acc174cfdfe26d Mon Sep 17 00:00:00 2001 From: mgiambalvo Date: Fri, 16 Dec 2016 11:27:07 -0800 Subject: [PATCH] feat(blockingproxy): Add synchronization with BlockingProxy. (#3813) This adds support for BlockingProxy behind the flag --useBlockingProxy. If set, the driver providers will start a proxy during their setup phase, passing the selenium address to the proxy and starting a webdriver client that talks to the proxy. Starting a proxy for each driver provider isn't strictly necessary. However, when we run with multiple capabilities it's easier to handle the logging if each Protractor instance has it's own proxy. Known issues: - Doesn't work with directConnect. You can get the address of chromedriver by mucking around in Selenium internals, but this probably changed for Selenium 3.0 and I doubt it's worth figuring out until we upgrade. - Doesn't yet work with webDriverProxy (but it's an easy fix) --- lib/bpRunner.ts | 62 +++++++++++++++++++++++++++ lib/browser.ts | 62 +++++++++++++++++++++++++-- lib/config.ts | 7 +++ lib/driverProviders/README.md | 6 +-- lib/driverProviders/attachSession.ts | 3 +- lib/driverProviders/browserStack.ts | 3 +- lib/driverProviders/direct.ts | 3 +- lib/driverProviders/driverProvider.ts | 40 +++++++++++++---- lib/driverProviders/hosted.ts | 2 +- lib/driverProviders/local.ts | 2 +- lib/driverProviders/mock.ts | 2 +- lib/driverProviders/sauce.ts | 2 +- lib/runner.ts | 8 +++- package.json | 3 +- scripts/test.js | 1 + spec/basic/polling_spec.js | 28 +++++++++++- spec/install/tsconfig.json | 2 +- tsconfig.json | 4 +- 18 files changed, 209 insertions(+), 31 deletions(-) create mode 100644 lib/bpRunner.ts diff --git a/lib/bpRunner.ts b/lib/bpRunner.ts new file mode 100644 index 000000000..80f9c2cca --- /dev/null +++ b/lib/bpRunner.ts @@ -0,0 +1,62 @@ +import {ChildProcess, fork} from 'child_process'; +import * as q from 'q'; + +import {Config} from './config'; +import {Logger} from './logger'; + +const BP_PATH = require.resolve('blocking-proxy/built/lib/bin.js'); + +let logger = new Logger('BlockingProxy'); + +export class BlockingProxyRunner { + bpProcess: ChildProcess; + public port: number; + + constructor(private config: Config) {} + + start() { + return q.Promise((resolve, reject) => { + this.checkSupportedConfig(); + + let args = [ + '--fork', '--seleniumAddress', this.config.seleniumAddress, '--rootElement', + this.config.rootElement + ]; + this.bpProcess = fork(BP_PATH, args, {silent: true}); + logger.info('Starting BlockingProxy with args: ' + args.toString()); + this.bpProcess + .on('message', + (data) => { + this.port = data['port']; + resolve(data['port']); + }) + .on('error', + (err) => { + reject(new Error('Unable to start BlockingProxy ' + err)); + }) + .on('exit', (code: number, signal: number) => { + reject(new Error('BP exited with ' + code)); + logger.error('Exited with ' + code); + logger.error('signal ' + signal); + }); + + this.bpProcess.stdout.on('data', (msg: Buffer) => { + logger.debug(msg.toString().trim()); + }); + + this.bpProcess.stderr.on('data', (msg: Buffer) => { + logger.error(msg.toString().trim()); + }); + + process.on('exit', () => { + this.bpProcess.kill(); + }) + }) + } + + checkSupportedConfig() { + if (this.config.directConnect) { + throw new Error('BlockingProxy not yet supported with directConnect!'); + } + } +} diff --git a/lib/browser.ts b/lib/browser.ts index 52ab787dc..d60f85dee 100644 --- a/lib/browser.ts +++ b/lib/browser.ts @@ -1,3 +1,4 @@ +import {BPClient} from 'blocking-proxy'; import {ActionSequence, By, Capabilities, Command as WdCommand, FileDetector, ICommandName, Options, promise as wdpromise, Session, TargetLocator, TouchSequence, until, WebDriver, WebElement} from 'selenium-webdriver'; import * as url from 'url'; @@ -142,6 +143,12 @@ export class ProtractorBrowser extends Webdriver { */ driver: WebDriver; + /** + * The client used to control the BlockingProxy. If unset, BlockingProxy is + * not being used and Protractor will handle client-side synchronization. + */ + bpClient: BPClient; + /** * Helper function for finding elements. * @@ -187,9 +194,26 @@ export class ProtractorBrowser extends Webdriver { * tests to become flaky. This should be used only when necessary, such as * when a page continuously polls an API using $timeout. * + * This property is deprecated - please use waitForAngularEnabled instead. + * + * @deprecated * @type {boolean} */ - ignoreSynchronization: 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; + } + + get ignoreSynchronization() { + return this.internalIgnoreSynchronization; + } + + internalIgnoreSynchronization: boolean; /** * Timeout in milliseconds to wait for pages to load when calling `get`. @@ -273,7 +297,7 @@ export class ProtractorBrowser extends Webdriver { constructor( webdriverInstance: WebDriver, opt_baseUrl?: string, opt_rootElement?: string, - opt_untrackOutstandingTimeouts?: boolean) { + opt_untrackOutstandingTimeouts?: boolean, opt_blockingProxyUrl?: string) { super(); // These functions should delegate to the webdriver instance, but should // wait for Angular to sync up before performing the action. This does not @@ -292,6 +316,10 @@ export class ProtractorBrowser extends Webdriver { }); this.driver = webdriverInstance; + if (opt_blockingProxyUrl) { + logger.info('Starting BP client for ' + opt_blockingProxyUrl); + this.bpClient = new BPClient(opt_blockingProxyUrl); + } this.element = buildElementHelper(this); this.$ = build$(this.element, By); this.$$ = build$$(this.element, By); @@ -342,6 +370,22 @@ export class ProtractorBrowser extends Webdriver { this.ExpectedConditions = new ProtractorExpectedConditions(this); } + /** + * If set to false, Protractor will not wait for Angular $http and $timeout + * tasks to complete before interacting with the browser. This can cause + * flaky tests, but should be used if, for instance, your app continuously + * polls an API with $timeout. + * + * Call waitForAngularEnabled() without passing a value to read the current + * state without changing it. + */ + waitForAngularEnabled(enabled: boolean = null): boolean { + if (enabled != null) { + this.ignoreSynchronization = !enabled; + } + return !this.ignoreSynchronization; + } + /** * Get the processed configuration object that is currently being run. This * will contain the specs and capabilities properties of the current runner @@ -462,7 +506,7 @@ export class ProtractorBrowser extends Webdriver { } let runWaitForAngularScript: () => wdpromise.Promise = () => { - if (this.plugins_.skipAngularStability()) { + if (this.plugins_.skipAngularStability() || this.bpClient) { return wdpromise.fulfilled(); } else if (this.rootEl) { return this.executeAsyncScript_( @@ -690,6 +734,12 @@ export class ProtractorBrowser extends Webdriver { 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(() => {}); @@ -790,6 +840,12 @@ export class ProtractorBrowser extends Webdriver { } } + if (this.bpClient) { + this.driver.controlFlow().execute(() => { + return this.bpClient.setSynchronization(!this.internalIgnoreSynchronization); + }); + } + this.driver.controlFlow().execute(() => { return this.plugins_.onPageStable().then(() => { deferred.fulfill(); diff --git a/lib/config.ts b/lib/config.ts index 3af3247e4..59ba6a99d 100644 --- a/lib/config.ts +++ b/lib/config.ts @@ -95,6 +95,13 @@ export interface Config { */ webDriverProxy?: string; + /** + * If specified, connect to webdriver through a proxy that manages client-side + * synchronization. Blocking Proxy is an experimental feature and may change + * without notice. + */ + useBlockingProxy?: boolean; + // ---- 3. To use remote browsers via Sauce Labs ----------------------------- /** diff --git a/lib/driverProviders/README.md b/lib/driverProviders/README.md index aa098b702..2cc0919a1 100644 --- a/lib/driverProviders/README.md +++ b/lib/driverProviders/README.md @@ -11,7 +11,7 @@ Each file exports a function which takes in the configuration as a parameter and * @return {q.promise} A promise which will resolve when the environment is * ready to test. */ -DriverProvider.prototype.setupEnv +DriverProvider.prototype.setupDriverEnv /** * @return {Array.} Array of existing webdriver instances. @@ -47,9 +47,9 @@ DriverProvider.prototype.updateJob Requirements ------------ - - `setupEnv` will be called before the test framework is loaded, so any + - `setupDriverEnv` will be called before the test framework is loaded, so any pre-work which might cause timeouts on the first test should be done there. - `getNewDriver` will be called once right after `setupEnv` to generate the + `getNewDriver` will be called once right after `setupDriverEnv` to generate the initial driver, and possibly during the middle of the test if users request additional browsers. diff --git a/lib/driverProviders/attachSession.ts b/lib/driverProviders/attachSession.ts index f5e102404..ff4ae9d86 100644 --- a/lib/driverProviders/attachSession.ts +++ b/lib/driverProviders/attachSession.ts @@ -22,11 +22,10 @@ export class AttachSession extends DriverProvider { /** * Configure and launch (if applicable) the object's environment. - * @public * @return {q.promise} A promise which will resolve when the environment is * ready to test. */ - setupEnv(): q.Promise { + protected setupDriverEnv(): q.Promise { logger.info('Using the selenium server at ' + this.config_.seleniumAddress); logger.info('Using session id - ' + this.config_.seleniumSessionId); return q(undefined); diff --git a/lib/driverProviders/browserStack.ts b/lib/driverProviders/browserStack.ts index d6c91a43b..9a42acf14 100644 --- a/lib/driverProviders/browserStack.ts +++ b/lib/driverProviders/browserStack.ts @@ -90,11 +90,10 @@ export class BrowserStack extends DriverProvider { /** * Configure and launch (if applicable) the object's environment. - * @public * @return {q.promise} A promise which will resolve when the environment is * ready to test. */ - setupEnv(): q.Promise { + protected setupDriverEnv(): q.Promise { let deferred = q.defer(); this.config_.capabilities['browserstack.user'] = this.config_.browserstackUser; this.config_.capabilities['browserstack.key'] = this.config_.browserstackKey; diff --git a/lib/driverProviders/direct.ts b/lib/driverProviders/direct.ts index 36529addb..7c81ec549 100644 --- a/lib/driverProviders/direct.ts +++ b/lib/driverProviders/direct.ts @@ -26,11 +26,10 @@ export class Direct extends DriverProvider { /** * Configure and launch (if applicable) the object's environment. - * @public * @return {q.promise} A promise which will resolve when the environment is * ready to test. */ - setupEnv(): q.Promise { + protected setupDriverEnv(): q.Promise { switch (this.config_.capabilities.browserName) { case 'chrome': logger.info('Using ChromeDriver directly...'); diff --git a/lib/driverProviders/driverProvider.ts b/lib/driverProviders/driverProvider.ts index 035293be5..863c8ccef 100644 --- a/lib/driverProviders/driverProvider.ts +++ b/lib/driverProviders/driverProvider.ts @@ -6,15 +6,18 @@ import * as q from 'q'; import {Builder, Session, WebDriver} from 'selenium-webdriver'; +import {BlockingProxyRunner} from '../bpRunner'; import {Config} from '../config'; -export class DriverProvider { +export abstract class DriverProvider { drivers_: WebDriver[]; config_: Config; + private bpRunner: BlockingProxyRunner; constructor(config: Config) { this.config_ = config; this.drivers_ = []; + this.bpRunner = new BlockingProxyRunner(config); } /** @@ -27,6 +30,10 @@ export class DriverProvider { return this.drivers_.slice(); // Create a shallow copy } + getBPUrl() { + return `http://localhost:${this.bpRunner.port}`; + } + /** * Create a new driver. * @@ -34,10 +41,17 @@ export class DriverProvider { * @return webdriver instance */ getNewDriver() { - let builder = new Builder() - .usingServer(this.config_.seleniumAddress) - .usingWebDriverProxy(this.config_.webDriverProxy) - .withCapabilities(this.config_.capabilities); + let builder: Builder; + if (this.config_.useBlockingProxy) { + builder = new Builder() + .usingServer(this.getBPUrl()) + .withCapabilities(this.config_.capabilities); + } else { + builder = new Builder() + .usingServer(this.config_.seleniumAddress) + .usingWebDriverProxy(this.config_.webDriverProxy) + .withCapabilities(this.config_.capabilities); + } if (this.config_.disableEnvironmentOverrides === true) { builder.disableEnvironmentOverrides(); } @@ -88,13 +102,23 @@ export class DriverProvider { }; /** - * Default setup environment method. - * @return a promise + * Default setup environment method, common to all driver providers. */ setupEnv(): q.Promise { - return q.fcall(function() {}); + let driverPromise = this.setupDriverEnv(); + if (this.config_.useBlockingProxy) { + // TODO(heathkit): If set, pass the webDriverProxy to BP. + return q.all([driverPromise, this.bpRunner.start()]); + } + return driverPromise; }; + /** + * Set up environment specific to a particular driver provider. Overridden + * by each driver provider. + */ + protected abstract setupDriverEnv(): q.Promise; + /** * Teardown and destroy the environment and do any associated cleanup. * Shuts down the drivers. diff --git a/lib/driverProviders/hosted.ts b/lib/driverProviders/hosted.ts index 388c229c4..f6778787a 100644 --- a/lib/driverProviders/hosted.ts +++ b/lib/driverProviders/hosted.ts @@ -22,7 +22,7 @@ export class Hosted extends DriverProvider { * @return {q.promise} A promise which will resolve when the environment is * ready to test. */ - setupEnv(): q.Promise { + protected setupDriverEnv(): q.Promise { logger.info('Using the selenium server at ' + this.config_.seleniumAddress); return q.fcall(function() {}); } diff --git a/lib/driverProviders/local.ts b/lib/driverProviders/local.ts index 3328cce5c..554a1de0d 100644 --- a/lib/driverProviders/local.ts +++ b/lib/driverProviders/local.ts @@ -77,7 +77,7 @@ export class Local extends DriverProvider { * @return {q.promise} A promise which will resolve when the environment is * ready to test. */ - setupEnv(): q.Promise { + protected setupDriverEnv(): q.Promise { let deferred = q.defer(); this.addDefaultBinaryLocs_(); diff --git a/lib/driverProviders/mock.ts b/lib/driverProviders/mock.ts index f4d02ebfd..b7a22d7d1 100644 --- a/lib/driverProviders/mock.ts +++ b/lib/driverProviders/mock.ts @@ -33,7 +33,7 @@ export class Mock extends DriverProvider { * @public * @return {q.promise} A promise which will resolve immediately. */ - setupEnv(): q.Promise { + protected setupDriverEnv(): q.Promise { return q.fcall(function() {}); } diff --git a/lib/driverProviders/sauce.ts b/lib/driverProviders/sauce.ts index c93e7f71c..8b4a24483 100644 --- a/lib/driverProviders/sauce.ts +++ b/lib/driverProviders/sauce.ts @@ -52,7 +52,7 @@ export class Sauce extends DriverProvider { * @return {q.promise} A promise which will resolve when the environment is * ready to test. */ - setupEnv(): q.Promise { + protected setupDriverEnv(): q.Promise { let deferred = q.defer(); this.sauceServer_ = new SauceLabs({ username: this.config_.sauceUser, diff --git a/lib/runner.ts b/lib/runner.ts index 2b45fe5b2..c720e0f69 100644 --- a/lib/runner.ts +++ b/lib/runner.ts @@ -186,8 +186,14 @@ export class Runner extends EventEmitter { let config = this.config_; let driver = this.driverprovider_.getNewDriver(); + let blockingProxyUrl: string; + if (config.useBlockingProxy) { + blockingProxyUrl = this.driverprovider_.getBPUrl(); + } + let browser_ = new ProtractorBrowser( - driver, config.baseUrl, config.rootElement, config.untrackOutstandingTimeouts); + driver, config.baseUrl, config.rootElement, config.untrackOutstandingTimeouts, + blockingProxyUrl); browser_.params = config.params; if (plugins) { diff --git a/package.json b/package.json index 4be7b6808..88af94eca 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,7 @@ "@types/q": "^0.0.32", "@types/selenium-webdriver": "2.53.38", "adm-zip": "0.4.7", + "blocking-proxy": "0.0.2", "chalk": "^1.1.3", "glob": "^7.0.3", "jasmine": "2.4.1", @@ -76,7 +77,7 @@ }, "license": "MIT", "engines": { - "node": ">=4.2.x" + "node": ">=6.9.x" }, "version": "4.0.14" } diff --git a/scripts/test.js b/scripts/test.js index 4213b1e7d..82a00b850 100755 --- a/scripts/test.js +++ b/scripts/test.js @@ -4,6 +4,7 @@ var Executor = require('./test/test_util').Executor; var passingTests = [ 'node built/cli.js spec/basicConf.js', + 'node built/cli.js spec/basicConf.js --useBlockingProxy', 'node built/cli.js spec/multiConf.js', 'node built/cli.js spec/altRootConf.js', 'node built/cli.js spec/onCleanUpAsyncReturnValueConf.js', diff --git a/spec/basic/polling_spec.js b/spec/basic/polling_spec.js index 376158229..80e05e253 100644 --- a/spec/basic/polling_spec.js +++ b/spec/basic/polling_spec.js @@ -30,8 +30,32 @@ describe('synchronizing with pages that poll', function() { }); }); + it('avoids timeouts using waitForAngularEnabled', function() { + var startButton = element(by.id('pollstarter')); + + var count = element(by.binding('count')); + expect(count.getText()).toEqual('0'); + + startButton.click(); + + // Turn this off to see timeouts. + browser.waitForAngularEnabled(false); + + expect(browser.waitForAngularEnabled()).toBeFalsy(); + + count.getText().then(function(text) { + expect(text).toBeGreaterThan(-1); + }); + + browser.sleep(2000); + + count.getText().then(function(text) { + expect(text).toBeGreaterThan(1); + }); + }); + afterEach(function() { - // Remember to turn it off when you're done! - browser.ignoreSynchronization = false; + // Remember to turn it back on when you're done! + browser.waitForAngularEnabled(true); }); }); diff --git a/spec/install/tsconfig.json b/spec/install/tsconfig.json index 366a1d730..c752f8b29 100644 --- a/spec/install/tsconfig.json +++ b/spec/install/tsconfig.json @@ -1,6 +1,6 @@ { "compilerOptions": { - "target": "es5", + "target": "es6", "module": "commonjs", "moduleResolution": "node", "sourceMap": false, diff --git a/tsconfig.json b/tsconfig.json index feba68aa3..f3889d4c1 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,9 +1,9 @@ { "compilerOptions": { - "target": "es5", + "target": "es6", "module": "commonjs", "moduleResolution": "node", - "sourceMap": false, + "sourceMap": true, "declaration": true, "removeComments": false, "noImplicitAny": true,