-
Notifications
You must be signed in to change notification settings - Fork 274
fix(typescript): ignore cursor mismatches in wire test mock server #11484
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
…xture Co-Authored-By: [email protected] <[email protected]>
|
|
||
| const mismatches = findMismatches(actualBody, expectedBody); | ||
| if (Object.keys(mismatches).filter((key) => !key.startsWith("pagination.")).length > 0) { | ||
| if (Object.keys(mismatches).filter((key) => !key.startsWith("pagination.") && key !== "cursor").length > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the key seems like it could lead to issues. Instead, is there a way for us to determine whether a field corresponds to a cursor used by the pagination feature? Isn't there metadata in the IR that specifies how the request/response fields correspond to pagination metadata that we could use to determine this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You raise a valid concern. The withJson.ts file is a static "as-is" file that gets copied into the generated SDK at build time, so it doesn't have access to IR metadata at runtime.
To use IR metadata for determining pagination cursor fields, we'd need to either:
-
Pass pagination field names to
withJson: ModifywithJsonto accept an optional list of fields to ignore, and haveTestGeneratorpass the pagination cursor field name(s) when generating tests. This would require changes to bothwithJson.tsand the test generation code. -
Generate endpoint-specific matchers: Instead of a generic
withJson, generate custom matchers per endpoint that know which fields are pagination-related based on the IR.
The current approach follows the same pattern as the existing pagination.* filter (which also makes assumptions about field naming). However, I agree that a more robust solution using IR metadata would be better.
Would you prefer I implement option 1 (passing pagination field names to withJson)? This would involve:
- Adding an optional
ignoredFields?: string[]parameter towithJson - Modifying
TestGenerator.buildExampleTestto extract pagination cursor field names fromendpoint.paginationand pass them to the mock server setup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, if possible it would make more sense to have the invoker of withJson tell it which fields to ignore rather than have withJson assume it can ignore any field called "cursor". Go ahead and implement this more robust approach.
…ection Instead of hardcoding 'cursor' as an ignored field, the mock server now accepts an optional ignoredFields parameter. The TestGenerator extracts pagination cursor field names from the IR and passes them to the mock server at code generation time. This is a more robust approach that: - Uses IR metadata to determine which fields are pagination cursors - Avoids false positives from legitimate fields named 'cursor' - Makes the ignored fields explicit in the generated test code Co-Authored-By: [email protected] <[email protected]>
…refactor Co-Authored-By: [email protected] <[email protected]>
Description
Fixes wire test failures for pagination endpoints where
getNextPage()sends a different cursor value than the original request.Link to Devin run: https://app.devin.ai/sessions/569f5162819c47c4ba95afbfde8313de
Requested by: @jsklan
Root Cause
When pagination tests call
getNextPage(), the SDK correctly sends the cursor from the response (e.g.,"next_cursor_value"), but the mock server was rejecting the request because it expected the original request's cursor value (e.g.,"cursor_value"). The mock server already filtered outpagination.*mismatches but not top-level cursor mismatches.Changes Made
WithJsonOptionsinterface towithJson.tswith optionalignoredFieldsparametermockEndpointBuilder.tsto passignoredFieldsthrough thejsonBodymethodTestGenerator.tsto extract pagination cursor field names from the IR and pass them to the mock server at code generation timeApproach
Instead of hardcoding field names to ignore (like
key !== "cursor"), the solution uses IR metadata to determine which fields are pagination cursors. TheTestGeneratorextracts the cursor field name fromendpoint.pagination.page.property.name.wireValuefor cursor pagination endpoints and passes it to the mock server'sjsonBodymethod. This approach:Note: Nested cursor fields (e.g.,
pagination.cursor) are already handled by the existing!key.startsWith("pagination.")filter.Testing
Review Checklist
WithJsonOptionsinterface andignoredFieldsparameter inwithJson.tsTestGenerator.tscorrectly extracts cursor field names from IR for top-level cursor paginationusers.ymlis valid