Skip to content

Commit

Permalink
Use prepared request in Performance
Browse files Browse the repository at this point in the history
This ensures that the method and body are correctly set for `req_perform_parallel()` and `req_perform_promise()`. Fixes #549
  • Loading branch information
hadley committed Sep 17, 2024
1 parent 11dfd1d commit f72044f
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 1 deletion.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# httr2 (development version)

* `req_perform_parallel()` and `req_perform_promise()` now correctly set up the method and body (#549).

# httr2 1.0.4

* `req_body_file()` now works with files >64kb once more (#524) and no longer leaks a connection if the response doesn't complete succesfully (#534).
Expand Down
2 changes: 1 addition & 1 deletion R/multi-req.R
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ Performance <- R6Class("Performance", public = list(
self$resp <- req
} else {
self$req_prep <- req_prepare(req)
self$handle <- req_handle(req)
self$handle <- req_handle(self$req_prep)

Check warning on line 171 in R/multi-req.R

View check run for this annotation

Codecov / codecov/patch

R/multi-req.R#L171

Added line #L171 was not covered by tests
curl::handle_setopt(self$handle, url = req$url)
}
},
Expand Down
5 changes: 5 additions & 0 deletions tests/testthat/test-multi-req.R
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ test_that("request and paths must match", {
expect_snapshot(req_perform_parallel(req, letters), error = TRUE)
})

test_that("correctly prepares request", {
reqs <- list(request_test("/post") %>% req_method("POST"))
expect_no_error(req_perform_parallel(reqs))
})

test_that("requests happen in parallel", {
# GHA MacOS builder seems to be very slow
skip_if(
Expand Down
5 changes: 5 additions & 0 deletions tests/testthat/test-req-perform-connection.R
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ test_that("validates inputs", {
})
})

test_that("correctly prepares request", {
req <- request_test("/post") %>% req_method("POST")
expect_no_error(resp <- req_perform_connection(req))
})

test_that("can read all data from a connection", {
resp <- request_test("/stream-bytes/2048") %>% req_perform_connection()
withr::defer(close(resp))
Expand Down
6 changes: 6 additions & 0 deletions tests/testthat/test-req-promise.R
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ test_that("returns a promise that resolves", {
expect_equal(resp_status(p2_value), 200)
})

test_that("correctly prepares request", {
req <- request_test("/post") %>% req_method("POST")
prom <- req_perform_promise(req)
expect_no_error(extract_promise(prom))
})

test_that("can promise to download files", {
req <- request_test("/json")
path <- withr::local_tempfile()
Expand Down

0 comments on commit f72044f

Please sign in to comment.