From f4cf27734a90d073eeca4f0893364be7f9cab600 Mon Sep 17 00:00:00 2001
From: Sammy Jelin <sjelin@gmail.com>
Date: Fri, 20 Jan 2017 14:52:01 -0800
Subject: [PATCH] chore(quitDriver): have quitDriver return a webdriver promise
 directly (#3992)

Wrapping it in a `q` promise is blocking https://github.com/angular/protractor/issues/3899

Closes https://github.com/angular/protractor/issues/3902

Custom frameworks might not make this change but it'll be fine.  It'll only be a
problem in edge cases and they probably weren't returning the right promise
before anyway.
---
 lib/driverProviders/README.md         |  4 +--
 lib/driverProviders/attachSession.ts  |  8 ++---
 lib/driverProviders/driverProvider.ts | 51 ++++++++++++++++-----------
 lib/runner.ts                         |  7 ++--
 4 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/lib/driverProviders/README.md b/lib/driverProviders/README.md
index 2cc0919a1..690b55ecd 100644
--- a/lib/driverProviders/README.md
+++ b/lib/driverProviders/README.md
@@ -25,12 +25,12 @@ DriverProvider.prototype.getNewDriver
 
 /**
  * @param {webdriver.WebDriver} The driver instance to quit.
+ * @return {webdriver.promise.Promise<void>} A promise which resolves when the instance has quit
  */
 DriverProvider.prototype.quitDriver
 
 /**
- * @return {q.promise} A promise which will resolve when the environment
- *     is down.
+ * @return {q.Promise<any>} A promise which will resolve when the environment is down.
  */
 DriverProvider.prototype.teardownEnv
 
diff --git a/lib/driverProviders/attachSession.ts b/lib/driverProviders/attachSession.ts
index ba3597a9d..dd342a77e 100644
--- a/lib/driverProviders/attachSession.ts
+++ b/lib/driverProviders/attachSession.ts
@@ -4,7 +4,7 @@
  *  it down, and setting up the driver correctly.
  */
 import * as q from 'q';
-import {WebDriver} from 'selenium-webdriver';
+import {promise as wdpromise, WebDriver} from 'selenium-webdriver';
 
 import {Config} from '../config';
 import {Logger} from '../logger';
@@ -50,9 +50,7 @@ export class AttachSession extends DriverProvider {
    *
    * @public
    */
-  quitDriver(): q.Promise<WebDriver> {
-    let defer = q.defer<WebDriver>();
-    defer.resolve(null);
-    return defer.promise;
+  quitDriver(): wdpromise.Promise<void> {
+    return wdpromise.when(undefined);
   }
 }
diff --git a/lib/driverProviders/driverProvider.ts b/lib/driverProviders/driverProvider.ts
index 9926b8a1d..f6655946f 100644
--- a/lib/driverProviders/driverProvider.ts
+++ b/lib/driverProviders/driverProvider.ts
@@ -4,7 +4,7 @@
  *  it down, and setting up the driver correctly.
  */
 import * as q from 'q';
-import {Builder, Session, WebDriver} from 'selenium-webdriver';
+import {Builder, promise as wdpromise, Session, WebDriver} from 'selenium-webdriver';
 
 import {BlockingProxyRunner} from '../bpRunner';
 import {Config} from '../config';
@@ -68,30 +68,44 @@ export abstract class DriverProvider {
    * @public
    * @param webdriver instance
    */
-  quitDriver(driver: WebDriver): q.Promise<WebDriver> {
+  quitDriver(driver: WebDriver): wdpromise.Promise<void> {
     let driverIndex = this.drivers_.indexOf(driver);
     if (driverIndex >= 0) {
       this.drivers_.splice(driverIndex, 1);
     }
 
-    let deferred = q.defer<WebDriver>();
     if (driver.getSession() === undefined) {
-      deferred.resolve();
+      return wdpromise.when(undefined);
     } else {
-      driver.getSession()
-          .then((session_: Session) => {
+      return driver.getSession()
+          .then<void>((session_: Session) => {
             if (session_) {
-              driver.quit().then(function() {
-                deferred.resolve();
-              });
-            } else {
-              deferred.resolve();
+              return driver.quit();
             }
           })
-          .catch((err: Error) => {
-            deferred.resolve();
-          });
+          .catch<void>(function(err: Error) {});
     }
+  }
+
+
+  /**
+   * Quits an array of drivers and returns a q promise instead of a webdriver one
+   *
+   * @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;
   }
 
@@ -126,12 +140,9 @@ export abstract class DriverProvider {
    * Shuts down the drivers.
    *
    * @public
-   * @return {q.promise} A promise which will resolve when the environment
-   *     is down.
+   * @return {q.Promise<any>} A promise which will resolve when the environment is down.
    */
-  teardownEnv(): q.Promise<q.Promise<WebDriver>[]> {
-    return q.all<any>(this.drivers_.map((driver: WebDriver) => {
-      return this.quitDriver(driver);
-    }));
+  teardownEnv(): q.Promise<any> {
+    return DriverProvider.quitDrivers(this, this.drivers_);
   }
 }
diff --git a/lib/runner.ts b/lib/runner.ts
index d38715b06..e4015b2eb 100644
--- a/lib/runner.ts
+++ b/lib/runner.ts
@@ -282,10 +282,9 @@ export class Runner extends EventEmitter {
    * @return {q.Promise} A promise which resolves on finish.
    * @private
    */
-  shutdown_(): q.Promise<any> {
-    return q.all(this.driverprovider_.getExistingDrivers().map((webdriver) => {
-      return this.driverprovider_.quitDriver(webdriver);
-    }));
+  shutdown_(): q.Promise<void> {
+    return DriverProvider.quitDrivers(
+        this.driverprovider_, this.driverprovider_.getExistingDrivers());
   }
 
   /**