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

Update exec_cfg_check to use config source path relative to the hub root #142

Merged
merged 13 commits into from
Oct 31, 2024
4 changes: 3 additions & 1 deletion R/exec_cfg_check.R
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ exec_cfg_check <- function(check_name, validations_cfg, caller_env, caller_call)
)
} else if (!is.null(fn_cfg[["source"]])) {
# TODO Validate source script.
source(fn_cfg[["source"]], local = TRUE)
hub_path <- rlang::env_get(env = caller_env, nm = "hub_path")
src <- fs::path(hub_path, fn_cfg[["source"]])
source(src, local = TRUE)
fn <- get(fn_cfg[["fn"]])
}

Expand Down
5 changes: 2 additions & 3 deletions tests/testthat/_snaps/execute_custom_checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,12 @@
# execute_custom_checks sourcing functions from scripts works

Code
test_custom_checks_caller(validations_cfg_path = testthat::test_path("testdata",
"config", "validations-src.yml"))
print(res)
zkamvar marked this conversation as resolved.
Show resolved Hide resolved
Message

-- 2023-05-08-hub-ensemble.parquet ----

i [src_check_works]: Sourcing custom functions WORKS! Also "Extra arguments passed"!!
i [check_1]: Sourcing custom functions WORKS! Also "Extra arguments passed"!!

# execute_custom_checks return early when appropriate

Expand Down
34 changes: 34 additions & 0 deletions tests/testthat/helper-custom-fns.R
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,37 @@ test_custom_checks_caller <- function(
# nolint end
execute_custom_checks(validations_cfg_path = validations_cfg_path)
}

stand_up_custom_check_hub <- function(
# nolint start
hub_path = system.file("testhubs/flusight", package = "hubValidations"),
new_path = NULL,
check_path = testthat::test_path("testdata/src/R/src_check_works.R"),
args = list("src_check_works" = list(extra_msg = 'Extra arguments passed'))
) {
if (is.null(new_path)) {
return()
}
fs::dir_copy(hub_path, new_path)
new_path <- fs::path(new_path, fs::path_file(hub_path))
script_path <- fs::path(new_path, "src/validations/R/test-check.R")
fs::dir_create(fs::path_dir(script_path))
fs::file_copy(check_path, script_path, overwrite = TRUE)
n <- seq_along(args)
fun <- lapply(n, function(fn) {
list(
fn = names(args)[[fn]],
source = fs::path_rel(script_path, start = new_path),
args = args[[fn]]
)
})
names(fun) <- paste0("check_", n)
yml <- list(
"default" = list(
"test_custom_checks_caller" = fun
)
)
# nolint end
writeLines(yaml::as.yaml(yml), fs::path(new_path, "hub-config", "validations.yml"))
return(new_path)
}
58 changes: 33 additions & 25 deletions tests/testthat/test-execute_custom_checks.R
Original file line number Diff line number Diff line change
Expand Up @@ -25,54 +25,62 @@ test_that("execute_custom_checks works", {
})

test_that("execute_custom_checks sourcing functions from scripts works", {
tmp <- withr::local_tempdir()
hub <- stand_up_custom_check_hub(new_path = tmp)

expect_snapshot(
test_custom_checks_caller(
validations_cfg_path = testthat::test_path(
"testdata",
"config",
"validations-src.yml"
)
)
)
expect_no_error({
withr::with_tempdir({
res <- test_custom_checks_caller(hub_path = hub)
})
})

expect_snapshot(print(res))
Copy link
Member

Choose a reason for hiding this comment

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

It's not that informative to take a snapshot of a generically named object as in the snapshot, it's not clear what the snapshot refers to. I'd prefer if the snapshot were of either:

  • more complete code generating the snapshot
  • a more descriptive name for the contents of the object being snapshotted

})


test_that("execute_custom_checks return early when appropriate", {
# When the first custom check returns an check_error class object, custom check
# execution should return early
early_ret_custom <- test_custom_checks_caller(
validations_cfg_path = testthat::test_path(
"testdata",
"config",
"validations-early-ret.yml"
tmp1 <- withr::local_tempdir()
the_check <- testthat::test_path("testdata/src/R/src_simple_example_check.R")
hub1 <- stand_up_custom_check_hub(new_path = tmp1,
check_path = the_check,
args = list(
simple_example_check = list(check = FALSE, error = TRUE),
simple_example_check = list(check = TRUE, error = FALSE)
)
)
early_ret_custom <- test_custom_checks_caller(hub_path = hub1)
expect_snapshot(early_ret_custom)
expect_length(early_ret_custom, 1L)
expect_false("check_2" %in% names(early_ret_custom))

# Same when the first custom check returns an exec_error class object
early_ret_exec_error <- test_custom_checks_caller(
validations_cfg_path = testthat::test_path(
"testdata",
"config",
"validations-exec-error.yml"
tmp2 <- withr::local_tempdir()
hub2 <- stand_up_custom_check_hub(new_path = tmp2,
check_path = the_check,
args = list(
simple_example_check = list(check = FALSE, error = TRUE, exec_error = TRUE),
simple_example_check = list(check = TRUE, error = FALSE)
)
)
early_ret_exec_error <- test_custom_checks_caller(hub2)
expect_snapshot(early_ret_exec_error)
expect_length(early_ret_exec_error, 1L)
expect_false("check_2" %in% names(early_ret_exec_error))
expect_false("check_2" %in% names(early_ret_custom))


# When the first custom check returns an check_failure class object, custom check
# execution should proceed
no_early_ret_custom <- test_custom_checks_caller(
validations_cfg_path = testthat::test_path(
"testdata",
"config",
"validations-no-early-ret.yml"
tmp3 <- withr::local_tempdir()
hub3 <- stand_up_custom_check_hub(new_path = tmp3,
check_path = the_check,
args = list(
simple_example_check = list(check = FALSE, error = FALSE),
simple_example_check = list(check = TRUE, error = FALSE)
)
)
no_early_ret_custom <- test_custom_checks_caller(hub3)
expect_snapshot(no_early_ret_custom)
expect_length(no_early_ret_custom, 2L)
expect_true("check_2" %in% names(no_early_ret_custom))
Expand Down
14 changes: 0 additions & 14 deletions tests/testthat/testdata/config/validations-early-ret.yml

This file was deleted.

15 changes: 0 additions & 15 deletions tests/testthat/testdata/config/validations-exec-error.yml

This file was deleted.

14 changes: 0 additions & 14 deletions tests/testthat/testdata/config/validations-no-early-ret.yml

This file was deleted.

7 changes: 0 additions & 7 deletions tests/testthat/testdata/config/validations-src.yml

This file was deleted.

Loading