chore(low-code cdk): add pagination to http resolver unit tests #440
Conversation
|
/autofix
|
📝 WalkthroughWalkthroughThe changes update the data extraction and pagination logic in the HTTP components resolver. The Changes
Sequence Diagram(s)sequenceDiagram
participant Test
participant HttpComponentsResolver
participant DefaultPaginator
participant API
Test->>HttpComponentsResolver: Invoke data retrieval
HttpComponentsResolver->>API: Send request for data
API-->>HttpComponentsResolver: Return JSON { data, has_more, next_cursor }
HttpComponentsResolver->>DefaultPaginator: Configure paginator with next_cursor & has_more
alt More Records Available
DefaultPaginator->>API: Request next page (using page_token_option)
API-->>DefaultPaginator: Return next page response
DefaultPaginator->>HttpComponentsResolver: Aggregate data
end
HttpComponentsResolver-->>Test: Return aggregated data
Would you like any further refinements or additional diagrams to illustrate other aspects of the changes, wdyt? ✨ Finishing Touches
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. 🪧 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
CodeRabbit Configuration File (
|
![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 (2)
unit_tests/sources/declarative/resolvers/test_http_components_resolver.py (2)
558-578: Good implementation of paginated mock responses.The test data structure has been updated to support pagination with a well-structured format that includes:
- A
dataarray containing the actual records- Pagination metadata (
has_more,next_cursor)- Two pages of responses with appropriate pagination signals
Perhaps we could explicitly set
next_cursor: nullin the last page for even more robust testing? This would verify that the code handles both the absence of a field and an explicit null value properly, wdyt?"data": [ {"id": 2, "name": "item_2"}, ], "has_more": False, + "next_cursor": None, }
284-297: Consider enhancing test assertions for pagination completeness.While the test correctly verifies the total number of streams and records, it doesn't explicitly confirm that records from both pages were processed. Would it be helpful to add assertions that specifically verify records from each page were included?
assert len(actual_catalog.streams) == 4 assert [stream.name for stream in actual_catalog.streams] == expected_stream_names assert len(records) == 4 actual_record_stream_names = [record.stream for record in records] actual_record_stream_names.sort() assert actual_record_stream_names == expected_stream_names + + # Verify that records from both pages were processed + record_ids = set() + for record in records: + # Extract the item ID from the stream name (format: parent_X_item_Y) + item_id = record.stream.split('_')[-1] + record_ids.add(item_id) + assert record_ids == {'1', '2'}, "Records from both pagination pages should be present"Also applies to: 558-578
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
unit_tests/sources/declarative/resolvers/test_http_components_resolver.py(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
unit_tests/sources/declarative/resolvers/test_http_components_resolver.py (1)
airbyte_cdk/test/mock_http/mocker.py (1) (1)
get(94-95)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: SDM Docker Image Build
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
unit_tests/sources/declarative/resolvers/test_http_components_resolver.py (2)
284-297: Enhanced extractor field path and added pagination support.The changes to the record selector's extractor field path and the addition of a paginator are well-implemented. This shifts the data extraction from the root level to a nested "data" field, which is a common pattern for paginated APIs.
The pagination strategy is well-configured, using cursor-based pagination that:
- Fetches the next page using the
page_cursorparameter- Gets the next cursor from the
next_cursorfield- Stops when
has_moreis false
569-570: Correctly implemented paginated request.The test correctly validates that the second request includes the page cursor parameter from the first response. This confirms that the pagination mechanism properly passes cursor values between requests.
lazebnyi
deleted the
lazebnyi/extend-http-resolver-unit-tests-with-pagination
branch
Summary by CodeRabbit
New Features
Tests