diff --git a/.github/workflows/testthat-editions.yaml b/.github/workflows/testthat-editions.yaml index 8ec2a58..6f8f3c7 100644 --- a/.github/workflows/testthat-editions.yaml +++ b/.github/workflows/testthat-editions.yaml @@ -13,7 +13,7 @@ jobs: matrix: config: - { os: ubuntu-20.04, r: 'release', testhat_e: '2', rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest" } - # - { os: ubuntu-20.04, r: 'release', testhat_e: '3', rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest" } + - { os: ubuntu-20.04, r: 'release', testhat_e: '3', rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest" } env: R_REMOTES_NO_ERRORS_FROM_WARNINGS: true diff --git a/DESCRIPTION b/DESCRIPTION index b29e62d..3eac533 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -42,3 +42,4 @@ Language: en-US RoxygenNote: 7.0.2 Roxygen: list(markdown = TRUE) VignetteBuilder: knitr +Config/testthat/edition: 3 diff --git a/NEWS.md b/NEWS.md index 8c335b4..75238ed 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,8 +1,17 @@ # httptest 3.3.0.9000 (under development) -* changed the filename `helper.R` to `setup.R` to comply with `testthat`'s latest recommendations (#44, @maelle). -* Mocking PUT and POST with a body consisting of only `httr::upload_file` no longer leaves a file connection open. + +## Changes for compatibility with the forthcoming testthat 3rd edition + +* `expect_header()` now emits a single warning with all of the headers that are included with the call (instead of one warning per header). This makes catching multiple headers easier and prevents excess warnings when using testthat 3e (#38, @jonkeane) +* Quiet an extraneous message about untracing `curl::form_file()` (#35, @dmenne) +* Numerous internal testing changes to ensure compatibility with testthat 3e + +## Other fixes and enhancements + +* Mocking PUT and POST with a body consisting of only `httr::upload_file()` no longer leaves a file connection open. * Mock files with special characters in the filename are now correctly found (#33, @natbprice) * Switch continuous integration to use GitHub Actions (#36, @jonkeane) +* The test file `helper.R` was renamed to `setup.R` to comply with `testthat`'s latest recommendations (#44, @maelle) # httptest 3.3.0 diff --git a/R/expect-header.R b/R/expect-header.R index f1f5252..0ce297e 100644 --- a/R/expect-header.R +++ b/R/expect-header.R @@ -25,9 +25,8 @@ expect_header <- function (..., ignore.case=TRUE) { tracer <- quote({ heads <- req$headers - for (h in names(heads)) { - warning(paste(h, heads[h], sep=": "), call.=FALSE) - } + msgs <- lapply(names(heads), function(h) paste(h, heads[h], sep=": ")) + warning(msgs, call.=FALSE) }) with_trace("request_prepare", exit=tracer, where=add_headers, expr={ expect_warning(..., ignore.case=ignore.case) diff --git a/R/trace.R b/R/trace.R index c627b34..5e9458e 100644 --- a/R/trace.R +++ b/R/trace.R @@ -59,7 +59,7 @@ quietly <- function (expr) { #' @return Nothing; called for its side effects #' @export stop_mocking <- function () { - safe_untrace(untrace(curl::form_file)) + safe_untrace(curl::form_file) invisible(safe_untrace("request_perform", add_headers)) } diff --git a/README.md b/README.md index 150244a..5aa8e2b 100644 --- a/README.md +++ b/README.md @@ -48,8 +48,8 @@ Adding `with_mock_api()` to your tests is straightforward. Given a very basic te ```r test_that("Requests happen", { - expect_is(GET("http://httpbin.org/get"), "response") - expect_is( + expect_s3_class(GET("http://httpbin.org/get"), "response") + expect_s3_class( GET("http://httpbin.org/response-headers", query=list(`Content-Type`="application/json")), "response" @@ -62,8 +62,8 @@ if we wrap the code in `with_mock_api()`, actual requests won't happen. ```r with_mock_api({ test_that("Requests happen", { - expect_is(GET("http://httpbin.org/get"), "response") - expect_is( + expect_s3_class(GET("http://httpbin.org/get"), "response") + expect_s3_class( GET("http://httpbin.org/response-headers", query=list(`Content-Type`="application/json")), "response" diff --git a/tests/testthat/setup.R b/tests/testthat/setup.R index f2e7fe5..6a7e6db 100644 --- a/tests/testthat/setup.R +++ b/tests/testthat/setup.R @@ -52,3 +52,21 @@ skip_on_R_older_than <- function (version) { skip(paste("Requires R >=", version)) } } + +# A quiet version of httr's content +quiet_content <- function(...) { + suppressMessages(content(...)) +} + +testthat_transition <- function(old, new) { + is_3e <- tryCatch(testthat::edition_get() == 3, error = function(e) FALSE) + if (is_3e) { + eval(new, envir = parent.frame()) + } else { + eval(old, envir = parent.frame()) + } +} + +# assign to global to be used inside of `public()` calls +third_edition <<- tryCatch(testthat::edition_get() == 3, error = function(e) FALSE) + diff --git a/tests/testthat/test-capture-requests.R b/tests/testthat/test-capture-requests.R index 098787e..b1ab6f9 100644 --- a/tests/testthat/test-capture-requests.R +++ b/tests/testthat/test-capture-requests.R @@ -1,5 +1,3 @@ -context("capture_requests") - d <- tempfile() dl_file <- tempfile() webp_file <- tempfile() @@ -34,7 +32,7 @@ test_that("We can record a series of requests (a few ways)", { )) ## Test the contents of the .R files teapot <- source(file.path(d, "httpbin.org/status/418.R"))$value - expect_is(teapot, "response") + expect_s3_class(teapot, "response") expect_identical(teapot$status_code, 418L) ## Make sure that our .html file has HTML expect_true(any(grepl("", @@ -71,11 +69,11 @@ test_that("We can then load the mocks it stores", { ## Compare the HTML as text because the parsed HTML (XML) object has a ## C pointer that is different between the two objects. expect_identical( - enc2native(content(m2, "text")), + enc2native(quiet_content(m2, "text")), enc2native(content(r2, "text")) ) - expect_true(grepl("", content(m2, "text"))) + expect_true(grepl("", quiet_content(m2, "text"))) expect_identical(content(m3), content(r3)) expect_identical(content(m4), content(r4)) expect_identical(content(m5), content(r5)) @@ -140,7 +138,7 @@ with_mock_api({ expect_true(setequal(dir(d3, recursive=TRUE), c("example.com/get.R", "api/object1.R", "httpbin.org/status/204.R"))) response <- source(file.path(d3, "example.com/get.R"))$value - expect_is(response, "response") + expect_s3_class(response, "response") expect_identical(content(response), content(GET("http://example.com/get/"))) }) diff --git a/tests/testthat/test-content-type.R b/tests/testthat/test-content-type.R index f896f9d..0ce93b5 100644 --- a/tests/testthat/test-content-type.R +++ b/tests/testthat/test-content-type.R @@ -1,5 +1,3 @@ -context("Content-Type parsing") - resp <- source("example.com/html.R")$value test_that("get_content_type handles valid Content-Types, including omitted", { diff --git a/tests/testthat/test-expect-header.R b/tests/testthat/test-expect-header.R index 81bb0a3..45345e7 100644 --- a/tests/testthat/test-expect-header.R +++ b/tests/testthat/test-expect-header.R @@ -1,20 +1,24 @@ -context("expect_header") - public({ with_fake_http({ test_that("expect_header with fake HTTP", { expect_GET(expect_success(expect_header(GET("http://httpbin.org/", config=add_headers(Accept="image/jpeg")), "Accept: image/jpeg"))) - expect_GET(expect_failure(expect_header(GET("http://httpbin.org/", - config=add_headers(Accept="image/png")), - "Accept: image/jpeg"))) + expect_GET(expect_failure(expect_warning( + expect_header(GET("http://httpbin.org/", + config=add_headers(Accept="image/png")), + "Accept: image/jpeg"), + "Accept: image/png" + ))) expect_POST(expect_success(expect_header(POST("http://httpbin.org/", config=add_headers(Accept="image/jpeg")), "Accept: image/jpeg"))) - expect_POST(expect_failure(expect_header(POST("http://httpbin.org/", - config=add_headers(Accept="image/png")), - "Accept: image/jpeg"))) + expect_POST(expect_failure(expect_warning( + expect_header(POST("http://httpbin.org/", + config=add_headers(Accept="image/png")), + "Accept: image/jpeg"), + "Content-Type: Accept: image/png" + ))) }) }) @@ -23,24 +27,32 @@ public({ expect_success(expect_header(GET("api/object1/", config=add_headers(Accept="image/jpeg")), "Accept: image/jpeg")) - expect_failure(expect_header(GET("api/object1/", - config=add_headers(Accept="image/png")), - "Accept: image/jpeg")) - expect_POST(expect_success(expect_header(POST("http://httpbin.org/", - config=add_headers(Accept="image/jpeg")), - "Accept: image/jpeg"))) - expect_failure(expect_header(expect_POST(POST("http://httpbin.org/", - config=add_headers(Accept="image/png")), silent=TRUE), + suppressWarnings( + expect_failure(expect_header(GET("api/object1/", + config=add_headers(Accept="image/png")), + "Accept: image/jpeg")) + ) + suppressWarnings( + expect_POST(expect_success(expect_header(POST("http://httpbin.org/", + config=add_headers(Accept="image/jpeg")), + "Accept: image/jpeg"))) + ) + skip_if(third_edition) + expect_failure(expect_header( + expect_POST(POST("http://httpbin.org/", + config=add_headers(Accept="image/png")), silent=TRUE), "Accept: image/jpeg")) }) test_that("expect_header ignore.case", { expect_success(expect_header(GET("api/object1/", config=add_headers(Accept="image/jpeg")), "accept: image/jpeg")) - expect_failure(expect_header(GET("api/object1/", - config=add_headers(Accept="image/jpeg")), - "accept: image/jpeg", - ignore.case=FALSE)) + suppressWarnings( + expect_failure(expect_header(GET("api/object1/", + config=add_headers(Accept="image/jpeg")), + "accept: image/jpeg", + ignore.case=FALSE)) + ) }) }) @@ -49,9 +61,12 @@ public({ expect_GET(expect_success(expect_header(GET("http://httpbin.org/", config=add_headers(Accept="image/jpeg")), "Accept: image/jpeg"))) - expect_GET(expect_failure(expect_header(GET("http://httpbin.org/", - config=add_headers(Accept="image/png")), - "Accept: image/jpeg"))) + expect_GET(expect_warning( + expect_failure(expect_header(GET("http://httpbin.org/", + config=add_headers(Accept="image/png")), + "Accept: image/jpeg")), + "Accept: image/png" + )) }) }) @@ -60,8 +75,11 @@ public({ expect_success(expect_header(GET("http://httpbin.org/get", config=add_headers(Accept="image/jpeg")), "Accept: image/jpeg")) - expect_failure(expect_header(GET("http://httpbin.org/get", - config=add_headers(Accept="image/png")), - "Accept: image/jpeg")) + expect_failure(expect_warning( + expect_header(GET("http://httpbin.org/get", + config=add_headers(Accept="image/png")), + "Accept: image/jpeg"), + "Accept: image/png" + )) }) }) diff --git a/tests/testthat/test-fake-http.R b/tests/testthat/test-fake-http.R index 7b09d95..d770e83 100644 --- a/tests/testthat/test-fake-http.R +++ b/tests/testthat/test-fake-http.R @@ -1,5 +1,3 @@ -context("Fake HTTP") - public({ with_fake_http({ test_that("fakeGET", { @@ -85,17 +83,25 @@ public({ }) test_that("fake_response returns a valid enough response even if you give it just a URL", { - expect_is(fake_response("http://httpbin.org/get"), "response") + expect_s3_class(fake_response("http://httpbin.org/get"), "response") }) test_that("fake_request gets covered directly (not just in tracer)", { - expect_is(fake_request(list(method="GET", url="http://httpbin.org/get")), - "response") - expect_is(fake_request( - list( - method="POST", - url="http://httpbin.org/get", - options=list(postfields=charToRaw("body")) - )), + expect_s3_class( + expect_message( + fake_request(list(method="GET", url="http://httpbin.org/get")), + "GET http://httpbin.org/get" + ), + "response") + expect_s3_class( + expect_message( + fake_request( + list( + method="POST", + url="http://httpbin.org/get", + options=list(postfields=charToRaw("body")) + )), + "POST http://httpbin.org/get body" + ), "response") }) diff --git a/tests/testthat/test-find-redactor.R b/tests/testthat/test-find-redactor.R index ff403f8..3f6629d 100644 --- a/tests/testthat/test-find-redactor.R +++ b/tests/testthat/test-find-redactor.R @@ -1,5 +1,3 @@ -context("Find package redactors") - expect_redactor <- function (expr) { expect_identical(names(formals(expr)), "response") } @@ -34,16 +32,34 @@ test_that("get_current_redactor edge cases", { with_mock_api({ test_that("Reading redactors from within a package (and install that package)", { newmocks <- tempfile() - expect_message( - capture_while_mocking(path=newmocks, { - ## Install the "testpkg" to a temp lib.loc _after_ we've already started recording - lib <- install_testpkg("testpkg") - library(testpkg, lib.loc=lib) - expect_true("testpkg" %in% names(sessionInfo()$otherPkgs)) + testthat_transition( + expect_message( + capture_while_mocking(path=newmocks, { + ## Install the "testpkg" to a temp lib.loc _after_ we've + ## already started recording + lib <- install_testpkg("testpkg") + library(testpkg, lib.loc=lib) + expect_true("testpkg" %in% names(sessionInfo()$otherPkgs)) - r <- GET("http://example.com/get") - }), - paste0("Using redact.R from ", dQuote("testpkg")) + r <- GET("http://example.com/get") + }), + paste0("Using redact.R from ", dQuote("testpkg")) + ), + expect_message( + expect_message( + capture_while_mocking(path=newmocks, { + ## Install the "testpkg" to a temp lib.loc _after_ we've + ## already started recording + lib <- install_testpkg("testpkg") + library(testpkg, lib.loc=lib) + expect_true("testpkg" %in% names(sessionInfo()$otherPkgs)) + + r <- GET("http://example.com/get") + }), + paste0("Using redact.R from ", dQuote("testpkg")) + ), + paste0("Using request.R from ", dQuote("testpkg")) + ) ) with_mock_path(newmocks, { r2 <- GET("http://example.com/get") @@ -84,7 +100,7 @@ with_mock_api({ }) expect_message( capture_while_mocking(path=newmocks3, { - pkgload::load_all("testpkg") + pkgload::load_all("testpkg", quiet = TRUE) expect_true("testpkg" %in% names(sessionInfo()$otherPkgs)) r <- GET("http://example.com/get") }), diff --git a/tests/testthat/test-json-equal.R b/tests/testthat/test-json-equal.R index 3e4fd50..2f0cea5 100644 --- a/tests/testthat/test-json-equal.R +++ b/tests/testthat/test-json-equal.R @@ -1,5 +1,3 @@ -context("JSON equivalence") - test_that("json_compare", { obj <- list(c=1, b=list(list(2, 3), list(d=9, f=5)), a=5) expect_false(json_compare(list(1, 2), list(2, 1))$equal) diff --git a/tests/testthat/test-mock-api.R b/tests/testthat/test-mock-api.R index 5df7bcf..7135ad6 100644 --- a/tests/testthat/test-mock-api.R +++ b/tests/testthat/test-mock-api.R @@ -1,5 +1,3 @@ -context("Mock API") - public({ with_mock_api({ test_that("Can load an object and file extension is added", { @@ -201,9 +199,9 @@ test_that("load_response invalid extension handling", { }) test_that("mock_request code paths are covered (outside of trace)", { - expect_is(mock_request(list(method="GET", url="api/")), + expect_s3_class(mock_request(list(method="GET", url="api/")), "response") - expect_is(mock_request(list(method="GET", url="http://example.com/html")), + expect_s3_class(mock_request(list(method="GET", url="http://example.com/html")), "response") expect_error(mock_request(list(method="PUT", url="api/"))) }) diff --git a/tests/testthat/test-mock-paths.R b/tests/testthat/test-mock-paths.R index b2c660a..26ebdde 100644 --- a/tests/testthat/test-mock-paths.R +++ b/tests/testthat/test-mock-paths.R @@ -1,5 +1,3 @@ -context("Setting different/multiple mock directories") - public({ test_that(".mockPaths works more or less like .libPaths", { expect_identical(.mockPaths(), ".") diff --git a/tests/testthat/test-offline.R b/tests/testthat/test-offline.R index 3b9c189..dfcb2cd 100644 --- a/tests/testthat/test-offline.R +++ b/tests/testthat/test-offline.R @@ -1,7 +1,8 @@ -context("Offline checking and skipping") - test_that("currently_offline interacts with the mock contexts", { - expect_false(with_fake_http(currently_offline())) + expect_message( + expect_false(with_fake_http(currently_offline())), + "GET http://httpbin.org/" + ) expect_true(without_internet(currently_offline())) }) @@ -13,9 +14,12 @@ public({ }) }) test_that("skip_if_disconnected when 'connected'", { - with_fake_http({ - skip_if_disconnected("This should not skip") - expect_failure(expect_true(FALSE)) - }) + expect_message( + with_fake_http({ + skip_if_disconnected("This should not skip") + expect_failure(expect_true(FALSE)) + }), + "GET http://httpbin.org/" + ) }) }) diff --git a/tests/testthat/test-public.R b/tests/testthat/test-public.R index 2758473..87714a4 100644 --- a/tests/testthat/test-public.R +++ b/tests/testthat/test-public.R @@ -1,5 +1,3 @@ -context("Public") - test_that("Functions not exported can be found", { expect_true(.internalFunction()) }) diff --git a/tests/testthat/test-redact.R b/tests/testthat/test-redact.R index 5c5149f..590b14d 100644 --- a/tests/testthat/test-redact.R +++ b/tests/testthat/test-redact.R @@ -1,5 +1,3 @@ -context("Redaction") - d <- tempfile() with_mock_api({ @@ -133,7 +131,7 @@ with_mock_api({ oauth <- GET("api/object1/", config(token = token)) }) test_that("The response has the 'auth_token' object'", { - expect_is(oauth$request$auth_token, "Token2.0") + expect_s3_class(oauth$request$auth_token, "Token2.0") }) test_that("But the mock doesn't have the auth_token", { diff --git a/tests/testthat/test-trace.R b/tests/testthat/test-trace.R index e7ef3b9..f19a0b5 100644 --- a/tests/testthat/test-trace.R +++ b/tests/testthat/test-trace.R @@ -1,5 +1,3 @@ -context("Tracing") - public({ test_that("safe_untrace makes mocking not error if not already traced", { expect_error(use_mock_api(), NA) diff --git a/tests/testthat/test-use-httptest.R b/tests/testthat/test-use-httptest.R index a909094..23a6868 100644 --- a/tests/testthat/test-use-httptest.R +++ b/tests/testthat/test-use-httptest.R @@ -1,5 +1,3 @@ -context("use_httptest") - test_add_to_desc <- function (str, msg="Adding 'httptest' to Suggests") { f <- tempfile() cat(str, file=f) @@ -60,7 +58,13 @@ expect_added_to_setup <- function (str, msg="Adding library\\(httptest\\)") { test_that("add to setup creates file if doesn't exist", { f <- tempfile() expect_false(file.exists(f)) - expect_message(add_httptest_to_setup(f), "Creating") + testthat_transition( + expect_message(add_httptest_to_setup(f), "Creating"), + expect_message( + expect_message(add_httptest_to_setup(f), "Creating"), + "Adding library\\(httptest\\) to" + ) + ) expect_identical(readLines(f), "library(httptest)") }) @@ -81,7 +85,19 @@ test_that("use_httptest integration test", { desc <- file.path(testpkg, "DESCRIPTION") cat("Title: Foo\n", file=desc) setup <- file.path(testpkg, "tests", "testthat", "setup.R") - expect_message(use_httptest(testpkg)) + testthat_transition( + expect_message(use_httptest(testpkg), "Adding 'httptest' to Suggests"), + expect_message( + expect_message( + expect_message( + use_httptest(testpkg), + "Adding 'httptest' to Suggests" + ), + "Creating " + ), + "Adding library\\(httptest\\) to " + ) + ) expect_identical(readLines(desc), c("Title: Foo", "Suggests: httptest")) expect_identical(readLines(setup), "library(httptest)") # It does nothing if you the package already uses httptest diff --git a/tests/testthat/test-vignette.R b/tests/testthat/test-vignette.R index 6035acb..8e2784a 100644 --- a/tests/testthat/test-vignette.R +++ b/tests/testthat/test-vignette.R @@ -1,5 +1,3 @@ -context("start_vignette") - rp <- httr:::request_perform g <- httr::GET path <- tempfile() diff --git a/tests/testthat/test-without-internet.R b/tests/testthat/test-without-internet.R index c42b2ba..fe48aca 100644 --- a/tests/testthat/test-without-internet.R +++ b/tests/testthat/test-without-internet.R @@ -1,5 +1,3 @@ -context("without_internet") - public({ test_that("Outside of without_internet, requests work", { skip_if_disconnected() @@ -49,11 +47,13 @@ public({ "http://httpbin.org/get", '{"t') ## Just to be explicit since the expectations do partial matching + skip_if(third_edition) expect_failure( expect_PUT(PUT("http://httpbin.org/get", body='{"test":true}'), - "http://httpbin.org/get", - '{"test":true}') + "http://httpbin.org/get", + '{"test":true}') ) + }) test_that("without_internet respects query params", {