Add support for WD_CHROME_PATH.

README.md

@@ -86,8 +86,7 @@ Webdrivers.cache_time = 86_400 # Default: 86,400 Seconds (24 hours)
 ```
 
 Alternatively, you can define this value via the `WD_CACHE_TIME` environment
-variable, which takes precedence over the `Webdrivers.cache_time` value. 
-**Only set one to avoid confusion**.
+variable. **Only set one to avoid confusion**.
 
 ##### Special exception for chromedriver
 
@@ -157,8 +156,9 @@ require 'webdrivers'
 load 'webdrivers/Rakefile'
 ```
 
-These tasks respect the `WD_INSTALL_DIR` and `WD_CACHE_TIME` environment
-variables which can also be passed through the `rake` command:
+These tasks respect the `WD_INSTALL_DIR`, `WD_CACHE_TIME`, and
+`WD_CHROME_PATH` environment variables, which can also be passed 
+through the `rake` command:
 
 ```bash
 $ bundle exec rake webdrivers:chromedriver:update[2.46] webdrivers:geckodriver:update[0.24.0] WD_CACHE_TIME=86_400 WD_INSTALL_DIR='my_dir'
@@ -197,6 +197,9 @@ You can override this behavior by providing a path to the browser binary you wan
 Selenium::WebDriver::Chrome.path = '/chromium/install/path/to/binary'
 ```
 
