-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix(forge): enable fail fast flag #11328
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
Conversation
zerosnacks
left a comment
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.
Makes sense 👍
yash-atreya
left a comment
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.
lgtm!
DaniPopes
left a comment
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.
lgtm, nits
crates/forge/src/runner.rs
Outdated
|
|
||
| /// Runs all tests for a contract whose names match the provided regular expression | ||
| pub fn run_tests(mut self, filter: &dyn TestFilter) -> SuiteResult { | ||
| pub fn run_tests(mut self, filter: &dyn TestFilter, fail_fast: &FailFast) -> SuiteResult { |
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.
can we set this in self? it can also be cloned just fine
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.
set in 9d61091
crates/forge/src/multi_runner.rs
Outdated
| filter: &dyn TestFilter, | ||
| tx: mpsc::Sender<(String, SuiteResult)>, | ||
| show_progress: bool, | ||
| fail_fast: bool, |
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.
set in self / tcfg instead?
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.
set in 9d61091
DaniPopes
left a comment
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.
lgtm
* fix(forge): enable fail fast flag * Cleanup after review
Motivation
forge test):--fail-fastflag does not work as it is not applied across multiple test suites #6529Solution
PR Checklist