chore(driverProviders): clean up driver provider q usage

- Remove driverProviderUseExistingWebDriver since the generation of the selenium server is already accomplished when providing a selenium address in driverProvider.ts. Also clean up docs and tests. - Use native promises over q promises.
angular · Nov 13, 2018 · 7ff7d2d · 7ff7d2d
1 parent 02b3cb9
commit 7ff7d2d
Show file tree

Hide file tree

Showing 18 changed files with 100 additions and 297 deletions.
diff --git a/docs/server-setup.md b/docs/server-setup.md
@@ -108,37 +108,3 @@ Protractor can test directly against Chrome and Firefox without using a Selenium
  - `directConnect: true` -  Your test script communicates directly Chrome Driver or Firefox Driver, bypassing any Selenium Server. If this is true, settings for `seleniumAddress` and `seleniumServerJar` will be ignored. If you attempt to use a browser other than Chrome or Firefox an error will be thrown.
 
 The advantage of directly connecting to browser drivers is that your test scripts may start up and run faster.
-
-Re-using an Existing WebDriver
-------------------------------
-
-The use case for re-using an existing WebDriver is when you have existing
-`selenium-webdriver` code and are already in control of how the WebDriver is
-created, but would also like Protractor to use the same browser, so you can
-use protractor's element locators and the rest of its API. This could be
-done with the `attachSession` driver provider, but the `attachSession` API is
-being removed in `selenium-webdriver` 4.0.0.
-
-Instead of a protractor config file, you create a config object in your test
-setup code, and add your already-created WebDriver object and base URL.
-
-```javascript
-const ProtractorConfigParser = require('protractor/built/configParser').ConfigParser;
-const ProtractorRunner = require('protractor/built/runner').Runner;
-
-const ptorConfig = new ProtractorConfigParser().config_;
-ptorConfig.baseUrl = myExistingBaseUrl;
-ptorConfig.seleniumWebDriver = myExistingWebDriver;
-ptorConfig.noGlobals = true; // local preference
-
-// looks similar to protractor/built/runner.js run()
-const ptorRunner = new ProtractorRunner(ptorConfig);
-ptorRunner.driverProvider_.setupEnv();
-const browser = ptorRunner.createBrowser();
-ptorRunner.setupGlobals_(browser); // now you can access protractor.$, etc.
-```
-
-Note that this driver provider leaves you in control of quitting the driver,
-but that also means Protractor API calls that expect the driver to properly
-quit and/or restart the browser, e.g. `restart`, `restartSync`, and
-`forkNewDriverInstance`, will not behave as documented.
diff --git a/lib/config.ts b/lib/config.ts
@@ -1,5 +1,3 @@
-import {WebDriver} from 'selenium-webdriver';
-
 import {PluginConfig} from './plugins';
 
 export interface Config {
@@ -230,12 +228,6 @@ export interface Config {
    */
   firefoxPath?: string;
 
-  // ---- 8. To re-use an existing WebDriver object ---------------------------
-
-  // This would not appear in a configuration file. Instead a configuration
-  // object would be created that includes an existing webdriver.
-  seleniumWebDriver?: WebDriver;
-
   // ---------------------------------------------------------------------------
   // ----- What tests to run ---------------------------------------------------
   // ---------------------------------------------------------------------------

diff --git a/lib/driverProviders/attachSession.ts b/lib/driverProviders/attachSession.ts
@@ -3,8 +3,7 @@
  *  It is responsible for setting up the account object, tearing
  *  it down, and setting up the driver correctly.
  */
-import * as q from 'q';
-import {promise as wdpromise, WebDriver} from 'selenium-webdriver';
+import {WebDriver} from 'selenium-webdriver';
 
 import {Config} from '../config';
 import {Logger} from '../logger';
@@ -25,10 +24,10 @@ export class AttachSession extends DriverProvider {
    * @return {q.promise} A promise which will resolve when the environment is
    *     ready to test.
    */
-  protected setupDriverEnv(): q.Promise<any> {
+  protected async setupDriverEnv(): Promise<any> {
     logger.info('Using the selenium server at ' + this.config_.seleniumAddress);
     logger.info('Using session id - ' + this.config_.seleniumSessionId);
-    return q(undefined);
+    return Promise.resolve();
   }
 
   /**
@@ -50,7 +49,7 @@ export class AttachSession extends DriverProvider {
    *
    * @public
    */
-  quitDriver(): wdpromise.Promise<void> {
-    return wdpromise.when(undefined);
+  quitDriver(): Promise<void> {
+    return Promise.resolve();
   }
 }
diff --git a/lib/driverProviders/browserStack.ts b/lib/driverProviders/browserStack.ts
@@ -3,8 +3,6 @@
  * It is responsible for setting up the account object, tearing
  * it down, and setting up the driver correctly.
  */
-import * as https from 'https';
-import * as q from 'q';
 import {Session, WebDriver} from 'selenium-webdriver';
 import * as util from 'util';
 
@@ -29,59 +27,54 @@ export class BrowserStack extends DriverProvider {
    * Hook to update the BrowserStack job status.
    * @public
    * @param {Object} update
-   * @return {q.promise} A promise that will resolve when the update is complete.
+   * @return {Promise} A promise that will resolve when the update is complete.
    */
-  updateJob(update: any): q.Promise<any> {
-    let deferredArray = this.drivers_.map((driver: WebDriver) => {
-      let deferred = q.defer();
+  async updateJob(update: any): Promise<any> {
+    let mappedDrivers = this.drivers_.map(async (driver: WebDriver) => {
+      let session = await driver.getSession();
 
-      driver.getSession().then((session: Session) => {
-
-        // Fetching BrowserStack session details.
-        this.browserstackClient.getSession(
-            session.getId(), function(error: Error, automate_session: any) {
-              if (error) {
+      // Fetching BrowserStack session details.
+      this.browserstackClient.getSession(
+          session.getId(), function(error: Error, automate_session: any) {
+            if (error) {
+              logger.info(
+                  'BrowserStack results available at ' +
+                  'https://www.browserstack.com/automate');
+            } else {
+              if (automate_session && automate_session.browser_url) {
+                logger.info('BrowserStack results available at ' + automate_session.browser_url);
+              } else {
                 logger.info(
                     'BrowserStack results available at ' +
                     'https://www.browserstack.com/automate');
-              } else {
-                if (automate_session && automate_session.browser_url) {
-                  logger.info('BrowserStack results available at ' + automate_session.browser_url);
-                } else {
-                  logger.info(
-                      'BrowserStack results available at ' +
-                      'https://www.browserstack.com/automate');
-                }
               }
-            });
+            }
+          });
 
-        let jobStatus = update.passed ? 'completed' : 'error';
-        let statusObj = {status: jobStatus};
+      let jobStatus = update.passed ? 'completed' : 'error';
+      let statusObj = {status: jobStatus};
 
-        // Updating status of BrowserStack session.
-        this.browserstackClient.updateSession(
-            session.getId(), statusObj, function(error: Error, automate_session: any) {
-              if (error) {
-                throw new BrowserError(
-                    logger, 'Error updating BrowserStack pass/fail status: ' + util.inspect(error));
-              } else {
-                logger.info(automate_session);
-                deferred.resolve();
-              }
-            });
-      });
-      return deferred.promise;
+      // Updating status of BrowserStack session.
+      this.browserstackClient.updateSession(
+          session.getId(), statusObj, function(error: Error, automate_session: any) {
+            if (error) {
+              throw new BrowserError(
+                  logger, 'Error updating BrowserStack pass/fail status: ' + util.inspect(error));
+            } else {
+              logger.info(automate_session);
+            }
+          });
     });
-    return q.all(deferredArray);
+
+    return Promise.all(mappedDrivers);
   }
 
   /**
    * Configure and launch (if applicable) the object's environment.
-   * @return {q.promise} A promise which will resolve when the environment is
+   * @return {promise} A promise which will resolve when the environment is
    *     ready to test.
    */
-  protected setupDriverEnv(): q.Promise<any> {
-    let deferred = q.defer();
+  protected async setupDriverEnv(): Promise<any> {
     this.config_.capabilities['browserstack.user'] = this.config_.browserstackUser;
     this.config_.capabilities['browserstack.key'] = this.config_.browserstackKey;
     this.config_.seleniumAddress = 'http://hub.browserstack.com/wd/hub';
@@ -99,8 +92,7 @@ export class BrowserStack extends DriverProvider {
           (':' + this.config_.specs.toString().replace(/^.*[\\\/]/, ''));
     }
 
-    logger.info('Using BrowserStack selenium server at ' + this.config_.seleniumAddress);
-    deferred.resolve();
-    return deferred.promise;
+    logger.info(`Using BrowserStack selenium server at ${this.config_.seleniumAddress}`);
+    return Promise.resolve();
   }
 }
diff --git a/lib/driverProviders/direct.ts b/lib/driverProviders/direct.ts
@@ -5,7 +5,6 @@
  */
 import * as fs from 'fs';
 import * as path from 'path';
-import * as q from 'q';
 import {Capabilities, WebDriver} from 'selenium-webdriver';
 import {Driver as ChromeDriver, ServiceBuilder as ChromeServiceBuilder} from 'selenium-webdriver/chrome';
 import {Driver as FirefoxDriver} from 'selenium-webdriver/firefox';
@@ -29,7 +28,7 @@ export class Direct extends DriverProvider {
    * @return {q.promise} A promise which will resolve when the environment is
    *     ready to test.
    */
-  protected setupDriverEnv(): q.Promise<any> {
+  protected async setupDriverEnv(): Promise<any> {
     switch (this.config_.capabilities.browserName) {
       case 'chrome':
         logger.info('Using ChromeDriver directly...');
@@ -43,7 +42,7 @@ export class Direct extends DriverProvider {
             'browserName ' + this.config_.capabilities.browserName +
                 ' is not supported with directConnect.');
     }
-    return q.fcall(function() {});
+    return Promise.resolve(function() {});
   }
 
   /**

diff --git a/lib/driverProviders/driverProvider.ts b/lib/driverProviders/driverProvider.ts
@@ -3,8 +3,7 @@
  *  It is responsible for setting up the account object, tearing
  *  it down, and setting up the driver correctly.
  */
-import * as q from 'q';
-import {Builder, promise as wdpromise, Session, WebDriver} from 'selenium-webdriver';
+import {Builder, Session, WebDriver} from 'selenium-webdriver';
 
 import {BlockingProxyRunner} from '../bpRunner';
 import {Config} from '../config';
@@ -68,22 +67,20 @@ export abstract class DriverProvider {
    * @public
    * @param webdriver instance
    */
-  quitDriver(driver: WebDriver): wdpromise.Promise<void> {
+  async quitDriver(driver: WebDriver): Promise<void> {
     let driverIndex = this.drivers_.indexOf(driver);
     if (driverIndex >= 0) {
       this.drivers_.splice(driverIndex, 1);
     }
 
-    if (driver.getSession() === undefined) {
-      return wdpromise.when(undefined);
-    } else {
-      return driver.getSession()
-          .then<void>((session_: Session) => {
-            if (session_) {
-              return driver.quit();
-            }
-          })
-          .catch<void>(function(err: Error) {});
+    try {
+      if (driver.getSession() !== undefined) {
+        const session = await driver.getSession();
+        if (session) {
+          return driver.quit();
+        }
+      }
+    } catch (_) {
     }
   }
 
@@ -93,56 +90,44 @@ export abstract class DriverProvider {
    *
    * @param drivers {webdriver.WebDriver[]} The webdriver instances
    */
-  static quitDrivers(provider: DriverProvider, drivers: WebDriver[]): q.Promise<void> {
-    let deferred = q.defer<void>();
-    wdpromise
-        .all(drivers.map((driver: WebDriver) => {
-          return provider.quitDriver(driver);
-        }))
-        .then(
-            () => {
-              deferred.resolve();
-            },
-            () => {
-              deferred.resolve();
-            });
-    return deferred.promise;
+  static async quitDrivers(provider: DriverProvider, drivers: WebDriver[]): Promise<void> {
+    await Promise.all(drivers.map(async (driver: WebDriver) => {
+      await provider.quitDriver(driver);
+    }));
   }
 
   /**
    * Default update job method.
    * @return a promise
    */
-  updateJob(update: any): q.Promise<any> {
-    return q.fcall(function() {});
+  updateJob(update: any): Promise<any> {
+    return Promise.resolve();
   };
 
   /**
    * Default setup environment method, common to all driver providers.
    */
-  setupEnv(): q.Promise<any> {
-    let driverPromise = this.setupDriverEnv();
+  async setupEnv(): Promise<any> {
+    await this.setupDriverEnv();
     if (this.config_.useBlockingProxy && !this.config_.blockingProxyUrl) {
-      // TODO(heathkit): If set, pass the webDriverProxy to BP.
-      return driverPromise.then(() => this.bpRunner.start());
+      await 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>;
+  protected async abstract setupDriverEnv(): Promise<any>;
 
   /**
    * Teardown and destroy the environment and do any associated cleanup.
    * Shuts down the drivers.
    *
    * @public
-   * @return {q.Promise<any>} A promise which will resolve when the environment is down.
+   * @return {Promise<any>} A promise which will resolve when the environment is down.
    */
-  teardownEnv(): q.Promise<any> {
-    return DriverProvider.quitDrivers(this, this.drivers_);
+  async teardownEnv(): Promise<any> {
+    await DriverProvider.quitDrivers(this, this.drivers_);
   }
 }
diff --git a/lib/driverProviders/hosted.ts b/lib/driverProviders/hosted.ts
@@ -3,8 +3,6 @@
  *  It is responsible for setting up the account object, tearing
  *  it down, and setting up the driver correctly.
  */
-import * as q from 'q';
-
 import {Config} from '../config';
 import {Logger} from '../logger';
 
@@ -19,11 +17,11 @@ export class Hosted extends DriverProvider {
   /**
    * Configure and launch (if applicable) the object's environment.
    * @public
-   * @return {q.promise} A promise which will resolve when the environment is
+   * @return {Promise} A promise which will resolve when the environment is
    *     ready to test.
    */
-  protected setupDriverEnv(): q.Promise<any> {
+  protected setupDriverEnv(): Promise<any> {
     logger.info('Using the selenium server at ' + this.config_.seleniumAddress);
-    return q.fcall(function() {});
+    return Promise.resolve();
   }
 }