Skip to content

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

Merged
Shivanshu-07 merged 5 commits into
masterfrom
PER-7348-readiness-in-serialize
May 27, 2026
Merged

feat: PER-7348 add waitForReady() call before serialize()#308
Shivanshu-07 merged 5 commits into
masterfrom
PER-7348-readiness-in-serialize

Conversation

@Shivanshu-07
Copy link
Copy Markdown
Contributor

Summary

Adopts the readiness gate from percy/cli#2184 (PER-7348). New waitForReady(jse, options) helper runs PercyDOM.waitForReady via executeAsyncScript (callback signal) before the existing serialize executeScript inside getSerializedDOM. Diagnostics are attached to readiness_diagnostics on the snapshot. Serialize is unchanged.

Contract

  • Config precedence: options["readiness"]cliConfig.snapshot.readiness → empty (CLI balanced default).
  • Backward compat: in-browser typeof PercyDOM.waitForReady === 'function' guard.
  • Disabled preset short-circuits. Graceful on any exception.

Visibility

getSerializedDOM is now package-private (was private) so tests can call it directly.

Tests

3 new Mockito tests in SdkTest:

  • ✅ diagnostics attached + readiness script contains waitForReady
  • ✅ disabled preset skips executeAsyncScript
  • ✅ readiness throw leaves serialize intact

Test results

Check Status Notes
Compile ✅ pass
Unit tests (mvn test -Dtest=...) 3 passed, 0 failed
Smoke test ⚠️ skipped: toolchain missing

Related

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>
Shivanshu-07 and others added 4 commits May 22, 2026 12:15
- 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>
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.
@Shivanshu-07 Shivanshu-07 marked this pull request as ready for review May 25, 2026 11:42
@Shivanshu-07 Shivanshu-07 requested a review from a team as a code owner May 25, 2026 11:42
@Shivanshu-07 Shivanshu-07 merged commit 39e6deb into master May 27, 2026
12 checks passed
@Shivanshu-07 Shivanshu-07 deleted the PER-7348-readiness-in-serialize branch May 27, 2026 05:31
Shivanshu-07 added a commit that referenced this pull request May 27, 2026
Bump version to 2.2.1-beta.0 to ship the readiness gate (waitForReady
before serialize, PER-7348) merged in #308.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants