-
Notifications
You must be signed in to change notification settings - Fork 111
Extensive testing for all examples #365
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: main
Are you sure you want to change the base?
Conversation
bec6b71 to
5fb8216
Compare
stylus-tools/src/utils/testing.rs
Outdated
| use alloy::primitives::{Address, TxHash}; | ||
| use eyre::Result; | ||
|
|
||
| pub trait ExampleContractTester { |
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.
What is the advantage of defining a trait for that?
How about moving those fn out of the trait, they don't rely on self, and change init to something like this?
pub fn init(expected_abi: String, expected_constructor: String) -> Result<(Node, Address)>
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 would rename this from init to something like init_and_test_all too.
Maybe there will be a future test scenario that we will only want to init, and do those other tests in the specific test implementation.
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.
The main reason was because I wanted to override some methods, e.g. deployer(), but I can have both.
|
|
||
| #[tokio::test] | ||
| async fn call() -> Result<()> { | ||
| let exporter = stylus_tools::Exporter::builder().build(); |
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 suppose that you didn't rely on stylus_tools::utils::testing::ExampleContractTester in this test, and other tests, since you want to confirm that this stylus_tools::utils::testing::ExampleContractTester strategy is better than replicating the code in every test, right?
I think not replicating is better in this case, so we can move forward with a strategy like stylus_tools::utils::testing::ExampleContractTester 🙂
0ae4d46 to
9b9cf2f
Compare
Description
All examples now test more stylus commands. Enabled the tuple example.
Checklist