dark-peak-analytics / assertHE

R package to assist in the verification of health economic decision models.
https://dark-peak-analytics.github.io/assertHE/
Other
4 stars 10 forks source link

Avoiding edge case failure for visualise_project() #77

Closed nialldavison closed 3 months ago

nialldavison commented 4 months ago

Bug description visualise_project() returns an uninformative error message if no tests are found in the specified test_path. The error means that the network plot is not rendered.

This may be problematic if a user copies example code for visualise_project() from the README, and the testthat folder does exist, but is not yet prepared.

To Reproduce

Here is some example code that can be run inside a new blank project...

library(assertHE)

# Create a couple of functions to be visualised
function1_content <- 'foo1 <- function(x) {
  return(x * 2)
}'
function2_content <- 'foo2 <- function(x) {
  return(x * 4)
}'

# Write the functions to files in the R folder
if (!dir.exists("R")) dir.create("R")
writeLines(function1_content, con = "R/foo1.R")
writeLines(function2_content, con = "R/foo2.R")

# Create a testthat folder and a couple of test files (suppressing usethis output for the sake of the reprex)
withr::local_options(usethis.quiet = T)
usethis::use_testthat()
usethis::use_test("foo1")
usethis::use_test("foo2")
withr::local_options(usethis.quiet = F)

# Print the default testthat file. Note that the foo1() function is not actually called
cat(paste(readLines("tests/testthat/test-foo1.R"),  collapse = "\n"))
#> test_that("multiplication works", {
#>   expect_equal(2 * 2, 4)
#> })

# Run visualise_project() with the testthat folder, and get a scary error message
visualise_project(
  project_path = ".",
  foo_path = "R",
  test_path = "tests/testthat")
#> Error in `$<-.data.frame`(`*tmp*`, "test_location", value = character(0)): replacement has 0 rows, data has 2

Created on 2024-06-21 with reprex v2.1.0

Expected behavior Would expect the code to identify no tests, but for the network to still plot.

Fix I believe this is due to the early return() in find_function_calls_in_folder(). When no tests are identified, the subsequent renaming to "test_location" does not occur because the function has already returned.

# Current
if(length(l_foo_test_paths) == 0) return(data.frame(foo_string = foo_strings, location = NA))
# Suggested
if(length(l_foo_test_paths) == 0) return(data.frame(foo_string = foo_strings, test_location = NA))

Having tested this code. The function then behaves as expected.

Tests This would require an update to the test in test-test_finder.R: "Check find_function_calls_in_folder works for sicksickerPack example".

# Current
expect_equal(object = df_output_2$location, expected = NA)
# Suggested
expect_equal(object = df_output_2$test_location, expected = NA)

R CMD Check devtools::check() was run with no errors or warnings.

There is one NOTE unrelated to this change: Imports includes 21 non-default packages. Importing from so many packages makes the package vulnerable to any of them becoming unavailable. Move as many as possible to Suggests and use conditionally.

Pull request I am submitting a simple pull request to address the above issue.