Conversation
There was a problem hiding this comment.
Pull request overview
This pull request simplifies event testing in the Soroban SDK by introducing a new ContractEvents struct that wraps XDR contract events. The change improves test debuggability by displaying actual event values instead of opaque object references when assertions fail. The new API returns XDR contract events directly while maintaining backward compatibility through PartialEq implementations.
Key Changes
- Introduced
ContractEventsstruct that wrapsstd::vec::Vec<xdr::ContractEvent>with filtering and comparison capabilities - Changed
env.events().all()return type fromVec<(Address, Vec<Val>, Val)>toContractEvents - Added
to_contract_event()method to theEventtrait for converting event structs to XDR format in tests
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| soroban-sdk/src/testutils.rs | Adds ContractEvents struct with PartialEq implementations for backward compatibility, includes filter_by_contract_id method |
| soroban-sdk/src/events.rs | Updates Events trait implementation to return ContractEvents, adds to_contract_event method to Event trait |
| soroban-sdk/src/address.rs | Changes contract_id and from_contract_id methods from pub(crate) to pub visibility |
| tests/events/src/lib.rs | Updates test examples to use new ContractEvents API with to_contract_event |
| tests/events_ref/src/lib.rs | Updates test examples using reference fields to use new API |
| soroban-sdk/src/tests/contract_event.rs | Refactors all event tests to use XDR format and adds new tests for backwards compatibility and filtering |
| tests-expanded/*.rs | Shows macro expansion results with updated event testing patterns |
| soroban-sdk/test_snapshots/* | Adds snapshot files for new test cases |
|
Looking at the OZ failures, the |
I thought about this, but ultimately decided against it. It felt weird adding arbitrary vec methods to |
|
@ozgunozerk @brozorec It looks like from the failures when we test this change with the OZ contracts repo (see the CI failures above) that the OZ contracts repo has some test utilities built around the
|
I'm supporting this change. Being able to directly use
I'd say no need to have both variants. We'll refactor once that's out. |
leighmcculloch
left a comment
There was a problem hiding this comment.
Looks good.
I pushed a couple tweaks that aren't critical. I had tried them locally because I wasn't sure if they were possible and they worked so pushed them instead of posting comments with them. If they cut across anything you were planning though we can revert them.
7be2477 to
9b288d7
Compare
What
Adds a testutils struct
ContractEventsto be returned byenv.events().all()that now returns the XDR variants of the emitted contract events. To simplify testing, Events defined bycontracteventscan be converted to the XDR variant withto_contract_event.Additional partial eq impl were added to maintain backwards compatibility with users who test with existing methods. This will impact tests where users directly check the length of the events array, via
all().len(), who will now need to writeall().events.len().Lastly, this also allows the debug information when testing events to display actual values, making it easier to determine what went wrong during test failures:
Before:
After:
An example test:
Why
Solves #1566. Makes it easier to validate events during tests.
Known limitations
None