Expose async persistence in FFI - #1287
Conversation
Pull Request Test Coverage Report for Build 21868383672Details
💛 - Coveralls |
b9ab24b to
aee5749
Compare
|
I see the integration test is skipping. In CI: and local: ======================================================================
ERROR: test.test_payjoin_integration_test (unittest.loader._FailedTest.test.test_payjoin_integration_test)
----------------------------------------------------------------------
ImportError: Failed to import test module: test.test_payjoin_integration_test
Traceback (most recent call last):
File "/opt/homebrew/Cellar/python@3.13/3.13.3/Frameworks/Python.framework/Versions/3.13/lib/python3.13/unittest/loader.py", line 396, in _find_test_path
module = self._get_module_from_name(name)
File "/opt/homebrew/Cellar/python@3.13/3.13.3/Frameworks/Python.framework/Versions/3.13/lib/python3.13/unittest/loader.py", line 339, in _get_module_from_name
__import__(name)
~~~~~~~~~~^^^^^^
File "/Users/dan/f/dev/payjoin/payjoin-ffi/python/test/test_payjoin_integration_test.py", line 11, in <module>
import payjoin.bitcoin as bitcoinffi
File "/Users/dan/f/dev/payjoin/payjoin-ffi/python/src/payjoin/bitcoin.py", line 645, in <module>
_UniffiLib.uniffi_bitcoin_ffi_fn_clone_address.argtypes = (
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/homebrew/Cellar/python@3.13/3.13.3/Frameworks/Python.framework/Versions/3.13/lib/python3.13/ctypes/__init__.py", line 403, in __getattr__
func = self.__getitem__(name)
File "/opt/homebrew/Cellar/python@3.13/3.13.3/Frameworks/Python.framework/Versions/3.13/lib/python3.13/ctypes/__init__.py", line 408, in __getitem__
func = self._FuncPtr((name_or_ordinal, self))
AttributeError: dlsym(0x6d0d1d60, uniffi_bitcoin_ffi_fn_clone_address): symbol not foundIs this entirely intentional? It was added in cc45199 #1191. I don't see discussion of just skipping the integration test there but still passing CI. Is this the intended behavior @chavic? |
DanGould
left a comment
There was a problem hiding this comment.
One of the python tests no-ops accidentally, that's the only thing I'd really want to fix.
Would be nice to have save/load/close coverage in FFI but for now imo it's ok without.
Yeah, that's the expected behavior |
|
In that case, when do we run & maintain the integration tests? |
ee65092 to
2f94194
Compare
arminsabouri
left a comment
There was a problem hiding this comment.
Noticed on inconsistency in the dart unit tests. Otherwise Ack .
IIRC the persister lifecycle is covered in the integration tests. |
O ya that seems sufficient as long as it's actually running edit: async & sync tho? |
currently sync only in integration tests. Same as in the core rust tests fwiw. Maybe worth revisiting at some point for fuller coverage. |
e51ac3a to
d8e5e31
Compare
This exposes the async persistence methods in payjoin-ffi and accompanying unit tests in the downstream languages.
d8e5e31 to
6314d3c
Compare
|
Rebased to fix CI and addressed Armin's comment |
There was a problem hiding this comment.
ACK 6314d3c I created an issue #1325 following this discussion to add fuller coverage
Addressed
payjoin#1287 added save() and save_async() coverage to the bindings but left load() and close() untested. Nothing pinned the binding layer's contract with a foreign persister: that a terminal save reaches through to close(), and that load() is a read the runtime can repeat rather than one that consumes the log. Cover the receiver and the sender in dart, javascript, python and csharp, sync and async. The two reach their terminal state by different routes: cancelling an initialized receiver is itself terminal, while cancelling a sender only moves it to a pending fallback and the session closes a step later, when the broadcast transition is saved. Assert the persister is open beforehand, closed afterwards, and that load() still returns every event.
This exposes the async persistence methods in payjoin-ffi and accompanying unit tests in downstream languages.
I wanted to address #1199 so that we wouldn't duplicate all the hardcoded test values so much, but it turned out to be more involved than I anticipated (see my comment on that issue).
Pull Request Checklist
Please confirm the following before requesting review:
AI
in the body of this PR.