kevin1024 / vcrpy

Automatically mock your HTTP interactions to simplify and speed up testing
MIT License
2.72k stars 388 forks source link

Fix HTTPX support #784

Closed parkerhancock closed 11 months ago

parkerhancock commented 11 months ago

Guys, VCRpy's support for httpx is hella broken. Like for serious. So I fixed it. Summary of the changes:

Changed patch target from .send to ._send_single_request

This change moves the patch target to client._send_single_request, which is the lowest, and most atomic request for data over the wire. By moving the patch lower down the stack, it fixes #684 (support for httpx.Auth), #619 (errors handling redirects), #600 (stubs trying to access an invalid property), #599 (encoding types), #597 (async httpx streaming doesn't work).

Removed attempt to decode raw body content

For some reason, in the record function, the serializer was trying to decode binary body content (i.e. content.decode("utf-8", "ignore"). This one line meant that any raw binary data that happened not to resolve to a valid Unicode code point was deleted, which meant subsequent requests for binary content would be corrupted. This fixes #656 (Binary upload data unsupported), and #597 (async httpx streaming).

Simplified Stubs & Test Suite

The stubs and tests have been dramatically pared down and simplified, because we've moved the patch target so much lower. We no longer need to test for redirects or auth flow. The only thing we need to hold httpx's hand on is cookies, which is still tested. Also added a test case for streaming data (#597).

Replaced mockbin with httpbin

Mockbin went commercial, which meant the whole test suite was broken. I moved all the tests to now target httpbin.org.

Other Benefits

I've also:

PLEASE PLEASE PLEASE MERGE THIS REQUEST AND SAVE US POOR HTTPX USERS!

jairhenrique commented 11 months ago

@parkerhancock can you help us moving mockbin tests to httpbin?

parkerhancock commented 11 months ago

@parkerhancock can you help us moving mockbin tests to httpbin?

Happy to! Maybe that will be a good next PR once this one is complete.

parkerhancock commented 11 months ago

@parkerhancock can you help us moving mockbin tests to httpbin?

Test suite was failing on this PR, so I fixed it for you. All tests on all backends now pass on py311, supported by the pytest-httpbin plugin (local server, no calls to external servers).

mockbin has been summarily executed.

codecov-commenter commented 11 months ago

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (defad28) 89.77% compared to head (5e6c20c) 90.10%.

Files Patch % Lines
vcr/serializers/compat.py 0.00% 0 Missing and 1 partial :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #784 +/- ## ========================================== + Coverage 89.77% 90.10% +0.33% ========================================== Files 27 27 Lines 1819 1809 -10 Branches 284 335 +51 ========================================== - Hits 1633 1630 -3 + Misses 141 134 -7 Partials 45 45 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jairhenrique commented 11 months ago

@parkerhancock LGTM

parkerhancock commented 11 months ago

@parkerhancock LGTM

πŸ™ YOU ARE MY HERO! LET'S GO πŸš€ πŸš€ πŸš€

Any idea when we can get this pushed out to PyPI in a release? It is a blocker to having a decent release over at patent_client

(And thanks again for your prompt review!)

ionelmc commented 11 months ago

I've tried this, it works. Just a small note, I found out that I needed to recreate some of my cassettes for tests that dealt with binary content as the previous implementation incorrectly saved responses as text instead of bytes.