feat(blockingproxy): Add synchronization with BlockingProxy. (#3813)

heathkit · juliemr · commit ef2fe605729d · 2016-12-19T13:13:59.000-08:00
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)
diff --git a/lib/bpRunner.ts 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
@@ -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';
 
@@ -141,6 +142,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.
    *
@@ -186,9 +193,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`.
@@ -272,7 +296,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
@@ -291,6 +315,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);
@@ -325,6 +353,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
@@ -445,7 +489,7 @@ export class ProtractorBrowser extends Webdriver {
     }
 
     let runWaitForAngularScript: () => wdpromise.Promise<any> = () => {
-      if (this.plugins_.skipAngularStability()) {
+      if (this.plugins_.skipAngularStability() || this.bpClient) {
         return wdpromise.fulfilled();
       } else if (this.rootEl) {
         return this.executeAsyncScript_(
@@ -668,6 +712,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(() => {});
@@ -768,6 +818,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
@@ -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
@@ -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.<webdriver.WebDriver>} 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
@@ -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<any> {
+  protected setupDriverEnv(): q.Promise<any> {
     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
@@ -89,11 +89,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<any> {
+  protected setupDriverEnv(): q.Promise<any> {
     var 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
@@ -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<any> {
+  protected setupDriverEnv(): q.Promise<any> {
     switch (this.config_.capabilities.browserName) {
       case 'chrome':
         logger.info('Using ChromeDriver directly...');
diff --git a/lib/driverProviders/driverProvider.ts b/lib/driverProviders/driverProvider.ts
@@ -5,17 +5,20 @@
  */
 import * as q from 'q';
 
+import {BlockingProxyRunner} from '../bpRunner';
 import {Config} from '../config';
 
 let webdriver = require('selenium-webdriver');
 
-export class DriverProvider {
+export abstract class DriverProvider {
   drivers_: webdriver.WebDriver[];
   config_: Config;
+  private bpRunner: BlockingProxyRunner;
 
   constructor(config: Config) {
     this.config_ = config;
     this.drivers_ = [];
+    this.bpRunner = new BlockingProxyRunner(config);
   }
 
   /**
@@ -28,17 +31,28 @@ export class DriverProvider {
     return this.drivers_.slice();  // Create a shallow copy
   }
 
+  getBPUrl() {
+    return `http://localhost:${this.bpRunner.port}`;
+  }
+
   /**
    * Create a new driver.
    *
    * @public
    * @return webdriver instance
    */
   getNewDriver() {
-    let builder = new webdriver.Builder()
-                      .usingServer(this.config_.seleniumAddress)
-                      .usingWebDriverProxy(this.config_.webDriverProxy)
-                      .withCapabilities(this.config_.capabilities);
+    let builder: webdriver.Builder;
+    if (this.config_.useBlockingProxy) {
+      builder = new webdriver.Builder()
+                    .usingServer(this.getBPUrl())
+                    .withCapabilities(this.config_.capabilities);
+    } else {
+      builder = new webdriver.Builder()
+                    .usingServer(this.config_.seleniumAddress)
+                    .usingWebDriverProxy(this.config_.webDriverProxy)
+                    .withCapabilities(this.config_.capabilities);
+    }
     if (this.config_.disableEnvironmentOverrides === true) {
       builder.disableEnvironmentOverrides();
     }
@@ -89,13 +103,23 @@ export class DriverProvider {
   };
 
   /**
-   * Default setup environment method.
-   * @return a promise
+   * Default setup environment method, common to all driver providers.
    */
   setupEnv(): q.Promise<any> {
-    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<any>;
+
   /**
    * 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
@@ -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<any> {
+  protected setupDriverEnv(): q.Promise<any> {
     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
@@ -75,7 +75,7 @@ export class Local extends DriverProvider {
    * @return {q.promise} A promise which will resolve when the environment is
    *     ready to test.
    */
-  setupEnv(): q.Promise<any> {
+  protected setupDriverEnv(): q.Promise<any> {
     let deferred = q.defer();
 
     this.addDefaultBinaryLocs_();
diff --git a/lib/driverProviders/mock.ts b/lib/driverProviders/mock.ts
@@ -28,7 +28,7 @@ export class Mock extends DriverProvider {
    * @public
    * @return {q.promise} A promise which will resolve immediately.
    */
-  setupEnv(): q.Promise<any> {
+  protected setupDriverEnv(): q.Promise<any> {
     return q.fcall(function() {});
   }
 
diff --git a/lib/driverProviders/sauce.ts b/lib/driverProviders/sauce.ts
@@ -51,7 +51,7 @@ export class Sauce extends DriverProvider {
    * @return {q.promise} A promise which will resolve when the environment is
    *     ready to test.
    */
-  setupEnv(): q.Promise<any> {
+  protected setupDriverEnv(): q.Promise<any> {
     let deferred = q.defer();
     this.sauceServer_ = new SauceLabs({
       username: this.config_.sauceUser,
diff --git a/lib/runner.ts b/lib/runner.ts
@@ -206,8 +206,14 @@ export class Runner extends EventEmitter {
     var config = this.config_;
     var driver = this.driverprovider_.getNewDriver();
 
+    let blockingProxyUrl: string;
+    if (config.useBlockingProxy) {
+      blockingProxyUrl = this.driverprovider_.getBPUrl();
+    }
+
     var 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
diff --git a/scripts/test.js b/scripts/test.js
diff --git a/spec/basic/polling_spec.js b/spec/basic/polling_spec.js
diff --git a/spec/install/tsconfig.json b/spec/install/tsconfig.json
diff --git a/tsconfig.json b/tsconfig.json

Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@ export class Hosted extends DriverProvider {`
`22`	`22`	`* @return {q.promise} A promise which will resolve when the environment is`
`23`	`23`	`* ready to test.`
`24`	`24`	`*/`
`25`		`- setupEnv(): q.Promise<any> {`
	`25`	`+ protected setupDriverEnv(): q.Promise<any> {`
`26`	`26`	`logger.info('Using the selenium server at ' + this.config_.seleniumAddress);`
`27`	`27`	`return q.fcall(function() {});`
`28`	`28`	`}`
Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ export class Mock extends DriverProvider {`
`28`	`28`	`* @public`
`29`	`29`	`* @return {q.promise} A promise which will resolve immediately.`
`30`	`30`	`*/`
`31`		`- setupEnv(): q.Promise<any> {`
	`31`	`+ protected setupDriverEnv(): q.Promise<any> {`
`32`	`32`	`return q.fcall(function() {});`
`33`	`33`	`}`
`34`	`34`