fix: support Appium-Python-Client v3+/Selenium 4.x command executor URL (PER-7786)

percy/common/__init__.py

@@ -10,3 +10,15 @@ def log(message, on_debug=None, error=False):
     if isinstance(on_debug, type(None)) or (isinstance(on_debug, bool) and PERCY_DEBUG):
         output = sys.stderr if error else sys.stdout
         print(f'{LABEL} {message}', file=output)
+
+def resolve_remote_url(command_executor):
+    # Support both old and new Appium-Python-Client / Selenium versions.
+    # New (Appium-Python-Client > 3 / Selenium 4.x):
+    #   command_executor._client_config.remote_server_addr
+    # Old: command_executor._url
+    client_config = getattr(command_executor, '_client_config', None)
+    if client_config:
+        remote_addr = getattr(client_config, 'remote_server_addr', None)
+        if remote_addr:
+            return remote_addr
+    return getattr(command_executor, '_url', '')

percy/metadata/driver_metadata.py

@@ -1,4 +1,5 @@
 from percy.lib.cache import Cache
+from percy.common import resolve_remote_url
 
 class DriverMetaData:
     def __init__(self, driver):
@@ -12,7 +13,7 @@ def session_id(self):
     def command_executor_url(self):
         url = Cache.get_cache(self.session_id, Cache.command_executor_url)
         if url is None:
-            url = self.driver.command_executor._url # pylint: disable=W0212
+            url = resolve_remote_url(self.driver.command_executor)
             Cache.set_cache(self.session_id, Cache.command_executor_url, url)
             return url
         return url

percy/metadata/metadata.py

@@ -1,7 +1,7 @@
 import json
 import os
 from abc import ABC, abstractmethod
-from percy.common import log
+from percy.common import log, resolve_remote_url
 
 
 _DEVICE_INFO_FILE_PATH = os.path.join(os.path.dirname(__file__), '..', 'configs', 'devices.json')
@@ -38,19 +38,7 @@ def os_version(self):
 
     @property
     def remote_url(self):
-        # Support both old and new Appium client versions
-        # New version > 3 : driver.command_executor._client_config.remote_server_addr
-        # Old version: driver.command_executor._url
-        command_executor = self.driver.command_executor
-        client_config = getattr(command_executor, '_client_config', None)
-
-        if client_config:
-            remote_addr = getattr(client_config, 'remote_server_addr', None)
-            if remote_addr:
-                return remote_addr
-
-        # Fallback to old version
-        return command_executor._url
+        return resolve_remote_url(self.driver.command_executor)
 
     def get_orientation(self, **kwargs):
         orientation = kwargs.get('orientation', self.capabilities.get('orientation', 'PORTRAIT'))

tests/test_common.py

@@ -7,7 +7,34 @@
 from unittest.mock import patch
 import percy.common
 
-from percy.common import log
+from percy.common import log, resolve_remote_url
+
+
+class TestResolveRemoteUrl(unittest.TestCase):
+    def test_prefers_client_config_remote_server_addr(self):
+        """New Appium-Python-Client (>3) / Selenium 4.x style."""
+        url = 'https://new-style-hub:4444/wd/hub'
+        executor = type('CE', (), {
+            '_client_config': type('Cfg', (), {'remote_server_addr': url})(),
+            '_url': 'https://should-not-be-used'
+        })()
+        self.assertEqual(resolve_remote_url(executor), url)
+
+    def test_falls_back_to_url_when_no_client_config(self):
+        """Older clients only expose ._url."""
+        executor = type('CE', (), {'_client_config': None, '_url': 'https://old-hub:4444/wd/hub'})()
+        self.assertEqual(resolve_remote_url(executor), 'https://old-hub:4444/wd/hub')
+
+    def test_falls_back_to_url_when_client_config_has_no_addr(self):
+        executor = type('CE', (), {
+            '_client_config': type('Cfg', (), {'remote_server_addr': None})(),
+            '_url': 'https://old-hub:4444/wd/hub'
+        })()
+        self.assertEqual(resolve_remote_url(executor), 'https://old-hub:4444/wd/hub')
+
+    def test_returns_empty_string_when_nothing_available(self):
+        executor = type('CE', (), {})()
+        self.assertEqual(resolve_remote_url(executor), '')
 
 
 class TestCommon(unittest.TestCase):