+Alternatively, you can define the path via the `WD_CHROME_PATH` environment
+variable.
+
 This is also required if Google Chrome is not installed in its 
 [default location](https://github.com/SeleniumHQ/selenium/wiki/ChromeDriver).
 

lib/webdrivers/chrome_finder.rb

@@ -1,10 +1,12 @@
 # frozen_string_literal: true
 
 module Webdrivers
+  #
+  # @api private
+  #
   class ChromeFinder
     class << self
       def version
-        location = Selenium::WebDriver::Chrome.path || send("#{System.platform}_location")
         version = send("#{System.platform}_version", location)
 
         raise VersionError, 'Failed to find Chrome binary or its version.' if version.nil? || version.empty?
@@ -13,23 +15,33 @@ def version
         version[/\d+\.\d+\.\d+\.\d+/] # Google Chrome 73.0.3683.75 -> 73.0.3683.75
       end
 
-      def win_location
-        return Selenium::WebDriver::Chrome.path unless Selenium::WebDriver::Chrome.path.nil?
+      def location
+        user_defined_location || send("#{System.platform}_location")
+      end
+
+      private
+
+      def user_defined_location
+        if Selenium::WebDriver::Chrome.path
+          Webdrivers.logger.debug "Selenium::WebDriver::Chrome.path: #{Selenium::WebDriver::Chrome.path}"
+          return Selenium::WebDriver::Chrome.path
+        end
+
+        return if ENV['WD_CHROME_PATH'].nil?
 
+        Webdrivers.logger.debug "WD_CHROME_PATH: #{ENV['WD_CHROME_PATH']}"
+        ENV['WD_CHROME_PATH']
+      end
+
+      def win_location
         envs = %w[LOCALAPPDATA PROGRAMFILES PROGRAMFILES(X86)]
         directories = ['\\Google\\Chrome\\Application', '\\Chromium\\Application']
         file = 'chrome.exe'
 
         directories.each do |dir|
           envs.each do |root|
             option = "#{ENV[root]}\\#{dir}\\#{file}"
-            next unless File.exist?(option)
-
-            # Fix for JRuby on Windows - #41 and #130.
-            # Escape space and parenthesis with backticks.
-            option = option.gsub(/([\s()])/, '`\1') if RUBY_PLATFORM == 'java'
-
-            return System.escape_path(option)
+            return option if File.exist?(option)
           end
         end
       end
@@ -42,7 +54,7 @@ def mac_location
         directories.each do |dir|
           files.each do |file|
             option = "#{dir}/#{file}"
-            return System.escape_path(option) if File.exist?(option)
+            return option if File.exist?(option)
           end
         end
       end
@@ -54,7 +66,7 @@ def linux_location
         directories.each do |dir|
           files.each do |file|
             option = "#{dir}/#{file}"
-            return System.escape_path(option) if File.exist?(option)
+            return option if File.exist?(option)
           end
         end
       end
@@ -64,11 +76,11 @@ def win_version(location)
       end
 
       def linux_version(location)
-        System.call("#{location} --product-version")&.strip
+        System.call(location, '--product-version')&.strip
       end
 
       def mac_version(location)
-        System.call("#{location} --version")&.strip
+        System.call(location, '--version')&.strip
       end
     end
   end

lib/webdrivers/common.rb

@@ -19,6 +19,7 @@ class NetworkError < StandardError
   end
 
   DEFAULT_CACHE_TIME = 86_400 # 24 hours
+  DEFAULT_INSTALL_DIR = File.expand_path(File.join(ENV['HOME'], '.webdrivers'))
 
   class << self
     attr_accessor :proxy_addr, :proxy_port, :proxy_user, :proxy_pass
@@ -30,15 +31,15 @@ class << self
     # are set, it defaults to 86,400 Seconds (24 hours).
     #
     def cache_time
-      (ENV['WD_CACHE_TIME'] || @cache_time || DEFAULT_CACHE_TIME).to_i
+      (@cache_time || ENV['WD_CACHE_TIME'] || DEFAULT_CACHE_TIME).to_i
     end
 
     #
     # Returns the install (download) directory path for the drivers.
     #
     # @return [String]
     def install_dir
-      @install_dir || ENV['WD_INSTALL_DIR'] || File.expand_path(File.join(ENV['HOME'], '.webdrivers'))
+      @install_dir || ENV['WD_INSTALL_DIR'] || DEFAULT_INSTALL_DIR
     end
 
     def logger
@@ -109,7 +110,7 @@ def remove
       #
       # @return [String]
       def driver_path
-        System.escape_path File.join(System.install_dir, file_name)
+        File.absolute_path File.join(System.install_dir, file_name)
       end
 
       private
@@ -145,7 +146,7 @@ def normalize_version(version)
       end
 
       def binary_version
-        version = System.call("#{driver_path} --version")
+        version = System.call(driver_path, '--version')
         Webdrivers.logger.debug "Current version of #{driver_path} is #{version}"
         version
       rescue Errno::ENOENT

lib/webdrivers/system.rb

@@ -2,6 +2,7 @@
 
 require 'rubygems/package'
 require 'zip'
+require 'English'
 
 module Webdrivers
   #
@@ -145,15 +146,16 @@ def bitsize
         Selenium::WebDriver::Platform.bitsize
       end
 
-      def call(cmd)
+      def call(process, arg = nil)
+        cmd = arg ? [process, arg] : process # Windows provides powershell command (process) only, no args.
         Webdrivers.logger.debug "making System call: #{cmd}"
-        `#{cmd}`
-      end
-
-      def escape_path(path)
-        return path.tr('/', '\\') if platform == 'win' # Windows
+        p = IO.popen(cmd)
+        out = p.read
+        p.close
+        raise "Failed to make system call: #{cmd}" unless $CHILD_STATUS.success?
 
-        Shellwords.escape(path) # Linux and macOS
+        Webdrivers.logger.debug "System call returned: #{out}"
+        out
       end
     end
   end

lib/webdrivers/tasks/chromedriver.rake

@@ -19,8 +19,6 @@ namespace :webdrivers do
     desc 'Remove and download updated chromedriver if necessary'
     task :update, [:version] do |_, args|
       args.with_defaults(version: 0)
-      Webdrivers.cache_time                     = ENV.fetch('WD_CACHE_TIME', 86_400)
-      Webdrivers.install_dir                    = ENV.fetch('WD_INSTALL_DIR', nil)
       Webdrivers::Chromedriver.required_version = args.version
       Webdrivers::Chromedriver.update
       Webdrivers.logger.info "Updated to chromedriver #{Webdrivers::Chromedriver.current_version}"

lib/webdrivers/tasks/geckodriver.rake

@@ -19,8 +19,6 @@ namespace :webdrivers do
     desc 'Remove and download updated geckodriver if necessary'
     task :update, [:version] do |_, args|
       args.with_defaults(version: 0)
-      Webdrivers.cache_time                    = ENV.fetch('WD_CACHE_TIME', 86_400)
-      Webdrivers.install_dir                   = ENV.fetch('WD_INSTALL_DIR', nil)
       Webdrivers::Geckodriver.required_version = args.version
       Webdrivers::Geckodriver.update
       Webdrivers.logger.info "Updated to geckodriver #{Webdrivers::Geckodriver.current_version}"

lib/webdrivers/tasks/iedriver.rake

@@ -19,8 +19,6 @@ namespace :webdrivers do
     desc 'Remove and download updated IEDriverServer if necessary'
     task :update, [:version] do |_, args|
       args.with_defaults(version: 0)
-      Webdrivers.cache_time                 = ENV.fetch('WD_CACHE_TIME', 86_400)
-      Webdrivers.install_dir                = ENV.fetch('WD_INSTALL_DIR', nil)
       Webdrivers::IEdriver.required_version = args.version
       Webdrivers::IEdriver.update
       Webdrivers.logger.info "Updated to IEDriverServer #{Webdrivers::IEdriver.current_version}"

spec/webdrivers/chrome_finder_spec.rb

@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Webdrivers::ChromeFinder do
+  let(:chrome_finder) { described_class }
+
+  context 'when the user relies on the gem to figure out the location of Chrome' do
+    it 'determines the location correctly based on the current OS' do
+      expect(chrome_finder.location).not_to be_nil
+    end
+  end
+
+  context 'when the user provides a path to the Chrome binary' do
+    it 'uses Selenium::WebDriver::Chrome.path when it is defined' do
+      Selenium::WebDriver::Chrome.path = chrome_finder.location
+      allow(chrome_finder).to receive(:win_location)
+      allow(chrome_finder).to receive(:mac_location)
+      allow(chrome_finder).to receive(:linux_location)
+      expect(chrome_finder.version).not_to be_nil
+      expect(chrome_finder).not_to have_received(:win_location)
+      expect(chrome_finder).not_to have_received(:mac_location)
+      expect(chrome_finder).not_to have_received(:linux_location)
+    end
+
+    it "uses ENV['WD_CHROME_PATH'] when it is defined" do
+      allow(ENV).to receive(:[]).with('WD_CHROME_PATH').and_return(chrome_finder.location)
+      allow(chrome_finder).to receive(:win_location)
+      allow(chrome_finder).to receive(:mac_location)
+      allow(chrome_finder).to receive(:linux_location)
+      expect(chrome_finder.version).not_to be_nil
+      expect(chrome_finder).not_to have_received(:win_location)
+      expect(chrome_finder).not_to have_received(:mac_location)
+      expect(chrome_finder).not_to have_received(:linux_location)
+    end
+
+    it 'uses Selenium::WebDriver::Chrome.path over WD_CHROME_PATH' do
+      Selenium::WebDriver::Chrome.path = chrome_finder.location
+      allow(ENV).to receive(:[]).with('WD_CHROME_PATH').and_return('my_wd_chrome_path')
+      expect(chrome_finder.version).not_to be_nil
+      expect(ENV).not_to have_received(:[]).with('WD_CHROME_PATH')
+    end
+  end
+end

spec/webdrivers/chromedriver_spec.rb

@@ -146,7 +146,7 @@
     it 'returns a Gem::Version instance if binary is on the system' do
       allow(chromedriver).to receive(:exists?).and_return(true)
       allow(Webdrivers::System).to receive(:call)
-        .with("#{chromedriver.driver_path} --version")
+        .with(chromedriver.driver_path, '--version')
         .and_return '71.0.3578.137'
 
       expect(chromedriver.current_version).to eq Gem::Version.new('71.0.3578.137')
@@ -253,8 +253,19 @@
   describe '#driver_path' do
     it 'returns full location of binary' do
       expected_bin = "chromedriver#{'.exe' if Selenium::WebDriver::Platform.windows?}"
-      expected_path = Webdrivers::System.escape_path("#{File.join(ENV['HOME'])}/.webdrivers/#{expected_bin}")
+      expected_path = File.absolute_path "#{File.join(ENV['HOME'])}/.webdrivers/#{expected_bin}"
       expect(chromedriver.driver_path).to eq(expected_path)
     end
   end
+
+  describe '#chrome_version' do
+    it 'returns a Gem::Version object' do
+      expect(chromedriver.chrome_version).to be_an_instance_of(Gem::Version)
+    end
+
+    it 'returns currently installed Chrome version' do
+      allow(Webdrivers::ChromeFinder).to receive(:version).and_return('72.0.0.0')
+      expect(chromedriver.chrome_version).to be Gem::Version.new('72.0.0.0')
+    end
+  end
 end

spec/webdrivers/geckodriver_spec.rb

@@ -112,7 +112,7 @@
 This program is subject to the terms of the Mozilla Public License 2.0.
 You can obtain a copy of the license at https://mozilla.org/MPL/2.0/"
 
-      allow(Webdrivers::System).to receive(:call).with("#{geckodriver.driver_path} --version").and_return return_value
+      allow(Webdrivers::System).to receive(:call).with(geckodriver.driver_path, '--version').and_return return_value
 
       expect(geckodriver.current_version).to eq Gem::Version.new('0.24.0')
     end
@@ -205,7 +205,7 @@
   describe '#driver_path' do
     it 'returns full location of binary' do
       expected_bin = "geckodriver#{'.exe' if Selenium::WebDriver::Platform.windows?}"
-      expected_path = Webdrivers::System.escape_path("#{File.join(ENV['HOME'])}/.webdrivers/#{expected_bin}")
+      expected_path = File.absolute_path "#{File.join(ENV['HOME'])}/.webdrivers/#{expected_bin}"
       expect(geckodriver.driver_path).to eq(expected_path)
     end
   end

spec/webdrivers/i_edriver_spec.rb

@@ -87,7 +87,7 @@
 
       return_value = 'something IEDriverServer.exe 3.5.1 something else'
 
-      allow(Webdrivers::System).to receive(:call).with("#{iedriver.driver_path} --version").and_return return_value
+      allow(Webdrivers::System).to receive(:call).with(iedriver.driver_path, '--version').and_return return_value
 
       expect(iedriver.current_version).to eq Gem::Version.new('3.5.1')
     end
@@ -186,7 +186,7 @@
 
   describe '#driver_path' do
     it 'returns full location of binary' do
-      expected_path = Webdrivers::System.escape_path("#{File.join(ENV['HOME'])}/.webdrivers/IEDriverServer.exe")
+      expected_path = File.absolute_path "#{File.join(ENV['HOME'])}/.webdrivers/IEDriverServer.exe"
       expect(iedriver.driver_path).to eq(expected_path)
     end
   end

spec/webdrivers/webdrivers_spec.rb

@@ -29,15 +29,15 @@
     end
 
     context 'when ENV variable WD_CACHE_TIME is set' do
-      before { described_class.cache_time = 86_400 }
+      before { described_class.cache_time = 3600 }
 
-      it 'uses cache time value from ENV variable over the Webdrivers.cache_time value' do
-        allow(ENV).to receive(:[]).with('WD_CACHE_TIME').and_return(999)
-        expect(described_class.cache_time).to be(999)
+      it 'uses cache time value from Webdrivers.cache_time over the ENV variable value' do
+        allow(ENV).to receive(:[]).with('WD_CACHE_TIME').and_return(900)
+        expect(described_class.cache_time).to be(3600)
       end
 
       it 'returns cache time as an Integer' do
-        allow(ENV).to receive(:fetch).with('WD_CACHE_TIME', 86_400).and_return('999')
+        allow(ENV).to receive(:fetch).with('WD_CACHE_TIME', 3600).and_return('999')
         expect(described_class.cache_time).to be_an_instance_of(Integer)
       end
     end
@@ -70,5 +70,17 @@
         end
       end
     end
+
+    context 'when both ENV variable WD_INSTALL_DIR and Webdrivers.install_dir are set' do
+      it 'uses path from Webdrivers.install_dir' do
+        begin
+          described_class.install_dir = 'my_install_dir_path'
+          allow(ENV).to receive(:[]).with('WD_INSTALL_DIR').and_return('my_env_path')
+          expect(described_class.install_dir).to be('my_install_dir_path')
+        ensure
+          described_class.install_dir = nil
+        end
+      end
+    end
   end
 end