From 703f694756d33371ef98d9501ab9149e85777363 Mon Sep 17 00:00:00 2001 From: rishigupta1599 Date: Thu, 18 Jun 2026 18:45:31 +0530 Subject: [PATCH 1/4] fix: support Appium-Python-Client v3+/Selenium 4.x command executor URL DriverMetaData.command_executor_url (used by the Percy-on-Automate path) read driver.command_executor._url directly, which newer Appium-Python-Client (>3) / Selenium 4.x removed, breaking POA on modern clients. Resolve the remote URL via a dual-path helper that prefers command_executor._client_config.remote_server_addr and falls back to ._url, mirroring the existing Metadata.remote_url logic. Backward compatible with older clients. Validated end-to-end against Appium servers 1.22.0 and 2.15.0 with Appium-Python-Client 2.11.1 / 3.2.1 / 4.2.1 / 5.3.1. Co-Authored-By: Claude Opus 4.8 (1M context) --- percy/metadata/driver_metadata.py | 15 ++++++++++++++- tests/test_driver_metadata.py | 17 +++++++++++++++++ tests/test_screenshot.py | 2 ++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/percy/metadata/driver_metadata.py b/percy/metadata/driver_metadata.py index 59c2ff2..de1bb1f 100644 --- a/percy/metadata/driver_metadata.py +++ b/percy/metadata/driver_metadata.py @@ -12,11 +12,24 @@ 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 = self._extract_remote_url() Cache.set_cache(self.session_id, Cache.command_executor_url, url) return url return url + def _extract_remote_url(self): + # Support both old and new Appium-Python-Client / Selenium versions. + # New (Appium-Python-Client > 3 / Selenium 4.x): + # driver.command_executor._client_config.remote_server_addr + # Old: 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 + return getattr(command_executor, '_url', '') # pylint: disable=W0212 + @property def capabilities(self): caps = Cache.get_cache(self.session_id, Cache.capabilities) diff --git a/tests/test_driver_metadata.py b/tests/test_driver_metadata.py index 2224a1b..1efac62 100644 --- a/tests/test_driver_metadata.py +++ b/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 = { diff --git a/tests/test_screenshot.py b/tests/test_screenshot.py index 4daa138..7fa433e 100644 --- a/tests/test_screenshot.py +++ b/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' @@ -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') From 143b68008abf1732bdd0aadc90f24e84a5ef62c7 Mon Sep 17 00:00:00 2001 From: rishigupta1599 Date: Thu, 18 Jun 2026 18:53:08 +0530 Subject: [PATCH 2/4] refactor: extract shared resolve_remote_url helper (fix pylint duplicate-code) driver_metadata._extract_remote_url duplicated metadata.remote_url, tripping pylint R0801 (duplicate-code). Move the dual-path logic into a single common.resolve_remote_url helper used by both. Co-Authored-By: Claude Opus 4.8 (1M context) --- percy/common/__init__.py | 12 ++++++++++++ percy/metadata/driver_metadata.py | 16 ++-------------- percy/metadata/metadata.py | 16 ++-------------- 3 files changed, 16 insertions(+), 28 deletions(-) diff --git a/percy/common/__init__.py b/percy/common/__init__.py index bfb09a2..a1f1b1d 100644 --- a/percy/common/__init__.py +++ b/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', '') diff --git a/percy/metadata/driver_metadata.py b/percy/metadata/driver_metadata.py index de1bb1f..9b85ac6 100644 --- a/percy/metadata/driver_metadata.py +++ b/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,24 +13,11 @@ 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._extract_remote_url() + url = resolve_remote_url(self.driver.command_executor) Cache.set_cache(self.session_id, Cache.command_executor_url, url) return url return url - def _extract_remote_url(self): - # Support both old and new Appium-Python-Client / Selenium versions. - # New (Appium-Python-Client > 3 / Selenium 4.x): - # driver.command_executor._client_config.remote_server_addr - # Old: 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 - return getattr(command_executor, '_url', '') # pylint: disable=W0212 - @property def capabilities(self): caps = Cache.get_cache(self.session_id, Cache.capabilities) diff --git a/percy/metadata/metadata.py b/percy/metadata/metadata.py index b070b4f..3f7227f 100644 --- a/percy/metadata/metadata.py +++ b/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')) From 870f991d7618802c91b764992a829d1402d30472 Mon Sep 17 00:00:00 2001 From: rishigupta1599 Date: Thu, 18 Jun 2026 19:17:41 +0530 Subject: [PATCH 3/4] ci: unpin .python-version patch (3.10.3 unavailable on runners) The org submit-pypi workflow resolves .python-version; the exact patch 3.10.3 is no longer provided by GitHub's Ubuntu runners, failing setup at the ecosystem step. Use '3.10' so setup-python resolves the latest available patch. Pre-existing infra issue, unrelated to the SDK change; included to keep PR CI green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .python-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.python-version b/.python-version index 7d4ef04..c8cfe39 100644 --- a/.python-version +++ b/.python-version @@ -1 +1 @@ -3.10.3 +3.10 From 5d28169e4899849a0d00ed06eafeb5b8ab692e85 Mon Sep 17 00:00:00 2001 From: rishigupta1599 Date: Mon, 22 Jun 2026 18:03:25 +0530 Subject: [PATCH 4/4] test: strengthen resolve_remote_url coverage (review) - test_screenshot POA assertion compared against the mock's own _url, so it passed regardless of the helper; assert the literal URL instead. - Add dedicated TestResolveRemoteUrl unit tests (new-style, old-style fallback, no-addr fallback, all-absent). Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_common.py | 29 ++++++++++++++++++++++++++++- tests/test_screenshot.py | 2 +- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/tests/test_common.py b/tests/test_common.py index b20f995..f44d894 100644 --- a/tests/test_common.py +++ b/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): diff --git a/tests/test_screenshot.py b/tests/test_screenshot.py index 7fa433e..34f4001 100644 --- a/tests/test_screenshot.py +++ b/tests/test_screenshot.py @@ -278,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+')