[js] Making SeleniumManager a thin wrapper (#13853)

* [js] Making SeleniumManager a thin wrapper * [js] Fixing imports and js doc * [js] Removing unused import
SeleniumHQ · Apr 22, 2024 · ec54309 · ec54309
1 parent e85bf8d
commit ec54309
Show file tree

Hide file tree

Showing 9 changed files with 77 additions and 69 deletions.
diff --git a/javascript/node/selenium-webdriver/chromium.js b/javascript/node/selenium-webdriver/chromium.js
@@ -78,7 +78,7 @@ const error = require('./lib/error')
 const Symbols = require('./lib/symbols')
 const webdriver = require('./lib/webdriver')
 const remote = require('./remote')
-const { getPath } = require('./common/driverFinder')
+const { getBinaryPaths } = require('./common/driverFinder')
 
 /**
  * Custom command names supported by Chromium WebDriver.
@@ -622,14 +622,17 @@ class Driver extends webdriver.WebDriver {
     } else {
       let service = opt_serviceExecutor || this.getDefaultService()
       if (!service.getExecutable()) {
-        const { driverPath, browserPath } = getPath(caps)
+        const { driverPath, browserPath } = getBinaryPaths(caps)
         service.setExecutable(driverPath)
-        const vendorOptions = caps.get(vendorCapabilityKey)
-        if (vendorOptions) {
-          vendorOptions['binary'] = browserPath
-          caps.set(vendorCapabilityKey, vendorOptions)
-        } else {
-          caps.set(vendorCapabilityKey, { binary: browserPath })
+        if (browserPath) {
+          const vendorOptions = caps.get(vendorCapabilityKey)
+          if (vendorOptions) {
+            vendorOptions['binary'] = browserPath
+            caps.set(vendorCapabilityKey, vendorOptions)
+          } else {
+            caps.set(vendorCapabilityKey, { binary: browserPath })
+          }
+          caps.delete(Capability.BROWSER_VERSION)
         }
       }
       onQuit = () => service.kill()
@@ -743,7 +746,7 @@ class Driver extends webdriver.WebDriver {
    * Set a permission state to the given value.
    *
    * @param {string} name A name of the permission to update.
-   * @param {('granted'|'denied'|'prompt')} state State to set permission to.
+   * @param {("granted"|"denied"|"prompt")} state State to set permission to.
    * @returns {!Promise<Object>} A promise that will be resolved when the
    *     command has finished.
    * @see <https://w3c.github.io/permissions/#permission-registry> for valid

diff --git a/javascript/node/selenium-webdriver/common/driverFinder.js b/javascript/node/selenium-webdriver/common/driverFinder.js
@@ -21,16 +21,19 @@
  *  Utility to find if a given file is present and executable.
  */
 
-const { driverLocation } = require('./seleniumManager')
+const path = require('node:path')
+const { binaryPaths } = require('./seleniumManager')
 
 /**
  * Determines the path of the correct Selenium Manager binary
+ * @param {Capabilities} capabilities browser options to fetch the driver
  * @returns {{browserPath: string, driverPath: string}} path of the driver
  * and browser location
  */
-function getPath(capabilities) {
+function getBinaryPaths(capabilities) {
   try {
-    return driverLocation(capabilities)
+    const args = getArgs(capabilities)
+    return binaryPaths(args)
   } catch (e) {
     throw Error(
       `Unable to obtain browser driver.
@@ -40,5 +43,34 @@ function getPath(capabilities) {
   }
 }
 
+function getArgs(options) {
+  let args = ['--browser', options.getBrowserName(), '--language-binding', 'javascript', '--output', 'json']
+
+  if (options.getBrowserVersion() && options.getBrowserVersion() !== '') {
+    args.push('--browser-version', options.getBrowserVersion())
+  }
+
+  const vendorOptions =
+    options.get('goog:chromeOptions') || options.get('ms:edgeOptions') || options.get('moz:firefoxOptions')
+  if (vendorOptions && vendorOptions.binary && vendorOptions.binary !== '') {
+    args.push('--browser-path', path.resolve(vendorOptions.binary))
+  }
+
+  const proxyOptions = options.getProxy()
+
+  // Check if proxyOptions exists and has properties
+  if (proxyOptions && Object.keys(proxyOptions).length > 0) {
+    const httpProxy = proxyOptions['httpProxy']
+    const sslProxy = proxyOptions['sslProxy']
+
+    if (httpProxy !== undefined) {
+      args.push('--proxy', httpProxy)
+    } else if (sslProxy !== undefined) {
+      args.push('--proxy', sslProxy)
+    }
+  }
+  return args
+}
+
 // PUBLIC API
-module.exports = { getPath }
+module.exports = { getBinaryPaths }
diff --git a/javascript/node/selenium-webdriver/common/seleniumManager.js b/javascript/node/selenium-webdriver/common/seleniumManager.js
@@ -25,7 +25,6 @@ const { platform } = require('node:process')
 const path = require('node:path')
 const fs = require('node:fs')
 const spawnSync = require('node:child_process').spawnSync
-const { Capability } = require('../lib/capabilities')
 const logging = require('../lib/logging')
 
 const log_ = logging.getLogger(logging.Type.DRIVER)
@@ -63,38 +62,12 @@ function getBinary() {
 
 /**
  * Determines the path of the correct driver
- * @param {Capabilities} options browser options to fetch the driver
+ * @param {string[]} args arguments to invoke Selenium Manager
  * @returns {{browserPath: string, driverPath: string}} path of the driver and
  * browser location
  */
 
-function driverLocation(options) {
-  let args = ['--browser', options.getBrowserName(), '--language-binding', 'javascript', '--output', 'json']
-
-  if (options.getBrowserVersion() && options.getBrowserVersion() !== '') {
-    args.push('--browser-version', options.getBrowserVersion())
-  }
-
-  const vendorOptions =
-    options.get('goog:chromeOptions') || options.get('ms:edgeOptions') || options.get('moz:firefoxOptions')
-  if (vendorOptions && vendorOptions.binary && vendorOptions.binary !== '') {
-    args.push('--browser-path', path.resolve(vendorOptions.binary))
-  }
-
-  const proxyOptions = options.getProxy()
-
-  // Check if proxyOptions exists and has properties
-  if (proxyOptions && Object.keys(proxyOptions).length > 0) {
-    const httpProxy = proxyOptions['httpProxy']
-    const sslProxy = proxyOptions['sslProxy']
-
-    if (httpProxy !== undefined) {
-      args.push('--proxy', httpProxy)
-    } else if (sslProxy !== undefined) {
-      args.push('--proxy', sslProxy)
-    }
-  }
-
+function binaryPaths(args) {
   const smBinary = getBinary()
   const spawnResult = spawnSync(smBinary, args)
   let output
@@ -120,11 +93,6 @@ function driverLocation(options) {
     throw new Error(`Error executing command for ${smBinary} with ${args}: ${e.toString()}`)
   }
 
-  // Once driverPath is available, delete browserVersion from payload
-  if (output.result.driver_path) {
-    options.delete(Capability.BROWSER_VERSION)
-  }
-
   logOutput(output)
   return {
     driverPath: output.result.driver_path,
@@ -144,4 +112,4 @@ function logOutput(output) {
 }
 
 // PUBLIC API
-module.exports = { driverLocation }
+module.exports = { binaryPaths }
diff --git a/javascript/node/selenium-webdriver/firefox.js b/javascript/node/selenium-webdriver/firefox.js
@@ -116,9 +116,9 @@ const io = require('./io')
 const remote = require('./remote')
 const webdriver = require('./lib/webdriver')
 const zip = require('./io/zip')
-const { Browser, Capabilities } = require('./lib/capabilities')
+const { Browser, Capabilities, Capability } = require('./lib/capabilities')
 const { Zip } = require('./io/zip')
-const { getPath } = require('./common/driverFinder')
+const { getBinaryPaths } = require('./common/driverFinder')
 const FIREFOX_CAPABILITY_KEY = 'moz:firefoxOptions'
 
 /**
@@ -541,7 +541,7 @@ class Driver extends webdriver.WebDriver {
       configureExecutor(executor)
     } else if (opt_executor instanceof remote.DriverService) {
       if (!opt_executor.getExecutable()) {
-        const { driverPath, browserPath } = getPath(caps)
+        const { driverPath, browserPath } = getBinaryPaths(caps)
         opt_executor.setExecutable(driverPath)
         firefoxBrowserPath = browserPath
       }
@@ -550,7 +550,7 @@ class Driver extends webdriver.WebDriver {
     } else {
       let service = new ServiceBuilder().build()
       if (!service.getExecutable()) {
-        const { driverPath, browserPath } = getPath(caps)
+        const { driverPath, browserPath } = getBinaryPaths(caps)
         service.setExecutable(driverPath)
         firefoxBrowserPath = browserPath
       }
@@ -566,6 +566,7 @@ class Driver extends webdriver.WebDriver {
       } else {
         caps.set(FIREFOX_CAPABILITY_KEY, { binary: firefoxBrowserPath })
       }
+      caps.delete(Capability.BROWSER_VERSION)
     }
 
     return /** @type {!Driver} */ (super.createSession(executor, caps, onQuit))

diff --git a/javascript/node/selenium-webdriver/ie.js b/javascript/node/selenium-webdriver/ie.js
@@ -33,7 +33,7 @@ const remote = require('./remote')
 const webdriver = require('./lib/webdriver')
 const { Browser, Capabilities } = require('./lib/capabilities')
 const error = require('./lib/error')
-const { getPath } = require('./common/driverFinder')
+const { getBinaryPaths } = require('./common/driverFinder')
 
 const OPTIONS_CAPABILITY_KEY = 'se:ieOptions'
 const SCROLL_BEHAVIOUR = {
@@ -223,7 +223,9 @@ class Options extends Capabilities {
 
   addBrowserCommandSwitches(...args) {
     let current = this.options_[Key.BROWSER_COMMAND_LINE_SWITCHES] || []
-    if (typeof current == 'string') current = current.split(' ')
+    if (typeof current == 'string') {
+      current = current.split(' ')
+    }
     this.options_[Key.BROWSER_COMMAND_LINE_SWITCHES] = current.concat(args).join(' ')
     return this
   }
@@ -239,7 +241,9 @@ class Options extends Capabilities {
 
   addArguments(...args) {
     let current = this.options_[Key.BROWSER_COMMAND_LINE_SWITCHES] || []
-    if (typeof current == 'string') current = current.split(' ')
+    if (typeof current == 'string') {
+      current = current.split(' ')
+    }
     this.options_[Key.BROWSER_COMMAND_LINE_SWITCHES] = current.concat(args).join(' ')
     return this
   }
@@ -447,7 +451,7 @@ class Driver extends webdriver.WebDriver {
       service = createServiceFromCapabilities(options)
     }
     if (!service.getExecutable()) {
-      service.setExecutable(getPath(options).driverPath)
+      service.setExecutable(getBinaryPaths(options).driverPath)
     }
 
     let client = service.start().then((url) => new http.HttpClient(url))

diff --git a/javascript/node/selenium-webdriver/safari.js b/javascript/node/selenium-webdriver/safari.js
@@ -25,10 +25,10 @@ const http = require('./http')
 const remote = require('./remote')
 const webdriver = require('./lib/webdriver')
 const { Browser, Capabilities } = require('./lib/capabilities')
-const { getPath } = require('./common/driverFinder')
+const { getBinaryPaths } = require('./common/driverFinder')
 
 /**
- * Creates {@link selenium-webdriver/remote.DriverService} instances that manage
+ * Creates {@link remote.DriverService} instances that manage
  * a [safaridriver] server in a child process.
  *
  * [safaridriver]: https://developer.apple.com/library/prerelease/content/releasenotes/General/WhatsNewInSafari/Articles/Safari_10_0.html#//apple_ref/doc/uid/TP40014305-CH11-DontLinkElementID_28
@@ -123,7 +123,7 @@ class Driver extends webdriver.WebDriver {
 
     let service = new ServiceBuilder(exe).build()
     if (!service.getExecutable()) {
-      service.setExecutable(getPath(caps).driverPath)
+      service.setExecutable(getBinaryPaths(caps).driverPath)
     }
     let executor = new http.Executor(service.start().then((url) => new http.HttpClient(url)))
 

diff --git a/javascript/node/selenium-webdriver/test/chrome/service_test.js b/javascript/node/selenium-webdriver/test/chrome/service_test.js
@@ -20,7 +20,7 @@
 const assert = require('node:assert')
 const chrome = require('../../chrome')
 const test = require('../../lib/test')
-const { getPath } = require('../../common/driverFinder')
+const { getBinaryPaths } = require('../../common/driverFinder')
 
 test.suite(
   function (_env) {
@@ -35,7 +35,7 @@ test.suite(
       it('can be started on a custom path', function () {
         service = new chrome.ServiceBuilder().setPath('/foo/bar/baz').build()
         if (!service.getExecutable()) {
-          service.setExecutable(getPath(new chrome.Options()).driverPath)
+          service.setExecutable(getBinaryPaths(new chrome.Options()).driverPath)
         }
         return service.start().then(function (url) {
           assert.ok(url.endsWith('/foo/bar/baz'), 'unexpected url: ' + url)

diff --git a/javascript/node/selenium-webdriver/test/edge/service_test.js b/javascript/node/selenium-webdriver/test/edge/service_test.js
@@ -20,7 +20,7 @@
 const assert = require('node:assert')
 const edge = require('../../edge')
 const test = require('../../lib/test')
-const { getPath } = require('../../common/driverFinder')
+const { getBinaryPaths } = require('../../common/driverFinder')
 
 test.suite(
   function (_env) {
@@ -35,7 +35,7 @@ test.suite(
 
       it('can start msedgedriver', async function () {
         service = new edge.ServiceBuilder().build()
-        service.setExecutable(getPath(new edge.Options()).driverPath)
+        service.setExecutable(getBinaryPaths(new edge.Options()).driverPath)
         let url = await service.start()
         assert(/127\.0\.0\.1/.test(url), `unexpected url: ${url}`)
       })

diff --git a/javascript/node/selenium-webdriver/testing/index.js b/javascript/node/selenium-webdriver/testing/index.js
@@ -41,7 +41,7 @@ const remote = require('../remote')
 const safari = require('../safari')
 const { Browser } = require('../lib/capabilities')
 const { Builder } = require('../index')
-const { getPath } = require('../common/driverFinder')
+const { getBinaryPaths } = require('../common/driverFinder')
 
 /**
  * Describes a browser targeted by a {@linkplain suite test suite}.
@@ -122,15 +122,15 @@ function getAvailableBrowsers() {
   info(`Searching for WebDriver executables installed on the current system...`)
 
   let targets = [
-    [getPath(new chrome.Options()), Browser.CHROME],
-    [getPath(new edge.Options()), Browser.EDGE],
-    [getPath(new firefox.Options()), Browser.FIREFOX],
+    [getBinaryPaths(new chrome.Options()), Browser.CHROME],
+    [getBinaryPaths(new edge.Options()), Browser.EDGE],
+    [getBinaryPaths(new firefox.Options()), Browser.FIREFOX],
   ]
   if (process.platform === 'win32') {
-    targets.push([getPath(new ie.Options()), Browser.INTERNET_EXPLORER])
+    targets.push([getBinaryPaths(new ie.Options()), Browser.INTERNET_EXPLORER])
   }
   if (process.platform === 'darwin') {
-    targets.push([getPath(new safari.Options()), Browser.SAFARI])
+    targets.push([getBinaryPaths(new safari.Options()), Browser.SAFARI])
   }
 
   let availableBrowsers = []