chore: use sleep_in_test over sleep in tests #4376
Conversation
The function fedimint_core::task::sleep_in_test should be used over fedimint_core::task::sleep in tests so the duration of the sleep and a comment describing the reason for sleeping can be logged. Closes: fedimint#1792
kernelkind
force-pushed
the
issue-1792
branch
from
3a83fcb
to
756f124
Compare
| @@ -617,6 +617,19 @@ impl<T: Sync> MaybeSync for T {} | |||
| #[cfg(target_family = "wasm")] | |||
| impl<T> MaybeSync for T {} | |||
|
|
|||
| // Used in tests when sleep functionality is desired so it can be logged. | |||
| // Must include comment describing the reason for sleeping. | |||
| pub async fn sleep_in_test(comment: impl AsRef<str>, duration: Duration) { | |||
There was a problem hiding this comment.
better to use tracing::Span instead of str comment
There was a problem hiding this comment.
Shouldn't comment be reason, and just another value with a fixed "Sleep" message? Or are there any other benefits? I'm not even sure how to do Span :D
There was a problem hiding this comment.
Span just lets you express variables better instead of using format!. also spans have parent-child relation.
something like this
async move {
info!(target: LOG_TEST, ?duration, "sleeping");
sleep(duration).await;
}.instrument(span).await
There was a problem hiding this comment.
caller is expected to use info_span! or debug_span! to create spans
There was a problem hiding this comment.
Is it useful to create a new span just for a sleep? I guess if you already have one it's nice, but what would one gain from adding it here otherwise?
| @@ -617,6 +617,19 @@ impl<T: Sync> MaybeSync for T {} | |||
| #[cfg(target_family = "wasm")] | |||
| impl<T> MaybeSync for T {} | |||
|
|
|||
| // Used in tests when sleep functionality is desired so it can be logged. | |||
| // Must include comment describing the reason for sleeping. | |||
| pub async fn sleep_in_test(comment: impl AsRef<str>, duration: Duration) { | |||
There was a problem hiding this comment.
Is it useful to create a new span just for a sleep? I guess if you already have one it's nice, but what would one gain from adding it here otherwise?
The function fedimint_core::task::sleep_in_test should be used over fedimint_core::task::sleep in tests so the duration of the sleep and a comment describing the reason for sleeping can be logged.
Closes: #1792