Improve Event testing api #3043
Conversation
kkawula
force-pushed
the
kkawula/3041-improve-eventspy-cheatcode-api
branch
from
0d983c7 to
a5c9b80
Compare
| @@ -2,6 +2,7 @@ use snforge_std::{ | |||
| declare, ContractClassTrait, DeclareResultTrait, spy_events, EventSpyAssertionsTrait, | |||
| EventSpyTrait, // Add for fetching events directly | |||
| Event, // A structure describing a raw `Event` | |||
| IsEmitted // Method for checking if a given event was ever emitted | |||
| @@ -104,16 +104,18 @@ impl EventSpyAssertionsTraitImpl< | |||
| } | |||
|
|
|||
| fn is_emitted<T, impl TEvent: starknet::Event<T>, impl TDrop: Drop<T>>( | |||
| self: @Events, expected_emitted_by: @ContractAddress, expected_event: @T | |||
| self: @Array<(ContractAddress, Event)>, | |||
| let mut expected_keys = array![]; | ||
| let mut expected_data = array![]; | ||
| expected_event.append_keys_and_data(ref expected_keys, ref expected_data); |
| if from == expected_emitted_by | ||
| && event.keys == @expected_keys |
There was a problem hiding this comment.
Use == here (now you have PartialEq, tuple also implements this)
| @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
|
|
|||
| ### Forge | |||
|
|
|||
| #### Added | |||
|
|
|||
| - Testing events api improvements. New `IsEmitted` trait. `Into` and `PartialEq` trait implementations for snforge_std Events. | |||
There was a problem hiding this comment.
Intowhat?PartialEqis implemented forEventnotEvents(maybe we should do it for both)?snforge_std::Event(markdown formatting)
| @@ -104,16 +104,18 @@ impl EventSpyAssertionsTraitImpl< | |||
| } | |||
|
|
|||
| fn is_emitted<T, impl TEvent: starknet::Event<T>, impl TDrop: Drop<T>>( | |||
There was a problem hiding this comment.
[nit] public should be above private
| @@ -127,6 +129,34 @@ fn is_emitted<T, impl TEvent: starknet::Event<T>, impl TDrop: Drop<T>>( | |||
| return is_emitted; | |||
| } | |||
|
|
|||
| pub trait IsEmitted { | |||
| fn is_emitted( | |||
| self: @Array<(ContractAddress, Event)>, | |||
There was a problem hiding this comment.
This should be implemented on @Events not on array imo (you always get Events from get_events and the events field is public). Ignore if it was consulted with the team previously
| ) -> bool { | ||
| let mut expected_keys = array![]; | ||
| let mut expected_data = array![]; | ||
| expected_event.append_keys_and_data(ref expected_keys, ref expected_data); | ||
|
|
||
| let mut i = 0; | ||
| let mut is_emitted = false; | ||
| while i < self.events.len() { | ||
| let (from, event) = self.events.at(i); | ||
| while i < self.len() { |
There was a problem hiding this comment.
You can use for in Cairo now to iterate over arrays
| @@ -127,6 +129,34 @@ fn is_emitted<T, impl TEvent: starknet::Event<T>, impl TDrop: Drop<T>>( | |||
| return is_emitted; | |||
| } | |||
|
|
|||
| pub trait IsEmitted { | |||
| fn is_emitted( | |||
There was a problem hiding this comment.
[nit] idk if is_emitted is good, maybe contains_event to distinct from assert_emitted, but idk
| 5. We can make sure that the event mock was emitted. | ||
| 6. Emitted events can be explicitly compared. |
There was a problem hiding this comment.
[nit] Idk if 5 and 6 shouldn't be before 3 and 4 since it is the default method.
Also a note, there is no example where snforge_std::Event is directly created, but it is out of scope
Closes #3041
Introduced changes
Checklist
CHANGELOG.md