feat: PER-7348 add waitForReady() call before serialize()

.python-version

@@ -1 +1 @@
-3.10.3
+3.10

package.json

@@ -4,6 +4,6 @@
     "test": "make test"
   },
   "devDependencies": {
-    "@percy/cli": "1.30.9"
+    "@percy/cli": "^1.31.14"
   }
 }

percy/snapshot.py

@@ -191,9 +191,127 @@ def _get_origin(url):
     parsed = urlparse(url)
     return f"{parsed.scheme}://{parsed.netloc}"
 
-def get_serialized_dom(driver, cookies, percy_dom_script=None, **kwargs):
+def _resolve_readiness_config(percy_config, kwargs):
+    """Shallow-merge global (.percy.yml) readiness config with per-snapshot
+    override. Per-snapshot keys win; unspecified keys are inherited.
+
+    Defensive against `config.snapshot` being None or non-dict — the CLI is
+    free to evolve its healthcheck payload and `None` should degrade to `{}`,
+    not raise AttributeError mid-snapshot."""
+    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(driver, percy_config, kwargs):
+    """Run readiness checks before serialize.
+
+    Sends PercyDOM.waitForReady via execute_async_script. The script checks
+    typeof PercyDOM.waitForReady in-browser so older CLI versions without the
+    method are a graceful no-op. Any failure is caught and logged at debug;
+    serialize still runs.
+
+    Returns readiness diagnostics (or None) so callers can attach it
+    to the domSnapshot for CLI-side logging.
+
+    Config precedence: per-snapshot `kwargs['readiness']` shallow-merged
+    over global `percy_config.snapshot.readiness`; per-snapshot keys win,
+    unspecified keys (e.g. a global `preset: disabled` kill switch) are
+    inherited. Skips entirely when the merged preset is 'disabled'.
+
+    The caller must 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.
+    """
+    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
+    # Match readiness.timeoutMs to the driver's async-script timeout so a
+    # higher user-configured readiness timeout isn't silently capped by
+    # WebDriver's default (~30s on Selenium 4, lower on some remotes).
+    timeout_ms = readiness_config.get('timeoutMs')
+    previous_timeout = None
+    if isinstance(timeout_ms, (int, float)) and timeout_ms > 0:
+        try:
+            # Selenium 4 exposes driver.timeouts.script (float seconds).
+            previous_timeout = getattr(driver.timeouts, 'script', None)
+            driver.set_script_timeout(timeout_ms / 1000 + 2)  # +2s buffer
+        except Exception:
+            previous_timeout = None  # older Selenium / unsupported — best effort
+    # JS-side hard timeout: geckodriver does not reliably honor selenium's
+    # script_timeout for async scripts whose pending work lives in microtasks
+    # (Promise.then chains), so tests can hang indefinitely. Wrap done() in
+    # a once-only guard and arm a setTimeout that calls it after the
+    # readiness deadline + 2s buffer, regardless of what waitForReady does.
+    deadline_ms = int((timeout_ms if isinstance(timeout_ms, (int, float)) and timeout_ms > 0
+                       else 10000) + 2000)
+    try:
+        # done() must be called ASYNCHRONOUSLY for execute_async_script to
+        # unblock — calling it synchronously within the script's body has
+        # historically hung geckodriver in CI for hours. fireDone() wraps
+        # done() in setTimeout(_, 0) so every code path defers the callback
+        # to the next event-loop tick.
+        diagnostics = driver.execute_async_script(
+            'var config = ' + json.dumps(readiness_config) + ';'
+            'var done = arguments[arguments.length - 1];'
+            'var doneFired = false;'
+            'function fireDone(v) {'
+            '  if (doneFired) return;'
+            '  doneFired = true;'
+            '  setTimeout(function() { done(v); }, 0);'
+            '}'
+            'setTimeout(function() { fireDone(); }, ' + str(deadline_ms) + ');'
+            'try {'
+            "  if (typeof PercyDOM !== 'undefined'"
+            "      && typeof PercyDOM.waitForReady === 'function') {"
+            '    PercyDOM.waitForReady(config)'
+            '      .then(function(r){ fireDone(r); })'
+            '      .catch(function(){ fireDone(); });'
+            '  } else { fireDone(); }'
+            '} catch(e) { fireDone(); }'
+        )
+        return diagnostics
+    except Exception as e:
+        log(f'waitForReady failed, proceeding to serialize: {e}', 'debug')
+        return None
+    finally:
+        if previous_timeout is not None:
+            try:
+                driver.set_script_timeout(previous_timeout)
+            except Exception:
+                pass
+
+
+def get_serialized_dom(driver, cookies, percy_config=None, percy_dom_script=None,
+                       skip_readiness=False, readiness_diagnostics=None, **kwargs):
+    # 0. Readiness gate before serialize. Graceful on old CLI.
+    #    `skip_readiness` lets responsive capture run readiness once before the
+    #    width loop and pass diagnostics through, instead of paying the cost
+    #    per width.
+    if not skip_readiness:
+        readiness_diagnostics = _wait_for_ready(driver, percy_config, kwargs)
+    # Strip `readiness` from kwargs before forwarding — it's an SDK-local
+    # concern; the CLI already has it from healthcheck and a top-level
+    # `readiness` in the POST body is brittle against future validators.
+    kwargs.pop('readiness', None)
     # 1. Serialize the main page first (this adds the data-percy-element-ids)
     dom_snapshot = driver.execute_script(f'return PercyDOM.serialize({json.dumps(kwargs)})')
