Closed dpprdan closed 4 months ago
That's the idea! To have support for it we need support for httr2 in webmockr as well as here. Any extra time to contribute to these changes?
Unfortunately I don't. I am not familiar with the webmockr/vcr codebase, so I'd need a lot of extra time too, I'm afraid.
Okay, no problem. Will try to get to it soon unless someone else has time
@sckott just wondering if you had any time to look into this? No worries if not, of course! (I would not know how :grimacing:)
not much yet unfortunately. i started a while back on webmockr https://github.com/ropensci/webmockr/compare/master...httr2 but nothing useable yet.
Almost there, left to do:
vcr::use_cassette("stuff", {
res_vcr1 <- request("https://google.com") %>% req_perform()
})
vcr::use_cassette("stuff", {
res_vcr2 <- request("https://google.com") %>% req_perform()
})
res_vcr1
#> <httr2_response>
#> GET https://google.com/
#> Status: 200 OK
#> Body: In memory (7070 bytes)
res_vcr2
#> <httr2_response>
#> GET https://google.com/
#> Status: 200 OK
#> Body: In memory (1 bytes)
httr2
with vcr, via use of httr2::local_mock
in wemockr::httr2_mock
. I'm not sure if this is a problem or notSetting global deferred event(s).
i These will be run:
* Automatically, when the R session ends.
* On demand, if you call `withr::deferred_run()`.
i Use `withr::deferred_clear()` to clear them without executing.
this is great! is there a public package where you're using httr2+vcr?
is there a public package where you're using httr2+vcr?
No, not yet. I shjould make a silly small pkg just to demonstrate though.
@maelle do you have a sense about that warning above from withr
? does that raise a red flag for you? I'm not sure how to suppress it, but i'm just not very familiar with withr
do you get that warning when running the tests or else?
I actually blogged about deferred_run()
recently which made me realize I had ignored that message forever no matter where I saw it :joy: https://masalmon.eu/2023/10/16/test-local-mess-reset/
When you run use_cassette()
that warning is thrown. It comes from here which uses httr2::local_mock
, which uses withr::local_options
, which uses withr::defer
. That's the "hook" mechanism hadley built into httr2
. I'll have a look at your blog post
wrt that webmockr
code, I cringe about having to change stuff in the global env, but that's the only way it works with the hook mechanism hadley used, 🤷🏽 🤷🏽 🤷🏽
Would looking at https://github.com/nealrichardson/httptest2/tree/main/R help? I'm not familiar with how it works under the hood.
(There was an issue I thought was related to the httr2 integration here, but was a redirect issue that's not germaine to httr2, see #267)
In testing this I don't see the withr
warnings when using vcr
in another package. So i'm thinking it's all good, but will keep an eye out
The test package for testing the httr2/vcr integration is up on my account https://github.com/sckott/rrr
wowsa - this thing about httr2
turning HTTP error codes into R errors is really painful - i'm surprised that decision was made - definitely a pkg made for end users and not developers
A few things i'm wokring on now that will hopefully be the last things:
httr2
's error behavior - that http errors are turned into R errors automatically. Considerations:
httr2::req_error(is_error = function(resp) FALSE)
to the intercepted real request and then running req_perform
. httr2
behavior we'd then have to use last_response
to get the response after a 400/500 series response, which would probably work, will try thathttr2
didn't have this behavior at all, but given it's there, it's more important to match the behavior of the pkg as close as possiblePretty sure above 2 are taken care of now with current state on httr2
bfranch
@maelle @dpprdan if either of you get a chance, would love any bug reports/etc. on the vcr/httr2 integration
Now that {httr2} is on CRAN and supports mocking, could {vcr} support it as well?