test: Cover visitDifferences node skipping - #7946
Draft
bthomee wants to merge 1 commit into
Draft
Conversation
`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.
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
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.
Part 9/9 of a stack (top). Base: part 8 (
bthomee/shamap-stack-08-move-entries-off-nodepathstack).Test-only: no production code changes.
visitDifferenceshad no test coverage:LedgerMaster::populateFetchPackisits only caller, and nothing exercised that path, so the private
hasLeafNodeand
hasInnerNodeit consults were untested too. Codecov flagged the resultinggap 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
visitNodeswalk sees, and a callback returning false stops the walkwhere
populateFetchPackwould 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
hasInnerNodeto always claim a match left every test passing, since the treewas 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
hasLeafNodein either direction, orhasInnerNode,now fails the case.
The maps are hashed via
getHashbefore being compared, since node hashes arecomputed on demand and
visitDifferencesreturns early while the root hash isstill zero. Without that the walk visits nothing and every assertion here
would hold vacuously.