Skip to content

Conversation

@seanmcguire12
Copy link
Member

@seanmcguire12 seanmcguire12 commented Dec 20, 2025

why

  • to add more coverage for the functions that captureHybridSnapshot() depends on

what changed

  • exported computeFramePrefixes() & mergeFramesIntoSnapshot() for testing
  • added snapshot-frame-merge.test.ts, which holds tests for the following functions:
    • computeFramePrefixes()
      • checks that when DOM.getFrameOwner resolves within the same CDP session, each child frame gets the iframe xpath prefix plus the encoded host id
      • checks that failing DOM.getFrameOwner calls (OOPIFs) inherit their parent prefix while succeeding siblings still receive explicit prefixes
      • checks that when a parent frame lacks the iframe entry in its xpath map, the child falls back to inheriting the parent prefix ("")
    • mergeFramesIntoSnapshot()
      • checks that root maps are copied as-is, child xpath entries are prefixed, urls get merged, and the child outline is injected into the parent tree
      • checks that frames without DOM maps or iframe-host mappings are skipped so their xpath entries stay undefined and the tree is left untouched
      • checks that if the root outline is missing, the function falls back to the first available outline (or empty string) as the combined tree
      • checks that when multiple children share the same parent iframe slot, only the last child outline injected into that slot remains (last write wins)

test plan

  • this is it

Summary by cubic

Add unit tests for computeFramePrefixes and mergeFramesIntoSnapshot to improve coverage for captureHybridSnapshot. Exported these functions from capture.ts for testing; no runtime behavior changes.

Written for commit 912b7c1. Summary will update automatically on new commits.

@changeset-bot
Copy link

changeset-bot bot commented Dec 20, 2025

⚠️ No Changeset found

Latest commit: 912b7c1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Architecture diagram
sequenceDiagram
    participant Test as "Test Suite"
    participant Capture as "capture.ts"
    participant Mock as "Mock Page/CDP"
    
    Note over Test,Mock: NEW: Unit Tests for "computeFramePrefixes"
    
    Test->>Capture: computeFramePrefixes(page, context, perFrameMaps)
    
    loop For each child frame in context
        Capture->>Mock: getSessionForFrame(parentId)
        Capture->>Mock: send("DOM.getFrameOwner", { frameId: childId })
        
        alt Success (Same Process / Local Frame)
            Mock-->>Capture: { backendNodeId: 200 }
            Capture->>Capture: Look up XPath for node 200 in parent map
            Note right of Capture: e.g., "/html/body/iframe[1]"
            Capture->>Capture: Store AbsPrefix = ParentPrefix + IframeXPath
            Capture->>Capture: Map ChildFrame to EncodedHostID
        else Failure (OOPIF / Cross-Process)
            Mock-->>Capture: Error or Empty
            Capture->>Capture: Fallback: Inherit ParentPrefix
            Note right of Capture: No explicit host mapping created
        end
    end
    
    Capture-->>Test: Return { absPrefix, iframeHostEncByChild }

    Note over Test,Mock: NEW: Unit Tests for "mergeFramesIntoSnapshot"

    Test->>Capture: mergeFramesIntoSnapshot(maps, outlines, prefixes...)
    
    loop For each frame
        Capture->>Capture: Retrieve AbsPrefix for frame
        Capture->>Capture: Prefix all entries in XPath map
        Capture->>Capture: Merge URL/Scroll maps into root
    end

    Capture->>Capture: Identify Root Frame Outline
    
    loop For each child frame
        alt Parent Iframe Host Exists
            Capture->>Capture: Locate encoded ID in Parent Outline
            Capture->>Capture: Inject Child Outline into Parent Tree
            opt Multiple children same slot
                Capture->>Capture: Overwrite previous child (Last write wins)
            end
        else Orphaned / No Host Map
            Capture->>Capture: Skip tree injection
        end
    end

    Capture-->>Test: Return Combined Snapshot (Tree + Maps)
Loading

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 20, 2025

Greptile Summary

Added comprehensive test coverage for frame prefix computation and snapshot merging logic that captureHybridSnapshot() depends on.

Key changes:

  • Exported computeFramePrefixes() and mergeFramesIntoSnapshot() from capture.ts to enable isolated unit testing
  • Created 7 test cases covering critical edge cases:
    • Same-session iframe prefix derivation with XPath lookup
    • OOPIF fallback behavior when DOM.getFrameOwner fails
    • Missing XPath mapping inheritance
    • Multi-frame merging with proper prefix application
    • Graceful handling of missing frame maps
    • Root outline fallback logic
    • Last-write-wins behavior for duplicate iframe host slots

The tests use mock CDP sessions and pages to isolate the frame tree traversal and merging logic without requiring a full browser context. All assertions validate both the computed data structures (absPrefix, iframeHostEncByChild) and the final snapshot outputs (combinedXpathMap, combinedUrlMap, combinedTree).

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • The changes are minimal and focused: only two functions are exported for testing (no implementation changes), and the new test file adds thorough coverage for complex frame merging logic. Tests are well-structured with clear edge case coverage including OOPIF handling, missing mappings, and tree injection scenarios. No production code behavior is modified.
  • No files require special attention

Important Files Changed

Filename Overview
packages/core/lib/v3/understudy/a11y/snapshot/capture.ts Exported two helper functions for testing without modifying their implementation
packages/core/tests/snapshot-frame-merge.test.ts Added comprehensive test coverage for frame prefix computation and snapshot merging logic with 7 edge cases

Sequence Diagram

sequenceDiagram
    participant Test as Test Suite
    participant Page as Mock Page
    participant Session as Mock CDP Session
    participant Compute as computeFramePrefixes
    participant Merge as mergeFramesIntoSnapshot
    
    Note over Test,Merge: Test: Frame Prefix Computation
    Test->>Page: Create mock page with sessions
    Test->>Compute: computeFramePrefixes(page, context, perFrameMaps)
    loop For each child frame
        Compute->>Page: parentSession(parent)
        Compute->>Session: DOM.getFrameOwner(frameId)
        alt Frame owner found
            Session-->>Compute: { backendNodeId }
            Compute->>Compute: Look up iframe XPath in parent maps
            Compute->>Compute: prefix = prefixXPath(parentAbs, iframeXPath)
        else Frame owner fails (OOPIF)
            Session-->>Compute: throw Error
            Compute->>Compute: prefix = inherit parent prefix
        end
        Compute->>Compute: Store absPrefix[child] and iframeHostEncByChild[child]
    end
    Compute-->>Test: { absPrefix, iframeHostEncByChild }
    
    Note over Test,Merge: Test: Snapshot Merging
    Test->>Merge: mergeFramesIntoSnapshot(context, maps, outlines, prefixes, hosts)
    loop For each frame
        Merge->>Merge: Get frame maps and absolute prefix
        alt Is root frame
            Merge->>Merge: Copy xpathMap and urlMap as-is
        else Is child frame
            Merge->>Merge: Prefix each xpath with absolute prefix
            Merge->>Merge: Merge urlMap
        end
    end
    Merge->>Merge: Build idToTree map from iframeHostEncByChild
    Merge->>Merge: injectSubtrees(rootOutline, idToTree)
    Merge-->>Test: HybridSnapshot with combined maps and tree
Loading

@seanmcguire12 seanmcguire12 merged commit d4a8db0 into main Dec 20, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants