fix: remove symbolize (#413)

Author: KazuCocoa

CHANGELOG.md

@@ -9,6 +9,7 @@ Read `release_notes.md` for commit level details.
 ### Bug fixes
 
 ### Deprecations
+- Removed forcefully converting keys of capabilities into symbol, which caused unexpected capabilities format issue [ruby_lib/945](https://github.com/appium/ruby_lib/issues/945)
 
 ## [5.4.0] - 2022-10-01
 

lib/appium_lib_core/common/base/bridge.rb

@@ -53,8 +53,8 @@ def browser
         # Override
         # Creates session handling.
         #
-        # @param [::Selenium::WebDriver::Remote::Capabilities, Hash] capabilities A capability
-        # @return [::Selenium::WebDriver::Remote::Capabilities]
+        # @param [::Appium::Core::Base::Capabilities, Hash] capabilities A capability
+        # @return [::Appium::Core::Base::Capabilities]
         #
         # @example
         #
@@ -89,10 +89,10 @@ def create_session(capabilities)
         # Append +appium:+ prefix for Appium following W3C spec
         # https://www.w3.org/TR/webdriver/#dfn-validate-capabilities
         #
-        # @param [::Selenium::WebDriver::Remote::Capabilities, Hash] capabilities A capability
-        # @return [::Selenium::WebDriver::Remote::Capabilities]
+        # @param [::Appium::Core::Base::Capabilities, Hash] capabilities A capability
+        # @return [::Appium::Core::Base::Capabilities]
         def add_appium_prefix(capabilities)
-          w3c_capabilities = ::Selenium::WebDriver::Remote::Capabilities.new
+          w3c_capabilities = ::Appium::Core::Base::Capabilities.new
 
           capabilities = capabilities.send(:capabilities) unless capabilities.is_a?(Hash)
 
@@ -116,7 +116,7 @@ def camel_case(str_or_sym)
         end
 
         def extension_prefix?(capability_name)
-          snake_cased_capability_names = ::Selenium::WebDriver::Remote::Capabilities::KNOWN.map(&:to_s)
+          snake_cased_capability_names = ::Appium::Core::Base::Capabilities::KNOWN.map(&:to_s)
           camel_cased_capability_names = snake_cased_capability_names.map { |v| camel_case(v) }
 
           # Check 'EXTENSION_CAPABILITY_PATTERN'
@@ -126,7 +126,7 @@ def extension_prefix?(capability_name)
         end
 
         def json_create(value)
-          ::Selenium::WebDriver::Remote::Capabilities.json_create(value)
+          ::Appium::Core::Base::Capabilities.json_create(value)
         end
 
         public
@@ -182,7 +182,7 @@ def get_timeouts
 
         # Port from MJSONWP
         def session_capabilities
-          ::Selenium::WebDriver::Remote::Capabilities.json_create execute(:get_capabilities)
+          ::Appium::Core::Base::Capabilities.json_create execute(:get_capabilities)
         end
 
         # Override for safe. Newer ruby selenium webdriver already has the same code
@@ -345,6 +345,9 @@ def unwrap_script_result(arg)
             element_id = element_id_from(arg)
             return ::Appium::Core::Element.new(self, element_id) if element_id
 
+            shadow_root_id = shadow_root_id_from(arg)
+            return ::Selenium::WebDriver::Remote::ShadowRoot.new self, shadow_root_id if shadow_root_id
+
             arg.each { |k, v| arg[k] = unwrap_script_result(v) }
           else
             arg

lib/appium_lib_core/common/base/capabilities.rb

@@ -15,22 +15,14 @@
 module Appium
   module Core
     class Base
-      module Capabilities
-        # @private
-        # @param [Hash] opts_caps Capabilities for Appium server. All capability keys are converted to lowerCamelCase when
-        #                         this client sends capabilities to Appium server as JSON format.
-        # @return [::Selenium::WebDriver::Remote::Capabilities] Return instance of Appium::Core::Base::Capabilities
-        #                         inherited ::Selenium::WebDriver::Remote::Capabilities
-        def self.create_capabilities(opts_caps = {})
-          # TODO: Move to 'Options' way instead of 'Capabilities'.
-          # Selenium 5 will have Options instead of 'Capabilities'.
-          # https://github.com/SeleniumHQ/selenium/blob/trunk/rb/lib/selenium/webdriver/common/options.rb
-          # Then, Ruby client also shoud move to the Options way.
-          # Appium's capabilities could change by depending on Appium versions. So it does not have
-          # standard options like chrome and firefox etc. So, the implementation should differ from
-          # other browsers. But here should inherit `Options` to follow Selenium.
-          ::Selenium::WebDriver::Remote::Capabilities.new(opts_caps)
-        end
+      class Capabilities < ::Selenium::WebDriver::Remote::Capabilities
+        # TODO: Move to 'Options' way instead of 'Capabilities'.
+        # Selenium 5 will have Options instead of 'Capabilities'.
+        # https://github.com/SeleniumHQ/selenium/blob/trunk/rb/lib/selenium/webdriver/common/options.rb
+        # Then, Ruby client also shoud move to the Options way.
+        # Appium's capabilities could change by depending on Appium versions. So it does not have
+        # standard options like chrome and firefox etc. So, the implementation should differ from
+        # other browsers. But here should inherit `Options` to follow Selenium.
       end
     end
   end

lib/appium_lib_core/driver.rb

@@ -293,9 +293,6 @@ def initialize(opts = {})
         @delegate_target = self # for testing purpose
         @automation_name = nil # initialise before 'set_automation_name'
 
-        opts = Appium.symbolize_keys opts
-        validate_keys(opts)
-
         @custom_url = opts.delete :url
         @caps = get_caps(opts)
 
@@ -371,7 +368,7 @@ def start_driver(server_url: nil,
         begin
           @driver = ::Appium::Core::Base::Driver.new(listener: @listener,
                                                      http_client: @http_client,
-                                                     capabilities: @caps, # ::Selenium::WebDriver::Remote::Capabilities
+                                                     capabilities: @caps, # ::Appium::Core::Base::Capabilities
                                                      url: @custom_url,
                                                      wait_timeout: @wait_timeout,
                                                      wait_interval: @wait_interval,
@@ -555,32 +552,9 @@ def extend_for(device:, automation_name:) # rubocop:disable Metrics/CyclomaticCo
         self
       end
 
-      # @private
-      def validate_keys(opts)
-        flatten_ops = flatten_hash_keys(opts)
-
-        raise Error::NoCapabilityError unless opts.member?(:caps) || opts.member?(:capabilities)
-
-        if !opts.member?(:appium_lib) && flatten_ops.member?(:appium_lib)
-          raise Error::CapabilityStructureError, 'Please check the value of appium_lib in the capability'
-        end
-
-        true
-      end
-
-      # @private
-      def flatten_hash_keys(hash, flatten_keys_result = [])
-        hash.each do |key, value|
-          flatten_keys_result << key
-          flatten_hash_keys(value, flatten_keys_result) if value.is_a?(Hash)
-        end
-
-        flatten_keys_result
-      end
-
       # @private
       def get_caps(opts)
-        Core::Base::Capabilities.create_capabilities(opts[:caps] || opts[:capabilities] || {})
+        Core::Base::Capabilities.new(opts[:caps] || opts[:capabilities] || {})
       end
 
       # @private

test/unit/appium_lib_core_test.rb

@@ -20,24 +20,6 @@ def test_version
       assert !::Appium::Core::VERSION.nil?
     end
 
-    def test_symbolize_keys
-      result = ::Appium.symbolize_keys({ 'a' => 1, b: 2 })
-      assert_equal({ a: 1, b: 2 }, result)
-    end
-
-    def test_symbolize_keys_nested
-      result = ::Appium.symbolize_keys({ 'a' => 1, b: { 'c' => 2, d: 3 } })
-      assert_equal({ a: 1, b: { c: 2, d: 3 } }, result)
-    end
-
-    def test_symbolize_keys_raise_argument_error
-      e = assert_raises ::Appium::Core::Error::ArgumentError do
-        ::Appium.symbolize_keys('no hash value')
-      end
-
-      assert_equal 'symbolize_keys requires a hash', e.message
-    end
-
     def test_url_param
       opts = {
         url: 'http://custom-host:8080/wd/hub.com',

test/unit/common_test.rb

@@ -133,11 +133,11 @@ def test_add_appium_prefix_already_have_appium_prefix
           someCapability2: 'someCapability2',
           'some_capability3' => 'string_shold_keep',
           'some_capability4' => {
-            'nested_key1': 1,
+            'nested_key1' => 1,
             nested_key2: 2
           }
         }
-        base_caps = Appium::Core::Base::Capabilities.create_capabilities(cap)
+        base_caps = Appium::Core::Base::Capabilities.new cap
 
         expected = {
           'platformName' => :ios,
@@ -149,7 +149,7 @@ def test_add_appium_prefix_already_have_appium_prefix
           'someCapability1' => 'some_capability1',
           'someCapability2' => 'someCapability2',
           'some_capability3' => 'string_shold_keep',
-          'some_capability4' => { 'nestedKey1' => 1, 'nestedKey2' => 2 }
+          'some_capability4' => { 'nested_key1' => 1, 'nestedKey2' => 2 }
         }
         assert_equal expected, base_caps.as_json
 
@@ -166,7 +166,7 @@ def test_add_appium_prefix_already_have_appium_prefix
           'appium:someCapability2' => 'someCapability2',
           'appium:some_capability3' => 'string_shold_keep',
           'appium:some_capability4' => {
-            'nested_key1': 1,
+            'nested_key1' => 1,
             nested_key2: 2
           }
         }
@@ -183,7 +183,7 @@ def test_add_appium_prefix_already_have_appium_prefix
           'appium:someCapability2' => 'someCapability2',
           'appium:some_capability3' => 'string_shold_keep',
           'appium:some_capability4' => {
-            'nestedKey1' => 1,
+            'nested_key1' => 1,
             'nestedKey2' => 2
           }
         }
@@ -192,7 +192,7 @@ def test_add_appium_prefix_already_have_appium_prefix
 
       def test_add_appium_prefix_has_no_parameter
         cap = {}
-        base_caps = Appium::Core::Base::Capabilities.create_capabilities(cap)
+        base_caps = Appium::Core::Base::Capabilities.new cap
         expected = {}
 
         assert_equal expected, @bridge.add_appium_prefix(base_caps).__send__(:capabilities)

test/unit/driver_test.rb

@@ -31,14 +31,6 @@ def initialize(opts)
       end
     end
 
-    def test_no_caps
-      opts = { no: { caps: {} }, appium_lib: {} }
-
-      assert_raises ::Appium::Core::Error::NoCapabilityError do
-        ExampleDriver.new(opts)
-      end
-    end
-
     def test_with_caps
       opts = { caps: { automationName: 'xcuitest' } }
       driver = ExampleDriver.new(opts)
@@ -60,25 +52,18 @@ def test_with_caps_and_appium_lib
       assert_equal driver.core.caps[:automationName], 'xcuitest'
     end
 
-    def test_with_caps_and_wrong_appium_lib
-      opts = { caps: { appium_lib: {} } }
-      assert_raises ::Appium::Core::Error::CapabilityStructureError do
-        ExampleDriver.new(opts)
-      end
-    end
-
     def test_verify_session_id_in_the_export_session_path
       @core.wait { assert File.size?(@core.export_session_path) }
     end
 
     def test_verify_appium_core_base_capabilities_create_capabilities
-      caps = ::Appium::Core::Base::Capabilities.create_capabilities(platformName: 'ios',
-                                                                    platformVersion: '11.4',
-                                                                    automationName: 'XCUITest',
-                                                                    deviceName: 'iPhone Simulator',
-                                                                    app: 'test/functional/app/UICatalog.app.zip',
-                                                                    some_capability1: 'some_capability1',
-                                                                    someCapability2: 'someCapability2')
+      caps = ::Appium::Core::Base::Capabilities.new(platformName: 'ios',
+                                                    platformVersion: '11.4',
+                                                    automationName: 'XCUITest',
+                                                    deviceName: 'iPhone Simulator',
+                                                    app: 'test/functional/app/UICatalog.app.zip',
+                                                    some_capability1: 'some_capability1',
+                                                    someCapability2: 'someCapability2')
 
       caps_with_json = JSON.parse(caps.to_json)
       assert_equal 'ios', caps_with_json['platformName']