fix: resolve hub URL on appium_lib_core 11+ and read viewportRect as a Hash (PPLT-6148)

.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

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'

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

percy/metadata/driver_metadata.rb

@@ -8,14 +8,31 @@ 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
 
     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

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)

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
 

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