fix: keep the legacy behavior partially, showing a warning message instead for now

CHANGELOG.md

@@ -7,8 +7,12 @@ Read `release_notes.md` for commit level details.
 ### Enhancements
 
 ### Bug fixes
+- Keep converting String to Symbol for `capabilities`, `caps` and `appium_lib` for the backward compatibility
+- Wrong `automationName` and `platformName` detection in this library before starting a session
 
 ### Deprecations
+- Converting `capabilities`, `caps` and `appium_lib` from String to Symbol
+    - They are expected to be Symbol. Nothing affects existing users who already give the above keys as Symbol for `Appium::Core.for`.
 
 ## [5.5.0] - 2022-10-09
 

lib/appium_lib_core.rb

@@ -21,24 +21,37 @@
 require_relative 'appium_lib_core/element'
 
 module Appium
-  # convert all keys (including nested) to symbols
+  # @private
+  #
+  # convert the top level keys to symbols.
   #
-  # based on deep_symbolize_keys & deep_transform_keys from rails
-  # https://github.com/rails/docrails/blob/a3b1105ada3da64acfa3843b164b14b734456a50/activesupport/lib/active_support/core_ext/hash/keys.rb#L84
   # @param [Hash] hash Hash value to make symbolise
-  def self.symbolize_keys(hash)
+  def self.symbolize_keys(hash, nested: false, enable_deprecation_msg: true)
+    # FIXME: As https://github.com/appium/ruby_lib/issues/945, we must remove this implicit string to symbol.
+    # But appium_lib_core's some capability handling expect to be symbol, so we should test to remove
+    # the mehotds which expect the symbol first.
     raise ::Appium::Core::Error::ArgumentError, 'symbolize_keys requires a hash' unless hash.is_a? Hash
 
     hash.each_with_object({}) do |pair, acc|
       key = begin
+        if enable_deprecation_msg && !(pair[0].is_a? Symbol)
+          ::Appium::Logger.warn("[Deprecation] The key '#{pair[0]}' must be a symbol while currently it " \
+                                "is #{pair[0].class.name}. Please define the key as a Symbol. " \
+                                'Converting it to Symbol for now.')
+        end
+
         pair[0].to_sym
       rescue StandardError => e
         ::Appium::Logger.warn(e.message)
         pair[0]
       end
 
       value = pair[1]
-      acc[key] = value.is_a?(Hash) ? symbolize_keys(value) : value
+      acc[key] = if nested
+                   value.is_a?(Hash) ? symbolize_keys(value, nested: false, enable_deprecation_msg: true) : value
+                 else
+                   value
+                 end
     end
   end
 

lib/appium_lib_core/common/error.rb

@@ -17,10 +17,6 @@ module Core
     module Error
       class CoreError < StandardError; end
 
-      # Capability related errors
-      class NoCapabilityError < CoreError; end
-      class CapabilityStructureError < CoreError; end
-
       # Appium related errors
       class NotSupportedAppiumServer < CoreError; end
       class NoSuchElementError < CoreError; end

lib/appium_lib_core/driver.rb

@@ -293,6 +293,12 @@ def initialize(opts = {})
         @delegate_target = self # for testing purpose
         @automation_name = nil # initialise before 'set_automation_name'
 
+        # TODO: Remove when we implement Options
+        # The symbolize_keys is to keep compatiility for the legacy code, which allows capabilities to give 'string' as the key.
+        # The toplevel `caps`, `capabilities` and `appium_lib` are expected to be symbol.
+        # FIXME: First, please try to remove `nested: true` to `nested: false`.
+        opts = Appium.symbolize_keys(opts, nested: true)
+
         @custom_url = opts.delete :url
         @caps = get_caps(opts)
 
@@ -486,14 +492,24 @@ def platform_version
 
       private
 
+      def convert_to_symbol(value)
+        if value.nil?
+          value
+        else
+          value.to_sym
+        end
+      end
+
       # @private
       def extend_for(device:, automation_name:) # rubocop:disable Metrics/CyclomaticComplexity
         extend Appium::Core
         extend Appium::Core::Device
 
-        case device
+        sym_automation_name = convert_to_symbol(automation_name)
+
+        case convert_to_symbol(device)
         when :android
-          case automation_name
+          case sym_automation_name
           when :espresso
             ::Appium::Core::Android::Espresso::Bridge.for self
           when :uiautomator2
@@ -504,7 +520,7 @@ def extend_for(device:, automation_name:) # rubocop:disable Metrics/CyclomaticCo
             ::Appium::Core::Android::Uiautomator1::Bridge.for self
           end
         when :ios, :tvos
-          case automation_name
+          case sym_automation_name
           when :safari
             ::Appium::Logger.debug('SafariDriver for iOS')
           when :xcuitest
@@ -513,7 +529,7 @@ def extend_for(device:, automation_name:) # rubocop:disable Metrics/CyclomaticCo
             ::Appium::Core::Ios::Uiautomation::Bridge.for self
           end
         when :mac
-          case automation_name
+          case sym_automation_name
           when :safari
             ::Appium::Logger.debug('SafariDriver for macOS')
           when :gecko
@@ -525,7 +541,7 @@ def extend_for(device:, automation_name:) # rubocop:disable Metrics/CyclomaticCo
             ::Appium::Logger.debug('macOS Native')
           end
         when :windows
-          case automation_name
+          case sym_automation_name
           when :gecko
             ::Appium::Logger.debug('Gecko Driver for Windows')
           else
@@ -535,7 +551,7 @@ def extend_for(device:, automation_name:) # rubocop:disable Metrics/CyclomaticCo
           # https://github.com/Samsung/appium-tizen-driver
           ::Appium::Logger.debug('tizen')
         else
