strawberry
strawberry copied to clipboard
Create initial structure for unified HTTP tests
This PR refactors the HTTP tests to remove code duplication. Working on this PR also surfaced some inconsistencies and missing features between the integrations.
To make this PR I created some sort of abstraction for the http clients, I think I want to publish this abstraction as a separated package at some point, but for now I'll keep here until I know the API makes sense.
Also this client overlaps with our test client, we should combine them somehow.
In terms of inconsistencies I found:
- Chalice didn't support returning a custom status code, fixed in this PR (breaking change?)
- Sanic didn't support returning a custom status code, fixed in this PR (breaking change)
- Aiohttp didn't support returning a custom status code, fixed in this PR (breaking change)
- Flask was returning 415 when doing a GET request to the GraphQL view with GraphiQL disabled, now it returns 404
- Some integrations weren't checking errors when parsing request data, fixed in this PR
- Chalice's process result receives the request like the other integrations (breaking change)
- [ ] Maybe we want to revert this (or do the same for flask)
- Same as above for sanic (breaking change) plus process result for sanic is async now
- Flask async view's get_root_value, get_context and process_results are now async
I've also introduce a class called TemporalResponse
, this will be used to override the status_code and headers, but it will need more updates:
- [ ] Add support for passing headers (in this PR)
- [ ] Make all integration use this new class (django has its own for example)
- [ ] Start using TypedDict for
get_context
return types (breaking change, mostly for typing)
Just to be clear, this work should lead to reducing the code duplication in the integrations (see also #1870, we might want to move the integration somewhere else, but that's to be discusses)
Codecov Report
Merging #1840 (e4c60ce) into main (c8f44e5) will decrease coverage by
0.96%
. The diff coverage is95.45%
.
Additional details and impacted files
@@ Coverage Diff @@
## main #1840 +/- ##
==========================================
- Coverage 97.75% 96.79% -0.97%
==========================================
Files 180 180
Lines 7362 7421 +59
Branches 1324 1331 +7
==========================================
- Hits 7197 7183 -14
- Misses 80 142 +62
- Partials 85 96 +11
I'm going to move some of these changes into individual PRs and then merge this when it only touches tests :)
Thanks for adding the RELEASE.md
file!
Here's a preview of the changelog:
This release does some internal refactoring of the HTTP views, hopefully it doesn't affect anyone. It mostly changes the status codes returned in case of errors (e.g. bad JSON, missing queries and so on).
It also improves the testing, and adds an entirely new test suite for the HTTP views, this means in future we'll be able to keep all the HTTP views in sync feature-wise.
Here's the preview release card for twitter:
Here's the tweet text:
🆕 Release (next) is out! Thanks to @patrick91 for the PR 👏
Get it here 👉 https://github.com/strawberry-graphql/strawberry/releases/tag/(next)