rb - update w3c implementation for latest Marionette release

Author: titusfortner

rb/lib/selenium/webdriver/common/options.rb

@@ -50,10 +50,9 @@ def add_cookie(opts = {})
         opts[:secure] ||= false
 
         if obj = opts.delete(:expires)
-          opts[:expiry] = seconds_from(obj)
+          opts[:expiry] = seconds_from(obj).to_i
         end
 
-
         @bridge.addCookie opts
       end
 

rb/lib/selenium/webdriver/firefox/w3c_bridge.rb

@@ -25,41 +25,14 @@ module Firefox
       class W3CBridge < Remote::W3CBridge
 
         def initialize(opts = {})
-          http_client = opts.delete(:http_client)
+          caps = opts[:desired_capabilities] ||= Remote::W3CCapabilities.firefox
+          Binary.path = caps[:firefox_binary] if caps[:firefox_binary]
 
-          opts.delete(:marionette)
-          caps = opts.delete(:desired_capabilities) { Remote::W3CCapabilities.firefox }
+          @service = Service.default_service(*extract_service_args(opts))
+          @service.start
+          opts[:url] = @service.uri
 
-          if opts.has_key?(:url)
-            url = opts.delete(:url)
-          else
-            Binary.path = caps[:firefox_binary] if caps[:firefox_binary]
-            if Firefox::Binary.version < 43
-              raise ArgumentError, "Firefox Version #{Firefox::Binary.version} does not support Marionette; Set firefox_binary in Capabilities to point to a supported binary"
-            end
-
-            @service = Service.default_service(*extract_service_args(opts))
-
-            if @service.instance_variable_get("@host") == "127.0.0.1"
-              @service.instance_variable_set("@host", 'localhost')
-            end
-
-            @service.start
-
-            url = @service.uri
-          end
-
-          unless opts.empty?
-            raise ArgumentError, "unknown option#{'s' if opts.size != 1}: #{opts.inspect}"
-          end
-
-          remote_opts = {
-            :url                  => url,
-            :desired_capabilities => caps
-          }
-
-          remote_opts.merge!(:http_client => http_client) if http_client
-          super(remote_opts)
+          super
         end
 
         def browser
@@ -83,13 +56,8 @@ def quit
         private
 
         def extract_service_args(opts)
-          args = []
-
-          if opts.has_key?(:service_log_path)
-            args << "--log-path=#{opts.delete(:service_log_path)}"
-          end
-
-          args
+          service_log_path = opts.delete(:service_log_path)
+          service_log_path ? ["--log-path=#{service_log_path}"] : []
         end
 
       end # W3CBridge

rb/lib/selenium/webdriver/remote/w3c_bridge.rb

@@ -62,7 +62,7 @@ def self.command(name, verb, url)
         #
 
         def initialize(opts = {})
-          if opts.key?(:desired_capabilities) && opts[:desired_capabilities][:browser_name] == 'MicrosoftEdge'
+          if opts.fetch(:desired_capabilities, {})[:browser_name] == 'MicrosoftEdge'
             require_relative '../edge/legacy_support'
             extend Edge::LegacySupport
           end
@@ -73,35 +73,25 @@ def initialize(opts = {})
           desired_capabilities = opts.delete(:desired_capabilities) { W3CCapabilities.firefox }
           url                  = opts.delete(:url) { "http://#{Platform.localhost}:4444/wd/hub" }
 
-          if opts.delete(:marionette) && !desired_capabilities.is_a?(W3CCapabilities)
-            caps = JSON.parse(desired_capabilities.to_json)
-            desired_capabilities = W3CCapabilities.firefox(caps.merge(:marionette => true))
+          desired_capabilities = W3CCapabilities.send(desired_capabilities) if desired_capabilities.is_a? Symbol
+
+          desired_capabilities[:marionette] = opts.delete(:marionette) unless opts[:marionette].nil?
+
+          if desired_capabilities[:marionette] && Firefox::Binary.version < 45
+              raise Error::WebDriverError, "Marionette is not supported in Firefox Version #{Firefox::Binary.version}"
           end
 
           unless opts.empty?
             raise ArgumentError, "unknown option#{'s' if opts.size != 1}: #{opts.inspect}"
           end
 
