Conversation
…eat/cmd_validator_tests
| async fn run_tests_with_timeout( | ||
| args: &TestValidatorArgs, | ||
| tests: &[ValidatorTestCase], | ||
| ) -> Vec<TestResult> { | ||
| let (tx, mut rx) = mpsc::channel::<TestResult>(100); | ||
| let mut test_iter = tests.iter().peekable(); | ||
|
|
||
| let timeout_result = tokio::time::timeout(args.test_config.timeout, async { | ||
| for &test_case in test_iter.by_ref() { | ||
| let result = run_single_test(args, test_case).await; | ||
| let _ = tx.send(result).await; | ||
| } | ||
| }) | ||
| .await; | ||
|
|
||
| // Collect all completed results | ||
| drop(tx); | ||
| let mut results = Vec::new(); | ||
| while let Ok(result) = rx.try_recv() { | ||
| results.push(result); | ||
| } | ||
|
|
||
| if timeout_result.is_err() | ||
| && let Some(&interrupted_test) = test_iter.peek() | ||
| { | ||
| results.push( | ||
| TestResult::new(interrupted_test.name()) | ||
| .fail(std::io::Error::other(ERR_TIMEOUT_INTERRUPTED)), | ||
| ); | ||
| } | ||
|
|
||
| results | ||
| } |
There was a problem hiding this comment.
This does not seem to handle the timeout properly.
Let's say we run only 1 tests, the test_iter is peakable, and inside the for &test_case in test_iter.by_ref() { it get one test and run.
Then the test timeout happens => the test_iter.peek() will return None => test result empty.
Validate by
cargo run -p pluto-cli -- alpha test validator \
--test-cases PingLoad \
--timeout 1ns
| } | ||
| }); | ||
|
|
||
| let _ = handle.await; |
There was a problem hiding this comment.
This only waits for the scheduler task, not the spawned ping worker tasks themselves. So when calling rx.try_recv(), some ping attempts may still be in flight and their RTTs are not included in the final score.
|
|
||
| let mut result = TestResult::new(ValidatorTestCase::PingLoad.name()); | ||
|
|
||
| let (tx, mut rx) = mpsc::channel::<Duration>(100); |
There was a problem hiding this comment.
100 is so small, charon uses math.MaxInt16
…ulav/cmd_validator_tests
PoulavBhowmick03
changed the title
| for &test_case in tests { | ||
| let remaining = timeout_deadline.saturating_duration_since(tokio::time::Instant::now()); | ||
|
|
||
| match tokio::time::timeout(remaining, run_single_test(args, test_case)).await { |
There was a problem hiding this comment.
The inner spawned tasks in run_single_test will still be running even after the timeout.
| } | ||
|
|
||
| /// Timeout error message | ||
| const ERR_TIMEOUT_INTERRUPTED: &str = "timeout"; |
| ) -> Vec<TestResult> { | ||
| let mut results = Vec::new(); | ||
| let timeout_deadline = tokio::time::Instant::now() | ||
| .checked_add(args.test_config.timeout) |
There was a problem hiding this comment.
Can we use saturating_add here?
2 participants
Closes #237
ping_test,ping_test,ping_load_testtocli/src/commands/test/validator.rsodinson charon-rs$pluto alpha test validator