-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: assert emitted events #34
base: main
Are you sure you want to change the base?
Conversation
Should be reviewed after this pr #31 |
…h derefed contract
# Conflicts: # crates/motsu/src/context.rs
# Conflicts: # .github/workflows/test.yml # CHANGELOG.md # crates/motsu/src/context.rs # crates/motsu/src/lib.rs # crates/motsu/src/prelude.rs # crates/motsu/src/router.rs # crates/motsu/src/shims.rs # crates/motsu/src/storage_access.rs
.expect("should ping successfully"); | ||
|
||
// Assert emitted events. | ||
assert!(Pinged { from: alice.address(), value: TEN }.emitted()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although it is not possible print events, during failure with this approach.
I think something like:
_ = ping
.sender(alice)
.ping(pong.address(), TEN)
.expect("should ping successfully");
// Assert emitted events.
ping.assert_emitted(Pinged { from: alice.address(), value: TEN });
would allow for printing events, and is also far more in line with motsu's current approach.
Alternatives
- a new macro like
assert_emitted!
that would be able to print events. - trait extension without macros:
Pinged { from: alice.address(), value: TEN }.assert_emitted();
I'd prefer the original proposal (contract.assert_emitted
) as it would allow focusing the assertion on specific contracts. EDIT: I'd be ok with the macro as well, see explanation below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I planned to just have assert_emitted!
macro for event's printing, but it would print all events from any contract in unit test. Assertion for events produced by specific contract ping.assert_emitted(Pinged { from: alice.address(), value: TEN });
can be quite convenient, I agree!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I planned to just have
assert_emitted!
macro for event's printing, but it would print all events from any contract in unit test.
The macro could accept the contract parameter as well, thus focusing the check:
assert_emitted!(ping, Pinged { from: alice.address(), value: TEN });
I'm fine with both contract.assert_emitted
and macro approach, as both seem idiomatic to Rust and convenient for motsu.
We could even provide both and let devs choose? 🤔
A nice(-to-have) feature would be to support specifying the number of times an event was emitted?
To give a dumb, and yet possible example where this would be useful would be a Vault that can execute multiple pending withdrawal requests in one transaction, collecting fees for the Vault owner and emitting something like FeeCollected
for each withdrawal.
#[test]
fn multiple_withdrawals_work(
owner: Account,
alice: Account,
bob: Account
) {
let vault = Contract::<Vault>::new();
// set up the vault
// alice and bob invest in the vault
// alice and bob initiate withdrawal requests
vault.withdraw_pending().expect("ok");
// Assert emitted exactly twice with macro
assert_emitted!(vault, FeeCollected { }, 2);
// or as a contract extension
vault.assert_emitted(FeeCollected { }).times(2);
}
# Conflicts: # crates/motsu/src/context.rs # crates/motsu/src/lib.rs
…vents # Conflicts: # crates/motsu/src/context.rs # crates/motsu/src/lib.rs
Resolves #9
PR Checklist