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

with_mock_api fails to handle httr::rerequest() #84

Open
krivard opened this issue Jul 3, 2023 · 3 comments
Open

with_mock_api fails to handle httr::rerequest() #84

krivard opened this issue Jul 3, 2023 · 3 comments

Comments

@krivard
Copy link

krivard commented Jul 3, 2023

I'm working on adding Cache-Control handling to a library I test using httptest, and to my dismay, it seems that with_mock_api doesn't handle httr::rerequest calls. Is rerequest support planned any time soon?

Example:

# test_rerequest.R
library(testthat)
library(httr)
library(httptest)

f <- function() GET("http://example.org")
g <- function(r) rerequest(r)

# setup: use capture_requests to seed the http cache                                                                                                                                                                                          
capture_requests(f())

with_mock_api({
  test_that("httptest::with_mock_api should handle httr::rerequest", {
    result <- f()
    expect_equal(result$status, 200)

    result <- g(result)
    expect_equal(result$status, 200)
  })
})

> testthat::test_file("test-rerequest.R")

══ Testing test-rerequest.R ════════════════════════════════════════════════
[ FAIL 1 | WARN 0 | SKIP 0 | PASS 1 ]

── Error (test-rerequest.R:16): httptest::with_mock_api should handle httr::rerequest ──
Error in `request_perform(resp$request, resp$handle)`: inherits(handle, "curl_handle") is not TRUE
Backtrace:
 1. g(result)
      at test-rerequest.R:16:4
 2. httr::rerequest(r)
      at test-rerequest.R:6:5
 3. httr:::reperform(r)
 4. httr:::request_perform(resp$request, resp$handle)
 5. base::stopifnot(is.request(req), inherits(handle, "curl_handle"))

[ FAIL 1 | WARN 0 | SKIP 0 | PASS 1 ]
@nealrichardson
Copy link
Owner

Hmm, so when you load a mock response, there was no actual request made, so there is no curl_handle in the response object, which I think explains the error message.

Also, in the default case, when you record a mock, the response headers aren't kept, only the response body, so there are no Cache-Control headers in the mock response when it is loaded, so you're always in the reperform path, which leads to the error. You can solve this by recording mocks with simplify = FALSE, so you'd keep all of the response headers (see https://enpiar.com/r/httptest/reference/capture_requests.html). But then that still may result in unexpected behavior if the cache control headers reference a timestamp, unless you edit it to be far into the future to ensure that the test always hits cache.

What behavior exactly are you trying to test?

@krivard
Copy link
Author

krivard commented Jul 3, 2023

Oh apologies; my test case wasn't clear.

I don't need to test the caching/rerequest behavior using with_mock_api (I can manually check that logic with mockery if I really want to); I'm most worried about the dozens of existing tests that worked with httr::request but are failing with httr::rerequest. Ideally with_mock_api would treat httr::rerequest just like httr::request, or in other words, the mocked request from httr::request would be one without cache-control headers, rerequest would do the reperform, but with_mock_api would intercept it so that you'd get another mocked request.

@nealrichardson
Copy link
Owner

Got it. It sounds like if the function that loaded the mock were to include a curl handle, httr::rerequest() would not error. And it looks like we have the request handle in the function that loads the mocks (https://github.com/nealrichardson/httptest/blob/main/R/mock-api.R#L42-L56), so we might just be able to poke it in there.

Would you be interested in submitting a PR to add this?

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

No branches or pull requests

2 participants