-          if desired_capabilities.kind_of?(Symbol)
-            unless W3CCapabilities.respond_to?(desired_capabilities)
-              raise Error::WebDriverError, "invalid desired capability: #{desired_capabilities.inspect}"
-            end
-
-            desired_capabilities = W3CCapabilities.send(desired_capabilities)
-          end
-
           uri = url.kind_of?(URI) ? url : URI.parse(url)
           uri.path += "/" unless uri.path =~ /\/$/
 
           http_client.server_url = uri
 
           @http          = http_client
           @capabilities  = create_session(desired_capabilities)
-
-          if @capabilities[:browser_name] == 'firefox' && @capabilities[:browser_version].to_i < 43
-            raise ArgumentError, "Server configuration does not support Marionette; start server with flag to Marionette binary -Dwebdriver.firefox.bin=/path/to/bin"
-          end
-
           @file_detector = nil
         end
 

rb/lib/selenium/webdriver/remote/w3c_capabilities.rb

@@ -87,21 +87,25 @@ def w3c?(opts = {})
           def json_create(data)
             data = data.dup
 
+            # Convert due to Remote Driver implementation
+            data["browserVersion"] = data.delete("version") if data["version"]
+            data["platformName"] = data.delete("platform") if data["platform"]
+
             caps = new
-            caps.browser_name = data.delete("browserName")
-            caps.browser_version = data.delete("browserVersion")
-            caps.platform_name = data.delete("platformName")
-            caps.platform_version = data.delete("platformVersion")
-            caps.accept_ssl_certs = data.delete("acceptSslCerts")
-            caps.takes_screenshot = data.delete("takesScreenshot")
-            caps.takes_element_screenshot = data.delete("takesElementScreenshot")
-            caps.page_load_strategy = data.delete("pageLoadStrategy")
-            caps.proxy = Proxy.json_create(data['proxy']) if data.has_key?('proxy')
+            caps.browser_name = data.delete("browserName") if data["browserName"]
+            caps.browser_version = data.delete("browserVersion") if data["browserVersion"]
+            caps.platform_name = data.delete("platformName") if data["platformName"]
+            caps.platform_version = data.delete("platformVersion") if data["platformVersion"]
+            caps.accept_ssl_certs = data.delete("acceptSslCerts") if data["acceptSslCerts"]
+            caps.takes_screenshot = data.delete("takesScreenshot") if data["takesScreenshot"]
+            caps.takes_element_screenshot = data.delete("takesElementScreenshot") if data["takesElementScreenshot"]
+            caps.page_load_strategy = data.delete("pageLoadStrategy") if data["pageloadStrategy"]
+            caps.proxy = Proxy.json_create(data['proxy']) if data['proxy']
 
             # Remote Server Specific
             caps[:remote_session_id] = data.delete('webdriver.remote.sessionid')
 
-            # obsolete capabilities returned by Remote Server
+            # Obsolete capabilities returned by Remote Server
             data.delete("javascriptEnabled")
             data.delete('cssSelectorsEnabled')
 

rb/spec/integration/selenium/webdriver/driver_spec.rb

@@ -282,7 +282,7 @@
       end
 
       # Edge BUG - https://connect.microsoft.com/IE/feedback/details/1849991/
-      not_compliant_on({:browser => [:edge, :marionette, :edge]},
+      not_compliant_on({:browser => [:edge, :marionette]},
                        {:driver => :remote, :browser => :phantomjs}) do
         it "times out if the callback is not invoked" do
           expect {

rb/spec/integration/selenium/webdriver/firefox/driver_spec.rb

@@ -27,8 +27,8 @@ module Firefox
         describe Driver do
           describe ".new" do
 
-            it "should take a binary path as an argument" do
-              pending unless ENV['MARIONETTE_PATH']
+            it "takes a binary path as an argument" do
+              pending "Set ENV['ALTERNATE_FIREFOX_PATH'] to test this" unless ENV['ALTERNATE_FIREFOX_PATH']
 
               begin
                 default_path = Firefox::Binary.path
@@ -37,7 +37,7 @@ module Firefox
                 default_version = driver1.capabilities[:version]
                 driver1.quit
 
-                caps = Remote::Capabilities.firefox(firefox_binary: ENV['MARIONETTE_PATH'])
+                caps = Remote::Capabilities.firefox(firefox_binary: ENV['ALTERNATE_FIREFOX_PATH'])
                 driver2 = Selenium::WebDriver.for :firefox, :desired_capabilities => caps
 
                 expect(driver2.capabilities[:version]).to_not be == default_version
@@ -47,7 +47,7 @@ module Firefox
               end
             end
 
-            it "should take a Firefox::Profile instance as argument" do
+            it "takes a Firefox::Profile instance as argument" do
               begin
                 profile = Selenium::WebDriver::Firefox::Profile.new
                 driver = Selenium::WebDriver.for :firefox, :profile => profile
@@ -60,8 +60,8 @@ module Firefox
           end
 
           it_behaves_like "driver that can be started concurrently", :firefox
-        end
 
+        end # Driver
       end
 
     end # Firefox

rb/spec/integration/selenium/webdriver/firefox/marionette_spec.rb

@@ -27,6 +27,27 @@ module WebDriver
         @opt = GlobalTestEnv.remote_server? ? {:url => GlobalTestEnv.remote_server.webdriver_url} : {}
       end
 
+      compliant_on :driver => :marionette do
+        it "takes a binary path as an argument" do
+          pending "Set ENV['ALTERNATE_FIREFOX_PATH'] to test this" unless ENV['ALTERNATE_FIREFOX_PATH']
+          pending "Set ENV['MARIONETTE_PATH'] to test this" unless ENV['MARIONETTE_PATH']
+
+          begin
+            caps = Remote::Capabilities.firefox(:firefox_binary => ENV['MARIONETTE_PATH'])
+            driver1 = Selenium::WebDriver.for :firefox, :desired_capabilities => caps, :marionette => true
+
+            default_version = driver1.capabilities.version
+            driver1.quit
+
+            caps = Remote::Capabilities.firefox(:firefox_binary => ENV['ALTERNATE_FIREFOX_PATH'])
+            driver2 = Selenium::WebDriver.for :firefox, :desired_capabilities => caps, :marionette => true
+
+            expect(driver2.capabilities.version).to_not be == default_version
+            driver2.quit
+          end
+        end
+      end
+
       context "when designated firefox binary includes Marionette" do
         before do
           unless ENV['MARIONETTE_PATH']
@@ -84,25 +105,32 @@ module WebDriver
       end
 
       compliant_on :browser => :firefox do
-        # TODO - Adjust specs when default Firefox version includes Marionette
-        # TODO - File bug for Unhandled error for remote / firefox
+        # These pass in isolation, but can not run in suite due to https://github.com/SeleniumHQ/selenium/issues/1150
         not_compliant_on :driver => :remote do
           context "when designated firefox binary does not include Marionette" do
-            let(:message) { /does not support Marionette/ }
+            let(:message) { /Marionette is not supported in Firefox Version \d\d/ }
+
+            before do
+              unless ENV['PRE_MARIONETTE_PATH']
+                pending "Set ENV['PRE_MARIONETTE_PATH'] to test features on firefox versions without marionette"
+              end
+            end
 
             it "Raises Wires Exception when setting marionette option in capabilities" do
-              caps = Selenium::WebDriver::Remote::Capabilities.firefox(:marionette => true)
+              caps = Selenium::WebDriver::Remote::Capabilities.firefox(:marionette => true,
+                                                                       :firefox_binary => ENV['PRE_MARIONETTE_PATH'])
               @opt.merge!(:desired_capabilities => caps)
-              expect { Selenium::WebDriver.for GlobalTestEnv.driver, @opt }.to raise_exception ArgumentError, message
+              expect { Selenium::WebDriver.for GlobalTestEnv.driver, @opt }.to raise_exception Error::WebDriverError, message
             end
 
             it "Raises Wires Exception when setting marionette option in driver initialization" do
-              @opt.merge!(:marionette => true)
-              expect{ Selenium::WebDriver.for GlobalTestEnv.driver, @opt}.to raise_exception ArgumentError, message
+              caps = Selenium::WebDriver::Remote::Capabilities.firefox(:firefox_binary => ENV['PRE_MARIONETTE_PATH'])
+              @opt.merge!(:marionette => true, :desired_capabilities => caps)
+              expect{ Selenium::WebDriver.for GlobalTestEnv.driver, @opt}.to raise_exception Error::WebDriverError, message
             end
           end
         end
       end
-    end
+    end # Firefox
   end # WebDriver
 end # Selenium

rb/spec/integration/selenium/webdriver/options_spec.rb

@@ -64,55 +64,53 @@ module WebDriver
         end
       end
 
-      not_compliant_on :browser => :marionette do
-        describe "cookie management" do
+      describe "cookie management" do
 
-          it "should get all" do
-            driver.navigate.to url_for("xhtmlTest.html")
-            driver.manage.add_cookie :name => "foo", :value => "bar"
+        it "should get all" do
+          driver.navigate.to url_for("xhtmlTest.html")
+          driver.manage.add_cookie :name => "foo", :value => "bar"
 
-            cookies = driver.manage.all_cookies
+          cookies = driver.manage.all_cookies
 
-            expect(cookies.size).to eq(1)
-            expect(cookies.first[:name]).to eq("foo")
-            expect(cookies.first[:value]).to eq("bar")
-          end
+          expect(cookies.size).to eq(1)
+          expect(cookies.first[:name]).to eq("foo")
+          expect(cookies.first[:value]).to eq("bar")
+        end
 
-          # Edge BUG - https://connect.microsoft.com/IE/feedbackdetail/view/1864122
-          not_compliant_on :browser => :edge do
-            it "should delete one" do
-              driver.navigate.to url_for("xhtmlTest.html")
-              driver.manage.add_cookie :name => "foo", :value => "bar"
+        # Edge BUG - https://connect.microsoft.com/IE/feedbackdetail/view/1864122
+        not_compliant_on :browser => :edge do
+          it "should delete one" do
+            driver.navigate.to url_for("xhtmlTest.html")
+            driver.manage.add_cookie :name => "foo", :value => "bar"
 
-              driver.manage.delete_cookie("foo")
-            end
+            driver.manage.delete_cookie("foo")
           end
+        end
 
-          # This is not a w3c supported spec
-          not_compliant_on :browser => :edge do
-            it "should delete all" do
-              driver.navigate.to url_for("xhtmlTest.html")
+        # This is not a w3c supported spec
+        not_compliant_on :browser => :edge do
+          it "should delete all" do
+            driver.navigate.to url_for("xhtmlTest.html")
 
-              driver.manage.add_cookie :name => "foo", :value => "bar"
-              driver.manage.delete_all_cookies
-              expect(driver.manage.all_cookies).to be_empty
-            end
+            driver.manage.add_cookie :name => "foo", :value => "bar"
+            driver.manage.delete_all_cookies
+            expect(driver.manage.all_cookies).to be_empty
           end
+        end
 
-          # Marionette BUG - Failed to convert expiry to Date
-          not_compliant_on :browser => [:android, :iphone, :safari, :marionette] do
-            it "should use DateTime for expires" do
-              driver.navigate.to url_for("xhtmlTest.html")
+        # Marionette BUG - Failed to convert expiry to Date
+        not_compliant_on :browser => [:android, :iphone, :safari, :marionette] do
+          it "should use DateTime for expires" do
+            driver.navigate.to url_for("xhtmlTest.html")
 
-              expected = DateTime.new(2039)
-              driver.manage.add_cookie :name => "foo",
-                                       :value => "bar",
-                                       :expires => expected
+            expected = DateTime.new(2039)
+            driver.manage.add_cookie :name => "foo",
+                                     :value => "bar",
+                                     :expires => expected
 
-              actual = driver.manage.cookie_named("foo")[:expires]
-              expect(actual).to be_kind_of(DateTime)
-              expect(actual).to eq(expected)
-            end
+            actual = driver.manage.cookie_named("foo")[:expires]
+            expect(actual).to be_kind_of(DateTime)
+            expect(actual).to eq(expected)
           end
         end
       end

rb/spec/integration/selenium/webdriver/spec_helper.rb

@@ -36,7 +36,7 @@ class Object
 RSpec.configure do |c|
   c.include(WebDriver::SpecSupport::Helpers)
   c.before(:suite) do
-    Selenium::WebDriver::Firefox::Binary.path = ENV['MARIONETTE_PATH'] if GlobalTestEnv.browser == :marionette
+    Selenium::WebDriver::Firefox::Binary.path = ENV['MARIONETTE_PATH'] if GlobalTestEnv.browser == :marionette && ENV['MARIONETTE_PATH']
     if GlobalTestEnv.driver == :remote
       server = GlobalTestEnv.remote_server
       if GlobalTestEnv.browser == :marionette