From 5eb6aa72f62f270fefc6a1f9b7613ec88394ec65 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Mon, 20 Apr 2026 11:21:19 +0530 Subject: [PATCH 1/5] feat: PER-7348 add waitForReady() call before serialize() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/main/java/io/percy/selenium/Percy.java | 58 ++++++++++++++- src/test/java/io/percy/selenium/SdkTest.java | 76 ++++++++++++++++++++ 2 files changed, 133 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/percy/selenium/Percy.java b/src/main/java/io/percy/selenium/Percy.java index 453c4bf..9489566 100644 --- a/src/main/java/io/percy/selenium/Percy.java +++ b/src/main/java/io/percy/selenium/Percy.java @@ -598,6 +598,54 @@ private String buildSnapshotJS(Map options) { return jsBuilder.toString(); } + /** + * Readiness gate (PER-7348): 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. + * + * Readiness config precedence: options["readiness"] > cliConfig.snapshot.readiness + * > empty (CLI applies balanced default). "disabled" preset skips the + * executeAsyncScript call entirely. Any exception is swallowed at debug level; + * serialize still runs. + * + * @return Readiness diagnostics to attach to the domSnapshot, or null. + */ + protected Object waitForReady(JavascriptExecutor jse, Map options) { + Object perSnapshot = options != null ? options.get("readiness") : null; + JSONObject readinessConfig; + if (perSnapshot instanceof Map) { + readinessConfig = new JSONObject((Map) perSnapshot); + } else if (perSnapshot instanceof JSONObject) { + readinessConfig = (JSONObject) perSnapshot; + } else if (cliConfig != null) { + JSONObject snapshotConfig = cliConfig.optJSONObject("snapshot"); + readinessConfig = snapshotConfig == null ? new JSONObject() + : snapshotConfig.optJSONObject("readiness"); + if (readinessConfig == null) { readinessConfig = new JSONObject(); } + } else { + readinessConfig = new JSONObject(); + } + if ("disabled".equals(readinessConfig.optString("preset", null))) { + return null; + } + 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; + } + } + static class FatalIframeException extends RuntimeException { FatalIframeException(String message, Throwable cause) { super(message, cause); @@ -673,10 +721,18 @@ private Map processFrame(WebElement frameElement, Map getSerializedDOM(JavascriptExecutor jse, Set cookies, Map options) { + Map getSerializedDOM(JavascriptExecutor jse, Set cookies, Map options) { + // Readiness gate before serialize (PER-7348). Graceful on old CLI. + Object readinessDiagnostics = waitForReady(jse, options); + Map domSnapshot = (Map) jse.executeScript(buildSnapshotJS(options)); Map 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 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 index 7a92677..d891603 100644 --- 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 (PER-7348) ----------------------------------------- + + @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 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 domSnap = new HashMap<>(); + domSnap.put("html", ""); + when(((JavascriptExecutor) mockedDriver).executeScript(any(String.class))).thenReturn(domSnap); + + Map result = mockedPercy.getSerializedDOM( + (JavascriptExecutor) mockedDriver, new HashSet<>(), new HashMap<>()); + + // Readiness script was sent via executeAsyncScript + ArgumentCaptor 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 domSnap = new HashMap<>(); + domSnap.put("html", ""); + when(((JavascriptExecutor) mockedDriver).executeScript(any(String.class))).thenReturn(domSnap); + + Map disabled = new HashMap<>(); + disabled.put("preset", "disabled"); + Map options = new HashMap<>(); + options.put("readiness", disabled); + + Map 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 domSnap = new HashMap<>(); + domSnap.put("html", ""); + when(((JavascriptExecutor) mockedDriver).executeScript(any(String.class))).thenReturn(domSnap); + + Map result = mockedPercy.getSerializedDOM( + (JavascriptExecutor) mockedDriver, new HashSet<>(), new HashMap<>()); + + // Serialize still ran; no diagnostics attached + assertNull(result.get("readiness_diagnostics")); + assertEquals("", result.get("html")); + } + private static Object invokePrivate(Object target, String methodName, Class[] paramTypes, Object... args) throws Exception { Method method = Percy.class.getDeclaredMethod(methodName, paramTypes); From 7b7c759606ba49bf352c558832d11705ad4a4822 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Fri, 22 May 2026 12:15:08 +0530 Subject: [PATCH 2/5] fix: address ce:review findings on readiness gate (PER-7348) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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) --- src/main/java/io/percy/selenium/Percy.java | 80 +++++++++++++++++----- 1 file changed, 62 insertions(+), 18 deletions(-) diff --git a/src/main/java/io/percy/selenium/Percy.java b/src/main/java/io/percy/selenium/Percy.java index 9489566..12110f9 100644 --- a/src/main/java/io/percy/selenium/Percy.java +++ b/src/main/java/io/percy/selenium/Percy.java @@ -543,6 +543,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); @@ -593,6 +597,8 @@ protected JSONObject request(String url, JSONObject json, String name) { private String buildSnapshotJS(Map 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(); @@ -605,31 +611,36 @@ private String buildSnapshotJS(Map options) { * typeof PercyDOM.waitForReady === 'function' so older CLI versions that * lack the method are a graceful no-op. * - * Readiness config precedence: options["readiness"] > cliConfig.snapshot.readiness - * > empty (CLI applies balanced default). "disabled" preset skips the - * executeAsyncScript call entirely. Any exception is swallowed at debug level; - * serialize still runs. + * 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 options) { - Object perSnapshot = options != null ? options.get("readiness") : null; - JSONObject readinessConfig; - if (perSnapshot instanceof Map) { - readinessConfig = new JSONObject((Map) perSnapshot); - } else if (perSnapshot instanceof JSONObject) { - readinessConfig = (JSONObject) perSnapshot; - } else if (cliConfig != null) { - JSONObject snapshotConfig = cliConfig.optJSONObject("snapshot"); - readinessConfig = snapshotConfig == null ? new JSONObject() - : snapshotConfig.optJSONObject("readiness"); - if (readinessConfig == null) { readinessConfig = new JSONObject(); } - } else { - readinessConfig = new JSONObject(); - } + 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() + ";" @@ -643,7 +654,40 @@ protected Object waitForReady(JavascriptExecutor jse, Map option } 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 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) 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 { From a0dc0ad2e8438f0bec9ce213c5cff93f2d77ff2a Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Fri, 22 May 2026 16:40:42 +0530 Subject: [PATCH 3/5] ci: retrigger CI after upstream maven central resolution issue (PER-7348) From 8245bc44336c477a1c5fc2217d154002da64bb61 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Sun, 24 May 2026 22:12:55 +0530 Subject: [PATCH 4/5] 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. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 72d9dfe..f313640 100644 --- 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" } } From 3349f33f5f7a8432ae75f4536f25a7d00399e3c3 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Mon, 25 May 2026 13:37:30 +0530 Subject: [PATCH 5/5] comments: remove JIRA ticket reference from code comments --- src/main/java/io/percy/selenium/Percy.java | 4 ++-- src/test/java/io/percy/selenium/SdkTest.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/percy/selenium/Percy.java b/src/main/java/io/percy/selenium/Percy.java index 12110f9..a360a30 100644 --- a/src/main/java/io/percy/selenium/Percy.java +++ b/src/main/java/io/percy/selenium/Percy.java @@ -605,7 +605,7 @@ private String buildSnapshotJS(Map options) { } /** - * Readiness gate (PER-7348): runs PercyDOM.waitForReady BEFORE serialize. + * 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 @@ -766,7 +766,7 @@ private Map processFrame(WebElement frameElement, Map getSerializedDOM(JavascriptExecutor jse, Set cookies, Map options) { - // Readiness gate before serialize (PER-7348). Graceful on old CLI. + // Readiness gate before serialize. Graceful on old CLI. Object readinessDiagnostics = waitForReady(jse, options); Map domSnapshot = (Map) jse.executeScript(buildSnapshotJS(options)); diff --git a/src/test/java/io/percy/selenium/SdkTest.java b/src/test/java/io/percy/selenium/SdkTest.java index d891603..1f1a0f5 100644 --- a/src/test/java/io/percy/selenium/SdkTest.java +++ b/src/test/java/io/percy/selenium/SdkTest.java @@ -1109,7 +1109,7 @@ public void captureResponsiveDomRefreshesDriverForEachWidthWhenReloadFlagSet() t } } - // --- Readiness gate (PER-7348) ----------------------------------------- + // --- Readiness gate ----------------------------------------- @Test public void readinessRunsBeforeSerializeAndAttachesDiagnostics() throws Exception {