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

json loading tests #425

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export(start_using_memoise)
export(stop_using_memoise)
export(test_cleaning)
export(test_download)
export(test_download_JSON)
export(test_processing)
export(test_return)
importFrom(R6,R6Class)
Expand Down Expand Up @@ -72,6 +73,7 @@ importFrom(dplyr,pull)
importFrom(dplyr,recode)
importFrom(dplyr,rename)
importFrom(dplyr,select)
importFrom(dplyr,slice_head)
importFrom(dplyr,slice_tail)
importFrom(dplyr,starts_with)
importFrom(dplyr,summarise)
Expand Down
43 changes: 43 additions & 0 deletions R/test-DataClass.R
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,49 @@ test_download <- function(DataClass_obj, download, snapshot_path) {
}
}


#' Test download method for JSON files works correctly
#' @description Test data can be downloaded if `download = TRUE`, or a requested
#' snapshot file is not found, and store a snap shot in the `snapshot_dir`. If
#' an existing snapshot file is found then load this data to use in future tests
#' @param DataClass_obj The R6Class object to perform checks on.
#' Must be a `DataClass` or `DataClass` child object.
#' @param download Logical check to download or use a snapshot of the data
#' @param snapshot_path character_array the path to save the downloaded
#' snapshot to.
#' @importFrom purrr map walk
#' @importFrom dplyr slice_head
#' @family tests
#' @concept tests
#' @export
test_download_JSON <- function(DataClass_obj, download, snapshot_path) {
if (!file.exists(snapshot_path)) {
download <- TRUE
}
if (download) {
testthat::test_that(
paste0(DataClass_obj$data_name, " downloads sucessfully"),
{
DataClass_obj$download_JSON()
walk(DataClass_obj$data$raw, function(data) {
testthat::expect_s3_class(data, "data.frame")
testthat::expect_true(nrow(data) > 0)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of these tests (which are copied straight over from the CSV code) sort of implicitly assume that we are getting table-like inputs from JSON but even with the Vietnam data the clean method is having to do more “heavy lifting”, and that’s just because we’re getting three parallel lists of tables (roughly) with a list to tie them together. Conceivably we could have other JSON input which was even further from our rectangular array assumptions but still valid JSON, useful data, and convertible (by cleaning) into something the rest of our class can work with.

I don’t necessarily think this needs to be changed now but I could imagine a case where we have another input which generates valid raw data which fails this test - and I’ve already made the argument about why cleaning should be done in the clean methods so I’m inclined to keep the download functions from starting to squeeze data into our rectangular table straitjacket.

Copy link
Contributor

@seabbs seabbs Sep 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes very much agree. Hmm that does leave us in a bit of a poser then regarding what to test. I suppose that lack of error and the existence of a JSON object as a minimum?

testthat::expect_true(ncol(data) >= 2
|| typeof(data[[1]]) == "list")
})
}
)
DataClass_obj$data$raw <- map(vn$data$raw,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will presumably fail if the input isn't a list of dataframes?

slice_head,
n = 2
)

saveRDS(DataClass_obj$data$raw, snapshot_path)
} else {
DataClass_obj$data$raw <- readRDS(snapshot_path)
}
}

#' Test clean method works correctly
#' @description Test data can be cleaned properly. The clean method is invoked
#' to generate clean data. This data is checked to ensure it is a data.frame,
Expand Down
1 change: 1 addition & 0 deletions man/expect_clean_cols.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions man/expect_columns_contain_data.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions man/expect_processed_cols.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions man/test_cleaning.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions man/test_download.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 33 additions & 0 deletions man/test_download_JSON.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions man/test_processing.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions man/test_return.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions tests/testthat/custom_data/mtcars.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[{"mpg":21,"cyl":6,"disp":160,"hp":110,"drat":3.9,"wt":2.62,"qsec":16.46,"vs":0,"am":1,"gear":4,"carb":4},{"mpg":21,"cyl":6,"disp":160,"hp":110,"drat":3.9,"wt":2.875,"qsec":17.02,"vs":0,"am":1,"gear":4,"carb":4},{"mpg":22.8,"cyl":4,"disp":108,"hp":93,"drat":3.85,"wt":2.32,"qsec":18.61,"vs":1,"am":1,"gear":4,"carb":1},{"mpg":21.4,"cyl":6,"disp":258,"hp":110,"drat":3.08,"wt":3.215,"qsec":19.44,"vs":1,"am":0,"gear":3,"carb":1},{"mpg":18.7,"cyl":8,"disp":360,"hp":175,"drat":3.15,"wt":3.44,"qsec":17.02,"vs":0,"am":0,"gear":3,"carb":2},{"mpg":18.1,"cyl":6,"disp":225,"hp":105,"drat":2.76,"wt":3.46,"qsec":20.22,"vs":1,"am":0,"gear":3,"carb":1}]
36 changes: 36 additions & 0 deletions tests/testthat/test-json_reader.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
test_path <- "custom_data/mtcars.json"
seabbs marked this conversation as resolved.
Show resolved Hide resolved
target <- tibble::as_tibble(head(mtcars))

test_that("json_reader can read in a simple dataset", {
seabbs marked this conversation as resolved.
Show resolved Hide resolved
test <- json_reader(test_path)
expect_s3_class(test, "tbl_df")
expect_equal(
colnames(test),
colnames(target)
)
attributes(test)$spec <- NULL
attributes(test)$problems <- NULL
expect_equal(
test,
target
)
})

test_that("json_reader verbosity is controlled as expected", {
expect_gte(
length(capture.output(tmp <- json_reader(test_path, verbose = TRUE),
type = "message"
)),
1
)
expect_equal(
length(capture.output(tmp <- json_reader(test_path, verbose = FALSE),
type = "message"
)),
0
)
})

test_that("json_reader fails as expected when given a file that doesn't exist", {
expect_error(json_reader("nonsense.json", verbose = FALSE))
})