test(drive): improve coverage for query conditions and filter matching#3441
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThe PR adds extensive unit test coverage to the query condition and filter modules, expanding test assertions for SQL value conversion, where clause parsing, operator validation, and document query filtering across multiple operator types and edge cases. No production code changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Review GateCommit:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3441 +/- ##
============================================
+ Coverage 80.66% 80.76% +0.10%
============================================
Files 2852 2852
Lines 285371 286583 +1212
============================================
+ Hits 230190 231463 +1273
+ Misses 55181 55120 -61
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/rs-drive/src/query/filter.rs (1)
1749-2691: Optional: extract common fixture/filter builders for test ergonomics.There’s substantial repeated setup for contract/filter scaffolding; small helpers would reduce noise and make intent-focused assertions stand out.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-drive/src/query/filter.rs` around lines 1749 - 2691, Extract small test helpers to reduce repetition: create a helper that wraps get_data_contract_fixture and returns the contract (used across tests), and one or more builder functions to construct DriveDocumentQueryFilter instances for a given document_type_name and DocumentActionMatchClauses (e.g., helper like make_contract_fixture() and make_filter(document_type_name, action_clauses)). Update tests to call these helpers and to construct InternalClauses / ValueClause inputs via small factory helpers (e.g., build_value_clause, build_internal_clauses) so repeated boilerplate around get_data_contract_fixture, DriveDocumentQueryFilter, DocumentActionMatchClauses, InternalClauses, and ValueClause is centralized and tests become concise.packages/rs-drive/src/query/conditions.rs (1)
3991-4099: Optional: reduce duplication in equal-valueless_thantests.These cases are clear, but a table-driven helper would keep the suite easier to maintain as value variants evolve.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-drive/src/query/conditions.rs` around lines 3991 - 4099, The tests duplicate the same equal-value less_than assertions across many Value variants; refactor by creating a table-driven helper that accepts a Value and the test name, then iterates over a list of (Value::U64(10), Value::U32(5), Value::I32(-3), Value::U16(100), Value::U8(7), Value::I8(-1), Value::U128(999), Value::Bytes(vec![1,2,3]), Value::Text("same".to_string()), Value::Float(2.5)) and for each constructs a WhereClause (using the existing WhereClause struct and Equal operator) and asserts less_than(&a, true).unwrap() is true and less_than(&a, false).unwrap() is false; replace the individual test functions (less_than_*_equal_values_with_allow_eq) with a single parameterized/test helper that calls the helper for each variant to eliminate duplication while keeping checks against the less_than method.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/rs-drive/src/query/conditions.rs`:
- Around line 3991-4099: The tests duplicate the same equal-value less_than
assertions across many Value variants; refactor by creating a table-driven
helper that accepts a Value and the test name, then iterates over a list of
(Value::U64(10), Value::U32(5), Value::I32(-3), Value::U16(100), Value::U8(7),
Value::I8(-1), Value::U128(999), Value::Bytes(vec![1,2,3]),
Value::Text("same".to_string()), Value::Float(2.5)) and for each constructs a
WhereClause (using the existing WhereClause struct and Equal operator) and
asserts less_than(&a, true).unwrap() is true and less_than(&a, false).unwrap()
is false; replace the individual test functions
(less_than_*_equal_values_with_allow_eq) with a single parameterized/test helper
that calls the helper for each variant to eliminate duplication while keeping
checks against the less_than method.
In `@packages/rs-drive/src/query/filter.rs`:
- Around line 1749-2691: Extract small test helpers to reduce repetition: create
a helper that wraps get_data_contract_fixture and returns the contract (used
across tests), and one or more builder functions to construct
DriveDocumentQueryFilter instances for a given document_type_name and
DocumentActionMatchClauses (e.g., helper like make_contract_fixture() and
make_filter(document_type_name, action_clauses)). Update tests to call these
helpers and to construct InternalClauses / ValueClause inputs via small factory
helpers (e.g., build_value_clause, build_internal_clauses) so repeated
boilerplate around get_data_contract_fixture, DriveDocumentQueryFilter,
DocumentActionMatchClauses, InternalClauses, and ValueClause is centralized and
tests become concise.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c3883eb0-3002-4614-84e1-63d410f5d089
📒 Files selected for processing (2)
packages/rs-drive/src/query/conditions.rspackages/rs-drive/src/query/filter.rs
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
2 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.
Summary
Adds 102 tests across 2 files in rs-drive covering query condition evaluation and document subscription filtering.
query/conditions.rssql_value_to_platform_value,from_componentsparsing,less_thancomparisons,value_shape_okvalidation,ValueClause::matches_value,WhereOperator::evalquery/filter.rsvalidate()for all transition types,matches_original_document,evaluate_clauses,get_value_by_pathTest plan
cargo test -p drive --lib— 1985 passedcargo fmt --allclean🤖 Generated with Claude Code
Summary by CodeRabbit