diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index dd55572..1002dce 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -21,7 +21,13 @@ jobs: matrix: os: [ubuntu-latest] ruby: ['2.6', '2.7', '3.0'] + include: + - os: ubuntu-latest + ruby: '3.3' + appium_lib: '~> 16.0' runs-on: ${{ matrix.os }} + env: + APPIUM_LIB_VERSION: ${{ matrix.appium_lib || '~> 12.0' }} steps: - uses: actions-ecosystem/action-regex-match@v2 id: regex-match diff --git a/Gemfile b/Gemfile index 87a0d44..652b814 100644 --- a/Gemfile +++ b/Gemfile @@ -2,14 +2,17 @@ source 'https://rubygems.org' -# Pin appium_lib to the 12.x line so all Ruby legs (2.6/2.7/3.0) resolve -# appium_lib_core ~> 5.0, which still exposes Appium::Core::Base::SearchContext. -# Unpinned, Ruby 3.0 resolved appium_lib 15 / appium_lib_core 9.x, which removed -# that constant and broke the specs. appium_console 3.0.0 requires appium_lib = 12.0.0. -gem 'appium_console', '~> 3.0' -gem 'appium_lib', '~> 12.0' +# appium_lib defaults to the 12.x line so the Ruby 2.6/2.7/3.0 legs resolve +# appium_lib_core ~> 5.0. CI also runs a leg on the current major via +# APPIUM_LIB_VERSION, since appium_lib_core 11+ changed where the driver keeps its +# hub URL. appium_console 3.0.0 requires appium_lib = 12.0.0, so it only joins the +# 12.x resolution. +appium_lib_version = ENV.fetch('APPIUM_LIB_VERSION', '~> 12.0') +gem 'appium_console', '~> 3.0' if appium_lib_version.include?('12.') +gem 'appium_lib', appium_lib_version gem 'dotenv' -gem 'minitest' +# minitest 6 moved Minitest::Mock into the separate minitest-mock gem. +gem 'minitest', '~> 5.0' gem 'rubocop', require: false gem 'tempfile' gem 'webmock', '~> 3.18', '>= 3.18.1' diff --git a/percy/metadata/android_metadata.rb b/percy/metadata/android_metadata.rb index 3723008..f218c5e 100644 --- a/percy/metadata/android_metadata.rb +++ b/percy/metadata/android_metadata.rb @@ -9,12 +9,11 @@ class AndroidMetadata < Percy::Metadata def initialize(driver) super(driver) @_bars = nil - # Intentionally left as the original lookup: this path already degrades to - # driver.get_system_bars consistently across all appium_lib_core versions - # (the rect read yields a non-Hash, so the rect arithmetic in - # get_system_bars rescues to nil and falls back), so it is out of scope for - # the snake_case capability fix. - @_viewport_rect = capabilities.to_json['viewportRect'] + # Read the rect as a Hash. The previous `capabilities.to_json['viewportRect']` + # indexed a JSON String, so the rect was never used and every call fell back + # to driver.get_system_bars, which appium_lib_core 13+ sends as the + # Appium 2-only `mobile: getSystemBars` command. + @_viewport_rect = viewport end def device_screen_size @@ -71,7 +70,8 @@ def navigation_bar end def viewport - capabilities.to_json['viewportRect'] + rect = get_capability_value('viewportRect') + rect.is_a?(Hash) ? rect : nil end def scale_factor diff --git a/percy/metadata/driver_metadata.rb b/percy/metadata/driver_metadata.rb index 97ee187..f4a0ca4 100644 --- a/percy/metadata/driver_metadata.rb +++ b/percy/metadata/driver_metadata.rb @@ -8,6 +8,23 @@ def initialize(driver) @driver = driver end + # Returns the hub URL the driver talks to. appium_lib_core <= 10 keeps it in + # the HTTP client's @server_url; 11+ (selenium-webdriver ClientConfig) moved it + # to client_config.server_url and leaves @server_url unset, which made every + # BrowserStack session look like a generic remote (no AppAutomate provider, + # no device name, empty executor URL for Percy on Automate). + def self.server_url(driver) + http = driver.instance_variable_get(:@bridge).instance_variable_get(:@http) + url = http.instance_variable_get(:@server_url).to_s + return url unless url.empty? + + client_config = http.respond_to?(:client_config) ? http.client_config : nil + client_config ||= http.instance_variable_get(:@client_config) + return '' unless client_config.respond_to?(:server_url) + + client_config.server_url.to_s.chomp('/') + end + def session_id @driver.session_id end @@ -15,7 +32,7 @@ def session_id def command_executor_url url = Percy::Cache.get_cache(session_id, Percy::Cache::COMMAND_EXECUTOR_URL) if url.nil? - url = @driver.instance_variable_get(:@bridge).instance_variable_get(:@http).instance_variable_get(:@server_url).to_s + url = self.class.server_url(@driver) Percy::Cache.set_cache(session_id, Percy::Cache::COMMAND_EXECUTOR_URL, url) end url diff --git a/percy/metadata/metadata.rb b/percy/metadata/metadata.rb index a4e08aa..6bdb524 100644 --- a/percy/metadata/metadata.rb +++ b/percy/metadata/metadata.rb @@ -3,6 +3,7 @@ require 'json' require 'pathname' require_relative '../common/common' +require_relative 'driver_metadata' DEVICE_INFO_FILE_PATH = File.join(File.dirname(__FILE__), '..', 'configs', 'devices.json') DEVICE_INFO = JSON.parse(File.read(DEVICE_INFO_FILE_PATH)) @@ -82,7 +83,7 @@ def os_version end def remote_url - driver.instance_variable_get(:@bridge).instance_variable_get(:@http).instance_variable_get(:@server_url).to_s + Percy::DriverMetadata.server_url(driver) end def get_orientation(**kwargs) diff --git a/specs/android_metadata.rb b/specs/android_metadata.rb index b090619..ee4994d 100644 --- a/specs/android_metadata.rb +++ b/specs/android_metadata.rb @@ -28,7 +28,7 @@ def test_viewport android_capabilities = get_android_capabilities @mock_webdriver.expect(:capabilities, android_capabilities.merge('viewportRect' => viewport)) - assert(viewport, @android_metadata.viewport) + assert_equal(viewport, @android_metadata.viewport) @mock_webdriver.verify end @@ -61,19 +61,30 @@ def test_device_screen_size_when_device_screen_size_is_present @mock_webdriver.verify end - def test_get_system_bars + def test_get_system_bars_falls_back_to_driver_without_viewport_rect system_bars = { 'statusBar' => { 'height' => 83 }, 'navigationBar' => { 'height' => 44 } } - android_capabilities = get_android_capabilities - session_id = 'session_id_123' - @mock_webdriver.expect(:session_id, session_id) - @mock_webdriver.expect(:session_id, session_id) - @mock_webdriver.expect(:capabilities, android_capabilities.merge('viewportRect' => nil)) - @mock_webdriver.expect(:get_system_bars, system_bars) + driver = Minitest::Mock.new + driver.expect(:capabilities, get_android_capabilities.merge('viewportRect' => nil)) + metadata = Percy::AndroidMetadata.new(driver) + driver.expect(:session_id, 'session_id_no_rect') + driver.expect(:session_id, 'session_id_no_rect') + driver.expect(:get_system_bars, system_bars) + + assert_equal(system_bars, metadata.get_system_bars) + driver.verify + end + + def test_get_system_bars_uses_viewport_rect + # viewportRect top=83, height=2153 on a 1080x2280 screen (see get_android_capabilities) + @mock_webdriver.expect(:session_id, 'session_id_rect') + @mock_webdriver.expect(:capabilities, get_android_capabilities) + + bars = @android_metadata.get_system_bars - assert(system_bars, @android_metadata.get_system_bars) + assert_equal({ 'statusBar' => { 'height' => 83 }, 'navigationBar' => { 'height' => 44 } }, bars) @mock_webdriver.verify end diff --git a/specs/driver_metadata.rb b/specs/driver_metadata.rb index 9ae2c5a..f618222 100644 --- a/specs/driver_metadata.rb +++ b/specs/driver_metadata.rb @@ -39,6 +39,38 @@ def test_command_executor_url assert(url, @metadata.command_executor_url) end + # appium_lib_core 11+ leaves the HTTP client's @server_url unset and keeps the + # hub URL in client_config.server_url (a URI with a trailing slash). + class ClientConfigHttp + attr_reader :client_config + + def initialize(client_config) + @client_config = client_config + end + end + + def driver_with_http(http) + bridge = Object.new + bridge.instance_variable_set(:@http, http) + driver = Object.new + driver.instance_variable_set(:@bridge, bridge) + driver.define_singleton_method(:session_id) { "session_#{object_id}" } + driver + end + + def test_command_executor_url_reads_client_config_when_server_url_unset + client_config = Struct.new(:server_url).new(URI('https://hub-cloud.browserstack.com/wd/hub/')) + driver = driver_with_http(ClientConfigHttp.new(client_config)) + + assert_equal('https://hub-cloud.browserstack.com/wd/hub', Percy::DriverMetadata.new(driver).command_executor_url) + end + + def test_server_url_is_empty_when_no_known_location_has_it + driver = driver_with_http(ClientConfigHttp.new(nil)) + + assert_equal('', Percy::DriverMetadata.server_url(driver)) + end + def test_capabilities session_id = 'session_id_123' 2.times do