-
Notifications
You must be signed in to change notification settings - Fork 13
Introduce an IT for consistency #343
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new Rust-based integration test for the C++/Rust driver wrapper that verifies consistency levels are applied correctly.
- Adds utility functions and test harness in
tests/integration/utils.rs
to run a 3-node dry-mode proxy cluster and inspect requests. - Implements a comprehensive
consistency_is_correctly_set_in_cql_requests
test intests/integration/consistency.rs
. - Adjusts visibility in
argconv.rs
and updates dev-dependencies to pull inscylla-cql
anditertools
.
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
tests/integration/utils.rs | Utilities for setting up a proxy cluster, error assertion macros, and request-forging helpers |
tests/integration/main.rs | Registers the consistency and utils modules for integration tests |
tests/integration/consistency.rs | New async integration test covering all consistency and serial-consistency settings |
src/argconv.rs | Changed several internal pointer conversion/borrowing methods to pub |
Cargo.toml | Added scylla-cql and itertools to dev-dependencies |
Comments suppressed due to low confidence (2)
scylla-rust-wrapper/src/argconv.rs:9
- [nitpick] Exposing
ptr_to_cstr
as a public API may unnecessarily broaden your crate's surface area. Consider reverting it topub(crate)
unless external consumers genuinely need this function.
pub unsafe fn ptr_to_cstr(ptr: *const c_char) -> Option<&'static str> {
scylla-rust-wrapper/src/argconv.rs:13
- [nitpick] Similarly,
ptr_to_cstr_n
is an internal helper and could be keptpub(crate)
to avoid exposing low-level unsafe conversions in your public API.
pub unsafe fn ptr_to_cstr_n(ptr: *const c_char, size: size_t) -> Option<&'static str> {
3b73ec3
to
99fb919
Compare
Rust integration tests can now use the `argconv` module's contents to convert between Rust and C types. This will soon allow us to write a fully-fledged Rust integration test for consistency settings.
99fb919
to
1d3a160
Compare
Rebased on master and appeased clippy. |
5089661
to
fead288
Compare
The test is based on an analogous one that is present in the Rust Driver. It finally convinces me that the consistencies are handled correctly.
fead288
to
289727b
Compare
This PR introduces the first integration test for the CPP-Rust Driver that is written in Rust.
The test is based on an analogous one that is present in the Rust Driver. It finally convinces me that the consistencies are handled correctly.
Pre-review checklist
[] I have implemented Rust unit tests for the features/changes introduced.[] I have enabled appropriate tests inMakefile
in{SCYLLA,CASSANDRA}_(NO_VALGRIND_)TEST_FILTER
.[] I added appropriateFixes:
annotations to PR description.