Add exhaustive tests for fvm sdk api #1681
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fridrik01
force-pushed
the
fvmsdk-syscall-testing
branch
3 times, most recently
from
feb3e46 to
177776d
Compare
Stebalien
reviewed
Feb 23, 2023
| assert_eq!(res, Err(ErrorNumber::IllegalArgument)); | ||
|
|
||
| // Test for invalid Cid pointer | ||
| let res = sdk::sys::ipld::block_open("invalid ptr".as_ptr()); |
There was a problem hiding this comment.
This is still a valid pointer (i.e., valid memory). I'd cast a large integer to a pointer.
(also, now that you've checked this, I wouldn't bother checking for "out of bounds" pointers on every syscall as that could get tedious and we use the same code-paths)
| let res = sdk::sys::ipld::block_open("invalid ptr".as_ptr()); | ||
| assert_eq!(res, Err(ErrorNumber::IllegalArgument)); | ||
|
|
||
| // TODO (fridrik): Test for very large cid |
There was a problem hiding this comment.
Yeah, this'll be a bit annoying as you'll have to manually construct it. Also not something that's super critical to test.
| sdk::sys::ipld::block_create(DAG_CBOR, test_bytes.as_ptr(), test_bytes.len() as u32); | ||
| assert_eq!(res, Err(ErrorNumber::LimitExceeded)); | ||
|
|
||
| // TODO (fridrik): Test for TooManyBlock error requires INT32_MAX blocks |
There was a problem hiding this comment.
I wouldn't bother with this one, honestly. Given the current gas model, there's no way to hit this limit at the moment.
fridrik01
force-pushed
the
fvmsdk-syscall-testing
branch
2 times, most recently
from
f84aaba to
d76fff3
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #1681 +/- ##
=======================================
Coverage 55.44% 55.44%
=======================================
Files 146 146
Lines 14031 14031
=======================================
Hits 7779 7779
Misses 6252 6252
|
fridrik01
force-pushed
the
fvmsdk-syscall-testing
branch
from
3484ca1 to
7955d21
Compare
fridrik01
changed the title
Stebalien
approved these changes
Mar 9, 2023
Comment on lines
10
to
15
| std::panic::set_hook(Box::new(|info| { | ||
| sdk::vm::abort( | ||
| ExitCode::USR_ASSERTION_FAILED.value(), | ||
| Some(&format!("{}", info)), | ||
| ) | ||
| })); |
There was a problem hiding this comment.
nit: I recommend just calling sdk::initialize().
testing/integration/tests/main.rs
Outdated
| panic!("non-zero exit code {}", res.msg_receipt.exit_code) | ||
| } | ||
| } | ||
|
|
||
| assert_eq!(res.msg_receipt.exit_code, ExitCode::OK); |
|
Feel free to merge when ready. |
…FVM and SDK This allows us to more easily assert on certain error conditions: Example (this failed before): let res = sdk::sys::ipld::block_open(buf.as_mut_ptr()); assert_eq!(res, Err(ErrorNumber::IllegalArgument));
This commit refactors the fil-create-actor actor, main differences are: - If test failed with user asserts, they are now printed in the test output (before they were ignored) - Tests now verify result for specific error numbers - Added more tests for resolve_address and get_actor_code_cid
When deleting an actor with 0 balance, then this sself::self_destruct will always return Ok() regardless of whether the actor has already been deleted, or the beneficiary address does not exist or is itself
fridrik01
force-pushed
the
fvmsdk-syscall-testing
branch
from
c3c1e6b to
cb0fa3b
Compare
|
Thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds multiple tests for our fvm sdk api to confirm they behave according to fvm sdk docs.
Fixes: #585
Note that there are still tests to be written for
actor::next_actor_addressandcrypto::verify_consensus_fault.