rb - fix remote server implementation for Marionette

Author: titusfortner

rb/build.desc

@@ -307,7 +307,8 @@ ruby_test(name = "remote-marionette",
   include = ["rb/spec/integration", "build/rb/lib"],
   deps = [
     "//java/server/test/org/openqa/selenium:server-with-tests:uber",
-    ":remote"
+    ":remote",
+    ":firefox"
   ]
 )
 

rb/lib/selenium/webdriver/remote/capabilities.rb

@@ -88,7 +88,7 @@ def edge(opts = {})
           end
 
           def firefox(opts = {})
-            return W3CCapabilities.firefox(opts) if opts.delete(:marionette)
+            return W3CCapabilities.firefox(opts) if opts[:marionette]
 
             new({
               :browser_name          => "firefox",

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

@@ -17,6 +17,8 @@
 # specific language governing permissions and limitations
 # under the License.
 
+require 'json'
+
 module Selenium
   module WebDriver
     module Remote
@@ -66,6 +68,11 @@ 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))
+          end
+
           unless opts.empty?
             raise ArgumentError, "unknown option#{'s' if opts.size != 1}: #{opts.inspect}"
           end
@@ -86,6 +93,10 @@ def initialize(opts = {})
           @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

@@ -64,22 +64,19 @@ def edge(opts = {})
           end
 
           def firefox(opts = {})
+            opts[:browser_version] = opts.delete :version
+            opts[:platform_name] = opts.delete :platform
+
             new({
-              :browser_name => "firefox"
+              :browser_name => "firefox",
+              :marionette => true
                 }.merge(opts))
           end
 
           alias_method :ff, :firefox
 
           def w3c?(opts = {})
-            intended = opts[:desired_capabilities].is_a?(W3CCapabilities) || !!opts.delete(:marionette)
-            if intended && Firefox::Binary.version < 43
-              raise ArgumentError, "Firefox Version #{Firefox::Binary.version} does not support Marionette; Set Selenium::WebDriver::Firefox::Binary.path to point to a supported binary"
-            elsif intended
-              true
-            else
-              false
-            end
+            opts[:desired_capabilities].is_a?(W3CCapabilities) || opts[:marionette]
           end
 
           #
@@ -92,14 +89,29 @@ def json_create(data)
             caps = new
             caps.browser_name = data.delete("browserName")
             caps.browser_version = data.delete("browserVersion")
-            caps.platform_name = data.delete("platformName").downcase.to_sym if data.has_key?('platform')
+            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_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')
 
+            # Remote Server Specific
+            caps[:remote_session_id] = data.delete('webdriver.remote.sessionid')
+
+            # obsolete capabilities returned by Remote Server
+            data.delete("javascriptEnabled")
+            data.delete('cssSelectorsEnabled')
+
+            # Marionette Specific
+            caps[:specification_level] = data.delete("specificaionLevel")
+            caps[:xul_app_id] = data.delete("XULappId")
+            caps[:raise_accessibility_exceptions] = data.delete('raisesAccessibilityExceptions')
+            caps[:rotatable] = data.delete('rotatable')
+            caps[:app_build_id] = data.delete('appBuildId')
+            caps[:device] = data.delete('device')
+
             # any remaining pairs will be added as is, with no conversion
             caps.merge!(data)
 

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

@@ -26,7 +26,7 @@
   end
 
   # Marionette BUG - AutomatedTester: "I need to add pagesource back and add it to the spec"
-  not_compliant_on :driver => :marionette do
+  not_compliant_on :browser => :marionette do
     it "should get the page source" do
       driver.navigate.to url_for("xhtmlTest.html")
       expect(driver.page_source).to match(%r[<title>XHTML Test Page</title>]i)

rb/spec/integration/selenium/webdriver/element_spec.rb

@@ -36,7 +36,7 @@
   end
 
   # Marionette BUG - AutomatedTester: "known bug with execute script"
-  not_compliant_on :driver => :marionette do
+  not_compliant_on :browser => :marionette do
     it "should submit" do
       driver.navigate.to url_for("formPage.html")
       wait(5).until {driver.find_elements(:id, "submitButton").size > 0}
