test(integration): port types tests to server v2 #22981
Conversation
📝 WalkthroughWalkthroughThis pull request focuses on refactoring pagination-related test files in the Changes
Sequence DiagramsequenceDiagram
participant Test as Test Function
participant Keeper as Bank Keeper
participant Query as Collection Query
participant Context as Context
Test->>Context: Create Context
Test->>Keeper: Initialize Bank Keeper
Test->>Query: CollectionPaginate/CollectionFilteredPaginate
Query-->>Keeper: Request Paginated Results
Keeper-->>Query: Return Paginated Data
Query-->>Test: Provide Paginated Results
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
tests/integration/v2/types/filtered_pagination_test.go (5)
45-45: Repeated pagination call
This appears to replicate the same query. If the purpose is to state that repeated calls yield the same result, consider clarifying via comments or removing duplication.
85-85: Offset-based pagination
It’s good to observe offset usage. Consider also testing offset that exactly equals total to ensure no off-by-one.
192-193: Fuzz output logging
Printing partial results is useful for debugging. Verify if structured logging might improve clarity.
222-228: Inlined function within the test
This local definition is short and specialized, which can be fine. If you need repeated usage, consider refactoring out to avoid duplication.
242-242: Boundary condition
Consider verifying the boundary offset precisely equals total balances to ensure no out-of-bounds.tests/integration/v2/types/pagination_test.go (2)
114-114: Local alias for s.queryClient
CapturingqueryClient := s.queryClientmight improve legibility.
223-223: SeparatequeryClientreference
ReassigningqueryClient := s.queryClientis optional but can be more readable.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/integration/v2/types/filtered_pagination_test.go(11 hunks)tests/integration/v2/types/fuzz_test.go(2 hunks)tests/integration/v2/types/pagination_test.go(22 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
tests/integration/v2/types/fuzz_test.go (3)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern **/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
tests/integration/v2/types/filtered_pagination_test.go (3)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern **/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
tests/integration/v2/types/pagination_test.go (3)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern **/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
🔇 Additional comments (45)
tests/integration/v2/types/fuzz_test.go (3)
1-1: Use of thetypespackage name
This change fromquery_testtotypesis consistent with the new layout. Ensure any references to the old name are removed from docstrings and comments.
9-9: Imports for collection-based pagination
The newly added imports fromcosmossdk.io/collections,cosmossdk.io/math, andcosmossdk.io/x/bank/testutilalign with your transition to the collection-based approach.
85-88: Recheck error handling in fuzz scenario
Currently,_ = query.CollectionPaginate(...)discards any error. If not intentional, consider at least logging or asserting errors to help diagnose unexpected issues in the fuzz environment.tests/integration/v2/types/filtered_pagination_test.go (15)
1-9: Switch totypespackage and updated imports
Shifting the package name fromquery_testtotypesand introducing packages likecontext,bankkeeper, andcollectionsis consistent with your new architecture. Verify that all references to the old package name are indeed updated.
39-39: Pagination limit check
Verifying behavior when limit exceeds total. Good to confirm the function returns only available items.
54-54: Confirm nextKey presence
Checkingres.NextKeyensures multi-page coverage. This test guards against pagination boundary mishaps.
64-64: Assert invalid request
Good to see you’re testing error handling when both key and offset are provided.
69-69: Valid nextKey usage
Probing the scenario of a remaining page helps ensure correct multi-page continuity.
77-77: Default limit on zero
Verifying that a limit of 0 triggers default behavior. Helpful coverage.
111-111: Reverse pagination with limit > total
Ensuring it defaults to the total record count is a valuable check.
117-117: Nil PageRequest
Testing the null request scenario helps ensure robust default logic.
126-126: Reverse default limit
Confirming consistency in reverse mode.
134-134: Partial results in reverse
This test ensures partial retrieval logic is correct when reversed.
144-144: Error path for reverse
Confirming that specifying both key and offset in reverse also produces an error.
149-149: Continuing reversed pagination
Ensures multi-chunk query flow works correctly.
158-158: Reversed final page
Checks noNextKeyis provided when returning the last chunk in reverse.
199-206: execFilterPaginate withCollectionFilteredPaginate
This function properly demonstrates filtered collection pagination. Consider advanced filters for negative or zero amounts if needed.
233-233: Offset usage check
Testing multiple offset scenarios ensures broad coverage.tests/integration/v2/types/pagination_test.go (27)
1-30: Package and imports update
Transitioning fromquery_testtotypeswhile introducing multiple new imports is consistent with the new v2 test structure. Make sure all references to the old package name are removed throughout.
46-53: paginationTestSuite fields
Addingctx,bankKeeper BaseKeeper, andqueryClient bankkeeper.Querieraligns with the shift to the new application design. Confirm each field is exercised in your tests.
61-85: SetupTest with integration.NewApp
Refactoring the setup via module config and app creation simplifies the environment. Confirm modules listed inmoduleConfigsare all necessary.
87-87: Context from StateLatestContext
Fetching the latest context ensures tests align with the most recent chain state.
89-89: Query client initialization
bankkeeper.NewQuerierintegrally ties into the new system.
135-135: Checking CountTotal
Verifies total record count is exposed properly when requested.
144-144: Over-limit boundary
Ensures retrieving more records than allowed gracefully caps at the expected maximum.
153-153: Partial retrieval test
Testing partial page requests withCountTotal = trueclarifies the total count in truncated responses.
162-162: Disable total
Correctly verifying thatTotalis zero whenCountTotalis false.
171-171: Key-based follow-up
Leveraging NextKey from the prior response is essential for multi-page verification.
180-180: Last page check
Ensures final chunk is smaller than the limit and NextKey is nil.
190-190: Offset-based scenario
Skipping records with offset coverage is well tested.
200-200: Detect conflicting offset and key
Helps capture invalid usage early.
209-209: Error scenario
Produces the expected error message for offset + nextKey usage.
216-216: Offset beyond total
Verifies zero records are returned when offset is out of range.
244-244: Reverse partial retrieval
Ensures reversed queries respect the limit and produce a valid NextKey if records remain.
252-252: High limit reversed
Testing large-limits in reverse checks boundary correctness.
261-261: Key-based reverse
Shows usage of NextKey in reverse mode.
270-270: Offset in reverse
Tests offset interplay with reversed pagination logic.
279-279: Last page reverse
Ensures final chunk is smaller than the limit with no NextKey.
288-288: Over-limit reversed
Confirms limit is enforced in reverse.
297-297: Multi-page reverse
Repeated calls with NextKey andCountTotal=falseconfirm multi-page retrieval.
309-309: Chaining reversed pages
Ensures continuity of NextKey across multiple reverse requests.
321-321: Reverse final chunk
Verifies the last chunk is returned and NextKey is nil in reverse pagination.
333-333: Invalid combination in reverse
Ensures consistent error for offset+NextKey usage in reverse mode.
340-340: Empty result on out-of-bound offset
Correct reverse logic for offset greater than total.
369-373: ManualCollectionPaginatesnippet
This example highlights advanced usage of the collection-based pagination in integration tests.
Description
ref: #20799
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
Refactoring
Tests
Code Structure
query_testtotypes