-
-
Notifications
You must be signed in to change notification settings - Fork 577
Test query batching with vars, ops, and extensions #3958
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
Test query batching with vars, ops, and extensions #3958
Conversation
Reviewer's GuideThis PR enhances GraphQL query batching tests by introducing a reusable batching client fixture, adding new test cases for various batching scenarios (single/multiple queries, variables, operation selection, extensions, and shared context), and extending the test schema with dynamic context retrieval and a mutation to update context. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @DoctorJohn - I've reviewed your changes - here's some feedback:
- Consider parameterizing the batch payload tests for variables, operationName, and extensions to reduce duplication and improve maintainability.
- Extract a small helper or fixture for sending batch requests with default headers to avoid repeating the same post call in every test.
- Rename the batching_http_client fixture to a more conventional name like
client
to align with other tests and reduce verbosity.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider parameterizing the batch payload tests for variables, operationName, and extensions to reduce duplication and improve maintainability.
- Extract a small helper or fixture for sending batch requests with default headers to avoid repeating the same post call in every test.
- Rename the batching_http_client fixture to a more conventional name like `client` to align with other tests and reduce verbosity.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Greptile Summary
This PR enhances the test coverage for GraphQL query batching functionality by adding comprehensive tests that validate variables, operation selection, extensions, and context sharing between batched operations.
The changes consist of two main modifications:
-
Test Schema Enhancement (
tests/views/schema.py
): The existing test schema is updated to support more flexible testing scenarios. Thevalue_from_context
method now accepts an optionalkey
parameter (defaulting to 'custom_value') allowing tests to access different context keys. A newupdate_context
mutation is added that enables modifying the context during execution, which is essential for testing context sharing between operations in a batch. -
Comprehensive Test Suite (
tests/http/test_query_batching.py
): The testing approach is refactored from a single test function to a fixture-based system with multiple focused test cases. The new tests validate:- Basic single and multiple query batching
- Variables being correctly supplied to individual queries in a batch
- Operation selection working independently for each query
- Extensions being handled per-query within batches
- Context sharing between operations (mutations can modify context that subsequent queries access)
These changes integrate well with Strawberry's existing GraphQL implementation by leveraging the standard strawberry.Info
context pattern and following established testing conventions. The modifications are focused on test infrastructure and don't affect production code behavior, making them safe additions to validate the robustness of the query batching feature.
Confidence score: 4/5
• This PR is very safe to merge as it only adds test coverage without changing production code
• Score reduced by 1 due to one test being conditionally skipped for GraphQL 3.2 due to formatting differences, which suggests potential edge cases that might need attention
• No files require additional attention - the changes are well-structured test enhancements
2 files reviewed, no comments
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3958 +/- ##
==========================================
- Coverage 94.41% 94.40% -0.01%
==========================================
Files 528 528
Lines 34342 34371 +29
Branches 1803 1803
==========================================
+ Hits 32423 32449 +26
- Misses 1627 1630 +3
Partials 292 292 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #3958 will not alter performanceComparing Summary
|
Description
This PR adds extra query batching tests, making sure supplying variables, selecting operations, using operation extensions, and context sharing work as expected.
Types of Changes
Summary by Sourcery
Add comprehensive tests for HTTP GraphQL query batching covering single/multiple queries, variables, operation selection, extensions, and shared context, and extend the test schema to support dynamic context key access and context updates.
Enhancements:
Tests: