Improve test coverage #93

dshemetov · 2023-04-29T02:40:40Z

Current coverage looks like:

File	       Lines	Relevant Covered Missed	Hits / Line	Coverage
R/covidcast.R	178	94	72	22	275	76.60%
R/epidatacall.R	270	89	74	15	21	83.15%
R/endpoints.R	1679	810	796	14	2	98.27%
R/model.R	181	95	94	1	186	98.95%
R/check.R	51	28	28	0	67	100.00%
R/request.R	44	18	18	0	10	100.00%
R/auth.R	35	18	18	0	8	100.00%
R/utils.R	19	6	6	0	47	100.00%

Generated with

library(covr)
report(file="test-report.html")

The text was updated successfully, but these errors were encountered:

dsweber2 · 2023-05-23T21:20:31Z

Notes on what's not covered yet:

the TLDR broad categories of untested:

printing utilities
some fetch errors
some http error codes
endpoint errors (mostly issues/lag being mutually exclusive
if statements which had a case missing

going to put two categories per file: to be tested and not to be tested. Feel free to move specific examples around as needed

covidcast.R:

to be tested:

https://github.com/cmu-delphi/epidatr/blob/dev/R/covidcast.R#L53 as.data.frame.covidcast_data_signal_list <- function(signals, ...) not sure this is actually called at all (maybe implicitly?), but flips from a list with sub-components to a table.
- Dmitry comment: this is easy to test with covidcast_epidata() by doing covidcast_epidata()$sources %>% as.data.frame, however, maybe we want a tibble conversion instead Should we support as.tibble instead of as.dataframe for covidcast_data_sources and covidcast_data_signals? #112
https://github.com/cmu-delphi/epidatr/blob/dev/R/covidcast.R#L150 as.data.frame.covidcast_data_source_list <- function(sources, ...) the source version of 53
- Dmitry comment: same as above
https://github.com/cmu-delphi/epidatr/blob/dev/R/covidcast.R#L115 dealing with http error codes
- Dmitry comment: no need to test specific ones, just mock out a HTTP 400 error and that should be good enough; consider using httpbin
(newly added code in parse source) make sure there are no duplicate source names in names(covidcast_epidata()$signals) (solving issue Use consistent naming for covidcast data_sources/sources #53 )

not to be tested:

https://github.com/cmu-delphi/epidatr/blob/dev/R/covidcast.R#L165 print.covidcast_epidata <- function(epidata, ...) printing again
https://github.com/cmu-delphi/epidatr/blob/dev/R/covidcast.R#L82 print.covidcast_data_source <- function(source, ...) printing again
https://github.com/cmu-delphi/epidatr/blob/dev/R/covidcast.R#L34 printing print.covidcast_data_signal
(just to make this a list)
Dmitry comment: fine to not test

request.R:

to be tested:

https://github.com/cmu-delphi/epidatr/blob/dev/R/covidcast.R#L34 http error code, specifically 414 request too long.

not to be tested:

(just to make this a list)

epidatacall.R:

to be tested:

https://github.com/cmu-delphi/epidatr/blob/dev/R/epidatacall.R#L124 fetch <- function(epidata_call, fields = NULL, disable_date_parsing = FALSE) classic call isn't tested in fetch
https://github.com/cmu-delphi/epidatr/blob/dev/R/epidatacall.R#L152 fetch_tbl <- function(epidata_call, fields = NULL, disable_date_parsing = FALSE) fetch_tbl called on an endpoint that only supports classic.
https://github.com/cmu-delphi/epidatr/blob/dev/R/epidatacall.R#L219 fetch_csv <- function(epidata_call, fields = NULL, disable_date_parsing = FALSE, disable_tibble_output = FALSE) fetch_csv called on an endpoint that only supports classic.
https://github.com/cmu-delphi/epidatr/blob/dev/R/epidatacall.R#L324 request_impl <- function(epidata_call, format_type, fields = NULL) another http error not tested
https://github.com/cmu-delphi/epidatr/blob/dev/R/epidatacall.R#L71 request_arguments <- function(epidata_call, format_type, fields = NULL) setting fields in the case that someone has handed non-NULL fields.
https://github.com/cmu-delphi/epidatr/blob/dev/R/epidatacall.R#L190 fetch_classic <- function(epidata_call, fields = NULL, disable_data_frame_parsing = TRUE) classic errors untested. FUTURE NOTE: I'm not entirely sure this is even possible to activate, as request_impl throws http errors rather than passing them up.
https://github.com/cmu-delphi/epidatr/blob/dev/R/epidatacall.R#L304 with_base_url <- function(epidata_call, base_url) also never tested. Are there even other base_url's to test this on? (besides mocking)

not to be tested:

https://github.com/cmu-delphi/epidatr/blob/dev/R/epidatacall.R#L265 fetch_debug <- function(epidata_call, format_type = "classic", fields = NULL) apparently never tested, though I think this is a helper tool for mocking.
https://github.com/cmu-delphi/epidatr/blob/dev/R/epidatacall.R#L93 print.epidata_call <- function(epidata_call) notify the user in the cli
https://github.com/cmu-delphi/epidatr/blob/dev/R/epidatacall.R#L230 fetch_csv <- function(epidata_call, fields = NULL, disable_date_parsing = FALSE, disable_tibble_output = FALSE) tibble output never disabled
https://github.com/cmu-delphi/epidatr/blob/dev/R/epidatacall.R#L248 fetch_csv <- function(epidata_call, fields = NULL, disable_date_parsing = FALSE, disable_tibble_output = FALSE) col_names never of length 0
https://github.com/cmu-delphi/epidatr/blob/dev/R/epidatacall.R#L356 info_to_type <- function(info, disable_date_parsing = FALSE) date parsing never disabled.
https://github.com/cmu-delphi/epidatr/blob/dev/R/epidatacall.R#L257 fetch_csv <- function(epidata_call, fields = NULL, disable_date_parsing = FALSE, disable_tibble_output = FALSE) info's type is never epiweek?
https://github.com/cmu-delphi/epidatr/blob/dev/R/epidatacall.R#L340 print.epidata_csv <- function(x, ...) another print
(just to make this a list)

model.R:

https://github.com/cmu-delphi/epidatr/blob/dev/R/model.R#L104-110 parse_value <- function(info, value, disable_date_parsing = FALSE) a couple of cases which aren't hit, like returning null if parsing a null, info$type is an epiweek, or a bool. NOTE: mostly covered, except the NULL; I'm not sure there even is a way to get a column that is actually a NULL.
https://github.com/cmu-delphi/epidatr/blob/dev/R/model.R#L126 parse_data_frame <- function(epidata_call, df, disable_date_parsing = FALSE) meta of length 0 never happens
https://github.com/cmu-delphi/epidatr/blob/dev/R/model.R#L147 parse_api_week <- function(value) not tested at all, strangely, ~~even though it comes up in other tests~~ it didn't come up in other tests, so I added a new fetch example, flusurv-epiweeks.rds
https://github.com/cmu-delphi/epidatr/blob/dev/R/model.R#L159 fields_to_predicate <- function(fields = NULL) always NULL in tests so mostly untested
https://github.com/cmu-delphi/epidatr/blob/dev/R/model.R#L199 parse_timeset_input <- function(value) error case goes untested

not to be tested:

https://github.com/cmu-delphi/epidatr/blob/dev/R/epidatacall.R#L82 request_arguments <- function(epidata_call, format_type, fields = NULL) format list argument; the only function called here format_list has tests though.
(just to make this a list)

endpoints.R:

not to be tested:

dsweber2 · 2023-06-01T23:52:22Z

In epidatacall.R fetch_classic, I don't think the check for response_content$result != 1 is the right way to go about this.
I think result is never going to be -1 or -2, as http errors 400, 401, 403, 405, 414, and 500 are all thrown as R errors earlier in request.R, while other errors make no guarantees of returning something parseable by httr::content and/or jsonlite::fromJSON. An example:

test_that("non-enumerated errors are passed up the chain", {
  response <- httr::RETRY("GET",
    url = "https://httpbin.org/status/418",
    query = list(),
    terminate_on = c(400, 401, 403, 405, 414, 500),
    http_headers,
    httr::authenticate("epidata", get_auth_key())
  ) %>% readr::write_rds(testthat::test_path("data/test-do_request-httpbin418.rds"))
  local_mocked_bindings(request_impl = function(...) readRDS(testthat::test_path("data/test-do_request-httpbin418.rds")))
  epidata_call <- delphi(system = "ec", epiweek = 201501)
  expect_error(epidata_call %>% fetch_classic(), class = "epidata_error")
})

So I'm leaving that one unchecked and untested for now

dshemetov · 2023-06-09T20:54:04Z

So there are 4 epidata status codes I know:

1 success
2 truncated
-2 no results
-1 generic error (e.g. messed up query params)

All of these come out of a non-error HTTP response.

But yes, any of the other HTTP error codes come prior to epidata status codes.

dshemetov added help wanted Extra attention is needed P1 medium priority labels Apr 29, 2023

dsweber2 self-assigned this May 17, 2023

dsweber2 mentioned this issue Jun 1, 2023

Extending tests #118

Merged

dsweber2 closed this as completed in #118 Jun 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve test coverage #93

Improve test coverage #93

dshemetov commented Apr 29, 2023 •

edited

Loading

dsweber2 commented May 23, 2023 •

edited

Loading

dsweber2 commented Jun 1, 2023

dshemetov commented Jun 9, 2023 •

edited

Loading

Improve test coverage #93

Improve test coverage #93

Comments

dshemetov commented Apr 29, 2023 • edited Loading

dsweber2 commented May 23, 2023 • edited Loading

Notes on what's not covered yet:

the TLDR broad categories of untested:

covidcast.R:

to be tested:

not to be tested:

request.R:

to be tested:

not to be tested:

epidatacall.R:

to be tested:

not to be tested:

model.R:

not to be tested:

endpoints.R:

not to be tested:

dsweber2 commented Jun 1, 2023

dshemetov commented Jun 9, 2023 • edited Loading

dshemetov commented Apr 29, 2023 •

edited

Loading

dsweber2 commented May 23, 2023 •

edited

Loading

dshemetov commented Jun 9, 2023 •

edited

Loading