Integration test for CachingSession#1237
Open
Bouncheck wants to merge 2 commits intoscylladb:branch-hackathonfrom
Open
Integration test for CachingSession#1237Bouncheck wants to merge 2 commits intoscylladb:branch-hackathonfrom
CachingSession#1237Bouncheck wants to merge 2 commits intoscylladb:branch-hackathonfrom
Conversation
|
|
Bouncheck
force-pushed
the
caching-session-test
branch
from
570b6ab to
ccb7dcc
Compare
Bouncheck
changed the title
CachingSession
Bouncheck
force-pushed
the
caching-session-test
branch
2 times, most recently
from
bdcbd63 to
d6f9285
Compare
Lorak-mmk
requested changes
Feb 13, 2025
wprzytula
approved these changes
Feb 13, 2025
Moves `SingleTargetLBP` used by `integration/tablets.rs` to `common/utils.rs` Code is identical except added `pub(crate)` to the struct and its `target` field. Moving it to allow usage in `integration/caching_session.rs` from subsequent commit and in other tests.
Bouncheck
force-pushed
the
caching-session-test
branch
from
d6f9285 to
6950c0c
Compare
Author
|
Pushed a new version. Let me know if should I mark conversations as resolved or if it's on the reviewer if he agrees with the new version. |
Bouncheck
commented
Feb 13, 2025
| } | ||
|
|
||
| #[derive(Debug)] | ||
| #[allow(dead_code)] |
Author
There was a problem hiding this comment.
I don't understand why I had to add this line here even though I use this struct in my test. Shouldn't it be not a dead_code because of that?
Adds the integration test `caching_session::ensure_cache_is_used` for `CachingSession`. The test sends through the `scylla_proxy` several queries and verifies that the number of prepare requests is exactly as expected. If the cache is working correctly no extra prepare requests should appear. If it's not, the assertions will bring attention to that.
Bouncheck
force-pushed
the
caching-session-test
branch
from
6950c0c to
6cbd476
Compare
Collaborator
In this repo usually the author of the PR marks them as resolved. Is that the best way? I have no idea, maybe the reviewer should do this - we can discuss it in the future. |
muzarski
approved these changes
Feb 16, 2025
11 tasks
Lorak-mmk
added
the
area/testing
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Adds the integration test
caching_session::ensure_cache_is_usedforCachingSession. The test sends through thescylla_proxyseveral queries and verifies that the number of prepare requests is exactly as expected. If the cache is working correctly no extra prepare requests should appear. If it's not, the assertions will bring attention to that.Pre-review checklist
No public items introduced
./docs/source/.No docs to change for the test. Docs for CachingSession are separate issue.
Fixes:annotations to PR description.No open issues regarding integration tests for CachingSession