Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
471dfa5
feat: PER-7348 add waitForReady() call before serialize()
Shivanshu-07 Apr 20, 2026
ded5420
fix: address ce:review findings on readiness gate (PER-7348)
Shivanshu-07 May 22, 2026
d79060f
fix: pylint offenses on PER-7348 changes
Shivanshu-07 May 22, 2026
94b1554
fix: pylint W0621 redefined-outer-name in test_screenshot (PER-7348)
Shivanshu-07 May 22, 2026
734e98b
chore: bump @percy/cli to ^1.31.15-beta.0 in tests (PER-7348)
Shivanshu-07 May 24, 2026
cea4d60
fix: hard JS-side timeout on readiness page.evaluate (PER-7348)
Shivanshu-07 May 24, 2026
511df17
fix: opt-in only — skip readiness when no config provided (PER-7348)
Shivanshu-07 May 25, 2026
342e217
fix: opt-in by kwarg presence + test deadline_ms arg (PER-7348)
Shivanshu-07 May 25, 2026
c744573
fix: stub page.evaluate via side_effect in readiness tests (PER-7348)
Shivanshu-07 May 25, 2026
c4a3837
test: skip readiness tests in playwright-python (PER-7348)
Shivanshu-07 May 25, 2026
f4f5e44
test: exclude readiness blocks from coverage in playwright-python
Shivanshu-07 May 25, 2026
c50ff7f
test: no-branch pragma on readiness skip in playwright-python
Shivanshu-07 May 25, 2026
5eed0ea
lint: break long line for pylint line-too-long
Shivanshu-07 May 25, 2026
b0faf54
comments: remove JIRA ticket reference from code comments
Shivanshu-07 May 25, 2026
9de71c3
ci: pin .python-version to 3.10 (was 3.10.3, unavailable on Ubuntu 24…
Shivanshu-07 May 25, 2026
e06ce19
test: replace skipped readiness tests with mock-based unit tests
Shivanshu-07 May 25, 2026
eef7c35
lint: move readiness-test imports to top of file
Shivanshu-07 May 25, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .python-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.10.3
3.10
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
"test": "make test"
},
"devDependencies": {
"@percy/cli": "^1.31.10"
"@percy/cli": "^1.31.15-beta.0"
}
}
115 changes: 110 additions & 5 deletions percy/screenshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 })"
Expand All @@ -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
Expand All @@ -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)

Expand Down Expand Up @@ -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,
Expand Down
130 changes: 129 additions & 1 deletion tests/test_screenshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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': '<html></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):
Expand Down
Loading