-
Notifications
You must be signed in to change notification settings - Fork 358
Cosmos: Regression tests for feed-range-scoped query fan-out #4705
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
Open
simorenoh
wants to merge
18
commits into
main
Choose a base branch
from
simorenoh/cosmos-feedrange-query-investigation
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 15 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
0efa7b8
Restrict cross-partition query fan-out to feed-range scope
simorenoh ba795b8
Add PR link to changelog entry
simorenoh 6c81640
Make feed-range fan-out tests observe the clip
simorenoh ad9e832
Mirror feed-range fan-out fix in driver CHANGELOG
simorenoh 96390fd
Pin disjoint-target error behavior with a test
simorenoh b97b0cf
Merge branch 'main' into simorenoh/cosmos-feedrange-query-investigation
simorenoh 19bb6d4
Add e2e regression test for feed-range-scoped queries
simorenoh 1d74c9d
Add feed-range test words to Cosmos cspell ignore list
simorenoh 9f3e986
Merge branch 'main' into simorenoh/cosmos-feedrange-query-investigation
simorenoh 6a29dde
Merge remote-tracking branch 'origin/main' into simorenoh/cosmos-feed…
simorenoh b817f11
Reframe #4705 changelog entries as regression tests
simorenoh 5c9b577
Attribute feed-range bugfix changelog to #4319
simorenoh 9d90f1a
Merge remote-tracking branch 'origin/main' into simorenoh/cosmos-feed…
simorenoh 0d29d2e
Assert per-range results in feed-range e2e test
simorenoh 0d3c8dc
Merge branch 'main' into simorenoh/cosmos-feedrange-query-investigation
simorenoh 194b406
Merge remote-tracking branch 'origin/main' into simorenoh/cosmos-feed…
simorenoh 140feda
Assert full per-range grouping in feed-range e2e test
simorenoh ab07e94
Hardcode per-range grouping in feed-range e2e test
simorenoh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
Oops, something went wrong.
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.
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.
Given that we're generating a fixed list of IDs, I feel like it would be better to just have a single
assert_eq!(vec![vec![...], ...], per_range_ids)that makes it easy to look at the IDs and say "Yep, that looks correct". This is all correct but it's asserting three separate properties (non-empty, disjoint results, fully spanning) which could also be trivially validated by looking at the actual ranges, and would be easier to read.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.
Switched to the hardcoded literal you asked for. To de-risk the coupling concern I raised above, I ran the grouping against a real Cosmos DB account and the local emulator: both split into the same two physical ranges at EPK
0x1FFF...FFand produced the identical per-range id grouping. The EPK hash and the split boundary are algorithm-driven, not environment-specific, so the literal is stable. It's now a singleassert_eq!(expected, per_range_ids)you can eyeball. Thanks!