tests/test_driver_metadata.py

@@ -14,6 +14,7 @@ def setUp(self, mock_appium) -> None:
         self.mock_webdriver.orientation = 'PORTRAIT'
         self.mock_webdriver.session_id = 'session_id_123'
         self.mock_webdriver.command_executor._url = 'https://example-hub:4444/wd/hub' # pylint: disable=W0212
+        self.mock_webdriver.command_executor._client_config = None # pylint: disable=W0212
         self.mock_webdriver.capabilities = {
             'platform': 'chrome_android',
             'browserVersion': '115.0.1'
@@ -38,6 +39,22 @@ def test_command_executor_url(self):
         url = 'https://example-hub:4444/wd/hub'
         self.assertEqual(self.metadata.command_executor_url, url)
 
+    @patch('percy.lib.cache.Cache.CACHE', {})
+    def test_command_executor_url_with_client_config(self):
+        # New Appium-Python-Client (>3) / Selenium 4.x exposes the address via
+        # command_executor._client_config.remote_server_addr
+        url = 'https://new-style-hub:4444/wd/hub'
+        self.mock_webdriver.command_executor._client_config = \
+            type('Config', (), {'remote_server_addr': url})() # pylint: disable=W0212
+        self.assertEqual(self.metadata.command_executor_url, url)
+
+    @patch('percy.lib.cache.Cache.CACHE', {})
+    def test_command_executor_url_fallback_to_url(self):
+        # Older clients only expose command_executor._url
+        self.mock_webdriver.command_executor._client_config = None # pylint: disable=W0212
+        self.mock_webdriver.command_executor._url = 'https://old-style-hub:4444/wd/hub' # pylint: disable=W0212
+        self.assertEqual(self.metadata.command_executor_url, 'https://old-style-hub:4444/wd/hub')
+
     @patch('percy.lib.cache.Cache.CACHE', {})
     def test_capabilities(self):
         capabilities = {

tests/test_screenshot.py

@@ -261,6 +261,7 @@ def test_posts_screenshot_poa(self):
         driver.desired_capabilities = { 'key': 'value' }
         driver.command_executor = Mock()
         driver.command_executor._url = 'https://hub-cloud.browserstack.com/wd/hub'
+        driver.command_executor._client_config = None # pylint: disable=W0212
 
         element = Mock()
         element.id = 'Dummy_id'
@@ -277,7 +278,7 @@ def test_posts_screenshot_poa(self):
         s1 = httpretty.latest_requests()[1].parsed_body
         self.assertEqual(s1['snapshotName'], 'Snapshot 1')
         self.assertEqual(s1['sessionId'], driver.session_id)
-        self.assertEqual(s1['commandExecutorUrl'], driver.command_executor._url) # pylint: disable=W0212
+        self.assertEqual(s1['commandExecutorUrl'], 'https://hub-cloud.browserstack.com/wd/hub')
         self.assertEqual(s1['capabilities'], dict(driver.capabilities))
         self.assertRegex(s1['client_info'], r'percy-appium-app/\d+')
         self.assertRegex(s1['environment_info'][0], r'appium/\d+')
@@ -298,6 +299,7 @@ def test_posts_screenshot_poa_with_sync(self):
         driver.desired_capabilities = { 'key': 'value' }
         driver.command_executor = Mock()
         driver.command_executor._url = 'https://hub-cloud.browserstack.com/wd/hub'
+        driver.command_executor._client_config = None # pylint: disable=W0212
         self.mock_webdriver.capabilities = { 'key': 'value' }
 
         self.assertEqual(percy_screenshot(driver, 'Snapshot 3', options = {'sync': True}), 'sync-data')