[sui move test] Use Sui's VM Conifg, Verifier Config, and Gas Meter in Unit Tests #25226
[sui move test] Use Sui's VM Conifg, Verifier Config, and Gas Meter in Unit Tests #25226tnowacki merged 6 commits intoMystenLabs:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
| let mut testing_config = | ||
| UnitTestingConfig::default_with_bound(Some(*MAX_UNIT_TEST_INSTRUCTIONS)); |
There was a problem hiding this comment.
If we just called sui move test we wouldn't need this here
crates/sui-move/src/unit_test.rs
Outdated
|
|
||
| impl SuiVMTestSetup { | ||
| pub fn new() -> Self { | ||
| let reference_gas_price = 500; |
There was a problem hiding this comment.
I could not find a good place to get this constant @dariorussi @tzakian
There was a problem hiding this comment.
There isn't one...this is probably as good as any ...
There was a problem hiding this comment.
I'll move it to a constant then
crates/sui-move/src/unit_test.rs
Outdated
| impl SuiVMTestSetup { | ||
| pub fn new() -> Self { | ||
| let reference_gas_price = 500; | ||
| let gas_price = reference_gas_price; |
There was a problem hiding this comment.
Leaving this in to make it clear we can configure them if needed
| }; | ||
|
|
||
| pub struct SuiGasMeter<'g>(pub &'g mut GasStatus); | ||
| pub struct SuiGasMeter<G: DerefMut<Target = GasStatus>>(pub G); |
There was a problem hiding this comment.
A bit strange, but this change allows for either owned values or &mut values to be used
There was a problem hiding this comment.
I'm fine with switching this to AsRef + AsMut if the DerefMut makes folks skittish... it is a sharp trait in that it does a lot of things automatically
There was a problem hiding this comment.
It's fancy, but I'm fine with keeping it.
| native_function_table: move_vm_runtime::native_functions::NativeFunctionTable, | ||
| } | ||
|
|
||
| impl Default for SuiVMTestSetup { |
There was a problem hiding this comment.
Infinite sadness (hatred?).
I have lost this battle. I will go default() now.
crates/sui-move/src/unit_test.rs
Outdated
|
|
||
| impl SuiVMTestSetup { | ||
| pub fn new() -> Self { | ||
| let reference_gas_price = 500; |
There was a problem hiding this comment.
There isn't one...this is probably as good as any ...
| /// # Safety | ||
| /// This function exists solely for propagating a VMConfig down into the unit testing | ||
| /// environment. It should never be called anywhere else. | ||
| pub unsafe fn allow_unpublishable_for_move_unit_testing(&mut self) { |
There was a problem hiding this comment.
We have the testing feature flag in the VM for enabling certain things only in CLI builds. Wondering if it would be useful plumbing that over into here for this function possibly?
| }; | ||
|
|
||
| pub struct SuiGasMeter<'g>(pub &'g mut GasStatus); | ||
| pub struct SuiGasMeter<G: DerefMut<Target = GasStatus>>(pub G); |
There was a problem hiding this comment.
It's fancy, but I'm fine with keeping it.
|
|
||
| /// This function exists solely for propagating a VMConfig down into the unit testing | ||
| /// environment. It should never be called anywhere else. | ||
| #[cfg(feature = "testing")] |
tzakian
left a comment
There was a problem hiding this comment.
LGTM! Thanks for adding in the cfg gating.
Description
Test plan
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.
sui move testnow uses Sui's gas meter and limits.