Merge pull request #387 from carpentries/always-provision-setup

always provision setup

NEWS.md

@@ -1,5 +1,12 @@
 # sandpaper 0.11.4
 
+BUG FIX
+-------
+
+* The setup page will always be provisioned for lessons regardless if it
+  exists in the `learners:` field in `config.yaml`
+  (reported: @zkamvar, #386; fixed: @zkamvar #387).
+
 PANDOC
 ------
 

R/build_markdown.R

@@ -3,17 +3,17 @@
 #' In the spirit of {hugodown}, This function will build plain markdown files
 #' as a minimal R package in the `site/` folder of your {sandpaper} lesson
 #' repository tagged with the hash of your file to ensure that only files that
-#' have changed are rebuilt. 
-#' 
+#' have changed are rebuilt.
+#'
 #' @param path the path to your repository (defaults to your current working
 #' directory)
 #' @param rebuild if `TRUE`, everything will be built from scratch as if there
 #' was no cache. Defaults to `FALSE`, which will only build markdown files that
-#' haven't been built before. 
-#' 
+#' haven't been built before.
+#'
 #' @return `TRUE` if it was successful, a character vector of issues if it was
 #'   unsuccessful.
-#' 
+#'
 #' @keywords internal
 #' @seealso [build_episode_md()]
 build_markdown <- function(path = ".", rebuild = FALSE, quiet = FALSE, slug = NULL) {
@@ -50,7 +50,7 @@ build_markdown <- function(path = ".", rebuild = FALSE, quiet = FALSE, slug = NU
   }
 
   # If the user accidentally used rmarkdown::render(), then they would end up
-  # with an html artifact in here and it will clog up the machinery. Best to 
+  # with an html artifact in here and it will clog up the machinery. Best to
   # remove it at the source.
   remove_rendered_html(sources)
 
@@ -66,7 +66,7 @@ build_markdown <- function(path = ".", rebuild = FALSE, quiet = FALSE, slug = NU
   # Copy the files to the assets directory -------------------------------------
   artifacts <- get_artifacts(path, "episodes")
   to_copy <- vapply(
-    c("data", "files", "fig"), 
+    c("data", "files", "fig"),
     FUN = function(i) enforce_dir(fs::path(episode_path, i)),
     FUN.VALUE = character(1)
   )
@@ -95,7 +95,7 @@ build_markdown <- function(path = ".", rebuild = FALSE, quiet = FALSE, slug = NU
     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 
+    # 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

R/test-fixtures.R

@@ -20,15 +20,16 @@
 #' @keywords internal
 #' @rdname fixtures
 create_test_lesson <- function() {
-  if (interactive()) {
+  noise <- interactive() || Sys.getenv("CI") == "true"
+  if (noise) {
     cli::cli_status("{cli::symbol$arrow_right} Bootstrapping example lesson")
   }
   # We explicitly need the package cache for tests
   options("sandpaper.use_renv" = renv_is_allowed())
   repodir <- fs::file_temp()
   fs::dir_create(repodir)
   repo <- fs::path(repodir, "lesson-example")
-  if (interactive()) {
+  if (noise) {
     cli::cli_status_update(
       "{cli::symbol$arrow_right} Bootstrapping example lesson in {repo}"
     )

R/utils-built-db.R

@@ -15,11 +15,11 @@ get_hash <- function(path, db = fs::path(path_built(path), "md5sum.txt")) {
 #' Get the database of built files and their hashes
 #'
 #' @param db the path to the database.
-#' @param filter regex describing files to include. 
+#' @param filter regex describing files to include.
 #' @return a data frame with three columns:
 #'   - file: the path to the source file
 #'   - checksum: the hash of the source file to generate the built file
-#'   - built: the path to the built file 
+#'   - built: the path to the built file
 #' @keywords internal
 #' @seealso [build_status()], [get_hash()]
 get_built_db <- function(db = "site/built/md5sum.txt", filter = "*R?md") {
@@ -34,14 +34,44 @@ get_built_db <- function(db = "site/built/md5sum.txt", filter = "*R?md") {
   return(files[are_markdown, , drop = FALSE])
 }
 
-#' Filter reserved files from the built db
+#' Filter reserved markdown files from the built db
+#'
+#' This provides a service for `build_site()` so that it does not build files
+#' that are used for aggregation, resource provision, or GitHub specific files
+#'
+#' @details
+#'
+#' There are three types of files that are reserved and we do not want to
+#' propogate to the HTML site
+#'
+#' ## GitHub specific files
+#'
+#' These are the README and CONTRIBUTING files. Both of these files provide
+#' information that is useful only in the context of GitHub
+#'
+#' ## Aggregation files
+#'
+#' These are files that are aggregated together with other files or have
+#' content appended to them:
+#'
+#'  - `index` and `learners/setup` are concatenated
+#'  - all markdown files in `profiles/` are concatenated
+#'  - `instructors/instructor-notes` have the inline instructor notes
+#'     concatenated.
+#'
+#' ## Resource provision files
+#'
+#' At the moment, there is one file that we use for resource provision and
+#' should not be propogated to the site: `links`. This provides global links
+#' for the lesson. It provides no content in and of itself.
 #'
 #' @param db the database from [get_built_db()]
-#' @return a data frame, but a bit shorter
+#' @return the same database with the above files filtered out
 #' @keywords internal
-#' @seealso [get_built_db()]
+#' @seealso [get_built_db()] that provides the database and [build_site()],
+#'   which uses the function
 reserved_db <- function(db) {
-  reserved <- c("index", "README", "CONTRIBUTING", "learners/setup", 
+  reserved <- c("index", "README", "CONTRIBUTING", "learners/setup",
     "profiles[/].*", "instructors[/]instructor-notes[.]*", "links")
   reserved <- paste(reserved, collapse = "|")
   reserved <- paste0("^(", reserved, ")[.]R?md")
@@ -53,7 +83,7 @@ write_build_db <- function(md5, db) write.table(md5, db, row.names = FALSE)
 #' Identify what files need to be rebuilt and what need to be removed
 #'
 #' This takes in a vector of files and compares them against a text database of
-#' files with checksums. It's been heavily adapted from blogdown to provide 
+#' files with checksums. It's been heavily adapted from blogdown to provide
 #' utilities for removal and updating of the old database.
 #'
 #' @details
@@ -72,10 +102,10 @@ write_build_db <- function(md5, db) write.table(md5, db, row.names = FALSE)
 #' @param rebuild if the files should be rebuilt, set this to TRUE (defaults to
 #'   FALSE)
 #' @param write if TRUE, the database will be updated, Defaults to FALSE,
-#' meaning that the database will remain the same. 
+#' meaning that the database will remain the same.
 #' @return a list of the following elements
 #'   - *build* absolute paths of files to build
-#'   - *new* a new data frame with three columns: 
+#'   - *new* a new data frame with three columns:
 #'      - file the relative path to the source file
 #'      - checksum the md5 sum of the source file
 #'      - built the relative path to the built file
@@ -166,7 +196,7 @@ build_status <- function(sources, db = "site/built/md5sum.txt", rebuild = FALSE,
   # also remove the files that no longer exist in the sources list.
   one <- one[match(sources, one$file), , drop = FALSE]
   # TODO: see if we can have rebuild be a vector matching the sources so that
-  #       we can indicate a vector of files to rebuild. 
+  #       we can indicate a vector of files to rebuild.
   if (rebuild) {
     files = one[['file']]
     to_remove <- old[['built']]

R/utils-paths-source.R

@@ -25,6 +25,17 @@ path_profiles <- function(inpath) {
   fs::path(home, "profiles")
 }
 
+
+provide_setup <- function(cfg, setup = "setup.md") {
+  if (is.null(cfg) || is.null(cfg$learners)) {
+    return(cfg)
+  }
+  if (get_slug(setup) %nin% get_slug(cfg$learners)) {
+    cfg$learners <- c(cfg$learners, setup)
+  }
+  return(cfg)
+}
+
 #' Get the full resource list of markdown files
 #'
 #' @param path path to the lesson
@@ -114,6 +125,9 @@ get_resource_list <- function(path, trim = FALSE, subfolder = NULL, warn = FALSE
   if (use_subfolder) {
     names(res) <- subfolder
   } else {
+    lrn <- res[["learners"]]
+    stp <- fs::path_file(lrn[get_slug(lrn) %in% "setup"])
+    cfg <- provide_setup(cfg, stp)
     subfolder <- c("episodes", "learners", "instructors", "profiles")
   }
 

README.Rmd

@@ -16,7 +16,6 @@ knitr::opts_chunk$set(
 
 <!-- badges: start -->
 [![R Universe](https://carpentries.r-universe.dev/badges/sandpaper)](https://carpentries.r-universe.dev/ui#builds)
-[![R build status](https://github.com/carpentries/sandpaper/workflows/R-CMD-check/badge.svg)](https://github.com/carpentries/sandpaper/actions)
 [![Codecov test coverage](https://codecov.io/gh/carpentries/sandpaper/branch/main/graph/badge.svg)](https://codecov.io/gh/carpentries/sandpaper?branch=main)
 [![Lifecycle: experimental](https://img.shields.io/badge/lifecycle-experimental-orange.svg)](https://www.tidyverse.org/lifecycle/#experimental)
 [![CRAN status](https://www.r-pkg.org/badges/version/sandpaper)](https://CRAN.R-project.org/package=sandpaper)

README.md

@@ -6,8 +6,6 @@
 
 [![R
 Universe](https://carpentries.r-universe.dev/badges/sandpaper)](https://carpentries.r-universe.dev/ui#builds)
-[![R build
-status](https://github.com/carpentries/sandpaper/workflows/R-CMD-check/badge.svg)](https://github.com/carpentries/sandpaper/actions)
 [![Codecov test
 coverage](https://codecov.io/gh/carpentries/sandpaper/branch/main/graph/badge.svg)](https://codecov.io/gh/carpentries/sandpaper?branch=main)
 [![Lifecycle:

tests/testthat/examples/s3.Rmd

@@ -12,7 +12,7 @@ this is an instructor note
 :::
 
 ```{r}
-#| name: example
+#| label: example-chunk
 point <- function(x, y) {
   stopifnot(is.numeric(x), is.numeric(y))
   structure(list(x = x, y = y), class = "point")

tests/testthat/helper-processing.R

@@ -0,0 +1,5 @@
+# This helper allows me to properly check the output of knitr without running
+# afoul of the weird escapes it has
+processing_ <- function (file) {
+  sprintf("processing file: .+?%s", sub("\\.", "\\\\.", basename(file)))
+}

tests/testthat/setup.R

@@ -6,7 +6,8 @@
   rmt <- fs::file_temp(pattern = "REMOTE-")
   setup_local_remote(repo = tmp, remote = rmt, verbose = FALSE)
 
-  if (interactive()) {
+  noise <- interactive() || Sys.getenv("CI") == "true"
+  if (noise) {
     cli::cli_alert_info("Current RENV_PATHS_ROOT {Sys.getenv('RENV_PATHS_ROOT')}")
     cli::cli_alert_info("Current renv::paths$root() {renv::paths$root()}")
     cli::cli_alert_info(
@@ -23,7 +24,8 @@ withr::defer({
   rem <- remove_local_remote(repo = tf)
   # remove the test fixture and report
   res <- tryCatch(fs::dir_delete(tf), error = function() FALSE)
-  if (interactive()) {
+  noise <- interactive() || Sys.getenv("CI") == "true"
+  if (noise) {
     status <- if (identical(res, FALSE)) "could not be" else "successfully"
     cli::cli_alert_info("{.file {tf}} {status} removed")
     status <- if (is.character(rem)) "successfully" else "could not be"

tests/testthat/test-build_episode.R

@@ -1,7 +1,6 @@
 res <- tmp <- restore_fixture()
 
 
-
 test_that("build_episode_html() returns nothing for an empty page", {
   skip_if_not(rmarkdown::pandoc_available("2.11"))
   tmp <- fs::path(res, "DELETEME")
@@ -16,7 +15,7 @@ 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))
-  
+
 
   skip_if_not(rmarkdown::pandoc_available("2.11"))
   # create a new file in extras
@@ -27,24 +26,24 @@ test_that("build_episode functions works independently", {
   skip_on_os("windows")
   expect_output({
     res <- build_episode_md(fun_file, workdir = dirname(fun_file))
-  }, "inline R code fragments")
+  }, processing_(fun_file))
 
   expect_equal(basename(res), "fun.md")
   expect_true(file.exists(file.path(tmp, "site", "built", "fun.md")))
   lines <- readLines(res)
   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. 
+  # 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")))
   expect_output({
-    build_episode_html(res, 
+    build_episode_html(res,
       fun_file,
       page_back = fun_file,
       page_forward = fun_file,
@@ -73,20 +72,20 @@ test_that("the chapter-links should be cromulent depending on the view", {
   expect_length(learn_links, 4L)
   expect_equal(xml2::xml_attr(learn_links, "href"), rep("fun.html", 4L))
 
-  learn_note<- xml2::xml_find_all(learn, 
+  learn_note<- xml2::xml_find_all(learn,
     ".//div[starts-with(@id, 'accordionInstructor')]")
   expect_length(learn_note, 0L)
 
   instruct_links <- xml2::xml_find_all(instruct, ".//a[@class='chapter-link']")
   expect_length(instruct_links, 4L)
-  expect_equal(xml2::xml_attr(instruct_links, "href"), 
+  expect_equal(xml2::xml_attr(instruct_links, "href"),
     rep("../instructor/fun.html", 4L))
 
   internal_instruct_links <- xml2::xml_find_all(instruct, ".//a[contains(text(), 'internal')]")
   expect_length(internal_instruct_links, 1L)
   expect_equal(xml2::xml_attr(internal_instruct_links, "href"), "fun.html")
 
-  instruct_note <- xml2::xml_find_all(instruct, 
+  instruct_note <- xml2::xml_find_all(instruct,
     ".//div[starts-with(@id, 'accordionInstructor')]")
   expect_length(instruct_note, 1L)
   IN_lines <- trimws(xml2::xml_text(instruct_note))