Add helpers to check events #392
Conversation
There was a problem hiding this comment.
Elegant and simple, love it - I was looking for something like this but I got braindead and couldn't figure it out.
Couple stylistic comments from my side. Naming issues are completely not important and I would not bring them up on their own, just gave my thought as I wanted to give some more relevant comments at particular line anyway - if you prefer your names don't bother with my suggestions on this field. However I would prefer to not take events by value as in one case I actually see benefit from checking for same event multiple times - don't see a reason to clone it. Also improving failure mesg is kind of imporant.
packages/multi-test/src/executor.rs
Outdated
| /// Like has_event but panics if no match | ||
| #[track_caller] | ||
| pub fn assert_event(&self, compare: Event) { | ||
| assert!(self.has_event(compare)); |
There was a problem hiding this comment.
I don't like how would failing test report looks like, it would be something like: "assertion failed, assert!(self.has_event(compare))" (probably somehow more details but in this shape). After all one would need to add some more prints to figure out what events actually got received (and also what was expected, but this can be probably read from tests itself). Hopefully assert macro may take additional arguments being the format string and its arguments for the failure case - but to make it work has_event cannot consume expected event.
| assert!(self.has_event(compare)); | |
| assert!(self.has_event(expected), "Expected to find an event {:?}, but received: {:?}", self, expected); |
|
Great feedback. Thanks |
|
I updated to what you suggested (except a small change on the assert error message) |
| assert_eq!("transfer", &reply_events[0].ty); | ||
| let reply = res.result.unwrap(); | ||
| assert_eq!(1, reply.events.len()); | ||
| // Note: this shows we must make the helpers more generic |
There was a problem hiding this comment.
I will merge this as soon as the CI passes.
@hashedone this is one example where we want to make it more generic (also to work with your test suites). Happy if you have some follow-up PRs to extend this pattern other places.
This may be a nice one for simplifying multi-tests.
Needs a unit test or two before merge.