-
Notifications
You must be signed in to change notification settings - Fork 334
Support developer tests #1988
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
Support developer tests #1988
Conversation
@@ -373,6 +373,10 @@ local_teardown_env <- function(frame = parent.frame()) { | |||
#' @export | |||
find_test_scripts <- function(path, filter = NULL, invert = FALSE, ..., full.names = TRUE, start_first = NULL) { | |||
files <- dir(path, "^test.*\\.[rR]$", full.names = full.names) | |||
if (env_var_is_true("CI") || env_var_is_true("NOT_CRAN")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly there is room for an explicit opt in/out env var that is very specific to this feature, like:
run_dev_tests <- function() {
if (env_var_is_true("RUN_DEV_TESTS")) {
return(TRUE)
}
if (env_var_is_false("RUN_DEV_TESTS")) {
return(FALSE)
}
env_var_is_true("CI") || env_var_is_true("NOT_CRAN")
}
Thinking a bit more about this after todays dicussion about how this works for reverse dependency checks. Perhaps the dev tests should be opt-in per package. So the variable should be named e.g. For example, if I want to run a reverse dependency check for |
Just chiming in to say that I would really like to see this capability added to testthat. My particular use case is that my package |
A few other questions to consider:
I think I'm overall sceptical that this is the correct approach compared to adding the appropriate skip calls to each individual test. I realise that is a lot of work, but it's really hard to reason about snapshots without it. (And it reduces the amount of code you need to read to tell if an individual test will run or not.) |
Okay perhaps this idea then is not a good fit for the testthat ecosystem incl devtools, revdepcheck, and snapshots. What I find it appealing is no so much the amount of work, but that to me it makes more sense to reason about developer test and deployment tests, rather than special casing each test with skip-on-xyz. But it see that it complicates the notion of snapshots in particular. But perhaps it makes more sense to do this on the package level by passing a |
I'm willing to be persuaded, but I think there's quite a lot of work more to do, and I suspect there are easier ways to get what you want. |
A cleaner pattern to distinguish between public tests and developer tests.
Developer tests only run in CI or devtools, and can be used for tests that are not appropriate for public infrastructure such as CRAN or other downstream use, e.g. tests that require special tooling, are flaky, or consume much resources.