Uh oh!
There was an error while loading. Please reload this page.
test(drive): improve identity fetch, balance, and public key hash coverage - #3443
Conversation
Add 67 new unit tests across the identity fetch subsystem targeting the lowest-coverage areas: queries, fetch_by_public_key_hashes, full_identity, balance, and contract_keys. Tests cover query construction (all branch variants), IdentityProveRequestType conversion, fetch with/without transactions, with-costs variants, negative balance/debt paths, estimated mode, public key hash lookups (unique and non-unique), batch fetches with missing identities, pagination with start_at (inclusive/exclusive, ascending/descending), and contract key fetches. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 12 minutes and 39 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds comprehensive unit test coverage to identity fetch modules in the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 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 |
⏳ Review in progress (commit d13c2ae) |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@## v3.1-dev #3443 +/- ##
============================================
+ Coverage 80.66% 81.35% +0.68%
============================================
Files 2852 2852 Lines 285371 291573 +6202 ============================================
+ Hits 230190 237203 +7013 + Misses 55181 54370 -811
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/rs-drive/src/drive/identity/fetch/contract_keys/mod.rs`:
- Around line 94-101: The test currently treats Err(_) as acceptable when
matching on result, weakening coverage; instead assert that result is Ok and
that the returned map is empty. Replace the match on result (the branch Ok(map)
=> assert!(map.is_empty()), Err(_) => { ... }) with a strict check such as let
map = result.expect("expected Ok(map) for non-existent contract-key");
assert!(map.is_empty()); and apply the same change to the second occurrence that
also matches on the variable result so any Err now fails the test.
In
`@packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/mod.rs`:
- Around line 284-291: The test currently uses conditional blocks like `if let
Some(key) = non_unique_key` which silently skip checks when `non_unique_key` is
None; change these to hard preconditions by asserting the presence of the key
(e.g., `assert!(non_unique_key.is_some())`) and then unwrapping it before
calling `key.public_key_hash()` and `drive.has_non_unique_public_key_hash(...)`;
apply the same change to all occurrences of `non_unique_key` in this module
(including the other ranges noted) so the assertions always run and the test
fails if a non-unique key was not produced.
In `@packages/rs-drive/src/drive/identity/fetch/queries/mod.rs`:
- Around line 703-716: The test should assert that the document type actually
influences the generated query path: update the
should_build_identities_contract_document_type_keys_query test that calls
Drive::identities_contract_document_type_keys_query so it verifies pq.path
encodes the supplied document type ("profile") (e.g., contains the document type
string or its expected encoded representation) in addition to the existing
non-empty check and limit assertion; reference
identities_contract_document_type_keys_query, pq.path, contract_id and the
"profile" document type when adding the assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c4e46b9-60c2-403b-86b4-51d59dc0412b
📒 Files selected for processing (6)
packages/rs-drive/src/drive/identity/fetch/balance/mod.rspackages/rs-drive/src/drive/identity/fetch/contract_keys/mod.rspackages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/mod.rspackages/rs-drive/src/drive/identity/fetch/full_identity/mod.rspackages/rs-drive/src/drive/identity/fetch/mod.rspackages/rs-drive/src/drive/identity/fetch/queries/mod.rs
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- contract_keys: assert Ok instead of matching Ok/Err - fetch_by_public_key_hashes: require non-unique key exists instead of conditional assertion - queries: document that doc type param doesn't affect query path Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
Summary
Adds 67 tests across 6 files in rs-drive identity fetch module, targeting the lowest-coverage areas.
fetch/queries/mod.rsIdentityProveRequestType::try_from, range queries with all pagination variantsfetch/fetch_by_public_key_hashes/mod.rsfetch/full_identity/mod.rsfetch/balance/mod.rsfetch/mod.rsfetch/contract_keys/mod.rsTest plan
cargo test -p drive --lib -- identity::fetchpassescargo fmt --allclean🤖 Generated with Claude Code
Summary by CodeRabbit