Skip to content

Commit

Permalink
Fix client state test cleanup
Browse files Browse the repository at this point in the history
The test `test_process_client_handling_stream_request_latest_voters_snapshot`
occasionally fails currently in an inconsistent manner.  This tests doesn't match the
cleanup steps of the other tests.  As a result it makes this test susceptible to async
task scheduling differences between runs.

In general the tasks being tested are considered "critical" and so such failures need
to result in a `panic!` if they occur.  However, with async-std's tasks such panics
cannot be easily intercepted / caught, especially with how they are spawned by the
program with `InternalClientMessageProcessingTask::new`.  So if the channels that
supply the task with the information coming in get `drop`ed and then the task attempts
to consume them, a panic will result.

The solution to this is to cancel the task as part of the test before the channels are
dropped.  This will ensure that the task itself no longer gets scheduled, and the
panic is avoided.
  • Loading branch information
Ayiga committed Aug 14, 2024
1 parent 06bf72f commit 68060a4
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion node-metrics/src/service/client_state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1308,7 +1308,7 @@ pub mod tests {
let (internal_client_message_sender, internal_client_message_receiver) = mpsc::channel(1);
let (server_message_sender_1, mut server_message_receiver_1) = mpsc::channel(1);
let (server_message_sender_2, mut server_message_receiver_2) = mpsc::channel(1);
let _process_internal_client_message_handle = InternalClientMessageProcessingTask::new(
let mut process_internal_client_message_handle = InternalClientMessageProcessingTask::new(
internal_client_message_receiver,
data_state,
client_thread_state,
Expand Down Expand Up @@ -1359,6 +1359,10 @@ pub mod tests {
voters_1, voters_2
]))),
);

if let Some(task_handle) = process_internal_client_message_handle.task_handle.take() {
assert_eq!(task_handle.cancel().await, None);
}
}

#[async_std::test]
Expand Down

0 comments on commit 68060a4

Please sign in to comment.