Merge pull request #289 from carpentries/fix-288

use `globalenv()` for `build_episode_md()`

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: sandpaper
 Title: Create and Curate Carpentries Lessons
-Version: 0.5.3
+Version: 0.5.4
 Authors@R: c(
     person(given = "Zhian N.",
            family = "Kamvar",

NEWS.md

@@ -1,3 +1,11 @@
+# sandpaper 0.5.4
+
+BUG FIX
+-------
+
+* `build_episode_md()` argument `workenv` now defaults to `globalenv()` to avoid
+  S3 dispatch issues that can occur in `new.env()` (see https://github.com/carpentries/sandpaper/issues/288)
+
 # sandpaper 0.5.3
 
 BUG FIX

R/build_episode.R

@@ -178,7 +178,8 @@ get_nav_data <- function(path_md, path_src = NULL, home = NULL,
 #'   is effectively useless.
 #' @param outdir the directory to write to
 #' @param workdir the directory where the episode should be rendered
-#' @param env a blank environment
+#' @param workenv an environment to use for evaluation. Defaults to the global
+#'   environment, which evaluates to the environment from [callr::r()]. 
 #' @param quiet if `TRUE`, output is suppressed, default is `FALSE` to show 
 #'   {knitr} output.
 #' @return the path to the output, invisibly
@@ -209,7 +210,7 @@ get_nav_data <- function(path_md, path_src = NULL, home = NULL,
 #' res <- build_episode_md(fun_file, outdir = fun_dir, workdir = fun_dir)
 build_episode_md <- function(path, hash = NULL, outdir = path_built(path), 
                              workdir = path_built(path), 
-                             workenv = new.env(), profile = "lesson-requirements", quiet = FALSE) {
+                             workenv = globalenv(), profile = "lesson-requirements", quiet = FALSE) {
 
   # define the output
   md <- fs::path_ext_set(fs::path_file(path), "md")

tests/testthat/examples/s3.Rmd

@@ -0,0 +1,29 @@
+---
+title: Fun times
+---
+
+
+# new page
+
+This is coming from `r R.version.string` with an [internal link](fun.Rmd)
+
+::: instructor
+this is an instructor note
+:::
+
+```{r}
+#| name: example
+point <- function(x, y) {
+  stopifnot(is.numeric(x), is.numeric(y))
+  structure(list(x = x, y = y), class = "point")
+}
+
+abs.point <- function(x) {
+  sqrt(x$x ^ 2 + x$y ^ 2)
+}
+
+points <- mapply(point, runif(5), runif(5), SIMPLIFY = FALSE)
+
+sapply(points, abs)
+```
+

tests/testthat/test-build_episode.R

@@ -13,7 +13,6 @@ test_that("build_episode_html() returns nothing for an empty page", {
 
 test_that("build_episode functions works independently", {
 
-
   withr::local_options(list(sandpaper.use_renv = FALSE))
   pkg <- pkgdown::as_pkgdown(file.path(tmp, "site"))
   expect_output(pkgdown::init_site(pkg))
@@ -22,19 +21,10 @@ test_that("build_episode functions works independently", {
   skip_if_not(rmarkdown::pandoc_available("2.11"))
   # create a new file in extras
   fun_file <- file.path(tmp, "episodes", "files", "fun.Rmd")
-  txt <- c(
-    "---\ntitle: Fun times\n---\n\n",
-    "# new page\n", 
-    "This is coming from `r R.version.string` with an [internal link](fun.Rmd)\n",
-    "::: instructor",
-    "this is an instructor note",
-    ":::"
-  )
-  file.create(fun_file)
-  writeLines(txt, fun_file)
+  file.copy(test_path("examples/s3.Rmd"), fun_file, overwrite = TRUE)
+  expect_true(fs::file_exists(fun_file))
 
   skip_on_os("windows")
-  manage_deps(tmp)
   expect_output({
     res <- build_episode_md(fun_file, workdir = dirname(fun_file))
   }, "inline R code fragments")
@@ -45,6 +35,11 @@ test_that("build_episode functions works independently", {
   expect_equal(lines[[2]], "title: Fun times")
   from_r <- grep("This is coming from", lines)
   expect_match(lines[from_r], "This is coming from R (version|Under)")
+
+  # Explicitly testing https://github.com/carpentries/sandpaper/issues/288
+  # If we specify a `new.env()`, then the S3 dispatch will not work, but when we
+  # default to `globalenv()`, the S3 dispatch works. 
+  expect_false(any(grepl("Error", lines)))
 
   expect_false(file.exists(file.path(tmp, "site", "docs", "fun.html")))
   expect_false(file.exists(file.path(tmp, "site", "docs", "instructor", "fun.html")))

tests/testthat/test-build_lesson.R

@@ -25,7 +25,12 @@ test_that("Lessons built for the first time are noisy", {
 
 })
 
-htmls <- read_all_html(sitepath)
+
+htmls <- NULL
+if (rmarkdown::pandoc_available("2.11")) {
+  htmls <- read_all_html(sitepath)
+}
+
 pkg <- pkgdown::as_pkgdown(fs::path_dir(sitepath))
 
 test_that("build_lesson() also builds the extra pages", {