Test: compare the renamed pre-shutdown snapshot and pin the head - #547
Merged
Conversation
The comparison read the pre-rename path, which no longer exists once the snapshot is moved aside. When both snapshots land on the same head the two names are identical, so filecmp compared the post-restart snapshot with itself and the assertion could never fail; when the heads differ the path is gone and the test dies with FileNotFoundError instead of its message. The heads differ whenever the irreversible-mode node is still short of the producer's final LIB. Killing the sole producer can leave its last block undelivered, and the fixed three second sleep was not a guarantee that the node had caught up. Wait on the node's own LIB instead, then take down the remaining peers so nothing can advance it between the two snapshots. Also compare with shallow=False so matching stat signatures cannot stand in for a byte comparison, and report a mismatched head directly rather than letting it surface as a confusing byte difference.
huangminghuang
approved these changes
Aug 6, 2026
huangminghuang
left a comment
huangminghuang
left a comment
Contributor
There was a problem hiding this comment.
Reviewed at 367da66. No actionable findings.
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
2 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.
nodeop_chainbase_allocation_testfailed in the ubsan job of run 31047433131 withFileNotFoundErrorrather than its own assertion message.The comparison read the pre-rename path, which no longer exists once the snapshot is moved aside. When both snapshots land on the same head the two names are identical, so
filecmpcompared the post-restart snapshot with itself and thesnapshot is not identicalassertion could never fail. When the heads differ the path is gone and the test dies insidefilecmp.The heads differ whenever the irreversible-mode node is still short of the producer's final LIB. Killing the sole producer can leave its last block undelivered, and the fixed three second sleep was not a guarantee that the node had caught up. In the failing run the snapshot landed one block short of the schedule promotion, so
global_property_objectwas not exercised either. The test now waits on the node's own LIB, then takes down the remaining peers so nothing can advance it between the two snapshots.It also compares with
shallow=Falseso matching stat signatures cannot stand in for a byte comparison, and reports a mismatched head directly instead of letting it surface as a confusing byte difference.Exercised locally across runs that landed on different heads, which is the variability that broke the previous version.