[test]: add tests for synchronous DOM node helpers #1443
Merged
Conversation
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
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
Greptile SummaryAdds unit test coverage for four synchronous DOM node helper functions used when parsing Test Coverage:
Minor Observation: Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Test as Test Suite
participant Helper as DOM Helper Functions
participant Node as Protocol.DOM.Node
Note over Test,Node: shouldExpandNode Testing
Test->>Helper: shouldExpandNode(node)
Helper->>Node: Check childNodeCount
Helper->>Node: Check children.length
Helper-->>Test: true/false (needs expansion?)
Note over Test,Node: mergeDomNodes Testing
Test->>Helper: mergeDomNodes(target, source)
Helper->>Node: Update childNodeCount
Helper->>Node: Update children array
Helper->>Node: Update shadowRoots
Helper->>Node: Update contentDocument
Helper-->>Test: void (target mutated)
Note over Test,Node: collectDomTraversalTargets Testing
Test->>Helper: collectDomTraversalTargets(node)
Helper->>Node: Collect children
Helper->>Node: Collect shadowRoots
Helper->>Node: Collect contentDocument
Helper-->>Test: Array of target nodes
Note over Test,Node: findNodeByBackendId Testing
Test->>Helper: findNodeByBackendId(root, backendId)
Helper->>Node: Stack-based traversal
Helper->>Node: Check children
Helper->>Node: Check shadowRoots
Helper-->>Test: Found node or undefined
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+28
to
+45
| describe("shouldExpandNode", () => { | ||
| it("returns true when declared children exceed realized children", () => { | ||
| const node = makeNode({ | ||
| childNodeCount: 2, | ||
| children: [makeNode()], | ||
| }); | ||
| expect(shouldExpandNode(node)).toBe(true); | ||
| }); | ||
|
|
||
| it("returns false when all declared children are realized", () => { | ||
| const child = makeNode(); | ||
| const node = makeNode({ | ||
| childNodeCount: 1, | ||
| children: [child], | ||
| }); | ||
| expect(shouldExpandNode(node)).toBe(false); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
style: Consider edge case: node with childNodeCount: 0 and no children array (should return false)
Suggested change
| describe("shouldExpandNode", () => { | |
| it("returns true when declared children exceed realized children", () => { | |
| const node = makeNode({ | |
| childNodeCount: 2, | |
| children: [makeNode()], | |
| }); | |
| expect(shouldExpandNode(node)).toBe(true); | |
| }); | |
| it("returns false when all declared children are realized", () => { | |
| const child = makeNode(); | |
| const node = makeNode({ | |
| childNodeCount: 1, | |
| children: [child], | |
| }); | |
| expect(shouldExpandNode(node)).toBe(false); | |
| }); | |
| }); | |
| describe("shouldExpandNode", () => { | |
| it("returns true when declared children exceed realized children", () => { | |
| const node = makeNode({ | |
| childNodeCount: 2, | |
| children: [makeNode()], | |
| }); | |
| expect(shouldExpandNode(node)).toBe(true); | |
| }); | |
| it("returns false when all declared children are realized", () => { | |
| const child = makeNode(); | |
| const node = makeNode({ | |
| childNodeCount: 1, | |
| children: [child], | |
| }); | |
| expect(shouldExpandNode(node)).toBe(false); | |
| }); | |
| it("returns false when node has no children", () => { | |
| const node = makeNode({ | |
| childNodeCount: 0, | |
| }); | |
| expect(shouldExpandNode(node)).toBe(false); | |
| }); | |
| }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/tests/snapshot-dom-tree-utils.test.ts
Line: 28:45
Comment:
**style:** Consider edge case: node with `childNodeCount: 0` and no `children` array (should return false)
```suggestion
describe("shouldExpandNode", () => {
it("returns true when declared children exceed realized children", () => {
const node = makeNode({
childNodeCount: 2,
children: [makeNode()],
});
expect(shouldExpandNode(node)).toBe(true);
});
it("returns false when all declared children are realized", () => {
const child = makeNode();
const node = makeNode({
childNodeCount: 1,
children: [child],
});
expect(shouldExpandNode(node)).toBe(false);
});
it("returns false when node has no children", () => {
const node = makeNode({
childNodeCount: 0,
});
expect(shouldExpandNode(node)).toBe(false);
});
});
```
How can I resolve this? If you propose a fix, please make it concise.
monadoid
approved these changes
Dec 19, 2025
seanmcguire12
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
why
DOM.getDocumentwhat changed
domTree.tsfor testingshouldExpandNode(),mergeDomNodes(),collectDomTraversalTargets(), andfindNodeByBackendId()test plan
Summary by cubic
Add unit tests for synchronous DOM tree helpers used when parsing DOM.getDocument. Supports STG-1094 by exporting helpers and adding tests for shouldExpandNode, mergeDomNodes, collectDomTraversalTargets, and findNodeByBackendId.
Written for commit 078e98c. Summary will update automatically on new commits.