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
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# hubValidations (development version)

* Custom checks no longer fail if validation is run outside of the root of the hub (#141)
* Downgrade result of missing model metadata file check from `check_error` to `check_failure` and suppress early return in case of check failure in `validate_model_file()` (#138).
* Add `check_file_n()` function to validate that the number of files submitted per round does not exceed the allowed number of submissions per team (#139).
* Ignore `NA`s in relevant `tbl` columns in `opt_check_tbl_col_timediff()` and `opt_check_tbl_horizon_timediff()` checks to ensure rows that may not be targeting relevant to modeling task do not cause false check failure. (#140).
Expand Down
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
3 changes: 1 addition & 2 deletions tests/testthat/_snaps/execute_custom_checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@
# execute_custom_checks sourcing functions from scripts works

Code
test_custom_checks_caller(validations_cfg_path = testthat::test_path("testdata",
"config", "validations-src.yml"))
validations_src_external
Message

-- 2023-05-08-hub-ensemble.parquet ----
Expand Down
31 changes: 31 additions & 0 deletions tests/testthat/helper-custom-fns.R
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,34 @@ 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_dir = testthat::test_path("testdata/src/R/"),
yaml_path = testthat::test_path("testdata/config/validations-src.yml")
) {
if (is.null(new_path)) {
return()
}
# make a copy of the hub
fs::dir_copy(hub_path, new_path)

# create the script path
new_path <- fs::path(new_path, fs::path_file(hub_path))
script_path <- fs::path(new_path, "src/validations/R")
# NOTE: this creates the `src/validatons` directory because if the `R/` dir
# exists, `dir_copy()` will place the copied directory _inside_ it, which is
# not what we want.
fs::dir_create(fs::path_dir(script_path))

# Copy the directory of the existing files to the new script path
fs::dir_copy(check_dir, script_path)

# Copy the provided yaml file to the config folder
new_yaml <- fs::path(new_path, "hub-config", "validations.yml")
fs::file_copy(yaml_path, new_yaml, overwrite = TRUE)
# nolint end
return(new_path)
}
53 changes: 25 additions & 28 deletions tests/testthat/test-execute_custom_checks.R
Original file line number Diff line number Diff line change
Expand Up @@ -25,54 +25,51 @@ test_that("execute_custom_checks works", {
})

test_that("execute_custom_checks sourcing functions from scripts works", {
tmp <- withr::local_tempdir()
the_config <- testthat::test_path("testdata/config/validations-src.yml")

expect_snapshot(
test_custom_checks_caller(
validations_cfg_path = testthat::test_path(
"testdata",
"config",
"validations-src.yml"
)
)
)
hub <- stand_up_custom_check_hub(new_path = tmp, yaml_path = the_config)

expect_no_error({
withr::with_tempdir({
validations_src_external <- test_custom_checks_caller(hub_path = hub)
})
})

expect_snapshot(validations_src_external)
})


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()
hub1 <- stand_up_custom_check_hub(new_path = tmp1,
yaml_path = testthat::test_path("testdata/config/validations-early-ret.yml")
)
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,
yaml_path = testthat::test_path("testdata/config/validations-exec-error.yml")
)
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,
yaml_path = testthat::test_path("testdata/config/validations-no-early-ret.yml")
)
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
4 changes: 2 additions & 2 deletions tests/testthat/testdata/config/validations-early-ret.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ default:
test_custom_checks_caller:
check_1:
fn: "simple_example_check"
source: !expr testthat::test_path("testdata/src/R/src_simple_example_check.R")
source: "src/validations/R/src_simple_example_check.R"
args:
check: FALSE
error: TRUE
check_2:
fn: "simple_example_check"
source: !expr testthat::test_path("testdata/src/R/src_simple_example_check.R")
source: "src/validations/R/src_simple_example_check.R"
args:
check: TRUE
error: FALSE
4 changes: 2 additions & 2 deletions tests/testthat/testdata/config/validations-exec-error.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ default:
test_custom_checks_caller:
check_1:
fn: "simple_example_check"
source: !expr testthat::test_path("testdata/src/R/src_simple_example_check.R")
source: "src/validations/R/src_simple_example_check.R"
args:
check: FALSE
error: TRUE
exec_error: TRUE
check_2:
fn: "simple_example_check"
source: !expr testthat::test_path("testdata/src/R/src_simple_example_check.R")
source: "src/validations/R/src_simple_example_check.R"
args:
check: TRUE
error: FALSE
4 changes: 2 additions & 2 deletions tests/testthat/testdata/config/validations-no-early-ret.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ default:
test_custom_checks_caller:
check_1:
fn: "simple_example_check"
source: !expr testthat::test_path("testdata/src/R/src_simple_example_check.R")
source: "src/validations/R/src_simple_example_check.R"
args:
check: FALSE
error: FALSE
check_2:
fn: "simple_example_check"
source: !expr testthat::test_path("testdata/src/R/src_simple_example_check.R")
source: "src/validations/R/src_simple_example_check.R"
args:
check: TRUE
error: FALSE
2 changes: 1 addition & 1 deletion tests/testthat/testdata/config/validations-src.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ default:
test_custom_checks_caller:
src_check_works:
fn: "src_check_works"
source: !expr testthat::test_path("testdata/src/R/src_check_works.R")
source: "src/validations/R/src_check_works.R"
args:
extra_msg: "Extra arguments passed"
Loading