From 758a8674aeff1de9b82fa0b200bd456a7d797398 Mon Sep 17 00:00:00 2001 From: Pranav Zinzurde Date: Fri, 3 Jul 2026 11:48:04 +0530 Subject: [PATCH 1/2] test: cover the 46 snapshot.py branches left untested by #226 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #226 (CORS iframes + closed shadow DOM parity) added new guard and error-fallback branches to percy/snapshot.py without tests for all of them, dropping total coverage from 100% to 94% and tripping the fail-under=100 gate in `make coverage` on every run since — both normal push CI (released @percy/cli) and the PER-9772 regression dispatch (@percy/cli built from git) fail with the identical missing-lines set, so the gap is deterministic, not CLI-flavor dependent. Add plain unit tests (no CLI needed) to tests/test_snapshot_units.py covering every previously missed line: - 36-37: invalid PERCY_RESIZE_SETTLE_SECONDS falls back to 0.5 - 247-248, 253-254: _should_skip_iframe invalid-origin / missing percyElementId skips - 272-274, 303-305, 316-317, 341-343, 364-366, 374, 377: process_frame_tree depth cap, missing element, document.URL read failure, empty serialize result, nested-enumeration failure, skipped child continue, nested capture collection - 408-409, 416: context-loss recovery (default_content failure at depth 1; PercyContextLost chaining the original error at depth > 1) - 460-462: _capture_cors_iframes outer error guard - 483-484, 549, 557-560, 564-565: expose_closed_shadow_roots capabilities failure, missing objectIds, resolveNode failure, getDocument + DOM.disable failures - 605, 607-608, 613-614: get_origin parse failures and _get_origin shim - 807-808: get_serialized_dom current_url read failure - 959-963: PERCY_DEBUG responsive snapshot dump (write + failure path) Verified locally on Python 3.9: full `make coverage` sequence (test_snapshot under `percy exec --testing` + the five plain modules) reports 759/759 statements, TOTAL 100%; pylint 10.00/10. Co-Authored-By: Claude Fable 5 --- tests/test_snapshot_units.py | 329 ++++++++++++++++++++++++++++++++++- 1 file changed, 328 insertions(+), 1 deletion(-) diff --git a/tests/test_snapshot_units.py b/tests/test_snapshot_units.py index 4c9472c..67e9a87 100644 --- a/tests/test_snapshot_units.py +++ b/tests/test_snapshot_units.py @@ -2,9 +2,10 @@ """Focused unit tests for snapshot.py helper functions and their error/ fallback paths. These run as plain unittests (no Percy CLI needed) and target branches the end-to-end suite in test_snapshot.py does not reach.""" +import importlib import os import unittest -from unittest.mock import patch, MagicMock +from unittest.mock import patch, MagicMock, Mock import percy.snapshot as local @@ -115,5 +116,331 @@ def test_invalid_min_height_falls_back_to_window_height(self): self.assertEqual(result, []) +class TestResizeSettleSecondsParsing(unittest.TestCase): + def test_invalid_env_value_falls_back_to_default(self): + # Module-level parse: an unparseable PERCY_RESIZE_SETTLE_SECONDS must + # fall back to the 0.5s default instead of raising at import time. + try: + with patch.dict(os.environ, {'PERCY_RESIZE_SETTLE_SECONDS': 'not-a-float'}): + importlib.reload(local) + self.assertEqual(local.RESIZE_SETTLE_SECONDS, 0.5) + finally: + # Restore module globals from the real (unpatched) environment. + importlib.reload(local) + + +class TestShouldSkipIframeEdgeCases(unittest.TestCase): + @staticmethod + def _meta(src, percy_id): + return { + 'src': src, 'srcdoc': None, 'percyElementId': percy_id, + 'dataPercyIgnore': False, 'matchesIgnoreSelector': False, 'index': 0, + } + + def test_unparseable_origin_is_skipped(self): + # src passes the unsupported-scheme check but has no scheme://netloc, + # so get_origin returns None -> "invalid URL" skip branch. + iframe = self._meta('not-a-url', 'pid-1') + self.assertTrue(local._should_skip_iframe(iframe, 'http://main.example.com')) + + def test_cross_origin_without_percy_element_id_is_skipped(self): + iframe = self._meta('https://other.example.com/widget', None) + self.assertTrue(local._should_skip_iframe(iframe, 'http://main.example.com')) + + +def _tree_ctx(max_depth=3): + return { + 'max_frame_depth': max_depth, + 'ignore_selectors': [], + 'serialize_options': {}, + 'percy_dom_script': 'PERCY_DOM', + } + + +class TestProcessFrameTreeGuards(unittest.TestCase): + @staticmethod + def _meta(src, percy_id, ignore=False): + return { + 'src': src, 'srcdoc': None, 'percyElementId': percy_id, + 'dataPercyIgnore': ignore, 'matchesIgnoreSelector': False, 'index': 0, + } + + def test_depth_beyond_max_returns_empty(self): + driver = MagicMock() + result = local.process_frame_tree( + driver, self._meta('https://x.example.com/', 'pid-x'), 4, set(), + _tree_ctx(max_depth=3)) + self.assertEqual(result, []) + driver.switch_to.frame.assert_not_called() + + def test_missing_iframe_element_returns_empty(self): + driver = MagicMock() + # querySelector by data-percy-element-id finds nothing (DOM mutated). + driver.execute_script.side_effect = [None] + result = local.process_frame_tree( + driver, self._meta('https://x.example.com/', 'pid-x'), 1, set(), + _tree_ctx()) + self.assertEqual(result, []) + driver.switch_to.frame.assert_not_called() + + def test_document_url_read_failure_is_treated_as_unsupported(self): + driver = MagicMock() + driver.execute_script.side_effect = [ + Mock(name='iframe_element'), # querySelector + Exception('document.URL blew up'), # post-switch URL read + ] + result = local.process_frame_tree( + driver, self._meta('https://x.example.com/', 'pid-x'), 1, set(), + _tree_ctx()) + self.assertEqual(result, []) + driver.switch_to.parent_frame.assert_called_once() + + def test_empty_serialize_result_returns_empty(self): + driver = MagicMock() + driver.execute_script.side_effect = [ + Mock(name='iframe_element'), # querySelector + 'https://x.example.com/page', # post-switch document.URL + None, # inject PercyDOM + None, # serialize returned nothing + ] + result = local.process_frame_tree( + driver, self._meta('https://x.example.com/page', 'pid-x'), 1, set(), + _tree_ctx()) + self.assertEqual(result, []) + + def test_nested_enumeration_failure_keeps_parent_capture(self): + driver = MagicMock() + driver.execute_script.side_effect = [ + Mock(name='iframe_element'), + 'https://x.example.com/page', + None, + {'snapshot': {'html': ''}, 'frameUrl': 'https://x.example.com/page'}, + Exception('nested enumerate blew up'), + ] + result = local.process_frame_tree( + driver, self._meta('https://x.example.com/page', 'pid-x'), 1, set(), + _tree_ctx()) + self.assertEqual(len(result), 1) + self.assertEqual(result[0]['iframeData']['percyElementId'], 'pid-x') + + def test_skipped_nested_child_is_not_recursed_into(self): + driver = MagicMock() + driver.execute_script.side_effect = [ + Mock(name='iframe_element'), + 'https://x.example.com/page', + None, + {'snapshot': {'html': ''}, 'frameUrl': 'https://x.example.com/page'}, + # nested enumeration returns one child carrying data-percy-ignore + [self._meta('https://c.example.com/', 'pid-c', ignore=True)], + ] + result = local.process_frame_tree( + driver, self._meta('https://x.example.com/page', 'pid-x'), 1, set(), + _tree_ctx()) + self.assertEqual(len(result), 1) + # Only one frame switch happened — the ignored child was never entered. + self.assertEqual(driver.switch_to.frame.call_count, 1) + + def test_nested_child_capture_is_collected(self): + driver = MagicMock() + driver.execute_script.side_effect = [ + # depth-1 frame + Mock(name='iframe_element_x'), + 'https://x.example.com/page', + None, + {'snapshot': {'html': ''}, 'frameUrl': 'https://x.example.com/page'}, + [self._meta('https://c.example.com/', 'pid-c')], + # nested depth-2 frame + Mock(name='iframe_element_c'), + 'https://c.example.com/', + None, + {'snapshot': {'html': ''}, 'frameUrl': 'https://c.example.com/'}, + [], + ] + result = local.process_frame_tree( + driver, self._meta('https://x.example.com/page', 'pid-x'), 1, set(), + _tree_ctx()) + pids = [e['iframeData']['percyElementId'] for e in result] + self.assertEqual(pids, ['pid-x', 'pid-c']) + + def test_default_content_failure_is_swallowed_at_depth_1(self): + driver = MagicMock() + driver.execute_script.side_effect = [ + Mock(name='iframe_element'), + 'https://x.example.com/page', + Exception('inject blew up'), + ] + driver.switch_to.parent_frame.side_effect = Exception('lost parent') + driver.switch_to.default_content.side_effect = Exception('lost everything') + # At depth 1 a total context loss must not raise — the walk just ends. + result = local.process_frame_tree( + driver, self._meta('https://x.example.com/page', 'pid-x'), 1, set(), + _tree_ctx()) + self.assertEqual(result, []) + + def test_context_lost_links_original_error_as_cause(self): + driver = MagicMock() + driver.execute_script.side_effect = [ + Mock(name='iframe_element'), + 'https://c.example.com/', + Exception('inject blew up'), + ] + driver.switch_to.parent_frame.side_effect = Exception('lost parent') + # depth > 1 with a captured processing error: PercyContextLost is + # raised and chains the original error for diagnosis. + with self.assertRaises(local.PercyContextLost) as cm: + local.process_frame_tree( + driver, self._meta('https://c.example.com/', 'pid-c'), 2, set(), + _tree_ctx()) + self.assertEqual(cm.exception.partial_capture, []) + self.assertIsNotNone(cm.exception.__cause__) + + +class TestCaptureCorsIframesErrorHandling(unittest.TestCase): + def test_unexpected_error_returns_empty_list(self): + driver = MagicMock() + # Enumeration "succeeds" but yields a malformed (non-dict) entry; + # the resulting AttributeError is swallowed by the outer guard. + driver.execute_script.side_effect = [[None]] + result = local._capture_cors_iframes( + driver, 'http://main.example.com/', _tree_ctx()) + self.assertEqual(result, []) + + +class TestExposeClosedShadowRootsEdgeCases(unittest.TestCase): + def test_capabilities_access_failure_is_treated_as_non_chromium(self): + cdp_calls = [] + + class StubDriver: + @property + def capabilities(self): + raise Exception('capabilities unavailable') + + def execute_cdp_cmd(self, cmd, params): + cdp_calls.append((cmd, params)) + + local.expose_closed_shadow_roots(StubDriver()) + self.assertEqual(cdp_calls, []) + + def test_missing_object_ids_are_skipped(self): + driver = MagicMock() + driver.capabilities = {'browserName': 'chrome'} + + def cdp(cmd, _params): + if cmd == 'DOM.getDocument': + return {'root': { + 'backendNodeId': 1, + 'shadowRoots': [{'backendNodeId': 2, 'shadowRootType': 'closed'}], + 'children': [], + }} + if cmd == 'DOM.resolveNode': + return {'object': {}} # no objectId -> pair is skipped + return {} + driver.execute_cdp_cmd.side_effect = cdp + + local.expose_closed_shadow_roots(driver) + cdp_cmds = [c.args[0] for c in driver.execute_cdp_cmd.call_args_list] + self.assertNotIn('Runtime.callFunctionOn', cdp_cmds) + + def test_resolve_node_failure_is_swallowed(self): + driver = MagicMock() + driver.capabilities = {'browserName': 'chrome'} + + def cdp(cmd, _params): + if cmd == 'DOM.getDocument': + return {'root': { + 'backendNodeId': 1, + 'shadowRoots': [{'backendNodeId': 2, 'shadowRootType': 'closed'}], + 'children': [], + }} + if cmd == 'DOM.resolveNode': + raise Exception('node gone') + return {} + driver.execute_cdp_cmd.side_effect = cdp + + local.expose_closed_shadow_roots(driver) # must not raise + + def test_get_document_failure_and_dom_disable_failure_are_swallowed(self): + driver = MagicMock() + driver.capabilities = {'browserName': 'chrome'} + + def cdp(cmd, _params): + if cmd == 'DOM.enable': + return {} + raise Exception(f'{cmd} failed') # DOM.getDocument AND DOM.disable + driver.execute_cdp_cmd.side_effect = cdp + + local.expose_closed_shadow_roots(driver) # must not raise + + +class TestGetOriginHelpers(unittest.TestCase): + def test_get_origin_returns_none_without_scheme_or_netloc(self): + self.assertIsNone(local.get_origin('not-a-url')) + self.assertIsNone(local.get_origin('http://')) + + def test_get_origin_returns_none_when_parsing_raises(self): + # urlparse raises AttributeError on non-str input + self.assertIsNone(local.get_origin(123)) + + def test_get_origin_compat_shim_returns_empty_string(self): + self.assertEqual(local._get_origin('https://a.example.com/x'), + 'https://a.example.com') + self.assertEqual(local._get_origin('not-a-url'), '') + + +class TestGetSerializedDomPageUrlFailure(unittest.TestCase): + def test_current_url_failure_degrades_to_no_page_url(self): + class StubDriver: + def __init__(self): + self.calls = 0 + + def execute_script(self, _script, *args): + self.calls += 1 + if self.calls == 1: + return {'html': ''} # main serialize + return [] # iframe enumeration + + @property + def current_url(self): + raise Exception('no url available') + + dom = local.get_serialized_dom( + StubDriver(), [{'name': 'k', 'value': 'v'}], + percy_dom_script='PERCY_DOM') + self.assertNotIn('corsIframes', dom) + self.assertEqual(dom['cookies'], [{'name': 'k', 'value': 'v'}]) + + +class TestResponsiveDebugDump(unittest.TestCase): + def tearDown(self): + if os.path.exists('output_file.json'): + os.remove('output_file.json') + + @staticmethod + def _driver(): + driver = MagicMock() + driver.get_window_size.return_value = {'width': 1000, 'height': 800} + return driver + + @patch('percy.snapshot.PERCY_DEBUG', True) + @patch('percy.snapshot.change_window_dimension_and_wait', MagicMock()) + @patch('percy.snapshot._wait_for_ready', MagicMock(return_value=None)) + @patch('percy.snapshot.get_responsive_widths', MagicMock(return_value=[])) + def test_debug_dump_is_written_when_percy_debug(self): + result = local.capture_responsive_dom( + self._driver(), {}, {}, percy_dom_script='PERCY_DOM') + self.assertEqual(result, []) + self.assertTrue(os.path.exists('output_file.json')) + + @patch('percy.snapshot.PERCY_DEBUG', True) + @patch('percy.snapshot.change_window_dimension_and_wait', MagicMock()) + @patch('percy.snapshot._wait_for_ready', MagicMock(return_value=None)) + @patch('percy.snapshot.get_responsive_widths', MagicMock(return_value=[])) + @patch('percy.snapshot.json.dump', side_effect=TypeError('not serializable')) + def test_debug_dump_failure_is_logged_not_raised(self, _mock_dump): + result = local.capture_responsive_dom( + self._driver(), {}, {}, percy_dom_script='PERCY_DOM') + self.assertEqual(result, []) + + if __name__ == '__main__': unittest.main() From e8d37d64aef31eba476bba2c8964ba2c79badce7 Mon Sep 17 00:00:00 2001 From: Pranav Zinzurde Date: Fri, 3 Jul 2026 12:20:48 +0530 Subject: [PATCH 2/2] test: rename unused stub arg to _args for pylint 2.17 (CI) Co-Authored-By: Claude Fable 5 --- tests/test_snapshot_units.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_snapshot_units.py b/tests/test_snapshot_units.py index 67e9a87..12492d8 100644 --- a/tests/test_snapshot_units.py +++ b/tests/test_snapshot_units.py @@ -393,7 +393,7 @@ class StubDriver: def __init__(self): self.calls = 0 - def execute_script(self, _script, *args): + def execute_script(self, _script, *_args): self.calls += 1 if self.calls == 1: return {'html': ''} # main serialize