Skip to content
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

Check for body in the reponse before to read it #37

Merged
merged 5 commits into from
Apr 25, 2024

Conversation

jmaspons
Copy link
Contributor

If the response has no body, write and empty file
FIX #35

@nealrichardson
Copy link
Owner

Thanks. This seems reasonable. Would you mind adding a test that covers this case?

@jmaspons
Copy link
Contributor Author

Yes, I would like to add a test. Could you give me an example about how to simulate and empty response? I've seen that webfakes is used but I have no experience with it

@nealrichardson
Copy link
Owner

httr2::response() makes responses, and as it turns out, with no arguments specified, the body will be empty. So you could test that save_response() can write that to a tempfile, and probably should test that we can read it back in with load_response().

@jmaspons
Copy link
Contributor Author

Tests added, @nealrichardson

Copy link
Owner

@nealrichardson nealrichardson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor suggestion but otherwise this looks good, thank you! Once I merge #38, can you rebase and add a NEWS bullet for this fix?

tests/testthat/test-capture-requests.R Outdated Show resolved Hide resolved
@jmaspons
Copy link
Contributor Author

Ready to merge, @nealrichardson

NEWS.md Outdated Show resolved Hide resolved
Copy link
Owner

@nealrichardson nealrichardson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again!

@nealrichardson nealrichardson merged commit a99f0a2 into nealrichardson:main Apr 25, 2024
10 checks passed
@jmaspons jmaspons deleted the empty_body branch April 25, 2024 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when using with_mock_dir and responses with empty body
2 participants