-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat: rewrite inline config using figment #9414
Conversation
invariant: config.invariant.clone(), | ||
..Default::default() | ||
}) | ||
.with_test_options(TestOptions::new(output, config.clone())?) |
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.
This wasn't parsing inline config, so it never worked for coverage
@@ -48,8 +48,9 @@ async fn test_invariant_with_alias() { | |||
|
|||
#[tokio::test(flavor = "multi_thread")] | |||
async fn test_invariant_filters() { | |||
let mut runner = TEST_DATA_DEFAULT.runner(); | |||
runner.test_options.invariant.runs = 10; | |||
let mut runner = TEST_DATA_DEFAULT.runner_with(|config| { |
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.
These are changed because test_options is not mutable after construction
self.config().project().expect("Failed to build project") | ||
} | ||
|
||
pub fn test_opts(&self, output: &ProjectCompileOutput) -> TestOptions { |
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.
These are moved to fn config
since test options uses Config now
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.
this is a lot better
/// Contains per-test specific "invariant" configurations. | ||
pub inline_invariant: InlineConfig<InvariantConfig>, | ||
/// The base configuration. | ||
pub config: Arc<Config>, |
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.
nice
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.
looks good! have single one comment re TODO, pls check
crates/forge/tests/it/fuzz.rs
Outdated
@@ -145,6 +147,8 @@ async fn test_persist_fuzz_failure() { | |||
assert_eq!(initial_calldata, new_calldata, "run {i}"); | |||
} | |||
|
|||
// TODO |
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.
will this be a followup PR or just a leftover?
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.
outdated
* feat: rewrite inline config using figment * wip * wip * fix: use same GasLimit type * wip * fixes * tests * test fixes * fmt * test update
Rewrite the current manual inline config value parser by using the existing
figment
infrastructure used for parsing the mainConfig
struct.Inline config is now just some maps that hold figment values, and that returns a
figment::Provider
when queried with a (contract, function) pair.Issues like #5371 are NOT fixed because this is not even implemented for the main foundry.toml parsing. Invalid keys are accepted even in foundry.toml. Once it's fixed for foundry.toml, it will easily be applied to inline config as well.
This also fixes inline config for coverage, which was not getting parsed before.