You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Replay persistence failure paths are unreachable in tests #1857
replay_event_log and replay_event_log_async translate a failing persister into InternalReplayError::PersistenceFailure in eight places — a load() site and a close() site in each of the sync and async function, in both receive/v2/session.rs and send/v2/session.rs.
None of them is reachable from a test. The only persisters in the tree, InMemoryPersister and InMemoryAsyncPersister, both declare:
so their load() and close() cannot fail, and PersistenceFailure appears only at those eight construction sites, never in an assertion.
The close() sites are the more interesting half: replay only calls close() after replay_events has already failed, so a failure there replaces the original error rather than surfacing alongside it.
Proposed scope
Add a fault-injecting persister with a real, non-Infallible storage error that can be made to fail on load() and on close() independently, then assert that replay_event_log and replay_event_log_async surface PersistenceFailure for both, in both the receive and send sessions.
Two things worth getting right while doing it:
Assert on the error variant, not its Display string, so rewording a message does not break the test. ReplayError wraps a private InternalReplayError, so this needs a small test-only accessor.
Cover the error-replacement chain described above with a comment, since a future refactor that stops closing on failure would break the test for non-obvious reasons.
Split out of #1835, which was retargeted to the FFI binding coverage #1325 asks for.
As @benalleng stated in #1325load() and close() test coverage should target the payjoin-cli since that is the actual implementation that we maintain in this repo. Closing this as not doing.
Activity
xstoicunicornx commentedon Sep 3, 2026
What is the purpose of testing an implementation that is not included in this library?
xstoicunicornx commentedon Sep 8, 2026
As @benalleng stated in #1325
load()andclose()test coverage should target thepayjoin-clisince that is the actual implementation that we maintain in this repo. Closing this as not doing.