@@ -142,7 +142,7 @@
   end
 
   # Location not currently supported in Spec, but should be?
-  not_compliant_on :driver => :marionette do
+  not_compliant_on :browser => :marionette do
     it "should get location" do
       driver.navigate.to url_for("xhtmlTest.html")
       loc = driver.find_element(:class, "header").location
@@ -165,7 +165,7 @@
   # Marionette BUG:
   # GET /session/f7082a32-e685-2843-ad2c-5bb6f376dac5/element/b6ff4468-ed6f-7c44-be4b-ca5a3ea8bf26/size
   # did not match a known command"
-  not_compliant_on :driver => :marionette do
+  not_compliant_on :browser => :marionette do
     it "should get size" do
       driver.navigate.to url_for("xhtmlTest.html")
       size = driver.find_element(:class, "header").size

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

@@ -23,69 +23,83 @@ module Selenium
   module WebDriver
 
     describe Firefox do
-      compliant_on :browser => :marionette do
-        context "when designated firefox binary includes Marionette" do
-          before(:each) do
-            Selenium::WebDriver::Firefox::Binary.path = ENV['MARIONETTE_PATH']
-          end
-
-          it "Uses Wires when initialized with W3C desired_capabilities" do
-            caps = Selenium::WebDriver::Remote::W3CCapabilities.firefox
-            expect do
-              @driver = Selenium::WebDriver.for :firefox, :desired_capabilities => caps
-            end.to_not raise_exception
-            @driver.quit
-          end
+      before(:each) do
+        @opt = GlobalTestEnv.remote_server? ? {:url => GlobalTestEnv.remote_server.webdriver_url} : {}
+      end
 
-          it "Uses Wires when initialized with non W3C desired_capabilities using marionette option" do
-            caps = Selenium::WebDriver::Remote::Capabilities.firefox(:marionette => true)
-            expect do
-              @driver = Selenium::WebDriver.for :firefox, :desired_capabilities => caps
-            end.to_not raise_exception
-            @driver.quit
+      context "when designated firefox binary includes Marionette" do
+        before(:each) do
+          unless ENV['MARIONETTE_PATH']
+            pending "Set ENV['MARIONETTE_PATH'] to test Marionette enabled Firefox installations"
           end
+        end
 
-          it "Uses Wires when initialized with marionette option" do
-            @driver = Selenium::WebDriver.for :firefox, {marionette: true}
-            expect(@driver.instance_variable_get('@bridge').instance_variable_get('@launcher')).to be_nil
-            @driver.quit
+        compliant_on :browser => :marionette do
+          # This passes in isolation, but can not run in suite due to combination of
+          # https://bugzilla.mozilla.org/show_bug.cgi?id=1228107 & https://github.com/SeleniumHQ/selenium/issues/1150
+          not_compliant_on :driver => :remote do
+            it "Uses Wires when setting marionette option in capabilities" do
+              cap_opts = {:marionette => true}
+              cap_opts.merge!(:firefox_binary => ENV['MARIONETTE_PATH']) unless GlobalTestEnv.driver == :remote
+              caps = Selenium::WebDriver::Remote::Capabilities.firefox cap_opts
+              @opt.merge!(:desired_capabilities => caps)
+              expect {@driver = Selenium::WebDriver.for GlobalTestEnv.driver, @opt}.to_not raise_exception
+              @driver.quit
+            end
           end
+        end
 
-          not_compliant_on :browser => :marionette do
-            it "Does not use wires by default" do
-              unless ENV['MARIONETTE_PATH']
-                pending "Set ENV['MARIONETTE_PATH'] to test Marionette enabled Firefox installations"
-              end
+        compliant_on :browser => :marionette do
+          # This passes in isolation, but can not run in suite due to combination of
+          # https://bugzilla.mozilla.org/show_bug.cgi?id=1228107 & https://github.com/SeleniumHQ/selenium/issues/1150
+          not_compliant_on :driver => :remote do
+            it "Uses Wires when setting marionette option in driver initialization" do
+              cap_opts = GlobalTestEnv.driver == :remote ? {} : {:firefox_binary => ENV['MARIONETTE_PATH']}
+              caps = Selenium::WebDriver::Remote::Capabilities.firefox cap_opts
+              @opt.merge!(:marionette => true, :desired_capabilities => caps)
+              @driver = Selenium::WebDriver.for GlobalTestEnv.driver, @opt
 
