-
Notifications
You must be signed in to change notification settings - Fork 31
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
chore(infra): add run_until utility fn #2320
Conversation
Artifacts upload workflows: |
Benchmark movements: |
commit-id:2d145538
commit-id:2959cc90
commit-id:4d0abfb5
commit-id:8453d204
c76ee1c
to
ce0295c
Compare
Benchmark movements: full_committer_flow performance improved 😺 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## spr/main/4d0abfb5 #2320 +/- ##
====================================================
Coverage ? 77.40%
====================================================
Files ? 387
Lines ? 40685
Branches ? 40685
====================================================
Hits ? 31493
Misses ? 6862
Partials ? 2330 ☔ View full report in Codecov by Sentry. |
746f4ba
to
ed3da77
Compare
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.
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Itay-Tsabary-Starkware)
crates/infra_utils/src/run_until_test.rs
line 20 at r1 (raw file):
// Run the function with a short interval and a maximum of 5 attempts. let result = run_until(100, 5, mock_executable, condition, None).await;
Don't you want to test that tracing works?
crates/infra_utils/src/run_until.rs
line 11 at r1 (raw file):
pub struct TraceConfig { pub level: LogLevel, pub message: String,
I think its more message_prefix than message.
Code quote:
message
crates/infra_utils/src/run_until.rs
line 79 at r1 (raw file):
if let Some(config) = &trace_config { let failure_message = format!("{}: Condition not met after {} attempts.", config.message, max_attempts);
I suggest something like: Condition not met after the maximum number of attempts - {}
Code quote:
Condition not met after {} attempts
ed3da77
to
2f0f137
Compare
There hasn't been any activity on this pull request recently, and in order to prioritize active work, it has been marked as stale. |
Stack: