Test replay persistence failure paths - #1858
Open
Jolah1 wants to merge 1 commit into
Open
Conversation
The in-memory persisters used by the existing tests are infallible, so the error paths where replay maps a storage failure into ReplayError were never exercised, and neither load() nor close() had direct coverage of their successful behavior. Add fault-injecting sync and async persisters under a single test-only module, and assert that sender and receiver replay surface a failure from load() and from the close() that replay performs when the event log is invalid. Assertions match on the error variant and downcast to the injected storage error rather than comparing Display strings, so rewording a message does not break them. Also cover the happy paths directly: a fresh persister loads nothing, load() replays events in save order without consuming the log, and close() marks the session closed while leaving the log readable.
Jolah1
requested review from
DanGould,
benalleng and
spacebear21
as code owners
September 3, 2026 10:37
Collaborator
Coverage Report for CI Build 33745316388Coverage increased (+0.07%) to 86.712%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
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.
Closes #1857.
What
Add a fault-injecting session persister, then assert that
replay_event_logandreplay_event_log_asyncsurfacePersistenceFailurewhenload()or whenclose()fails, in both the receive and send sessions.Also covers the in-memory persisters' happy paths directly: a fresh persister loads nothing,
load()replays events in save order without consuming the log, andclose()marks the session closed while leaving the log readable.Why
The eight
PersistenceFailureconstruction sites were unreachable from any test. The only persisters in the tree declaretype InternalStorageError = std::convert::Infallible;, so theirload()andclose()cannot fail, and the variant appeared in no assertion.Notes
Split out of #1835, which was retargeted to the FFI binding coverage #1325 asks for. This carries the review feedback from that PR:
#[cfg(test)] pub(crate) mod test_supportDisplaystringsreplay_surfaces_close_failuredocuments the error-replacement chain: replay closes the session on an invalid log, so a close failure replaces the replay errorTests
cargo test -p payjoin --all-featurescargo clippy -p payjoin --all-targets --all-features -- -D warningscargo fmt --all -- --checkDisclosure: co-authored by Claude Code. The tests, the commit message and this PR body were generated by Claude Code, reviewed and verified by me.
🤖 Generated with Claude Code
https://claude.ai/code/session_01VLcgR2FZhENW7ZfxHS2Rse