+    # Attach readiness diagnostics so the CLI can log timing and pass/fail.
+    # `is not None` preserves legitimate falsy returns (e.g. `{}` meaning
+    # "gate ran, no notable diagnostics").
+    if readiness_diagnostics is not None and isinstance(dom_snapshot, dict):
+        dom_snapshot['readiness_diagnostics'] = readiness_diagnostics
     # 2. Process CORS IFrames
     try:
         page_origin = _get_origin(driver.current_url)
@@ -307,6 +425,12 @@ def capture_responsive_dom(driver, cookies, config, percy_dom_script=None, **kwa
     resize_count = 0
     # Initialize resize listener once before the loop
     driver.execute_script("PercyDOM.waitForResize()")
+    # Run readiness ONCE before the per-width loop. With N widths and a
+    # `timeoutMs` of e.g. 10s, running readiness per width can cost up to
+    # N*timeout seconds of sequential waits — almost always undesirable.
+    # Per-width DOM mutations after viewport changes are handled by the
+    # `waitForResize` instrumentation above, not by re-running readiness.
+    responsive_readiness_diagnostics = _wait_for_ready(driver, config, kwargs)
     target_height = current_height
 
     if PERCY_RESPONSIVE_CAPTURE_MIN_HEIGHT:
@@ -339,7 +463,11 @@ def capture_responsive_dom(driver, cookies, config, percy_dom_script=None, **kwa
         print(f'{width}x{height} ready, taking snapshot...')
         _responsive_sleep()
         dom_snapshot = get_serialized_dom(
-            driver, cookies, percy_dom_script=percy_dom_script, **kwargs)
+            driver, cookies, percy_config=config,
+            percy_dom_script=percy_dom_script,
+            skip_readiness=True,
+            readiness_diagnostics=responsive_readiness_diagnostics,
+            **kwargs)
         dom_snapshot['width'] = width
         print(f'Taken snapshot for width: {width}, height: {height}')
         dom_snapshots.append(dom_snapshot)
@@ -383,10 +511,15 @@ def percy_snapshot(driver, name, **kwargs):
             )
         else:
             dom_snapshot = get_serialized_dom(
-                driver, cookies, percy_dom_script=percy_dom_script, **kwargs)
+                driver, cookies, percy_config=data.get('config'),
+                percy_dom_script=percy_dom_script, **kwargs)
 
