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

Shivanshu-07 · claude · web-flow · commit 39e6deb3facb · 2026-05-27T11:01:48.000+05:30
* feat: PER-7348 add waitForReady() call before serialize() Adds the readiness gate from percy/cli#2184. New waitForReady() helper runs PercyDOM.waitForReady via executeAsyncScript (callback signal) before the existing PercyDOM.serialize executeScript inside getSerializedDOM. Diagnostics are attached to the mutable snapshot as readiness_diagnostics. serialize is unchanged. Config precedence: options['readiness'] > cliConfig.snapshot.readiness > empty. Backward compat via in-browser typeof guard. Disabled preset short-circuits. Graceful on exception. Visibility: getSerializedDOM is now package-private so tests can call it directly; it was previously private. Tests (Mockito): diagnostics attached + readiness script contains waitForReady, disabled preset skips executeAsyncScript, readiness throw leaves serialize intact. Local: mvn test → 3 passed, 0 failed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: address ce:review findings on readiness gate (PER-7348) - Replaced exclusive precedence (`else if`) with shallow-merge in a new `resolveReadinessConfig()` helper. Per-snapshot keys win, global keys inherited — a partial per-snapshot override no longer silently drops a global `preset: disabled` kill switch. - Set driver script timeout to match `readiness.timeoutMs` (+ 2s buffer) around the executeAsyncScript call, with finally-restore. WebDriver's default ~30s script timeout was silently capping higher user-configured readiness timeouts via ScriptTimeoutException. - Strip `readiness` from `buildSnapshotJS` (so it doesn't leak into PercyDOM.serialize args) and from `postSnapshot` JSON (so it doesn't round-trip to the CLI which already has it via healthcheck). `mvn compile` passes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: retrigger CI after upstream maven central resolution issue (PER-7348) * chore: bump @percy/cli to ^1.31.15-beta.0 in tests (PER-7348) CLI 1.31.15-beta.0 ships PercyDOM.waitForReady (the readiness gate). The SDK changes in this PR call waitForReady end-to-end in tests; old CLI pins (1.30.9, 1.31.10) don't have it, causing the typeof guard's done() callback path to never quite settle in geckodriver's async-script handling. Bump so tests run against a CLI that actually has the feature. * comments: remove JIRA ticket reference from code comments --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
diff --git a/package.json b/package.json
@@ -4,6 +4,6 @@
     "test": "npx percy exec --testing -- mvn test"
   },
   "devDependencies": {
-    "@percy/cli": "1.31.10"
+    "@percy/cli": "^1.31.15-beta.0"
   }
 }
