Request coordinator tests #1352
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`test_shard_out_of_range()` was put in `load_balancing.rs`, while `shards.rs` seems to be a much better place for it.
The only test that is left there regards latency latency_awareness, so such rename makes sense. And module_inception is no longer there.
Vector type is definitely more about a CQL collection than about the Session.
…ns.rs The test is definitely more about CQL collections than about the Session.
This test is about load balancing more than about the Session.
This obviously belongs to `metadata` category.
These are tests that simply check that driver keeps metadata of some kind. As these tests are similar in their concept, I believe it makes sense to have them all in one file.
Materialized views test suits metadata/contents.rs as well.
Having read the contents of the test, I was confused what it actually tests. `git blame` shed some light, so I pasted the commit message as a doc comment: "This test case indicates that we support enough CQL types to parse schema keyspace information.". A-ha! But we no longer type check nor deserialize rows eagerly! The deserialization refactor seems to have rendered this test case useless. To fix this, I materialise all rows as Row, which actually performs the type check and deserialization. This way I virtually bring back the old, intended semantics of this test.
Test `test_unusual_valuelists()` is moved from `session/session.rs` to `types/valuelists.rs`.
As `Session::get_keyspace()` returns the name of the keyspace that was set in `Session::use_keyspace()`, it makes sense for tests of both functionalities to reside in the same place.
Test `test_keyspaces_to_fetch()` is extracted from `session/session.rs` and now comprises `metadata/keyspace_filtering.rs`.
Tests `tests_timestamp()` and `test_timestamp_generator()` are extracted from `session/session.rs` into `statement/timestamps.rs`
There is a bunch of tests which check that statements are reprepared if the DB responds with DbError::Unprepared. They are extracted from `session/session.rs` into `statements/unprepared_reprepare.rs`.
Test `test_unprepared_statement()` is extracted into `statements/unprepared/basic.rs`.
Test `test_prepared_statement()` is extracted into `statements/prepared/basic.rs`.
Test `test_prepared_batch()` is extracted into `statements/batch/basic.rs`.
The test is extracted into `statements/batch/counter_batch.rs`.
The test is extracted into `statements/batch/multi_table_batch.rs`.
The test is extracted into `statements/prepared/prepared_inherits_config.rs`.
The test is about creating a new Session, so it fits there well.
Tests: - test_iter_works_when_retry_policy_returns_ignore_write_error() - test_iter_methods_with_modification_statements() are extracted into `session/pager.rs`.
The test is extracted into `statements/prepared/partitioner.rs`.
The test is extracted into `statements/prepared/token_calculation.rs`.
The test is extracted to `statements/request_timeout.rs`.
The test case brings no value to our suite. There are plenty of other tests that call `Session::await_schema_agreement()`.
The test is extracted to `statements/named_bind_markers.rs`.
The remaining two tests in session.rs: - test_db_errors(), - test_rate_limit_exceeded_exception(), regard DB errors, so the file is renamed accordingly. This finishes dissolution of the old overloaded `session_test.rs` module. Hooray!
The test is extracted into `prepared/col_specs.rs`.
The only tests remaining in `statement.rs` are about coordinator enforcing, so the file is renamed accordingly.
The test is strictly about unprepared statements, so it's moved to the corresponding module.
The test is strictly about prepared statements, so it's moved to the corresponding module.
The file will soon contain more coordinator-related tests than only enforcing-related ones.
The test checks that if a node is enforced as the coordinator of a request, the [Coordinator] struct exposed on the request result (`QueryResult` and `QueryPager`) contains that `Node`. Fixes: scylladb#1347
wprzytula
added
the
area/testing
|
|
muzarski
approved these changes
May 14, 2025
Lorak-mmk
approved these changes
May 16, 2025
No, I can test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stacked on: #1351
Adds a test case for exposed
Coordinator. The test checks that if a node is enforced as the coordinator of a request, theCoordinatorstruct exposed on the request result (QueryResultandQueryPager) contains thatNode.Fixes: #1347
Pre-review checklist
[ ] I have provided docstrings for the public items that I want to introduce.[ ] I have adjusted the documentation in./docs/source/.Fixes:annotations to PR description.