+        # Strip SDK-local `readiness` from the snapshot POST body. The CLI
+        # already has it via healthcheck; sending it again here risks future
+        # CLI-side validators rejecting unknown top-level fields.
+        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, **{
+        response = requests.post(f'{PERCY_CLI_API}/percy/snapshot', json={**post_kwargs, **{
             'client_info': CLIENT_INFO,
             'environment_info': ENV_INFO,
             'dom_snapshot': dom_snapshot,

tests/test_snapshot.py

@@ -13,7 +13,12 @@
 from selenium.webdriver.remote.webelement import WebElement
 from selenium.webdriver.remote.remote_connection import RemoteConnection
 from selenium.webdriver.safari.remote_connection import SafariRemoteConnection
-from percy.snapshot import create_region
+from percy.snapshot import (
+    create_region,
+    _resolve_readiness_config,
+    _wait_for_ready,
+    get_serialized_dom,
+)
 
 from percy import percy_snapshot, percySnapshot, percy_screenshot
 import percy.snapshot as local
@@ -119,6 +124,7 @@ def mock_screenshot(fail=False, data=False):
         }),
         status=(500 if fail else 200))
 
+# pylint: disable=too-many-public-methods
 class TestPercySnapshot(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
@@ -472,6 +478,143 @@ def test_raise_error_poa_token_with_snapshot(self):
         " For more information on usage of PercyScreenshot, refer https://www.browserstack.com"\
         "/docs/percy/integrate/functional-and-visual", str(context.exception))
 
+
+class TestReadinessGate(unittest.TestCase):
+    """Unit tests for _wait_for_ready / _resolve_readiness_config using a
+    fully-mocked WebDriver. Bypasses real geckodriver/Firefox 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):
+        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):
+        driver = Mock()
+        result = _wait_for_ready(driver, percy_config={}, kwargs={})
+        self.assertIsNone(result)
+        driver.execute_async_script.assert_not_called()
+
+    def test_wait_for_ready_runs_when_kwargs_opt_in(self):
+        diagnostics = {'passed': True, 'preset': 'balanced'}
+        driver = Mock()
+        driver.execute_async_script.return_value = diagnostics
+        driver.timeouts.script = 30
+
+        result = _wait_for_ready(driver, percy_config={}, kwargs={'readiness': {}})
+
+        self.assertEqual(result, diagnostics)
+        self.assertEqual(driver.execute_async_script.call_count, 1)
+        script = driver.execute_async_script.call_args.args[0]
+        self.assertIn('PercyDOM.waitForReady', script)
+        self.assertIn('typeof PercyDOM', script)
+
+    def test_wait_for_ready_runs_when_global_config_opts_in(self):
+        driver = Mock()
+        driver.execute_async_script.return_value = None
+        driver.timeouts.script = 30
+        percy_config = {'snapshot': {'readiness': {'preset': 'balanced'}}}
+
+        _wait_for_ready(driver, percy_config=percy_config, kwargs={})
+
+        self.assertEqual(driver.execute_async_script.call_count, 1)
+
+    def test_wait_for_ready_skips_disabled_preset(self):
+        driver = Mock()
+        result = _wait_for_ready(
+            driver, percy_config={}, kwargs={'readiness': {'preset': 'disabled'}})
+        self.assertIsNone(result)
+        driver.execute_async_script.assert_not_called()
+
+    def test_wait_for_ready_inlines_per_snapshot_config_into_script(self):
+        driver = Mock()
+        driver.execute_async_script.return_value = None
+        driver.timeouts.script = 30
+        cfg = {'preset': 'strict', 'stabilityWindowMs': 500}
+
+        _wait_for_ready(driver, percy_config={}, kwargs={'readiness': cfg})
+
+        script = driver.execute_async_script.call_args.args[0]
+        self.assertIn('"preset": "strict"', script)
+        self.assertIn('"stabilityWindowMs": 500', script)
+
+    def test_wait_for_ready_sets_and_restores_script_timeout(self):
+        driver = Mock()
+        driver.execute_async_script.return_value = None
+        driver.timeouts.script = 30  # selenium 4 default (seconds)
+
+        _wait_for_ready(
+            driver, percy_config={},
+            kwargs={'readiness': {'timeoutMs': 5000}})
+
+        # set to readiness.timeoutMs/1000 + 2s buffer, then restored
+        driver.set_script_timeout.assert_any_call(7)  # 5000/1000 + 2
+        driver.set_script_timeout.assert_any_call(30)  # restored
+
+    def test_wait_for_ready_swallows_exception_and_returns_none(self):
+        driver = Mock()
+        driver.execute_async_script.side_effect = RuntimeError('boom')
+
+        with patch('percy.snapshot.log') as mock_log:
+            result = _wait_for_ready(driver, 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_pops_readiness_from_serialize_call(self):
+        """`readiness` is SDK-local; PercyDOM.serialize must not see it."""
+        driver = Mock()
+        driver.execute_script.return_value = {'html': '<html></html>'}
+        driver.execute_async_script.return_value = None
+        driver.current_url = 'http://localhost:8000/'
+        driver.get_cookies.return_value = []
+
+        get_serialized_dom(driver, cookies=[], percy_config={},
+                           readiness={'preset': 'balanced'})
+
+        serialize_call = next(
+            c for c in driver.execute_script.call_args_list
+            if 'PercyDOM.serialize' in c.args[0]
+        )
+        self.assertNotIn('readiness', serialize_call.args[0])
+
+    def test_get_serialized_dom_attaches_diagnostics(self):
+        driver = Mock()
+        driver.execute_script.return_value = {'html': '<html></html>'}
+        driver.execute_async_script.return_value = {'passed': True}
+        driver.current_url = 'http://localhost:8000/'
+        driver.get_cookies.return_value = []
+
+        dom_snapshot = get_serialized_dom(
+            driver, cookies=[], percy_config={}, readiness={'preset': 'balanced'})
+
+        self.assertEqual(dom_snapshot['readiness_diagnostics'], {'passed': True})
+
 class TestPercyScreenshot(unittest.TestCase):
     @classmethod
     def setUpClass(cls):