-
Notifications
You must be signed in to change notification settings - Fork 190
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
tests: refactor and add new tests #1405
Conversation
@kariy so to verify if two any other ideas to test the |
Without adding more complexity, you may try to use a |
Yeah I think that should probably work but I was wondering if there is any better way to do it |
Yeah, as @glihm mentioned, |
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.
Good work on that!
May worth ordering the test by tested field for easier navigation and verification of which test is here.
…e specifying `private_key` to be consistent with `keystore`
@@ -33,7 +33,6 @@ pub struct AccountOptions { | |||
|
|||
#[arg(long = "password", env = DOJO_KEYSTORE_PASSWORD_ENV_VAR)] | |||
#[arg(value_name = "PASSWORD")] | |||
#[arg(requires = "keystore_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.
removed this require so that when passing keystore_password
we can still get keystore_path
from environment metadata
pub struct AccountOptions { | ||
#[arg(long, env = DOJO_ACCOUNT_ADDRESS_ENV_VAR)] | ||
pub account_address: Option<FieldElement>, | ||
|
||
#[arg(long, env = DOJO_PRIVATE_KEY_ENV_VAR)] | ||
#[arg(requires = "account_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.
removed to be consistent with the behaviour of keystore
.
user can specify account_address
in Scarb.toml
and provide private_key
from commandline.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1405 +/- ##
==========================================
+ Coverage 67.13% 67.51% +0.37%
==========================================
Files 231 231
Lines 20858 21023 +165
==========================================
+ Hits 14003 14193 +190
+ Misses 6855 6830 -25 ☔ View full report in Codecov by Sentry. |
follow up: #1358