chore: Add code comments + refactor if statement for when snapshots are taken#34026
Merged
Merged
Conversation
…ss Test Replay In a headless run with the capture protocol enabled, the driver cloned the entire AUT <body> (document.importNode + stylesheet collection + iframe replacement) on every command via Log.snapshot() -> createSnapshot(). Those captured body/htmlAttrs are dropped at protocol ingestion and never used: Test Replay reconstructs the DOM from its own cy:protocol-snapshot -> dom:full-snapshot path, so this was wasted per-command work. Emit a metadata-only snapshot (name/timestamp) in that case while still toggling the highlight attribute so the protocol can resolve elementsToHighlight. Gate on !isInteractive && isProtocolEnabled && numTestsKeptInMemory === 0 so Open Mode (e.g. Studio AI) and the driver snapshot tests (numTestsKeptInMemory: 1) keep full-body snapshots. The two serialization short-circuits now share the same predicate. https://claude.ai/code/session_017CFfoAmczKsjQFduBMHf82
![cypress-bot[bot]](https://avatars.githubusercontent.com/in/24384?s=60&v=4){kind=small}
jennifer-shehane
changed the title
cypress
 |
||||||||||||||||||||||||||||
| Project |
cypress
|
| Branch Review |
claude/headless-cypress-perf-B94TM
|
| Run status |
|
| Run duration | 18m 52s |
| Commit |
|
| Committer | Jennifer Shehane |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
 |
0
|
 |
6
|
 |
1096
|
 |
0
|
 |
24806
|
| View all changes introduced in this branch ↗︎ | |
Warning
No Report: Something went wrong and we could not generate a report for the Application Quality products.
Reverts the runtime change that skipped the per-command DOM body-clone in headless Test Replay runs, since the capture protocol may replace cy.createSnapshot under numTestsKeptInMemory === 0 (handled on the Test Replay/consumer side). Leaves explanatory comments at the snapshot gate in cypress/log.ts and at createSnapshotBody in cy/snapshots.ts so the redundant clone is noticeable to anyone optimizing this path. Also removes the no-longer -applicable changelog entry. https://claude.ai/code/session_017CFfoAmczKsjQFduBMHf82
Replaces the precedence-sensitive `A || (B || C) && D` snapshot gate with named boolean locals (isCrossOriginLogOnPrimary / reporterWillUseSnapshot / protocolWillUseSnapshot) and explicit grouping. Behavior is identical (verified across all input combinations); only readability changes. https://claude.ai/code/session_017CFfoAmczKsjQFduBMHf82
cypress-bot
Bot
dismissed
their stale review
Cursor Bugbot risk assessment is no longer Low Risk. Auto-approval dismissed; manual review required.
jennifer-shehane
changed the title
Covers the refactored snapshot() bail condition across headless/Open Mode, protocol on/off, in-memory retention, and cross-origin/spec-bridge cases by asserting whether createSnapshot is invoked. Verified to fail under a mutated gate (mutation testing). https://claude.ai/code/session_017CFfoAmczKsjQFduBMHf82
ryanthemanuel
approved these changes
Jun 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR is documentation plus a readability refactor — no functional or behavioral change. It clarifies the per-command snapshot gate and documents a redundant DOM clone in headless Test Replay runs. The actual optimization (skipping the clone) is being handled on the Test Replay capture service side, so the driver's runtime behavior here is intentionally left unchanged.
Background
In a headless
cypress runrecorded with Test Replay,Log.snapshot()callscy.createSnapshotfor every command, which clones the entire DOM<body>(createSnapshotBody→document.importNode). That clone is effectively wasted for Test Replay:body/htmlAttrsare dropped at ingestion — onlyname/timestamp/elementsToHighlightare consumed.cy:protocol-snapshot→dom:full-snapshot), not from this clone.cy.createSnapshotoutright whennumTestsKeptInMemory === 0, depending on testing type.Because skipping the clone must be coordinated with (and may already be handled by) the Test Replay capture service, this PR leaves runtime behavior unchanged and instead documents the situation where a future reader would notice the redundant work.
What changed
packages/driver/src/cypress/log.ts— Rewrote thesnapshot()bail condition. The previousA || (B || C) && Dform relied on&&binding tighter than||, which was easy to misread. It is now expressed with named booleans —shouldDeferSnapshotToSpecBridge,needsTimeTravelSnapshot,needsProtocolSnapshot, andshouldTakeSnapshot— so the intent reads directly: take a snapshot only when something will consume it (interactive command-log time-travel, or Test Replay capture), and defer cross-origin logs to the spec bridge. Behavior is identical — verified equivalent to the original across every combination of cross-origin /isInteractive/numTestsKeptInMemory/isProtocolEnabled.packages/driver/src/cy/snapshots.ts— Added a Test Replay note abovecreateSnapshotdocumenting that theimportNodebody clone runs per command during protocol runs even though the resulting body is discarded downstream.Behavioral impact
None. No config, public API, command-log, or Test Replay output changes. Open Mode time-travel, cross-origin snapshots, and protocol capture all behave exactly as before.
Steps to test
There is nothing functional to exercise — the gate refactor is behavior-preserving and the comments are documentation. Existing driver snapshot specs continue to pass.
How has the user experience changed?
No user-facing change. This is internal documentation plus a readability refactor. (The actual performance win lands when the redundant clone is removed, which is being handled on the Test Replay capture service side.)
PR Tasks
cypress-documentation? (N/A)type definitions? (N/A — no public API changes)https://claude.ai/code/session_017CFfoAmczKsjQFduBMHf82
Note
Low Risk
Readability and documentation only; snapshot gating logic is preserved and now covered by explicit unit tests.
Overview
Refactors
Log.snapshot()so when snapshots run is expressed with named flags (shouldDeferSnapshotToSpecBridge,needsTimeTravelSnapshot,needsProtocolSnapshot,shouldTakeSnapshot) instead of a hard-to-read||/&&mix. Intent is unchanged: snapshot only for command-log time-travel (interactive + tests kept in memory) or protocol/Test Replay, and skip on the primary origin for cross-origin logs.Documents in
createSnapshotthat headless Test Replay still triggers per-commandimportNodebody clones whosebody/htmlAttrsare not used downstream (ingestion drops them; replay DOM comes from protocol capture).Adds unit tests in
log.spec.tsthat lock in snapshot gating for headless vs open mode, protocol on/off, memory retention, and cross-origin / spec-bridge cases.Reviewed by Cursor Bugbot for commit 00b7aeb. Bugbot is set up for automated code reviews on this repo. Configure here.