[move testing] allow introspection on emitted events in tests #17699
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@sblackshear is attempting to deploy a commit to the Mysten Labs Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Thanks! Tagging @tzakian and @damirka for naming concerns.
@tzakian thoughts on the private generics/native function issue? We had discussed usually staggering these by a release--where we add the Rust implementations one release and then add the native functions the next release. Do you think it matters for test code? I think it will but just wanted to be sure
|
|
||
| #[test_only] | ||
| /// Get the total number of events emitted during execution so far | ||
| public native fun num_events(): u32; |
There was a problem hiding this comment.
num_user_events exists in test_scenario. Do we need this one here too?
| #[test_only] | ||
| /// Get all events of type `T` emitted during execution. | ||
| /// Can only be used in testing, | ||
| public native fun get_events_by_type<T: copy + drop>(): vector<T>; |
There was a problem hiding this comment.
Probably just events_by_type. But I'm wondering if we should add this to test scenario for discoverability. Not sure
There was a problem hiding this comment.
I think it probably makes sense here--the event module is where I would be inclined to look for a function related to testing events, and this is definitely useful outside of code that uses test_scenario (e.g., unit tests that call complex third-party code that emits events + want to check that they look as expected).
On naming: agreed, shorter is always better!
| @@ -160,8 +161,12 @@ fn verify_private_event_emit( | |||
| type_arguments: &[SignatureToken], | |||
| ) -> Result<(), String> { | |||
| let fident = view.identifier_at(fhandle.name); | |||
| if fident == GET_EVENTS_TEST_FUNCTION { | |||
There was a problem hiding this comment.
Might be worthwhile to have something like PUBLIC_EVENT_FUNCTIONS to match the other private generics code, but this is probably fine as is
| assert_eq(event::get_events_by_type<S1>()[1], e3); | ||
| assert_eq(event::num_events(), 4); | ||
| } | ||
| } |
I believe you are correct, and we might want to stagger the release of the natives and framework for this. In this case, the issue will arise where if you are running the current CLI (which I believe is released along with mainnet) and then if you try to run tests with your Sui framework set to devnet/testnet as this is rolling out then it will fail to run since the framework will be looking for the natives that aren't there. |
Gotcha--so I should modify this to contain the |
|
Yea, I think that's the right thing to do :) Then once those changes make their way to mainnet, at that point the framework part of this PR can be landed, and everything shouldn't break (and/or if it does there's an easy fix of "update your CLI") |
| @@ -1018,4 +1020,32 @@ module sui::test_scenario_tests { | |||
| scenario.next_tx(sender); | |||
| abort 42 | |||
| } | |||
|
|
|||
| public struct E1(u64) has copy, drop; | |||
There was a problem hiding this comment.
Thanks for this feature, enjoyed not defining a dummy field :)
Roger! Made that update. Left the rest of the code in comments so it's easy to bring back |
- Extend the `event` module with test-only native functions for grabbing emitted events by type and getting the total event count. This will make it possible to write tests that assert things about emitted events. - Add tests to show that the new functions and `test_scenario` behave as expected. Fortunately, `test_scenario` dumps the object runtime state (including events) between txes, so it works without any mods--calling `get_events_by_type` during a tx only shows events from the current transaction, not from previous ones.
eventmodule with test-only native functions for grabbing emitted events by type and getting the total event count. This will make it possible to write tests that assert things about emitted events.test_scenariobehave as expected. Fortunately,test_scenariodumps the object runtime state (including events) between txes, so it works without any mods--callingget_events_by_typeduring a tx only shows events from the current transaction, not from previous ones.