diff --git a/R/download_cnefe.R b/R/download_cnefe.R index bef671a..23a19c5 100644 --- a/R/download_cnefe.R +++ b/R/download_cnefe.R @@ -34,7 +34,7 @@ download_cnefe <- function(state = "all", progress = TRUE, cache = TRUE) { ) if (!cache) { - data_dir <- tempfile("standardized_cnefe") + data_dir <- fs::path_norm(tempfile("standardized_cnefe")) } else { data_dir <- get_cache_dir() } @@ -91,11 +91,7 @@ download_files <- function(files_to_download, progress) { dest_files <- fs::path(download_dir, basename(files_to_download)) - responses <- httr2::req_perform_parallel( - requests, - paths = dest_files, - progress = ifelse(progress == TRUE, "Downloading CNEFE data", FALSE) - ) + responses <- perform_requests_in_parallel(requests, dest_files, progress) response_errored <- purrr::map_lgl( responses, @@ -104,13 +100,28 @@ download_files <- function(files_to_download, progress) { # TODO: improve this error if (any(response_errored)) { - invalid_dest_files <- dest_files[response_errored] - fs::file_delete(invalid_dest_files) - stop("Could not download data for one of the states, uh-oh!") } return(dest_files) } +perform_requests_in_parallel <- function(requests, dest_files, progress) { + # we create this wrapper around httr2::req_perform_parallel just for testing + # purposes. it's easier to mock this function when testing than to mock a + # function from another package. + # + # related test: "errors if could not download the data for one or more states" + # in test-download_cnefe + # + # related help page: + # https://testthat.r-lib.org/reference/local_mocked_bindings.html + + httr2::req_perform_parallel( + requests, + paths = dest_files, + on_error = "continue", + progress = ifelse(progress == TRUE, "Downloading CNEFE data", FALSE) + ) +} diff --git a/tests/testthat/test-download_cnefe.R b/tests/testthat/test-download_cnefe.R new file mode 100644 index 0000000..04077dc --- /dev/null +++ b/tests/testthat/test-download_cnefe.R @@ -0,0 +1,69 @@ +tester <- function(state = "all", progress = TRUE, cache = TRUE) { + download_cnefe(state, progress, cache) +} + +test_that("errors with incorrect input", { + expect_error(tester(1)) + expect_error(tester("oie")) + + expect_error(tester(progress = 1)) + expect_error(tester(progress = NA)) + expect_error(tester(progress = c(TRUE, TRUE))) + + expect_error(tester(cache = 1)) + expect_error(tester(cache = NA)) + expect_error(tester(cache = c(TRUE, TRUE))) +}) + +test_that("returns the path to the downloaded files", { + result <- tester("AL") + expect_identical( + result, + file.path(get_cache_dir(), "estado=AL/part-0.parquet") + ) + + states <- c("AC", "AL") + result <- tester(states) + expect_identical( + result, + file.path(get_cache_dir(), glue::glue("estado={states}/part-0.parquet")) + ) +}) + +test_that("cache usage is controlled by the cache argument", { + result <- tester("AL", cache = TRUE) + expect_identical( + result, + file.path(get_cache_dir(), "estado=AL/part-0.parquet") + ) + + result <- tester("AL", cache = FALSE) + expect_true( + grepl(fs::path(fs::path_norm(tempdir()), "standardized_cnefe"), result) + ) +}) + +test_that("errors if could not download the data for one or more states", { + local_mocked_bindings( + perform_requests_in_parallel = function(...) { + httr2::req_perform_parallel( + list(httr2::request("FAILURE")), + on_error = "continue" + ) + } + ) + + expect_error(tester("AL", cache = FALSE)) +}) + +test_that("would download the data of all states if state='all'", { + local_mocked_bindings( + perform_requests_in_parallel = function(requests, ...) { + if (length(requests) == 27) { + cli::cli_abort("Too much to download", class = "state_all_succeeded") + } + } + ) + + expect_error(tester("all", cache = FALSE), class = "state_all_succeeded") +})