tests: introducing keep-alive tests #1241
Conversation
introducing test that validates the keep-alive behavier those tests are based on: `com.datastax.oss.driver.core.heartbeat.HeartbeatIT`
| ) { | ||
| let mut total_keep_alives: u32 = 0; | ||
| let start = tokio::time::Instant::now(); | ||
| while total_keep_alives < expected_number_of_keep_alive && start.elapsed() < timeout { |
There was a problem hiding this comment.
I'm not sure this is the best idiomatic way of doing such a thing
There was a problem hiding this comment.
It is not. I'd do it roughly like that (didn't test the code, so it may not compile):
tokio::time::timeout(timeout, async move {
for _ in 0..expected_number_of_keep_alive {
request_rx.recv().await.unwrap();
}
});
| #[cfg(not(scylla_cloud_tests))] | ||
| async fn should_send_heartbeat_on_regular_connection() { | ||
| /* | ||
| // Prime a simple query so we get at least some results |
There was a problem hiding this comment.
didn't got to implementing all of the tests, left the java code as skeletons
|
test seems to be failing in CI, when working locally... |
|
|
| /// Waits for a specified number of keep-alive messages to be received within a given timeout period. | ||
| /// | ||
| /// # Arguments | ||
| /// | ||
| /// * `request_rx` - An `UnboundedReceiver` that receives the keep-alive messages. | ||
| /// * `expected_number_of_keep_alive` - The number of keep-alive messages expected to be received. | ||
| /// * `timeout` - The maximum duration to wait for the expected number of keep-alive messages. | ||
| /// | ||
| /// # Panics | ||
| /// | ||
| /// This function will panic if the number of received keep-alive messages does not match the | ||
| /// expected number within the timeout period. | ||
| /// | ||
| async fn wait_for_keep_alive<T>( |
There was a problem hiding this comment.
👍 I really like that you put such extensive comment, such things are sadly missing in many of our tests.
There was a problem hiding this comment.
It's mostly copilot, I just removed a few unneeded things after him
| .unwrap(); | ||
|
|
||
| // TODO: No way to get node status, no as in java-driver | ||
|
|
There was a problem hiding this comment.
WDYM by node status? Do you need information if the driver is connected to a given node?
If so you can get it with session.cluster_state().get_nodes_info()[node_idx].is_connected()
How are you running the tests? I started to look into "should_not_send_heartbeat_during_protocol_initialization", and I don't understand why it fails. |
The reason is very simple (yet not so easy to spot). These tests are placed under all tests are passing. These tests should be moved to |
introducing test that validates the keep-alive behavier
those tests are based on:
com.datastax.oss.driver.core.heartbeat.HeartbeatITPre-review checklist
./docs/source/.Fixes:annotations to PR description.