-
-
Notifications
You must be signed in to change notification settings - Fork 590
Extend graphql over ws operation selection tests #3868
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
Extend graphql over ws operation selection tests #3868
Conversation
Reviewer's GuideExtends and parameterizes operation selection tests across both GraphQL over WS protocols by adding new subscription tests and refactoring existing single-result query tests with dynamic payload handling, updated send methods, and assertions. File-Level Changes
Possibly linked issues
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.
PR Summary
Extended operation selection tests for both GraphQL WebSocket protocols by introducing parameterized test cases to verify different operation selection scenarios.
- Added parameterized tests in
tests/websockets/test_graphql_transport_ws.py
to cover both subscription and single-result operations - Replaced single operation test with parameterized version in
tests/websockets/test_graphql_ws.py
to test default, None, and named operations - Enhanced test coverage for operation selection with different payload configurations
- Added verification for correct message handling and operation execution based on operation names
💡 (3/5) Reply to the bot's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!
2 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
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:
- Extract the duplicated parametrization data into a shared constant or fixture to DRY up the operation selection tests across both WebSocket test modules.
- Unify the test client API calls (send_json vs send_message/send_legacy_message) across both protocols to keep the test interfaces consistent and clear.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3868 +/- ##
=======================================
Coverage 95.01% 95.02%
=======================================
Files 509 509
Lines 33194 33208 +14
Branches 1728 1728
=======================================
+ Hits 31539 31555 +16
+ Misses 1373 1371 -2
Partials 282 282 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #3868 will not alter performanceComparing Summary
|
Description
This PR extends the operation selection tests of both GraphQL over WS protocols.
There's still some work left in this area, but I'll do that in a separate PR to keep things reviewable.
Types of Changes
Summary by Sourcery
Extend operation selection test coverage for both GraphQL over WS protocols by parametrizing multi‐operation queries to test default, null, and named operationName payloads in subscription and single‐result scenarios.
Tests: