Skip to content
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

[WIP] feat(fuzz) - add test progress #7914

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

grandizzy
Copy link
Collaborator

@grandizzy grandizzy commented May 13, 2024

Motivation

Closes #585
Closes #3277
Closes #4452

Solution

  • make use of indicatif crate
  • add --show-progress arg (to give more time for feedback / OS tests, then make it default and add a --stream-results flag for legacy output)
  • long running tasks (fuzz and invariant tests) get their own entry / progress bar in test suite progress. Fuzz tests shows [current / total] runs; Invariants show [current / total] runs and then the [current shrink / max shrink] runs (if case)
  • when a test suite is finished it is moved from running tasks to summary section showing the result summary
  • when all test suits are completed then detailed results are streamed

test-out

TBD: rn the fail fast flag doesn't stop test case execution (rayon par iter continue to run until all tasks are completed), need to find a way to exit early when such

@grandizzy grandizzy marked this pull request as ready for review May 13, 2024 09:47
@mattsse
Copy link
Member

mattsse commented May 13, 2024

rn the fail fast flag doesn't stop test case execution (rayon par iter continue to run until all tasks are completed), need to find a way to exit early when such

I think we could add a termination check with something like an Arc<AtomicBool> to exit early

@grandizzy grandizzy changed the title feat(forge) - add test progress feat(fuzz) - add test progress May 14, 2024
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DaniPopes should be proceed with this given that this is opt in for now and do a larger cleanup for the nested suite par_iter and exit early separately or do this here?

@grandizzy grandizzy requested review from klkvr May 20, 2024 15:36
@@ -166,6 +166,61 @@ macro_rules! update_progress {
};
}

/// Creates progress object and progress bar.
#[macro_export]
macro_rules! init_tests_progress {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is how it exists already, but I'd like these macros to be helper functions and/or structs instead, since that's all they are

let result = self.run_tests(id, contract, db.clone(), filter, &handle);
let _ = tx.send((id.identifier(), result));
})
if show_progress {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok with this for now, but I think we want to have one single par_iter here and also handle all output in between progress bars

@grandizzy grandizzy changed the title feat(fuzz) - add test progress [WIP] feat(fuzz) - add test progress May 27, 2024
@grandizzy grandizzy marked this pull request as draft May 27, 2024 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants