test(drive-abci): add comprehensive tests for group query modules#3268
Merged
Conversation
Add 59 unit tests across all 4 group query v0 modules to achieve >93% code coverage: - group_info/v0: 9 tests covering validation errors, prove/no-prove paths, empty state, and populated group data with member entries - group_infos/v0: 11 tests covering validation, limit errors, start position boundaries, prove/no-prove, and populated groups prove - group_actions/v0: 25 tests covering all validation paths plus populated data tests for every token event type (Mint, Burn, Freeze, Unfreeze, DestroyFrozenFunds, EmergencyAction Pause/Resume, ConfigUpdate, ChangePriceForDirectPurchase with None/Single/Variable) - group_action_signers/v0: 14 tests covering validation, boundaries, prove/no-prove, and populated action with verified signer data Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds comprehensive test suites to four group query modules exercising validation, boundary conditions, empty-state and prove paths, and integration scenarios for group info, group infos, group actions, and group action signers. No production code or public API signatures were modified. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (6)
packages/rs-drive-abci/src/query/group_queries/group_infos/v0/mod.rs (2)
355-403: Add one populated non-prove assertion for returned group payload.This test verifies only the proof branch. Add a non-prove populated test that checks
group_contract_position, members, andgroup_required_powerso serialization/mapping regressions are caught in this module too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-drive-abci/src/query/group_queries/group_infos/v0/mod.rs` around lines 355 - 403, Add a new test (or extend the existing test_query_group_infos_with_populated_groups_prove) that sends a GetGroupInfosRequestV0 with prove: false and the same populated groups, then call platform.query_group_infos_v0(request, &state, version) and assert result.errors.is_empty() and that result.data contains GetGroupInfosResponseV0 with a Result::GroupInfos (non-proof) payload; inside that payload verify the group's group_contract_position equals 0 (or the inserted key), group_required_power equals 5, and the members map contains the member Identifier ([2u8;32]) with power 5 to catch serialization/mapping regressions.
182-211: Tighten invalid-limit tests to assert the exact failure shape.Line 194 and Line 210 currently assert only
is_err(), which can pass on unrelated failures. Please match the specificInvalidLimitpath to make these tests deterministic and intention-revealing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-drive-abci/src/query/group_queries/group_infos/v0/mod.rs` around lines 182 - 211, Update the two tests (test_invalid_limit_zero and test_invalid_limit_exceeds_max) to assert the specific InvalidLimit failure instead of only is_err(): call platform.query_group_infos_v0(...), take the Err via unwrap_err() or match on the result, and assert that the error matches the InvalidLimit variant/path (the exact Query/Validation error produced by query_group_infos_v0 for bad count). Keep references to the same request type GetGroupInfosRequestV0 and function query_group_infos_v0 so the assertion targets the specific InvalidLimit variant rather than any error.packages/rs-drive-abci/src/query/group_queries/group_info/v0/mod.rs (1)
253-358: Consider extracting shared populated-group setup into a helper.The arrangement logic is repeated in both populated tests; a small helper would reduce maintenance overhead and keep intent-focused assertions shorter.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-drive-abci/src/query/group_queries/group_info/v0/mod.rs` around lines 253 - 358, Both tests test_query_group_info_with_populated_group and test_query_group_info_with_populated_group_prove duplicate the same populated-group setup; extract that into a small helper (e.g., fn create_populated_group(platform: &PlatformType, state: &StateType, version: &VersionType) -> (Identifier, Identifier) or a helper that returns (contract_id, member_id, state, version) ) that performs the Identifier::from(...) creation, builds the BTreeMap members, constructs Group::V0(GroupV0 { ... }), inserts it into platform.drive.add_new_groups(...), and returns the contract_id and member_id (or the full request). Replace the duplicated setup in both tests with calls to this helper and only keep the test-specific request/assert logic (GetGroupInfoRequestV0 creation and assertions).packages/rs-drive-abci/src/query/group_queries/group_actions/v0/mod.rs (2)
638-1164: Consider consolidating repeated request/response scaffolding in event tests.A tiny helper for “query + unwrap
GroupActions” would remove repeated boilerplate and make each test focus only on event-specific assertions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-drive-abci/src/query/group_queries/group_actions/v0/mod.rs` around lines 638 - 1164, Add a small test helper that runs GetGroupActionsRequestV0 against query_group_actions_v0 and returns the unwrapped GroupActions (or its group_actions Vec) so tests only assert event specifics; locate usage around setup_group_and_add_action, Query method query_group_actions_v0, request type GetGroupActionsRequestV0, response type GetGroupActionsResponseV0 and GroupActions and implement a function (e.g., fetch_group_actions_or_panic) that builds the request (contract_id, group_contract_position:0, status:0, start_at_action_id:None, count:None, prove:bool), calls platform.query_group_actions_v0(...), ensures no errors and that result contains get_group_actions_response_v0::Result::GroupActions(GroupActions { group_actions }), and returns the group_actions for tests to use, then replace the repeated scaffolding in each test with calls to this helper.
920-1078: Strengthen a few event tests with explicit event-type assertions.Line 948, Line 1032, and Line 1074 currently only assert result length. Add event-type checks (as done in neighboring tests) so mapping regressions are caught.
Proposed assertion strengthening
@@ - assert_eq!(group_actions.len(), 1); + assert_eq!(group_actions.len(), 1); + let event = group_actions[0] + .event + .as_ref() + .expect("expected event") + .event_type + .as_ref(); + assert!(matches!( + event, + Some(group_action_event::EventType::TokenEvent(t)) + if matches!(t.r#type.as_ref(), Some(token_event::Type::EmergencyAction(_))) + )); @@ - assert_eq!(group_actions.len(), 1); + assert_eq!(group_actions.len(), 1); + let event = group_actions[0] + .event + .as_ref() + .expect("expected event") + .event_type + .as_ref(); + assert!(matches!( + event, + Some(group_action_event::EventType::TokenEvent(t)) + if matches!(t.r#type.as_ref(), Some(token_event::Type::UpdatePrice(_))) + )); @@ - assert_eq!(group_actions.len(), 1); + assert_eq!(group_actions.len(), 1); + let event = group_actions[0] + .event + .as_ref() + .expect("expected event") + .event_type + .as_ref(); + assert!(matches!( + event, + Some(group_action_event::EventType::TokenEvent(t)) + if matches!(t.r#type.as_ref(), Some(token_event::Type::UpdatePrice(_))) + ));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-drive-abci/src/query/group_queries/group_actions/v0/mod.rs` around lines 920 - 1078, For each of the three tests (test_query_group_actions_with_emergency_action_resume, test_query_group_actions_with_change_price_single_price, test_query_group_actions_with_change_price_variable_prices) add an explicit assertion that the returned group_actions[0].event.event_type is the expected variant (like the existing check in test_query_group_actions_with_change_price_no_schedule). Specifically: in test_query_group_actions_with_emergency_action_resume assert the event is Some(group_action_event::EventType::TokenEvent(t)) and that t.r#type is EmergencyAction with TokenEmergencyAction::Resume; in test_query_group_actions_with_change_price_single_price assert t.r#type is UpdatePrice with TokenPricingSchedule::SinglePrice(42) (or matching SinglePrice); and in test_query_group_actions_with_change_price_variable_prices assert t.r#type is UpdatePrice with TokenPricingSchedule::SetPrices (matching the inserted map). Use the same pattern of unwrapping event and using matches!() as in GetGroupActionsResponseV0 / group_action_event::EventType checks to keep tests consistent.packages/rs-drive-abci/src/query/group_queries/group_action_signers/v0/mod.rs (1)
386-556: Optional: factor duplicated populated-action setup into a local helper.Both populated tests repeat the same group/action arrangement; extracting it will keep future signer test additions cheaper and less error-prone.
🤖 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-abci/src/query/group_queries/group_actions/v0/mod.rs`:
- Around line 638-1164: Add a small test helper that runs
GetGroupActionsRequestV0 against query_group_actions_v0 and returns the
unwrapped GroupActions (or its group_actions Vec) so tests only assert event
specifics; locate usage around setup_group_and_add_action, Query method
query_group_actions_v0, request type GetGroupActionsRequestV0, response type
GetGroupActionsResponseV0 and GroupActions and implement a function (e.g.,
fetch_group_actions_or_panic) that builds the request (contract_id,
group_contract_position:0, status:0, start_at_action_id:None, count:None,
prove:bool), calls platform.query_group_actions_v0(...), ensures no errors and
that result contains
get_group_actions_response_v0::Result::GroupActions(GroupActions { group_actions
}), and returns the group_actions for tests to use, then replace the repeated
scaffolding in each test with calls to this helper.
- Around line 920-1078: For each of the three tests
(test_query_group_actions_with_emergency_action_resume,
test_query_group_actions_with_change_price_single_price,
test_query_group_actions_with_change_price_variable_prices) add an explicit
assertion that the returned group_actions[0].event.event_type is the expected
variant (like the existing check in
test_query_group_actions_with_change_price_no_schedule). Specifically: in
test_query_group_actions_with_emergency_action_resume assert the event is
Some(group_action_event::EventType::TokenEvent(t)) and that t.r#type is
EmergencyAction with TokenEmergencyAction::Resume; in
test_query_group_actions_with_change_price_single_price assert t.r#type is
UpdatePrice with TokenPricingSchedule::SinglePrice(42) (or matching
SinglePrice); and in test_query_group_actions_with_change_price_variable_prices
assert t.r#type is UpdatePrice with TokenPricingSchedule::SetPrices (matching
the inserted map). Use the same pattern of unwrapping event and using matches!()
as in GetGroupActionsResponseV0 / group_action_event::EventType checks to keep
tests consistent.
In `@packages/rs-drive-abci/src/query/group_queries/group_info/v0/mod.rs`:
- Around line 253-358: Both tests test_query_group_info_with_populated_group and
test_query_group_info_with_populated_group_prove duplicate the same
populated-group setup; extract that into a small helper (e.g., fn
create_populated_group(platform: &PlatformType, state: &StateType, version:
&VersionType) -> (Identifier, Identifier) or a helper that returns (contract_id,
member_id, state, version) ) that performs the Identifier::from(...) creation,
builds the BTreeMap members, constructs Group::V0(GroupV0 { ... }), inserts it
into platform.drive.add_new_groups(...), and returns the contract_id and
member_id (or the full request). Replace the duplicated setup in both tests with
calls to this helper and only keep the test-specific request/assert logic
(GetGroupInfoRequestV0 creation and assertions).
In `@packages/rs-drive-abci/src/query/group_queries/group_infos/v0/mod.rs`:
- Around line 355-403: Add a new test (or extend the existing
test_query_group_infos_with_populated_groups_prove) that sends a
GetGroupInfosRequestV0 with prove: false and the same populated groups, then
call platform.query_group_infos_v0(request, &state, version) and assert
result.errors.is_empty() and that result.data contains GetGroupInfosResponseV0
with a Result::GroupInfos (non-proof) payload; inside that payload verify the
group's group_contract_position equals 0 (or the inserted key),
group_required_power equals 5, and the members map contains the member
Identifier ([2u8;32]) with power 5 to catch serialization/mapping regressions.
- Around line 182-211: Update the two tests (test_invalid_limit_zero and
test_invalid_limit_exceeds_max) to assert the specific InvalidLimit failure
instead of only is_err(): call platform.query_group_infos_v0(...), take the Err
via unwrap_err() or match on the result, and assert that the error matches the
InvalidLimit variant/path (the exact Query/Validation error produced by
query_group_infos_v0 for bad count). Keep references to the same request type
GetGroupInfosRequestV0 and function query_group_infos_v0 so the assertion
targets the specific InvalidLimit variant rather than any error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b7e9616c-7d79-492c-bc34-bfea8a709962
📒 Files selected for processing (4)
packages/rs-drive-abci/src/query/group_queries/group_action_signers/v0/mod.rspackages/rs-drive-abci/src/query/group_queries/group_actions/v0/mod.rspackages/rs-drive-abci/src/query/group_queries/group_info/v0/mod.rspackages/rs-drive-abci/src/query/group_queries/group_infos/v0/mod.rs
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3268 +/- ##
============================================
- Coverage 68.30% 66.49% -1.82%
============================================
Files 3292 3292
Lines 253958 255279 +1321
============================================
- Hits 173470 169744 -3726
- Misses 80488 85535 +5047
🚀 New features to boost your workflow:
|
Member
Author
|
Duplicate of #3267 which has more tests (44 vs 41) and addressed CodeRabbit feedback. |
Address CodeRabbit review feedback: - Add explicit event-type assertions for emergency_action_resume, change_price_single_price, and change_price_variable_prices tests instead of only checking result length - Tighten invalid-limit tests to match the specific InvalidLimit error variant rather than a generic is_err() check Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Member
Author
|
Reopening - this PR has 59 tests (including all token event type coverage and CodeRabbit feedback addressed) vs 44 in #3267. The latest push also addresses CodeRabbit review feedback by strengthening assertions and using specific error type matching. |
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
1 participant
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.
Issue being fixed or feature implemented
The 4 group query modules in
packages/rs-drive-abci/src/query/group_queries/had 0% test coverage. This PR adds comprehensive unit tests to achieve >93% coverage.What was done?
Added 59 unit tests across all 4 group query v0 modules:
Tests follow existing patterns from other query modules (
data_contract,balance,prefunded_specialized_balances).How Has This Been Tested?
Breaking Changes
None.
Checklist:
🤖 Generated with Claude Code
Summary by CodeRabbit