-              Selenium::WebDriver::Firefox::Binary.path = ENV['MARIONETTE_PATH']
-              @driver = Selenium::WebDriver.for :firefox
-              expect(@driver.instance_variable_get('@bridge').instance_variable_get('@launcher')).to_not be_nil
+              expect(@driver.capabilities[:takes_element_screenshot]).to_not be_nil
               @driver.quit
             end
           end
+        end
+
+        # test with firefox due to https://bugzilla.mozilla.org/show_bug.cgi?id=1228121
+        compliant_on :browser => :firefox do
+          it "Does not use wires when marionette option is not set" do
+            @driver = Selenium::WebDriver.for GlobalTestEnv.driver, @opt
+
+            expect(@driver.capabilities[:takes_element_screenshot]).to be_nil
+            @driver.quit
+          end
+        end
 
+        compliant_on :browser => :marionette do
+          # https://bugzilla.mozilla.org/show_bug.cgi?id=1228107
           not_compliant_on :browser => :marionette do
             it_behaves_like "driver that can be started concurrently", :marionette
           end
         end
       end
 
       compliant_on :browser => :firefox do
-        context "when designated firefox binary does not include Marionette" do
-          let(:message) { /Firefox Version \d\d does not support Marionette/ }
+        # TODO - Adjust specs when default Firefox version includes Marionette
+        # TODO - File bug for Unhandled error for remote / firefox
+        not_compliant_on :driver => :remote do
+          context "when designated firefox binary does not include Marionette" do
+            let(:message) { /does not support Marionette/ }
 
-          it "Raises Wires Exception when initialized with :desired_capabilities" do
-            caps = Selenium::WebDriver::Remote::W3CCapabilities.firefox
-            opt = {:desired_capabilities => caps}
-            expect { Selenium::WebDriver.for :firefox, opt }.to raise_exception ArgumentError, message
-          end
-
-          it "Raises Wires Exception when initialized with marionette option" do
-            expect{Selenium::WebDriver.for :firefox, {marionette: true}}.to raise_exception ArgumentError, message
-          end
+            it "Raises Wires Exception when setting marionette option in capabilities" do
+              caps = Selenium::WebDriver::Remote::Capabilities.firefox(:marionette => true)
+              @opt.merge!(:desired_capabilities => caps)
+              expect { Selenium::WebDriver.for GlobalTestEnv.driver, @opt }.to raise_exception ArgumentError, message
+            end
 
-          it "Raises Wires Exception when initialized with marionette option" do
-            expect{Selenium::WebDriver.for :firefox, {marionette: true}}.to raise_exception ArgumentError, message
+            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
+            end
           end
         end
       end

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

@@ -66,7 +66,7 @@ module WebDriver
         end
       end
 
-      not_compliant_on({:browser => :ie}) do
+      not_compliant_on({:browser => [:ie, :marionette]}) do
         describe "cookie management" do
           it "should get all" do
             driver.navigate.to url_for("xhtmlTest.html")

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

@@ -29,9 +29,12 @@ module Remote
             expect(driver.session_id).to be_kind_of(String)
           end
 
-          it "should expose remote status" do
-            expect(driver).to be_kind_of(DriverExtensions::HasRemoteStatus)
-            expect(driver.remote_status).to be_kind_of(Hash)
+          # status is not w3c compliant
+          not_compliant_on :browser => :marionette do
+            it "should expose remote status" do
+              expect(driver).to be_kind_of(DriverExtensions::HasRemoteStatus)
+              expect(driver.remote_status).to be_kind_of(Hash)
+            end
           end
         end
       end

rb/spec/integration/selenium/webdriver/remote/element_spec.rb

@@ -32,7 +32,7 @@ module WebDriver
           driver.file_detector = nil
         end
 
-        not_compliant_on :browser => [:phantomjs, :safari] do
+        not_compliant_on :browser => [:phantomjs, :safari, :marionette] do
           it "uses the file detector" do
             driver.navigate.to url_for("upload.html")