Fix: Httpx Human Readable Responses

[parkerhancock]

Little quality of life upgrade for #784 and httpx support. We fixed a lot of the functional issues by forcing a binary content type on all files regardless of mime type. But that makes the bodies not human readable in the cassettes, which is one of the big advantages of using VCRpy. This fixes it by attempting to convert all response bodies to text using utf-8, and if it fails to do so, then records binary content.

In no circumstance should this optional encoding of response bodies change the actual data coming to or from the client.

Test coverage also included!

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (dd97b02) 90.10% compared to head (bbf1055) 90.13%.

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

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #788      +/-   ##
==========================================
+ Coverage   90.10%   90.13%   +0.03%     
==========================================
  Files          27       27              
  Lines        1809     1815       +6     
  Branches      335      336       +1     
==========================================
+ Hits         1630     1636       +6     
  Misses        134      134              
  Partials       45       45              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jairhenrique]

LGTM

[jairhenrique]

@parkerhancock please, rebase with master!

[jairhenrique]

@parkerhancock can you fix lint errors!?

[parkerhancock]

Hey! Just tried to. Ruff on my computer doesn't show any errors. Not sure what to do here..

[hartwork]

sure

Hi @parkerhancock, are you running the same version as CI: ruff 0.1.7? It says:

Would reformat: tests/integration/test_httpx.py
Would reformat: vcr/stubs/httpx_stubs.py

(at https://github.com/kevin1024/vcrpy/actions/runs/7183525284/job/19562496307?pr=788#step:5:9).

The other issue looks like a flaky thing and should go away after re-running so ruff is likely our main and only thing to address here.

[hartwork]

The other issue looks like a flaky thing and should go away after re-running so ruff is likely our main and only thing to address here.

@parkerhancock that just got proven correct be rerunning the CI.

@jairhenrique I'm using pre-commit to run the same checks for local commits and in CI at multiple places and it would be the perfect fit for our use of ruff and make sure everyone is on the same version as a side-effect. Is that a direction that you would support in general? I can make a pull request about it but I'd want to be sure to not waste my time. Here's what I'm using with git-delete-merged-branches for example:

• Config example: https://github.com/hartwork/git-delete-merged-branches/blob/master/.pre-commit-config.yaml
• CI 1: https://github.com/hartwork/git-delete-merged-branches/blob/master/.github/workflows/pre-commit.yml
• CI 2 (bonus): https://github.com/hartwork/git-delete-merged-branches/blob/master/.github/workflows/pre-commit-detect-outdated.yml

What do you think?

[hartwork]

@jairhenrique PS: In any case the CI should invoke ruff in a way where we can see a diff of the changes produced, using git diff --exit-status if we need to, ruff build-in if we can. Not seeing a diff just proves to not be nice to contributors.

[jairhenrique]

@hartwork personally I don't like pre-commit because it is a tool that can be bypassed with git's --no-verify parameter. It might be useful, but I prefer to just trust the ci result.

We can add ruff parameters that show a diff on errors.

[jairhenrique]

@parkerhancock i run, ruff format .

[hartwork]

@hartwork personally I don't like pre-commit because it is a tool that can be bypassed with git's --no-verify parameter. It might be useful, but I prefer to just trust the ci result.

@jairhenrique I think there is a misunderstanding here: the CI still enforces ruff, it just uses pre-commit to do that — command pre-commit run --all-files --show-diff-on-failure — and as a result the local pre-commit hooks and the what the CI enforces always match 100%: --no-verify cannot take that away, it would be as strict as before. This https://github.com/hartwork/git-delete-merged-branches/actions/runs/7172651706/job/19530292191#step:4:129 is where it would read "ruff" instead and where a diff would be shown below also in case of deviation.

[jairhenrique]

@hartwork feel free to open a pr with pre-commit feature!

About this pr, do you think we can merge?

[hartwork]

@hartwork feel free to open a pr with pre-commit feature!

@jairhenrique alright, it will take a few minutes, probably before 20:00 UTC+1 though.

About this pr, do you think we can merge?

I haven't looked into httpx integration here at all yet, I would want leave that topic to the two of you for now.