From 37392145cfa170825c31cae601db05a369c1ba87 Mon Sep 17 00:00:00 2001
From: wlandau-lilly <will.landau@lilly.com>
Date: Wed, 15 May 2024 11:42:01 -0400
Subject: [PATCH] migrate testing

---
 R/make.R                        | 12 +---
 tests/testthat/helper-testrun.R |  6 +-
 tests/testthat/test-2-build.R   |  3 +-
 tests/testthat/test-2-lock.R    | 97 ---------------------------------
 4 files changed, 5 insertions(+), 113 deletions(-)
 delete mode 100644 tests/testthat/test-2-lock.R

diff --git a/R/make.R b/R/make.R
index 33d726d89..621f4a4ed 100644
--- a/R/make.R
+++ b/R/make.R
@@ -53,15 +53,7 @@
 #' For each target that is still problematic  (e.g.
 #' `https://github.com/rstudio/gt/issues/297`)
 #' you can safely run the command in its own special `callr::r()` process.
-#' Example: `https://github.com/rstudio/gt/issues/297#issuecomment-497778735`. # nolint
-#'
-#' If that fails, you can run `make(plan, lock_envir = FALSE)`
-#' to suppress environment-locking for all targets.
-#' However, this is not usually recommended.
-#' There are legitimate use cases for `lock_envir = FALSE`
-#' (example: `https://books.ropensci.org/drake/hpc.html#parallel-computing-within-targets`) # nolint
-#' but most workflows should stick with the default `lock_envir = TRUE`.
-#'
+#' Example: `https://github.com/rstudio/gt/issues/297#issuecomment-497778735`. # nolin
 #' @section Cache locking:
 #' When `make()` runs, it locks the cache so other processes cannot modify it.
 #' Same goes for [outdated()], [vis_drake_graph()], and similar functions
@@ -169,7 +161,7 @@ make <- function(
   memory_strategy = "speed",
   layout = NULL,
   spec = NULL,
-  lock_envir = TRUE,
+  lock_envir = NULL,
   history = TRUE,
   recover = FALSE,
   recoverable = TRUE,
diff --git a/tests/testthat/helper-testrun.R b/tests/testthat/helper-testrun.R
index 0764b3be8..f9890a0ff 100644
--- a/tests/testthat/helper-testrun.R
+++ b/tests/testthat/helper-testrun.R
@@ -16,8 +16,7 @@ testrun <- function(config) {
       lazy_load = config$settings$lazy_load,
       session_info = config$settings$session_info,
       fetch_cache = config$fetch_cache,
-      caching = config$caching,
-      lock_envir = !any(grepl("staged", config$settings$parallelism))
+      caching = config$caching
     )
   )
 }
@@ -38,8 +37,7 @@ testconfig <- function(config) {
     lazy_load = config$settings$lazy_load,
     session_info = config$settings$session_info,
     fetch_cache = config$fetch_cache,
-    caching = config$caching,
-    lock_envir = !any(grepl("staged", config$settings$parallelism))
+    caching = config$caching
   )
   out$plan <- config$plan
   out$targets <- config$targets
diff --git a/tests/testthat/test-2-build.R b/tests/testthat/test-2-build.R
index 2881c435b..8bdf20ca1 100644
--- a/tests/testthat/test-2-build.R
+++ b/tests/testthat/test-2-build.R
@@ -8,8 +8,7 @@ test_with_dir("drake_build() works as expected", {
   con <- drake_config(
     plan = pl,
     session_info = FALSE,
-    envir = e,
-    lock_envir = TRUE
+    envir = e
   )
   # can run before any make()
   o <- drake_build_impl(
diff --git a/tests/testthat/test-2-lock.R b/tests/testthat/test-2-lock.R
deleted file mode 100644
index 6faf6dcc3..000000000
--- a/tests/testthat/test-2-lock.R
+++ /dev/null
@@ -1,97 +0,0 @@
-drake_context("lock")
-
-test_with_dir("lock_environment()", {
-  skip_on_cran()
-  skip_if_not_installed("tibble")
-  scenario <- get_testing_scenario()
-  e <- eval(parse(text = scenario$envir))
-  jobs <- scenario$jobs
-  parallelism <- scenario$parallelism
-  caching <- scenario$caching
-  plan <- drake_plan(
-    x = try(
-      assign("a", 1L, envir = parent.env(drake_envir("targets"))),
-      silent = TRUE
-    )
-  )
-  make(
-    plan,
-    envir = e,
-    jobs = jobs,
-    parallelism = parallelism,
-    caching = caching,
-    lock_envir = TRUE,
-    verbose = 1L,
-    session_info = FALSE
-  )
-  expect_true(inherits(readd(x), "try-error"))
-  e$a <- 123
-  e$plan$four <- "five"
-  plan <- drake_plan(
-    x = assign("a", 1, envir = drake_envir("targets"))
-  )
-  make(
-    plan,
-    envir = e,
-    jobs = jobs,
-    parallelism = parallelism,
-    caching = caching,
-    lock_envir = FALSE,
-    verbose = 0L,
-    session_info = FALSE
-  )
-  expect_true("x" %in% cached())
-  expect_equal(readd(x), 1L)
-})
-
-test_with_dir("Try to modify a locked environment", {
-  skip_on_cran()
-  e <- new.env()
-  lock_environment(e)
-  plan <- drake_plan(x = {
-    e$a <- 1
-    2
-  })
-  expect_error(
-    make(plan, session_info = FALSE, cache = storr::storr_environment()),
-    regexp = "Self-invalidation"
-  )
-})
-
-test_with_dir("unlock_environment()", {
-  skip_on_cran()
-  expect_error(
-    unlock_environment(NULL),
-    regexp = "use of NULL environment is defunct"
-  )
-  expect_error(
-    unlock_environment("x"),
-    regexp = "not an environment"
-  )
-  e <- new.env(parent = emptyenv())
-  e$y <- 1
-  expect_false(environmentIsLocked(e))
-  assign(x = ".x", value = "x", envir = e)
-  expect_equal(get(x = ".x", envir = e), "x")
-  lock_environment(e)
-  msg1 <- "cannot change value of locked binding"
-  msg2 <- "cannot add bindings to a locked environment"
-  expect_true(environmentIsLocked(e))
-  assign(x = ".x", value = "y", envir = e)
-  expect_equal(get(x = ".x", envir = e), "y")
-  expect_error(assign(x = "y", value = "y", envir = e), regexp = msg1)
-  expect_error(assign(x = "a", value = "x", envir = e), regexp = msg2)
-  expect_error(assign(x = "b", value = "y", envir = e), regexp = msg2)
-  unlock_environment(e)
-  assign(x = ".x", value = "1", envir = e)
-  assign(x = "y", value = "2", envir = e)
-  assign(x = "a", value = "x", envir = e)
-  expect_equal(get(x = ".x", envir = e), "1")
-  expect_equal(get(x = "y", envir = e), "2")
-  expect_equal(get(x = "a", envir = e), "x")
-  expect_false(environmentIsLocked(e))
-  unlock_environment(e)
-  assign(x = "b", value = "y", envir = e)
-  expect_equal(get(x = "b", envir = e), "y")
-  expect_false(environmentIsLocked(e))
-})