-
Notifications
You must be signed in to change notification settings - Fork 7
Add cheatcode test #435
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
base: master
Are you sure you want to change the base?
Add cheatcode test #435
Conversation
This reverts commit cc75e46.
| // SKIP it should not affect pallet-revive execution | ||
| // revive_cheat_test_original!(test_env, "Env"); | ||
| // EXTCODECOPY compilation issue | ||
| revive_cheat_test_original!(test_etch, "Etch"); |
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.
@smiasojed etch, prank, mock tests were already ported by me and passing here:
| pub mod cheat_mock_call; |
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.
yes I saw it, I will clean this up. But anyway original etch is failing - probably issue with EXTCODECOPY - but it should not affect our EVM path.
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.
yeah EXTCODECOPY compilation was the reason I modified. the test slightly.
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 think we should not error out in this case, because it means that we do not have pvm code which is fine for evm path. @pkhry WDYT?
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.
error out
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.
It means that if any project uses opcodes not supported in PVM, the EVM tests will fail. I’m not sure if this is the right approach.
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.
It means that if any project uses opcodes not supported in PVM, the EVM tests will fail. I’m not sure if this is the right approach.
then just disable pvm for now i guess
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.
or if compilation for pvm fails just skip this bytecode in dualcompiledcontract - the user will not be able to switch to pvm in test
|
for |
| #[case::evm(ReviveRuntimeMode::Evm)] | ||
| #[tokio::test(flavor = "multi_thread")] | ||
| async fn $test_name(#[case] runtime_mode: ReviveRuntimeMode) { | ||
| let filter = Filter::new(".*", ".*", &format!(".*/{}/{}.*", $dir, $file_pattern)); |
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.
shouldn't this be */{}/{}.t.* to correctly grab tests by their filename?
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.
yes there is a bug
How to reproduce:
cargo test --package forge --test it -- revive::cheats_individual --nocapturefailures: