Skip to content

Commit

Permalink
Make tests more isolated. (#133)
Browse files Browse the repository at this point in the history 
Some tests were calling `orderly_run` without an `envir` parameter,
which ends up running the report in the global environment. This
pollutes that namespace and might affect subsequent tests.

One particular issue I was hitting was a test depending on the global
`data` name from `utils`, while other tests were defining a global
variable with that (unfortunately very common) name.

I've added a `envir = new.env()` parameter to all the `orderly_run`
calls I could find, and changed the test that depended on `data` to use
the fully qualified name `utils::base`.

Running the whole test suite now leaves an unmodified global
environment:

```r
ls()
# character(0)
devtools::test()
ls()
# character(0)
```
  • Loading branch information
@plietar
plietar authored Mar 20, 2024
1 parent ef4a592 commit 9222c8f
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 24 deletions.
2 changes: 1 addition & 1 deletion tests/testthat/test-cleanup.R
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ test_that("Don't call cleanup on an active packet", {
append_lines(file.path(path_src, "data.R"),
"orderly2::orderly_cleanup_status()")
expect_error(
orderly_run_quietly("data", root = path),
orderly_run_quietly("data", root = path, envir = new.env()),
"Don't call 'orderly2::orderly_cleanup_status()' from a running packet",
fixed = TRUE)
})
Expand Down
6 changes: 4 additions & 2 deletions tests/testthat/test-init.R
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ test_that("can initialise a repo with orderly but no .outpack directory", {
base <- basename(path)
unlink(file.path(path, ".outpack"), recursive = TRUE)
err <- expect_error(
withr::with_dir(parent, orderly_run_quietly("data", root = base)),
withr::with_dir(parent,
orderly_run_quietly("data", root = base, envir = new.env())),
sprintf("orderly directory '%s' not initialise", base))
expect_equal(
err$body,
Expand All @@ -60,7 +61,8 @@ test_that("can initialise a repo with orderly but no .outpack directory", {
root <- root_open(path, FALSE, TRUE)
expect_true(is_directory(file.path(path, ".outpack")))

id <- withr::with_dir(parent, orderly_run_quietly("data", root = base))
id <- withr::with_dir(parent,
orderly_run_quietly("data", root = base, envir = new.env()))
expect_type(id, "character")
})

Expand Down
7 changes: 4 additions & 3 deletions tests/testthat/test-parameters.R
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,15 @@ test_that("parameters must be atomic scalars", {
"x" = "Values must be character, numeric or boolean, but were not for:",
"*" = "a"))
err <- expect_error(
check_parameters(list(a = data, b = 2), list(a = NULL, b = NULL)),
check_parameters(list(a = utils::data, b = 2), list(a = NULL, b = NULL)),
"Invalid parameter value\\b")
expect_equal(
err$body,
c("x" = "Values must be character, numeric or boolean, but were not for:",
"*" = "a"))
err <- expect_error(
check_parameters(list(a = data, b = 2 + 1i), list(a = NULL, b = NULL)),
check_parameters(list(a = utils::data, b = 2 + 1i),
list(a = NULL, b = NULL)),
"Invalid parameter values\\b")
expect_equal(
err$body,
Expand All @@ -177,7 +178,7 @@ test_that("defaults must be valid", {
expect_equal(err$body, c("x" = "Values must be scalar, but were not for:",
"*" = "a"))
err <- expect_error(
static_orderly_parameters(list(a = data)),
static_orderly_parameters(list(a = utils::data)),
"Invalid parameter default")
expect_equal(
err$body,
Expand Down
42 changes: 24 additions & 18 deletions tests/testthat/test-run.R
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,8 @@ test_that("with strict mode, only declared files are copied, running fails", {
writeLines(c("orderly2::orderly_strict_mode()", code), path_src)
err <- suppressWarnings(tryCatch(read.csv("data.csv"), error = identity))
expect_error(
suppressWarnings(orderly_run_quietly("implicit", root = path)),
suppressWarnings(
orderly_run_quietly("implicit", root = path, envir = new.env())),
err$message,
fixed = TRUE)
})
Expand All @@ -390,7 +391,7 @@ test_that("with strict mode, indicate unknown files as potential artefacts", {
code),
path_src)
res <- testthat::evaluate_promise(
id <- orderly_run("implicit", root = path))
id <- orderly_run("implicit", root = path, envir = new.env()))
expect_match(res$messages,
"orderly produced unexpected files:\n - mygraph.png",
all = FALSE)
Expand All @@ -404,7 +405,7 @@ test_that("without strict mode, detect modified files", {
path <- test_prepare_orderly_example("implicit")
file.create(file.path(path, "src", "implicit", "mygraph.png"))
res <- testthat::evaluate_promise(
id <- orderly_run("implicit", root = path))
id <- orderly_run("implicit", root = path, envir = new.env()))
expect_match(
res$messages,
"inputs modified; these are probably artefacts:\n - mygraph.png",
Expand All @@ -425,15 +426,15 @@ test_that("disallow multiple calls to strict mode", {
code),
path_src)
expect_error(
orderly_run_quietly("implicit", root = path),
orderly_run_quietly("implicit", root = path, envir = new.env()),
"orderly function 'orderly_strict_mode' can only be used at the top level")

writeLines(c("orderly2::orderly_strict_mode()",
"orderly2::orderly_strict_mode()",
code),
path_src)
expect_error(
orderly_run_quietly("implicit", root = path),
orderly_run_quietly("implicit", root = path, envir = new.env()),
"Only one call to 'orderly2::orderly_strict_mode' is allowed")
})

Expand Down Expand Up @@ -573,9 +574,9 @@ test_that("don't copy artefacts over when not needed", {
test_that("can pull resources programmatically", {
path <- test_prepare_orderly_example("programmatic-resource")
id1 <- orderly_run_quietly("programmatic-resource", list(use = "a"),
root = path)
root = path, envir = new.env())
id2 <- orderly_run_quietly("programmatic-resource", list(use = "b"),
root = path)
root = path, envir = new.env())
meta1 <- orderly_metadata(id1, root = path)
meta2 <- orderly_metadata(id2, root = path)

Expand All @@ -598,9 +599,9 @@ test_that("can pull resources programmatically, strictly", {
"programmatic-resource.R")
prepend_lines(path_src, "orderly2::orderly_strict_mode()")
id1 <- orderly_run_quietly("programmatic-resource", list(use = "a"),
root = path)
root = path, envir = new.env())
id2 <- orderly_run_quietly("programmatic-resource", list(use = "b"),
root = path)
root = path, envir = new.env())
meta1 <- meta <- orderly_metadata(id1, root = path)
meta2 <- meta <- orderly_metadata(id2, root = path)

Expand Down Expand Up @@ -697,7 +698,9 @@ test_that("Can select location when querying dependencies for a report", {
for (nm in c("us", "prod", "dev")) {
path[[nm]] <- test_prepare_orderly_example(c("data", "depends"))
if (nm != "us") {
ids[[nm]] <- orderly_run_quietly("data", root = path[[nm]])
ids[[nm]] <- orderly_run_quietly("data", root = path[[nm]],
envir = new.env())

orderly_location_add(nm, "path", list(path = path[[nm]]),
root = path[["us"]])
orderly_location_pull_metadata(nm, root = path[["us"]])
Expand All @@ -707,24 +710,27 @@ test_that("Can select location when querying dependencies for a report", {
}
}
## Run extra local copy - this is the most recent.
ids[["us"]] <- orderly_run_quietly("data", root = path[["us"]])
ids[["us"]] <- orderly_run_quietly("data", root = path[["us"]],
envir = new.env())

## Without locations we prefer the local one:
id1 <- orderly_run_quietly("depends", root = path[["us"]])
id1 <- orderly_run_quietly("depends", root = path[["us"]], envir = new.env())
expect_equal(orderly_metadata(id1, path[["us"]])$depends$packet,
ids[["us"]])

## Filter to only allow prod:
id2 <- orderly_run_quietly("depends",
search_options = list(location = "prod"),
root = path[["us"]])
root = path[["us"]],
envir = new.env())
expect_equal(orderly_metadata(id2, path[["us"]])$depends$packet,
ids[["prod"]])

## Allow any location:
id3 <- orderly_run_quietly("depends",
search_options = list(location = c("prod", "dev")),
root = path[["us"]])
root = path[["us"]],
envir = new.env())
expect_equal(orderly_metadata(id3, path[["us"]])$depends$packet,
ids[["dev"]])
})
Expand Down Expand Up @@ -1070,7 +1076,7 @@ test_that("can use quote for queries queries", {
path_src <- file.path(path, "src", "depends", "depends.R")
src <- readLines(path_src)
writeLines(sub('"latest"', "quote(latest())", src, fixed = TRUE), path_src)
id2 <- orderly_run_quietly("depends", root = path)
id2 <- orderly_run_quietly("depends", root = path, envir = new.env())
expect_equal(orderly_metadata(id2, root = path)$depends$packet, id1[[2]])
})

Expand Down Expand Up @@ -1189,7 +1195,7 @@ test_that("can read about assigned resources", {
code <- c(code, 'writeLines(r, "resources.txt")')
writeLines(code, file.path(path_src, "directories.R"))

id <- orderly_run_quietly("directories", root = path)
id <- orderly_run_quietly("directories", root = path, envir = new.env())
expect_setequal(
readLines(file.path(path, "archive", "directories", id, "resources.txt")),
c("data/a.csv", "data/b.csv"))
Expand Down Expand Up @@ -1220,7 +1226,7 @@ test_that("can read about assigned shared resources", {
code <- c(code, 'saveRDS(r, "resources.rds")')
writeLines(code, file.path(path_src, "shared-dir.R"))

id <- orderly_run_quietly("shared-dir", root = path)
id <- orderly_run_quietly("shared-dir", root = path, envir = new.env())
r <- readRDS(file.path(path, "archive", "shared-dir", id, "resources.rds"))
expect_equal(
r,
Expand Down Expand Up @@ -1250,7 +1256,7 @@ test_that("can read about dependencies", {
code <- c(code, 'saveRDS(r, "depends.rds")')
writeLines(code, file.path(path_src, "depends.R"))

id2 <- orderly_run_quietly("depends", root = path)
id2 <- orderly_run_quietly("depends", root = path, envir = new.env())
r <- readRDS(file.path(path, "archive", "depends", id2, "depends.rds"))
expect_equal(r$id, id1)
expect_equal(r$name, "data")
Expand Down

0 comments on commit 9222c8f

Please sign in to comment.