-          case automation_name
+          case sym_automation_name
           when :youiengine
             # https://github.com/YOU-i-Labs/appium-youiengine-driver
             ::Appium::Logger.debug('YouiEngine')
@@ -567,6 +583,7 @@ def get_appium_lib_opts(opts)
       # The path can be local, HTTP/S, Windows Share and other path like 'sauce-storage:'.
       # Use @caps[:app] without modifications if the path isn't HTTP/S or local path.
       def set_app_path
+        # FIXME: maybe `:app` should check `app` as well.
         return unless @caps && @caps[:app] && !@caps[:app].empty?
         return if @caps[:app] =~ URI::DEFAULT_PARSER.make_regexp
 
@@ -605,15 +622,18 @@ def set_appium_lib_specific_values(appium_lib_opts)
       # @private
       def set_appium_device
         # https://code.google.com/p/selenium/source/browse/spec-draft.md?repo=mobile
-        @device = @caps[:platformName]
+        # TODO: check if the Appium.symbolize_keys(opts, nested: false) enoug with this
+        @device = @caps[:platformName] || @caps['platformName']
         return @device unless @device
 
         @device = @device.is_a?(Symbol) ? @device.downcase : @device.downcase.strip.intern
       end
 
       # @private
       def set_automation_name
-        @automation_name = @caps[:automationName] if @caps[:automationName]
+        # TODO: check if the Appium.symbolize_keys(opts, nested: false) enoug with this
+        candidate = @caps[:automationName] || @caps['automationName']
+        @automation_name = candidate if candidate
         @automation_name = if @automation_name
                              @automation_name.is_a?(Symbol) ? @automation_name.downcase : @automation_name.downcase.strip.intern
                            end

test/unit/appium_lib_core_test.rb

@@ -20,6 +20,50 @@ def test_version
       assert !::Appium::Core::VERSION.nil?
     end
 
+    # TODO: Should be removed in the future
+    def test_symbolize_keys
+      result = ::Appium.symbolize_keys({ 'a' => 1, b: 2 })
+      assert_equal({ a: 1, b: 2 }, result)
+    end
+
+    def test_not_symbolize_keys_nested1
+      result = ::Appium.symbolize_keys(
+        { 'caps': { 'automationName' => 'xcuitest', platformName: :ios } }, nested: true
+      )
+      assert_equal({ caps: { automationName: 'xcuitest', platformName: :ios } }, result)
+    end
+
+    def test_not_symbolize_keys_nested2
+      result = ::Appium.symbolize_keys(
+        { 'caps': { 'automationName' => 'xcuitest', platformName: :ios, other_caps: { 'something1': 1, something2: 2 } } },
+        nested: true
+      )
+      assert_equal(
+        { caps: { automationName: 'xcuitest', platformName: :ios, other_caps: { 'something1': 1, something2: 2 } } },
+        result
+      )
+    end
+
+    def test_not_symbolize_keys_nested3
+      result = ::Appium.symbolize_keys(
+        { 'caps': { 'automationName' => 'xcuitest', platformName: :ios, other_caps: { 'something1': 1, something2: 2 } } },
+        nested: false
+      )
+      assert_equal(
+        { caps: { 'automationName' => 'xcuitest', platformName: :ios, other_caps: { 'something1': 1, something2: 2 } } },
+        result
+      )
+    end
+
+    # TODO: Should be removed in the future
+    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

@@ -43,7 +43,16 @@ def setup
         app: "#{Dir.pwd}/test/functional/app/api.apk.zip",
         platformVersion: '7.1.1',
         deviceName: 'Android Emulator',
-        appPackage: 'io.appium.android.apis'
+        'appPackage' => 'io.appium.android.apis',
+        'custom_cap' => 'custom_value',
+        'custom_cap_2' => {
+          'custom_nested_key' => 'custom_value'
+        },
+        'custom_cap_3' => {
+          'custom_nested_key_2' => {
+            'custom_nested_key_3' => 'custom_value'
+          }
+        }
       }.freeze
 
       APPIUM_PREFIX_CAPS = {
@@ -52,7 +61,16 @@ def setup
         'appium:app' => "#{Dir.pwd}/test/functional/app/api.apk.zip",
         'appium:platformVersion' => '7.1.1',
         'appium:deviceName' => 'Android Emulator',
-        'appium:appPackage' => 'io.appium.android.apis'
+        'appium:appPackage' => 'io.appium.android.apis',
+        'appium:custom_cap' => 'custom_value',
+        'appium:custom_cap_2' => {
+          'custom_nested_key' => 'custom_value'
+        },
+        'appium:custom_cap_3' => {
+          'custom_nested_key_2' => {
+            'custom_nested_key_3' => 'custom_value'
+          }
+        }
       }.freeze
 
       def test_create_session_w3c
@@ -139,6 +157,9 @@ def test_add_appium_prefix_already_have_appium_prefix
         }
         base_caps = Appium::Core::Base::Capabilities.new cap
 
+        assert_equal base_caps[:platformName], :ios
+        assert_equal base_caps['platformName'], nil
+
         expected = {
           'platformName' => :ios,
           'automationName' => 'XCUITest',

test/unit/driver_test.rb

@@ -46,7 +46,7 @@ def test_with_capabilities
     end
 
     def test_with_caps_and_appium_lib
-      opts = { caps: { automationName: 'xcuitest' }, appium_lib: {} }
+      opts = { 'caps' => { 'automationName': 'xcuitest' }, appium_lib: {} }
       driver = ExampleDriver.new(opts)
       refute_nil driver
       assert_equal driver.core.caps[:automationName], 'xcuitest'