diff --git a/src/main/java/io/percy/selenium/Percy.java b/src/main/java/io/percy/selenium/Percy.java
@@ -564,6 +564,10 @@ private JSONObject postSnapshot(
 
         // Build a JSON object to POST back to the agent node process
         JSONObject json = new JSONObject(options);
+        // `readiness` is SDK-local — the CLI already has it via healthcheck.
+        // Strip before posting to avoid round-tripping and to stay forward-
+        // compatible with future CLI-side validators rejecting unknown fields.
+        json.remove("readiness");
         json.put("url", url);
         json.put("name", name);
         json.put("domSnapshot", domSnapshot);
@@ -614,11 +618,99 @@ protected JSONObject request(String url, JSONObject json, String name) {
     private String buildSnapshotJS(Map<String, Object> options) {
         StringBuilder jsBuilder = new StringBuilder();
         JSONObject json = new JSONObject(options);
+        // `readiness` is consumed by waitForReady upstream — not a serialize arg.
+        json.remove("readiness");
         jsBuilder.append(String.format("return PercyDOM.serialize(%s)\n", json.toString()));
 
         return jsBuilder.toString();
     }
 
+    /**
+     * Readiness gate: runs PercyDOM.waitForReady BEFORE serialize.
+     *
+     * Uses executeAsyncScript with a callback signal. The embedded JS checks
+     * typeof PercyDOM.waitForReady === 'function' so older CLI versions that
+     * lack the method are a graceful no-op.
+     *
+     * Config precedence: per-snapshot options["readiness"] is shallow-merged
+     * over cliConfig.snapshot.readiness so a partial per-snapshot override
+     * inherits global keys (notably preset: disabled) instead of dropping
+     * them. The merged "disabled" preset skips the executeAsyncScript entirely.
+     *
+     * @return Readiness diagnostics to attach to the domSnapshot, or null.
+     */
+    protected Object waitForReady(JavascriptExecutor jse, Map<String, Object> options) {
+        JSONObject readinessConfig = resolveReadinessConfig(options);
+        if ("disabled".equals(readinessConfig.optString("preset", null))) {
+            return null;
+        }
+        // Match the driver's async-script timeout to readiness.timeoutMs so
+        // a higher user-configured timeout isn't silently capped by WebDriver
+        // firing ScriptTimeoutException before the in-page Promise resolves.
+        long timeoutMs = readinessConfig.optLong("timeoutMs", 0L);
+        Duration previousTimeout = null;
+        if (timeoutMs > 0) {
+            try {
+                previousTimeout = jse instanceof org.openqa.selenium.WebDriver
+                        ? ((org.openqa.selenium.WebDriver) jse).manage().timeouts().getScriptTimeout()
+                        : null;
+                if (jse instanceof org.openqa.selenium.WebDriver) {
+                    ((org.openqa.selenium.WebDriver) jse).manage().timeouts()
+                            .scriptTimeout(Duration.ofMillis(timeoutMs + 2000L));
+                }
+            } catch (Exception e) {
+                previousTimeout = null; // best-effort; older Selenium / unsupported
+            }
+        }
+        try {
+            String script =
+                "var cfg = " + readinessConfig.toString() + ";"
+                + "var done = arguments[arguments.length - 1];"
+                + "try {"
+                + "  if (typeof PercyDOM !== 'undefined' && typeof PercyDOM.waitForReady === 'function') {"
+                + "    PercyDOM.waitForReady(cfg).then(function(r){ done(r); }).catch(function(){ done(); });"
+                + "  } else { done(); }"
+                + "} catch (e) { done(); }";
+            return jse.executeAsyncScript(script);
+        } catch (Exception e) {
+            log("waitForReady failed, proceeding to serialize: " + e.getMessage(), "debug");
+            return null;
+        } finally {
+            if (previousTimeout != null && jse instanceof org.openqa.selenium.WebDriver) {
+                try {
+                    ((org.openqa.selenium.WebDriver) jse).manage().timeouts()
+                            .scriptTimeout(previousTimeout);
+                } catch (Exception ignored) { /* best effort */ }
+            }
+        }
+    }
+
+    /**
+     * Shallow-merge of global (cliConfig.snapshot.readiness) and per-snapshot
+     * (options["readiness"]) readiness config. Per-snapshot keys win, global
+     * keys are inherited. Defensive against null / wrong-type values.
+     */
+    @SuppressWarnings("unchecked")
+    private JSONObject resolveReadinessConfig(Map<String, Object> options) {
+        JSONObject merged = new JSONObject();
+        if (cliConfig != null) {
+            JSONObject snapshotConfig = cliConfig.optJSONObject("snapshot");
+            JSONObject global = snapshotConfig == null ? null : snapshotConfig.optJSONObject("readiness");
+            if (global != null) {
+                for (String key : global.keySet()) merged.put(key, global.get(key));
+            }
+        }
+        Object perSnapshot = options != null ? options.get("readiness") : null;
+        if (perSnapshot instanceof Map) {
+            JSONObject perJson = new JSONObject((Map<String, Object>) perSnapshot);
+            for (String key : perJson.keySet()) merged.put(key, perJson.get(key));
+        } else if (perSnapshot instanceof JSONObject) {
+            JSONObject perJson = (JSONObject) perSnapshot;
+            for (String key : perJson.keySet()) merged.put(key, perJson.get(key));
+        }
+        return merged;
+    }
+
     static class FatalIframeException extends RuntimeException {
         FatalIframeException(String message, Throwable cause) {
             super(message, cause);
@@ -694,10 +786,18 @@ private Map<String, Object> processFrame(WebElement frameElement, Map<String, Ob
         return result;
     }
 
-    private Map<String, Object> getSerializedDOM(JavascriptExecutor jse, Set<Cookie> cookies, Map<String, Object> options) {
+    Map<String, Object> getSerializedDOM(JavascriptExecutor jse, Set<Cookie> cookies, Map<String, Object> options) {
+        // Readiness gate before serialize. Graceful on old CLI.
+        Object readinessDiagnostics = waitForReady(jse, options);
+
         Map<String, Object> domSnapshot = (Map<String, Object>) jse.executeScript(buildSnapshotJS(options));
         Map<String, Object> mutableSnapshot = new HashMap<>(domSnapshot);
         mutableSnapshot.put("cookies", cookies);
+
+        // Attach readiness diagnostics so the CLI can log timing and pass/fail
+        if (readinessDiagnostics != null) {
+            mutableSnapshot.put("readiness_diagnostics", readinessDiagnostics);
+        }
         try {
             String pageOrigin = getOrigin(driver.getCurrentUrl());
             List<WebElement> iframes = driver.findElements(By.tagName("iframe"));
diff --git a/src/test/java/io/percy/selenium/SdkTest.java b/src/test/java/io/percy/selenium/SdkTest.java
@@ -1109,6 +1109,82 @@ public void captureResponsiveDomRefreshesDriverForEachWidthWhenReloadFlagSet() t
         }
     }
 
+    // --- Readiness gate -----------------------------------------
+
+    @Test
+    public void readinessRunsBeforeSerializeAndAttachesDiagnostics() throws Exception {
+      RemoteWebDriver mockedDriver = mock(RemoteWebDriver.class);
+      Percy mockedPercy = new Percy(mockedDriver);
+      setField(mockedPercy, "isPercyEnabled", true);
+      setField(mockedPercy, "cliConfig", new JSONObject().put("snapshot", new JSONObject()));
+
+      Map<String, Object> diagnostics = new HashMap<>();
+      diagnostics.put("ok", true);
+      diagnostics.put("timed_out", false);
+      // executeAsyncScript (readiness)
+      when(((JavascriptExecutor) mockedDriver).executeAsyncScript(any(String.class))).thenReturn(diagnostics);
+      // executeScript (serialize + any other sync scripts)
+      Map<String, Object> domSnap = new HashMap<>();
+      domSnap.put("html", "<html></html>");
+      when(((JavascriptExecutor) mockedDriver).executeScript(any(String.class))).thenReturn(domSnap);
+
+      Map<String, Object> result = mockedPercy.getSerializedDOM(
+          (JavascriptExecutor) mockedDriver, new HashSet<>(), new HashMap<>());
+
+      // Readiness script was sent via executeAsyncScript
+      ArgumentCaptor<String> scriptCap = ArgumentCaptor.forClass(String.class);
+      verify((JavascriptExecutor) mockedDriver, atLeastOnce()).executeAsyncScript(scriptCap.capture());
+      assertTrue(scriptCap.getValue().contains("waitForReady"),
+          "readiness script should mention waitForReady");
+      // Diagnostics propagated to the snapshot
+      assertEquals(diagnostics, result.get("readiness_diagnostics"));
+    }
+
+    @Test
+    public void readinessSkippedWhenPresetDisabled() throws Exception {
+      RemoteWebDriver mockedDriver = mock(RemoteWebDriver.class);
+      Percy mockedPercy = new Percy(mockedDriver);
+      setField(mockedPercy, "isPercyEnabled", true);
+
+      Map<String, Object> domSnap = new HashMap<>();
+      domSnap.put("html", "<html></html>");
+      when(((JavascriptExecutor) mockedDriver).executeScript(any(String.class))).thenReturn(domSnap);
+
+      Map<String, Object> disabled = new HashMap<>();
+      disabled.put("preset", "disabled");
+      Map<String, Object> options = new HashMap<>();
+      options.put("readiness", disabled);
+
+      Map<String, Object> result = mockedPercy.getSerializedDOM(
+          (JavascriptExecutor) mockedDriver, new HashSet<>(), options);
+
+      // executeAsyncScript must NOT have been called
+      verify((JavascriptExecutor) mockedDriver, never()).executeAsyncScript(any(String.class));
+      // serialize still ran; no diagnostics attached
+      assertNull(result.get("readiness_diagnostics"));
+    }
+
+    @Test
+    public void snapshotSurvivesReadinessThrow() throws Exception {
+      RemoteWebDriver mockedDriver = mock(RemoteWebDriver.class);
+      Percy mockedPercy = new Percy(mockedDriver);
+      setField(mockedPercy, "isPercyEnabled", true);
+      setField(mockedPercy, "cliConfig", new JSONObject().put("snapshot", new JSONObject()));
+
+      when(((JavascriptExecutor) mockedDriver).executeAsyncScript(any(String.class)))
+          .thenThrow(new RuntimeException("readiness boom"));
+      Map<String, Object> domSnap = new HashMap<>();
+      domSnap.put("html", "<html></html>");
+      when(((JavascriptExecutor) mockedDriver).executeScript(any(String.class))).thenReturn(domSnap);
+
+      Map<String, Object> result = mockedPercy.getSerializedDOM(
+          (JavascriptExecutor) mockedDriver, new HashSet<>(), new HashMap<>());
+
+      // Serialize still ran; no diagnostics attached
+      assertNull(result.get("readiness_diagnostics"));
+      assertEquals("<html></html>", result.get("html"));
+    }
+
     private static Object invokePrivate(Object target, String methodName, Class<?>[] paramTypes, Object... args)
         throws Exception {
       Method method = Percy.class.getDeclaredMethod(methodName, paramTypes);

Original file line number	Diff line number	Diff line change
`@@ -4,6 +4,6 @@`
`4`	`4`	`"test": "npx percy exec --testing -- mvn test"`
`5`	`5`	`},`
`6`	`6`	`"devDependencies": {`
`7`		`- "@percy/cli": "1.31.10"`
	`7`	`+ "@percy/cli": "^1.31.15-beta.0"`
`8`	`8`	`}`
`9`	`9`	`}`