diff --git a/CHANGELOG.md b/CHANGELOG.md index 119ca8a6..5a2b9b10 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lib/appium_lib_core.rb b/lib/appium_lib_core.rb index dfa7546c..de02c92b 100644 --- a/lib/appium_lib_core.rb +++ b/lib/appium_lib_core.rb @@ -21,16 +21,25 @@ 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) @@ -38,7 +47,11 @@ def self.symbolize_keys(hash) 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 diff --git a/lib/appium_lib_core/common/error.rb b/lib/appium_lib_core/common/error.rb index 01125fb5..bcfcd842 100644 --- a/lib/appium_lib_core/common/error.rb +++ b/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 diff --git a/lib/appium_lib_core/driver.rb b/lib/appium_lib_core/driver.rb index 777aed81..6419479a 100644 --- a/lib/appium_lib_core/driver.rb +++ b/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,7 +622,8 @@ 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 @@ -613,7 +631,9 @@ def set_appium_device # @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 diff --git a/test/unit/appium_lib_core_test.rb b/test/unit/appium_lib_core_test.rb index 546976da..37ea5c0b 100644 --- a/test/unit/appium_lib_core_test.rb +++ b/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', diff --git a/test/unit/common_test.rb b/test/unit/common_test.rb index b6699b1b..7efcc610 100644 --- a/test/unit/common_test.rb +++ b/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', diff --git a/test/unit/driver_test.rb b/test/unit/driver_test.rb index 7d2d6ef3..d649ecff 100644 --- a/test/unit/driver_test.rb +++ b/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'