Merge pull request #315 from carpentries/fix-314

Ensure optional config.yaml parameter `fail_on_error` is `false` by default

DESCRIPTION

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

NEWS.md

@@ -1,3 +1,11 @@
+# sandpaper 0.7.1
+
+BUG FIX
+-------
+
+* A bug where `fail_on_error` defaulted to `true` has been fixed. This will
+  default to `false` if they key is not present in `config.yaml` (#314, @zkamvar).
+
 # sandpaper 0.7.0
 
 NEW FEATURE

R/build_markdown.R

@@ -93,10 +93,14 @@ build_markdown <- function(path = ".", rebuild = FALSE, quiet = FALSE, slug = NU
   if (any(needs_building)) {
     # Render the episode files to the built directory --------------------------
     renv_check_consent(path, quiet, sources)
+    # determine if we need to fail when errors are triggered
+    fail_on_error <- this_metadata$get()[["fail_on_error"]]
+    # this is `error` in the knitr sense of `error = TRUE` means 
+    # fail_on_error = FALSE.
+    error <- is.null(fail_on_error) || !fail_on_error
+    # exclude files that do not need to be rebuilt
     build_me <- db$build[needs_building]
     slugs    <- get_slug(build_me)
-    error    <- this_metadata$get()[["fail_on_error"]]
-    error    <- !is.null(error) && !error
     if (!error && !quiet) {
       cli::cli_alert_info("{.code fail_on_error: true}. Use {.code error=TRUE} in code chunks for demonstrative errors")
     }

R/utils-metadata.R

@@ -24,8 +24,8 @@ metadata_url <- function(cfg) {
 }
 
 initialise_metadata <- function(path = ".") {
+  cfg <- get_config(path)
   if (length(this_metadata$get()) == 0) {
-    cfg <- get_config(path)
     this_metadata$set(NULL, cfg)
     this_metadata$set("metadata_template", readLines(template_metadata()))
     this_metadata$set("pagetitle", cfg$title)
@@ -36,6 +36,8 @@ initialise_metadata <- function(path = ".") {
     # TODO: implement custom DESCRIPTION
     # For the Description, it would be good to take this from an ABOUT page
     # where the description paragraph can be found under the Description header
+  } else {
+    this_metadata$update(cfg)
   }
   this_metadata$set(c("date", "modified"), format(Sys.Date(), "%F"))
   this_metadata$set(c("date", "published"), format(Sys.Date(), "%F"))

tests/testthat/test-build_markdown.R

@@ -299,6 +299,8 @@ test_that("Removing partially matching slugs will not have side-effects", {
 })
 
 test_that("setting `fail_on_error: true` in config will cause build to fail", {
+  # fail_on_error is NULL by default
+  expect_null(this_metadata$get()[["fail_on_error"]])
   old_yaml <- withr::local_tempfile()
   old_episode <- withr::local_tempfile()
   suppressMessages(episode <- get_episodes(res, trim = FALSE)[[1]])
@@ -327,12 +329,14 @@ test_that("setting `fail_on_error: true` in config will cause build to fail", {
   # 
   # The first chunk is allowed to show the error in the document, the second
   # is not. When we check for the text of the second error, that confirms that
-  # the first error is passed over.
+  # the first error is passed over
   suppressMessages({
     out <- capture.output({
       build_markdown(res, quiet = FALSE) %>%
         expect_message("use error=TRUE") %>%
         expect_error("in the name of love")
     })
   })
+  # fail on error is true
+  expect_true(this_metadata$get()[["fail_on_error"]])
 })