tests: added unit test in fedimint-core/src/runtime.rs #5141
Conversation
Signed-off-by: sivasathyaseeelan <dnsiva.sathyaseelan.chy21@iitbhu.ac.in>
There was a problem hiding this comment.
Please don't use sleep, or at least not with 1 second values. CI time is a scarce resource. Where you really need to guarantee sleep, maybe use 1 millisecond. where you need coordination between threads use https://docs.rs/tokio/latest/tokio/sync/oneshot/index.html to unblock things.
Signed-off-by: sivasathyaseeelan <dnsiva.sathyaseelan.chy21@iitbhu.ac.in>
Hi @dpc, I have changed from 1 second to 1 millisecond. Can you please review this pr again? |
Signed-off-by: sivasathyaseeelan <dnsiva.sathyaseelan.chy21@iitbhu.ac.in>
| assert!(result.is_ok()); | ||
| } | ||
| _ = rx => { | ||
| assert!(false, "Timeout occurred before future completed"); |
There was a problem hiding this comment.
assert(false is just panic!, AFAIK.
| async fn test_timeout_elapsed() { | ||
| let duration = Duration::from_millis(1); | ||
| let future = async { | ||
| tokio::time::sleep(duration * 2).await; |
There was a problem hiding this comment.
std::future::pending().await can be used if you just want to test timeout
| async fn test_sleep() { | ||
| let duration = Duration::from_millis(1); | ||
| let start = Instant::now(); | ||
| sleep(duration).await; |
There was a problem hiding this comment.
Meh. This might be a test in tokio, it doesn't test our code really.
| let future = async move { | ||
| // Simulating some work | ||
| tokio::time::sleep(Duration::from_millis(1)).await; | ||
| let _ = tx.send("done"); |
There was a problem hiding this comment.
I don't understand what this test is trying to test.
But probably the future should be only rx.recv();
Then spawn that future as a task, then tx.send() to unblock it, then await on the result.
| }; | ||
|
|
||
| tokio::select! { | ||
| result = timeout(duration, future) => { |
There was a problem hiding this comment.
Here also, timeout, sleep, everything seems to be outside code, so I don't understand what we're testing.
My point with oneshot channels was that rx.recv() can be used as sleep, that you can "wake up" from anywhere with tx.send() thus guaranteeing order of operations, instead of relying on sleeps waiting up in a given sequence (which is never really guranteed).
|
Closing. Feel free to reopen with improvements or just create a different PR. |
Fix
This PR adds unit test fin fedimint/fedimint-core/src/runtime.rs and closes #5140