test: stop pinning rustledger's directive hash in snapshots - #280
Merged
Conversation
test_get_ledger_data.json pinned 62 literal SHA-256 digests. They are
rustledger's directive-identity hash: an implementation detail of a
separate repo, covered by its own tests, and nothing about rustfava's
behaviour is asserted by their exact values. Pinning them means every
change to that hash function turns this suite red and forces a lockstep
snapshot regeneration - which is what rustledger#1984 just did.
The redaction the snapshot machinery already performs was meant to cover
this. It did not, because both patterns require the underscore:
_hash": ?"[0-9a-f]+
That matches entry_hash and misses the bare hash key inside meta, so one
hash was normalised and the other 62 leaked into the file.
Anchoring on the opening quote fixes it without touching entry_hash, and
requiring a full-length digest keeps user metadata that happens to be
called hash from being silently swallowed.
Verified against both components: test_get_ledger_data now passes
against the pinned v0.21.0 release asset AND against a component built
from rustledger main, which it could not do before - the two produce
different digests for identical input.
Sabotage-checked, so this is not just a blanket that hides everything:
changing a lineno in the snapshot still fails the test, a short
hash-valued metadata entry survives redaction untouched, and entry_hash
is unaffected by the new pattern.
Full suite against the pinned v0.21.0 asset: 667 passed, 1 skipped.
This does not make the whole suite green against rustledger main. Three
snapshots still differ, and all three are genuine behaviour changes on
main rather than churn, each verified correct:
- test_api_errors: a spurious "interpolation failed" error is gone.
beancount itself reports no such error for that fixture, so main now
matches it and the released component was wrong.
- test_serialise_entry_types: number-value metadata "10" became "20".
The fixture says 10 + 10, and beancount evaluates metadata
arithmetic, so 20 is correct.
- account_report and test_serialise_entry_types: 101.0 became 101,
-1.0 became -1. Every source literal involved is an integer, so the
trailing zero was spurious.
Those three cannot be fixed here: their expected values differ between
the pinned release and main, so any single value is red on one side.
They belong in whichever change advances RUSTLEDGER_VERSION.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018bGRsKA42peqSnz4VMreBG
robcohen
added a commit
that referenced
this pull request
Aug 10, 2026
Resolves the conflict in test_get_ledger_data.json by taking main's version. This branch's update to that file is obsolete: #280 stopped pinning rustledger's directive hash in snapshots at all, so the 62 digests this branch was refreshing no longer appear in the file, and that test now passes against both the pinned release asset and a component built from rustledger main. What remains here is the part #280 could not fix - three snapshots whose expected values genuinely differ between the pinned component and main, so no single committed value is green on both sides. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018bGRsKA42peqSnz4VMreBG
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.
The coupling
test_get_ledger_data.jsonpinned 62 literal SHA-256 digests. Those are rustledger's directive-identity hash — an implementation detail of a separate repo, covered by its own tests. Nothing about rustfava's behaviour is asserted by their exact values. Pinning them means every change to that hash function turns this suite red and forces a lockstep regeneration, which is exactly what rustledger#1984 just did.The snapshot machinery already tries to normalise hashes. It missed these because both patterns require the underscore:
That matches
entry_hashand misses the barehashkey insidemeta— so one hash was normalised and the other 62 leaked into the file. Anchoring on the opening quote fixes it without touchingentry_hash; requiring a full-length digest keeps user metadata that happens to be calledhashfrom being silently swallowed.Verification
test_get_ledger_datanow passes against both components, which it could not do before — they produce different digests for identical input:v0.21.0release assetmainFull suite against the pinned
v0.21.0asset: 667 passed, 1 skipped.Sabotage-checked, so this is a normalisation and not a blanket that hides everything:
linenoin the snapshothashmetadata valueentry_hashunder the new patternWhat this does not fix
Three snapshots still differ against rustledger
main. All three are genuine behaviour changes, not churn, and I verified each is a correctness improvement:test_api_errors— a spuriousinterpolation failed: multiple postings missing amounts...error is gone. I ran beancount directly against that fixture: it reports 3 errors and no interpolation error, somainnow matches beancount and the released component was wrong.test_serialise_entry_types—number-valuemetadata"10"→"20". The fixture isnumber-value: 10 + 10, and beancount evaluates metadata arithmetic, so20is correct.account_report_off_by_one_journalandtest_serialise_entry_types—101.0→101,-1.0→-1. Every source literal involved is an integer (1 COM {100 USD}, price101 USD), so the trailing zero was spurious.These cannot be fixed in this PR: their expected values genuinely differ between the pinned release and
main, so any single committed value is red on one side. I confirmed that empirically — regenerating all four againstmainmakes the same four tests fail againstv0.21.0, just inverted. They belong in whichever change advancesRUSTLEDGER_VERSION.🤖 Generated with Claude Code
https://claude.ai/code/session_018bGRsKA42peqSnz4VMreBG