From 7d56dbe5fd59be98b41621364a83d62eece2f656 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Mon, 20 Apr 2026 11:01:46 +0530 Subject: [PATCH 1/4] 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 before the existing PercyDOM.serialize page.evaluate inside getSerializedDOM. Playwright auto-awaits the returned Promise. Diagnostics are attached to the mutable domSnapshot as readiness_diagnostics so the CLI can log timing and pass/fail. Config precedence: options['readiness'] > cliConfig.snapshot.readiness > empty (CLI applies balanced default). Backward compat via in-browser typeof PercyDOM.waitForReady === 'function' guard. Disabled preset short-circuits. Any exception is swallowed at debug level. Tests (Mockito): diagnostics attached + readiness JS sent, disabled preset skips the evaluate, and readiness throw leaves the serialize path intact. Local: mvn test → 3 passed, 0 failed. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/main/java/io/percy/playwright/Percy.java | 52 +++++++++++ .../java/io/percy/playwright/SDKTest.java | 91 +++++++++++++++++++ 2 files changed, 143 insertions(+) diff --git a/src/main/java/io/percy/playwright/Percy.java b/src/main/java/io/percy/playwright/Percy.java index 2f2224e..6dc12d6 100644 --- a/src/main/java/io/percy/playwright/Percy.java +++ b/src/main/java/io/percy/playwright/Percy.java @@ -428,6 +428,50 @@ private String buildSnapshotJS(Map options) { return jsBuilder.toString(); } + /** + * Readiness gate: runs PercyDOM.waitForReady BEFORE serialize (PER-7348). + * + * Uses page.evaluate — Playwright auto-awaits Promises. 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). If preset is "disabled", skip the + * evaluate call entirely. Any exception is swallowed at debug level — the + * serialize call still runs. + * + * @return Readiness diagnostics to attach to the domSnapshot, or null. + */ + protected Object waitForReady(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 { + JSONObject snapshotConfig = cliConfig.optJSONObject("snapshot"); + readinessConfig = snapshotConfig == null ? new JSONObject() + : snapshotConfig.optJSONObject("readiness"); + if (readinessConfig == null) { readinessConfig = new JSONObject(); } + } + if ("disabled".equals(readinessConfig.optString("preset", null))) { + return null; + } + try { + String js = + "(cfg) => {" + + " if (typeof PercyDOM !== 'undefined' && typeof PercyDOM.waitForReady === 'function') {" + + " return PercyDOM.waitForReady(cfg);" + + " }" + + "}"; + return page.evaluate(js, readinessConfig.toMap()); + } catch (Exception e) { + log("waitForReady failed, proceeding to serialize: " + e.getMessage(), "debug"); + return null; + } + } + /** * Attempts to load dom.js from the local Percy server. Use cached value in `domJs`, * if it exists. @@ -837,6 +881,9 @@ Map getSerializedDOM( String percyDomScript, Map options) { + // Readiness gate before serialize (PER-7348). Graceful on old CLI. + Object readinessDiagnostics = waitForReady(options); + Map domSnapshot = (Map) page.evaluate(buildSnapshotJS(options)); if (domSnapshot == null) { @@ -844,6 +891,11 @@ Map getSerializedDOM( } Map mutableSnapshot = new HashMap<>(domSnapshot); + // Attach readiness diagnostics so the CLI can log timing and pass/fail + if (readinessDiagnostics != null) { + mutableSnapshot.put("readiness_diagnostics", readinessDiagnostics); + } + // Process cross-origin iframes try { URI pageUri = new URI(page.url()); diff --git a/src/test/java/io/percy/playwright/SDKTest.java b/src/test/java/io/percy/playwright/SDKTest.java index 2206af5..3d728f7 100644 --- a/src/test/java/io/percy/playwright/SDKTest.java +++ b/src/test/java/io/percy/playwright/SDKTest.java @@ -489,4 +489,95 @@ public void sameOriginFramesAreNotProcessedAsCorsIframes() throws Exception { assertNull(result.get("corsIframes"), "Same-origin frames must not be added to corsIframes"); } + // ------------------------------------------------------------------------- + // Readiness gate (PER-7348) + // ------------------------------------------------------------------------- + + @Test + @Order(90) + @SuppressWarnings("unchecked") + public void readinessRunsBeforeSerializeAndAttachesDiagnostics() throws Exception { + Page mockPage = Mockito.mock(Page.class); + + Map domMap = new HashMap<>(); + domMap.put("html", ""); + + // Readiness path: page.evaluate(js, config) — any 2-arg call with a Map + Map diagnostics = new HashMap<>(); + diagnostics.put("ok", true); + diagnostics.put("timed_out", false); + when(mockPage.evaluate(anyString(), any(Map.class))).thenReturn(diagnostics); + + // Serialize path: single-arg evaluate (buildSnapshotJS) + when(mockPage.evaluate(anyString())).thenReturn(domMap); + when(mockPage.url()).thenReturn("http://example.com"); + when(mockPage.frames()).thenReturn(new ArrayList<>()); + + Percy percyInstance = new Percy(mockPage); + + Map result = percyInstance.getSerializedDOM( + new ArrayList<>(), "// percy dom script", new HashMap<>()); + + assertNotNull(result); + // waitForReady script was sent via the 2-arg evaluate overload + verify(mockPage, atLeastOnce()).evaluate(contains("waitForReady"), any(Map.class)); + // Diagnostics propagated onto the domSnapshot + assertEquals(diagnostics, result.get("readiness_diagnostics")); + } + + @Test + @Order(91) + @SuppressWarnings("unchecked") + public void readinessSkippedWhenPresetDisabled() throws Exception { + Page mockPage = Mockito.mock(Page.class); + + Map domMap = new HashMap<>(); + domMap.put("html", ""); + when(mockPage.evaluate(anyString())).thenReturn(domMap); + when(mockPage.url()).thenReturn("http://example.com"); + when(mockPage.frames()).thenReturn(new ArrayList<>()); + + Percy percyInstance = new Percy(mockPage); + + Map disabled = new HashMap<>(); + disabled.put("preset", "disabled"); + Map options = new HashMap<>(); + options.put("readiness", disabled); + + Map result = percyInstance.getSerializedDOM( + new ArrayList<>(), "// percy dom script", options); + + assertNotNull(result); + // Readiness evaluate(js, config) must NOT have been called + verify(mockPage, never()).evaluate(contains("waitForReady"), any(Map.class)); + // Serialize still ran — and no diagnostics attached + assertNull(result.get("readiness_diagnostics")); + } + + @Test + @Order(92) + @SuppressWarnings("unchecked") + public void snapshotSurvivesReadinessThrow() throws Exception { + Page mockPage = Mockito.mock(Page.class); + + Map domMap = new HashMap<>(); + domMap.put("html", ""); + + // 2-arg evaluate (readiness) blows up; 1-arg evaluate (serialize) still works + when(mockPage.evaluate(anyString(), any(Map.class))).thenThrow(new RuntimeException("readiness boom")); + when(mockPage.evaluate(anyString())).thenReturn(domMap); + when(mockPage.url()).thenReturn("http://example.com"); + when(mockPage.frames()).thenReturn(new ArrayList<>()); + + Percy percyInstance = new Percy(mockPage); + + Map result = percyInstance.getSerializedDOM( + new ArrayList<>(), "// percy dom script", new HashMap<>()); + + assertNotNull(result); + // domSnapshot was still built; no diagnostics attached + assertNull(result.get("readiness_diagnostics")); + assertEquals("", result.get("html")); + } + } From e9e90a32c0faa6c0c1ea76ec48581a4ea2a0d08d Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Fri, 22 May 2026 12:21:47 +0530 Subject: [PATCH 2/4] 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 (`if perSnapshot else fallback`) with shallow-merge in a new `resolveReadinessConfig` helper. Per-snapshot options["readiness"] keys win, global cliConfig.snapshot.readiness keys inherited — a partial per-snapshot override no longer silently drops a global preset:disabled kill switch. - Strip `readiness` from `buildSnapshotJS` (consumed by waitForReady upstream, not a PercyDOM.serialize argument) and from `postSnapshot` JSON body (CLI already has it via healthcheck). `mvn compile` passes. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/main/java/io/percy/playwright/Percy.java | 55 +++++++++++++------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/src/main/java/io/percy/playwright/Percy.java b/src/main/java/io/percy/playwright/Percy.java index 6dc12d6..75f78d1 100644 --- a/src/main/java/io/percy/playwright/Percy.java +++ b/src/main/java/io/percy/playwright/Percy.java @@ -376,6 +376,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-trip and stay forward-compatible + // with future CLI-side validators. + json.remove("readiness"); json.put("url", url); json.put("name", name); json.put("domSnapshot", domSnapshot); @@ -423,6 +427,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("PercyDOM.serialize(%s)\n", json.toString())); return jsBuilder.toString(); @@ -431,30 +437,15 @@ private String buildSnapshotJS(Map options) { /** * Readiness gate: runs PercyDOM.waitForReady BEFORE serialize (PER-7348). * - * Uses page.evaluate — Playwright auto-awaits Promises. 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). If preset is "disabled", skip the - * evaluate call entirely. Any exception is swallowed at debug level — the - * serialize call 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. If the merged preset is "disabled", skip the evaluate call. * * @return Readiness diagnostics to attach to the domSnapshot, or null. */ protected Object waitForReady(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 { - JSONObject snapshotConfig = cliConfig.optJSONObject("snapshot"); - readinessConfig = snapshotConfig == null ? new JSONObject() - : snapshotConfig.optJSONObject("readiness"); - if (readinessConfig == null) { readinessConfig = new JSONObject(); } - } + JSONObject readinessConfig = resolveReadinessConfig(options); if ("disabled".equals(readinessConfig.optString("preset", null))) { return null; } @@ -472,6 +463,30 @@ protected Object waitForReady(Map options) { } } + /** + * Shallow-merge of global (cliConfig.snapshot.readiness) and per-snapshot + * (options["readiness"]) readiness config. Per-snapshot keys win; global + * keys (notably preset: disabled) inherited. + */ + @SuppressWarnings("unchecked") + private JSONObject resolveReadinessConfig(Map options) { + JSONObject merged = new JSONObject(); + JSONObject snapshotConfig = cliConfig == null ? null : 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; + } + /** * Attempts to load dom.js from the local Percy server. Use cached value in `domJs`, * if it exists. From dfbb4f08d78c6765c6aaa5854b79d5592c558422 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Sun, 24 May 2026 22:13:07 +0530 Subject: [PATCH 3/4] 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 9e8de3a..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 82144cc4d3e4bb6f54662f980ca3cada5a4be6c3 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Mon, 25 May 2026 13:38:52 +0530 Subject: [PATCH 4/4] comments: remove JIRA ticket reference from code comments --- src/main/java/io/percy/playwright/Percy.java | 4 ++-- src/test/java/io/percy/playwright/SDKTest.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/percy/playwright/Percy.java b/src/main/java/io/percy/playwright/Percy.java index 75f78d1..d46a134 100644 --- a/src/main/java/io/percy/playwright/Percy.java +++ b/src/main/java/io/percy/playwright/Percy.java @@ -435,7 +435,7 @@ private String buildSnapshotJS(Map options) { } /** - * Readiness gate: runs PercyDOM.waitForReady BEFORE serialize (PER-7348). + * Readiness gate: runs PercyDOM.waitForReady BEFORE serialize. * * Config precedence: per-snapshot options["readiness"] is shallow-merged * over cliConfig.snapshot.readiness so a partial per-snapshot override @@ -896,7 +896,7 @@ Map getSerializedDOM( String percyDomScript, Map options) { - // Readiness gate before serialize (PER-7348). Graceful on old CLI. + // Readiness gate before serialize. Graceful on old CLI. Object readinessDiagnostics = waitForReady(options); Map domSnapshot = diff --git a/src/test/java/io/percy/playwright/SDKTest.java b/src/test/java/io/percy/playwright/SDKTest.java index 3d728f7..d2a8505 100644 --- a/src/test/java/io/percy/playwright/SDKTest.java +++ b/src/test/java/io/percy/playwright/SDKTest.java @@ -490,7 +490,7 @@ public void sameOriginFramesAreNotProcessedAsCorsIframes() throws Exception { } // ------------------------------------------------------------------------- - // Readiness gate (PER-7348) + // Readiness gate // ------------------------------------------------------------------------- @Test