Skip to content

test: Cover visitDifferences node skipping - #7946

Draft
bthomee wants to merge 1 commit into
bthomee/shamap-stack-08-move-entries-off-nodepathstackfrom
bthomee/shamap-stack-09-visit-differences-tests
Draft

test: Cover visitDifferences node skipping#7946
bthomee wants to merge 1 commit into
bthomee/shamap-stack-08-move-entries-off-nodepathstackfrom
bthomee/shamap-stack-09-visit-differences-tests

Conversation

@bthomee

@bthomee bthomee commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Part 9/9 of a stack (top). Base: part 8 (bthomee/shamap-stack-08-move-entries-off-nodepathstack).

Test-only: no production code changes.

visitDifferences had no test coverage: LedgerMaster::populateFetchPack is
its only caller, and nothing exercised that path, so the private hasLeafNode
and hasInnerNode it consults were untested too. Codecov flagged the resulting
gap against #7938, since that commit happened to touch a line inside
hasLeafNode.

Four cases: a map missing one item reports only the nodes on that item's path,
an identical map reports nothing, a null comparison map reports every node the
plain visitNodes walk sees, and a callback returning false stops the walk
where populateFetchPack would stop once a pack is full.

The first case uses 64 shared items rather than a couple, so the shared leaves
build inner nodes of their own and the walk has whole matching subtrees to
skip. Verified by mutation: with only two shared items, breaking
hasInnerNode to always claim a match left every test passing, since the tree
was too shallow for that branch to be taken. It also asserts that some inner
nodes are reported but not all, which is what distinguishes skipping a subtree
from walking it. Breaking hasLeafNode in either direction, or hasInnerNode,
now fails the case.

The maps are hashed via getHash before being compared, since node hashes are
computed on demand and visitDifferences returns early while the root hash is
still zero. Without that the walk visits nothing and every assertion here
would hold vacuously.

`visitDifferences` had no test coverage: `LedgerMaster::populateFetchPack` is
its only caller, and nothing exercised that path, so the private `hasLeafNode`
and `hasInnerNode` it consults were untested too. Codecov flagged the resulting
gap against an unrelated refactor, since that commit happened to touch a line
inside `hasLeafNode`.

Four cases: a map missing one item reports only the nodes on that item's path,
an identical map reports nothing, a null comparison map reports every node the
plain `visitNodes` walk sees, and a callback returning false stops the walk
where `populateFetchPack` would stop once a pack is full.

The first case uses 64 shared items rather than a couple, so the shared leaves
build inner nodes of their own and the walk has whole matching subtrees to
skip. Verified by mutation: with only two shared items, breaking
`hasInnerNode` to always claim a match left every test passing, since the tree
was too shallow for that branch to be taken. It also asserts that some inner
nodes are reported but not all, which is what distinguishes skipping a subtree
from walking it. Breaking `hasLeafNode` in either direction, or `hasInnerNode`,
now fails the case.

The maps are hashed via `getHash` before being compared, since node hashes are
computed on demand and `visitDifferences` returns early while the root hash is
still zero. Without that the walk visits nothing and every assertion here
would hold vacuously.
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.

1 participant