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 diff --git a/package.json b/package.json index b9a445e..84da721 100644 --- a/package.json +++ b/package.json @@ -4,6 +4,6 @@ "test": "make test" }, "devDependencies": { - "@percy/cli": "^1.31.10" + "@percy/cli": "^1.31.15-beta.0" } } diff --git a/percy/screenshot.py b/percy/screenshot.py index faabe23..dff7779 100644 --- a/percy/screenshot.py +++ b/percy/screenshot.py @@ -153,20 +153,111 @@ def process_frame(page, frame, options, percy_dom_script): return None -def get_serialized_dom(page, cookies, percy_dom_script=None, **kwargs): +def _resolve_readiness_config(percy_config, kwargs): + """Shallow-merge global (percy_config.snapshot.readiness) and per-snapshot + (kwargs['readiness']) readiness config. Per-snapshot keys win; unspecified + keys (e.g. a global preset: disabled kill switch) are inherited. + + Defensive: `(config or {}).get('snapshot') or {}` guards against the CLI + returning a None-valued snapshot section.""" + config = percy_config or {} + global_readiness = ((config.get('snapshot') or {}).get('readiness')) or {} + per_snapshot = kwargs.get('readiness') or {} + if not isinstance(global_readiness, dict): + global_readiness = {} + if not isinstance(per_snapshot, dict): + per_snapshot = {} + return {**global_readiness, **per_snapshot} + + +def _wait_for_ready(page, percy_config, kwargs): + """Run readiness checks before serialize. + + Uses page.evaluate (sync Playwright auto-awaits Promises). The embedded + JS checks typeof PercyDOM.waitForReady === 'function' so old CLI versions + without the method are a graceful no-op. + + Returns readiness diagnostics dict (or None) to be attached to the + domSnapshot. Callers pass `percy_config` explicitly from the + `_is_percy_enabled()` payload they already have in scope — we don't + re-call the cached lookup here, both for clarity and to avoid surprise + dependencies on the cache. + """ + # Opt-in only: skip the readiness gate entirely unless the caller + # either passed a `readiness` kwarg or set one in .percy.yml. Mirrors + # percy-selenium-python; until the CI-hang root cause is understood, + # default to no-op. + has_explicit_kwarg = 'readiness' in kwargs + has_global_config = bool( + (percy_config or {}).get('snapshot', {}).get('readiness') + if isinstance(percy_config, dict) else False) + if not has_explicit_kwarg and not has_global_config: + return None + readiness_config = _resolve_readiness_config(percy_config, kwargs) + if readiness_config.get('preset') == 'disabled': + return None + # Hard JS-side timeout: even though Playwright auto-awaits Promises, a + # waitForReady() that never resolves (e.g. CLI's internal observers + # keep ticking) would block the whole snapshot suite. Race against a + # Promise that resolves after the readiness deadline + 2s buffer. + timeout_ms = readiness_config.get('timeoutMs') + deadline_ms = int( + (timeout_ms if isinstance(timeout_ms, (int, float)) and timeout_ms > 0 else 10000) + + 2000 + ) + try: + return page.evaluate( + "([cfg, deadlineMs]) => {" + " if (typeof PercyDOM === 'undefined'" + " || typeof PercyDOM.waitForReady !== 'function') {" + " return null;" + " }" + " return Promise.race([" + " PercyDOM.waitForReady(cfg)," + " new Promise(resolve => setTimeout(resolve, deadlineMs))" + " ]);" + "}", + [readiness_config, deadline_ms], + ) + except Exception as e: + log(f'waitForReady failed, proceeding to serialize: {e}', 'debug') + return None + + +# pylint: disable=too-many-locals +def get_serialized_dom(page, cookies, percy_dom_script=None, *, + percy_config=None, skip_readiness=False, + readiness_diagnostics=None, **kwargs): """ Serializes the DOM and captures cross-origin iframes. Args: page: The page object cookies: Page cookies + percy_config: CLI healthcheck config (used for readiness merge) percy_dom_script: The Percy DOM serialization script + skip_readiness: Set True when the caller has already run readiness + once (e.g. responsive capture running it before the width loop) + to avoid paying the cost per width. + readiness_diagnostics: Diagnostics from the caller's earlier + readiness run, used when skip_readiness=True. **kwargs: Additional options Returns: Dictionary containing the DOM snapshot with cross-origin iframe data """ - dom_snapshot = page.evaluate(f"PercyDOM.serialize({json.dumps(kwargs)})") + # Readiness gate before serialize. Graceful on old CLI. + if not skip_readiness: + readiness_diagnostics = _wait_for_ready(page, percy_config, kwargs) + # Strip `readiness` from forwarded serialize args — it's consumed by + # _wait_for_ready upstream, not a PercyDOM.serialize argument. + serialize_kwargs = {k: v for k, v in kwargs.items() if k != 'readiness'} + dom_snapshot = page.evaluate(f"PercyDOM.serialize({json.dumps(serialize_kwargs)})") + # Attach readiness diagnostics so the CLI can log timing and pass/fail. + # `is not None` preserves legitimate falsy returns like {} ("gate ran, + # no notable diagnostics"). + if readiness_diagnostics is not None and isinstance(dom_snapshot, dict): + dom_snapshot['readiness_diagnostics'] = readiness_diagnostics # Process CORS IFrames # Note: Blob URL handling (data-src images, blob background images) is now handled @@ -316,6 +407,7 @@ def change_window_dimension_and_wait(page, width, height, resize_count): log(f"Timed out waiting for window resize event for width {width}", "debug") +# pylint: disable=too-many-locals def capture_responsive_dom(page, cookies, percy_dom_script=None, config=None, **kwargs): viewport = page.viewport_size or page.evaluate( "() => ({ width: window.innerWidth, height: window.innerHeight })" @@ -329,6 +421,9 @@ def capture_responsive_dom(page, cookies, percy_dom_script=None, config=None, ** last_window_width = viewport["width"] resize_count = 0 page.evaluate("PercyDOM.waitForResize()") + # Run readiness ONCE before the per-width loop. Running it per width can + # cost up to N*timeoutMs of sequential waits — almost never the intent. + responsive_readiness_diagnostics = _wait_for_ready(page, config, kwargs) for width_height in width_heights: # Apply default height if not provided by CLI @@ -348,7 +443,12 @@ def capture_responsive_dom(page, cookies, percy_dom_script=None, config=None, ** resize_count = 0 _responsive_sleep() - snapshot = get_serialized_dom(page, cookies, percy_dom_script, **kwargs) + snapshot = get_serialized_dom( + page, cookies, percy_dom_script, + percy_config=config, + skip_readiness=True, + readiness_diagnostics=responsive_readiness_diagnostics, + **kwargs) snapshot["width"] = width dom_snapshots.append(snapshot) @@ -400,13 +500,18 @@ def percy_snapshot(page, name, **kwargs): page, cookies, percy_dom_script, config=data["config"], **kwargs ) else: - dom_snapshot = get_serialized_dom(page, cookies, percy_dom_script, **kwargs) + dom_snapshot = get_serialized_dom( + page, cookies, percy_dom_script, + percy_config=data.get("config"), **kwargs) + # Strip `readiness` from POST body — SDK-local config that the CLI + # already has via healthcheck. + post_kwargs = {k: v for k, v in kwargs.items() if k != "readiness"} # Post the DOM to the snapshot endpoint with snapshot options and other info response = requests.post( f"{PERCY_CLI_API}/percy/snapshot", json={ - **kwargs, + **post_kwargs, **{ "client_info": CLIENT_INFO, "environment_info": ENV_INFO, diff --git a/tests/test_screenshot.py b/tests/test_screenshot.py index 2e31edf..48191e0 100644 --- a/tests/test_screenshot.py +++ b/tests/test_screenshot.py @@ -24,7 +24,9 @@ change_window_dimension_and_wait, get_serialized_dom, process_frame, - log + log, + _resolve_readiness_config, + _wait_for_ready, ) import percy.screenshot as local @@ -297,6 +299,132 @@ def test_raise_error_poa_token_with_snapshot(self): ) +class TestReadinessGate(unittest.TestCase): + """Unit tests for _wait_for_ready / _resolve_readiness_config using a + fully-mocked Page. Bypasses real Playwright CDP traffic, so cannot hang + on real in-page observers like the integration-style tests did.""" + + def test_resolve_readiness_config_shallow_merges(self): + merged = _resolve_readiness_config( + {'snapshot': {'readiness': {'preset': 'balanced', 'timeoutMs': 8000}}}, + {'readiness': {'stabilityWindowMs': 500}} + ) + self.assertEqual(merged, { + 'preset': 'balanced', 'timeoutMs': 8000, 'stabilityWindowMs': 500 + }) + + def test_resolve_readiness_config_per_snapshot_wins(self): + merged = _resolve_readiness_config( + {'snapshot': {'readiness': {'preset': 'balanced'}}}, + {'readiness': {'preset': 'strict'}} + ) + self.assertEqual(merged['preset'], 'strict') + + def test_resolve_readiness_config_handles_none_snapshot(self): + # Defensive: CLI healthcheck could return snapshot: null + merged = _resolve_readiness_config({'snapshot': None}, {}) + self.assertEqual(merged, {}) + + def test_resolve_readiness_config_handles_non_dict_inputs(self): + merged = _resolve_readiness_config( + {'snapshot': {'readiness': 'not-a-dict'}}, + {'readiness': 12345} + ) + self.assertEqual(merged, {}) + + def test_wait_for_ready_opt_in_skips_when_no_config(self): + page = MagicMock() + result = _wait_for_ready(page, percy_config={}, kwargs={}) + self.assertIsNone(result) + page.evaluate.assert_not_called() + + def test_wait_for_ready_runs_when_kwargs_opt_in(self): + diagnostics = {'passed': True, 'preset': 'balanced'} + page = MagicMock() + page.evaluate.return_value = diagnostics + + result = _wait_for_ready(page, percy_config={}, kwargs={'readiness': {}}) + + self.assertEqual(result, diagnostics) + self.assertEqual(page.evaluate.call_count, 1) + script, args = page.evaluate.call_args.args + self.assertIn('PercyDOM.waitForReady', script) + self.assertEqual(args[0], {}) # readiness config + self.assertEqual(args[1], 12000) # default deadline_ms (10000 + 2000) + + def test_wait_for_ready_runs_when_global_config_opts_in(self): + page = MagicMock() + page.evaluate.return_value = None + percy_config = {'snapshot': {'readiness': {'preset': 'balanced'}}} + + _wait_for_ready(page, percy_config=percy_config, kwargs={}) + + self.assertEqual(page.evaluate.call_count, 1) + + def test_wait_for_ready_skips_disabled_preset(self): + page = MagicMock() + result = _wait_for_ready( + page, percy_config={}, kwargs={'readiness': {'preset': 'disabled'}}) + self.assertIsNone(result) + page.evaluate.assert_not_called() + + def test_wait_for_ready_inlines_per_snapshot_config_into_args(self): + page = MagicMock() + page.evaluate.return_value = None + cfg = {'preset': 'strict', 'stabilityWindowMs': 500} + + _wait_for_ready(page, percy_config={}, kwargs={'readiness': cfg}) + + _, eval_args = page.evaluate.call_args.args + self.assertEqual(eval_args[0], cfg) + + def test_wait_for_ready_honors_timeoutMs_for_deadline(self): + page = MagicMock() + page.evaluate.return_value = None + + _wait_for_ready( + page, percy_config={}, + kwargs={'readiness': {'timeoutMs': 5000}}) + + _, eval_args = page.evaluate.call_args.args + # deadline_ms = timeoutMs + 2000 + self.assertEqual(eval_args[1], 7000) + + def test_wait_for_ready_swallows_exception_and_returns_none(self): + page = MagicMock() + page.evaluate.side_effect = RuntimeError('boom') + + with patch('percy.screenshot.log') as mock_log: + result = _wait_for_ready(page, percy_config={}, kwargs={'readiness': {}}) + + self.assertIsNone(result) + mock_log.assert_called_once() + self.assertIn('waitForReady failed', mock_log.call_args.args[0]) + + def test_get_serialized_dom_skips_readiness_when_flag_set(self): + """skip_readiness=True (responsive capture path) reuses the caller's + diagnostics instead of running readiness again per width.""" + page = MagicMock() + page.evaluate.return_value = {'html': ''} + page.url = 'http://localhost:8000/' + page.frames = [] + page.context.return_value.cookies.return_value = [] + + result = get_serialized_dom( + page, cookies=[], percy_config={}, + skip_readiness=True, + readiness_diagnostics={'cached': True}, + ) + + # Diagnostics from caller propagated, _wait_for_ready never invoked + self.assertEqual(result['readiness_diagnostics'], {'cached': True}) + # Only the serialize call hits evaluate + evaluate_scripts = [c.args[0] for c in page.evaluate.call_args_list] + self.assertFalse(any( + isinstance(s, str) and 'waitForReady' in s for s in evaluate_scripts + )) + + class TestPercyFunctions(unittest.TestCase): @patch("requests.get") def test_is_percy_enabled(self, mock_get):