Merge pull request #511 from TymekDev/main

Add --no-update flag to spell-check

Author: lorenzwalthert

R/testing.R

@@ -86,8 +86,9 @@ run_test <- function(hook_name,
 #' @param expect_success Whether or not an exit code 0 is expected. This can
 #'   be derived from `std_err`, but sometimes, non-empty stderr does not mean
 #'   error, but just a message.
-#' @param read_only If `TRUE` and `artifacts` are not `NULL`, then assert that hook
-#'   did not modify the artifacts.
+#' @param read_only If `TRUE`, then assert that no new files were created.
+#'   Additionally, if `artifacts` are not `NULL`, then assert that hook did not
+#'   modify the artifacts.
 #' @keywords internal
 run_test_impl <- function(path_executable,
                           path_candidate,
@@ -112,6 +113,7 @@ run_test_impl <- function(path_executable,
   )
   path_stderr <- tempfile()
   path_stdout <- tempfile()
+  files_before_hook <- fs::dir_ls(tempdir, all = TRUE, recurse = TRUE)
   exit_status <- hook_state_create(
     tempdir,
     path_candidate_temp,
@@ -133,12 +135,17 @@ run_test_impl <- function(path_executable,
     std_out,
     exit_status
   )
-  if (isTRUE(read_only) && !is.null(artifacts)) {
-    purrr::iwalk(artifacts, function(reference_path, temp_path) {
-      artifact_before_hook <- readLines(testthat::test_path(reference_path))
-      artifact_after_hook <- readLines(fs::path_join(c(tempdir, temp_path)))
-      testthat::expect_equal(artifact_before_hook, artifact_after_hook)
-    })
+  if (isTRUE(read_only)) {
+    files_after_hook <- fs::dir_ls(tempdir, all = TRUE, recurse = TRUE)
+    testthat::expect_equal(files_before_hook, files_after_hook)
+
+    if (!is.null(artifacts)) {
+      purrr::iwalk(artifacts, function(reference_path, temp_path) {
+        artifact_before_hook <- readLines(reference_path)
+        artifact_after_hook <- readLines(fs::path_join(c(tempdir, temp_path)))
+        testthat::expect_equal(artifact_before_hook, artifact_after_hook)
+      })
+    }
   }
 }
 

inst/hooks/exported/spell-check.R

@@ -2,10 +2,11 @@
 
 "Spell check for files
 Usage:
-  spell-check [--lang=<language>] <files>...
+  spell-check [--lang=<language>] [--read-only] <files>...
 
 Options:
   --lang=<language> Passed to `spelling::spell_check_files()` [default: en_US]
+  --read-only       Don't update inst/WORDLIST with errors (new words)
 
 " -> doc
 
@@ -16,10 +17,12 @@ if (file.exists(path_wordlist)) {
   ignore <- readLines(path_wordlist, encoding = "UTF-8")
   action <- "update"
 } else {
-  if (!dir.exists(dirname(path_wordlist))) {
-    dir.create(dirname(path_wordlist))
+  if (isFALSE(arguments$read_only)) {
+    if (!dir.exists(dirname(path_wordlist))) {
+      dir.create(dirname(path_wordlist))
+    }
+    file.create(path_wordlist)
   }
-  file.create(path_wordlist)
   ignore <- character()
   action <- "create"
 }
@@ -34,24 +37,26 @@ spelling_errors <- spelling::spell_check_files(
 if (nrow(spelling_errors) > 0) {
   cat("The following spelling errors were found:\n")
   print(spelling_errors)
-  ignore_df <- data.frame(
-    original = unique(c(ignore, spelling_errors$word))
-  )
-  ignore_df$lower <- tolower(ignore_df$original)
-  ignore_df <- ignore_df[order(ignore_df$lower, method = "radix"), ]
-  ignore <- ignore_df$original[ignore_df$lower != ""] # drop blanks if any
-  writeLines(ignore, path_wordlist)
-  cat(
-    "All spelling errors found were copied to inst/WORDLIST assuming they were",
-    "not spelling errors and will be ignored in the future. Please ",
-    "review the above list and for each word that is an actual typo:\n",
-    "- fix it in the source code.\n",
-    "- remove it again manually from inst/WORDLIST to make sure it's not\n",
-    "  ignored in the future.\n",
-    "Then, try committing again.\n"
-  )
+  if (isTRUE(arguments$read_only)) {
+    cat("Hint: you can enable an automatic updating of WORDLIST with errors (new words) by removing the --read-only flag.\n")
+  } else {
+    ignore_df <- data.frame(
+      original = unique(c(ignore, spelling_errors$word))
+    )
+    ignore_df$lower <- tolower(ignore_df$original)
+    ignore_df <- ignore_df[order(ignore_df$lower, method = "radix"), ]
+    ignore <- ignore_df$original[ignore_df$lower != ""] # drop blanks if any
+    writeLines(ignore, path_wordlist)
+    cat(
+      "All spelling errors found were copied to inst/WORDLIST assuming they were",
+      "not spelling errors and will be ignored in the future. Please",
+      "review the above list and for each word that is an actual typo:\n",
+      "- fix it in the source code.\n",
+      "- remove it again manually from inst/WORDLIST to make sure it's not\n",
+      "  ignored in the future.\n",
+      "Then, try committing again.\n",
+      "Hint: you can disable this behavior by providing a --read-only flag.\n"
+    )
+  }
   stop("Spell check failed", call. = FALSE)
-} else {
-  ignore <- ignore[ignore != ""] # drop blanks if any
-  writeLines(ignore, path_wordlist)
 }

man/run_test.Rd

@@ -0,0 +1,3 @@
+# a ttle
+
+This is a fsssile.

man/run_test_impl.Rd

@@ -219,8 +219,24 @@ run_test(
 # success
 run_test("spell-check", suffix = "-success.md", std_err = NULL)
 
-# basic failure
-run_test("spell-check", suffix = "-fail.md", std_err = "Spell check failed")
+# failure with --read-only does not create WORDLIST
+run_test(
+  "spell-check",
+  suffix = "-fail.md",
+  std_err = "Spell check failed",
+  cmd_args = "--read-only",
+  read_only = TRUE
+)
+
+# failure with --read-only does not update WORDLIST
+run_test(
+  "spell-check",
+  suffix = "-fail-2.md",
+  std_err = "Spell check failed",
+  cmd_args = "--read-only",
+  artifacts = c("inst/WORDLIST" = test_path("in/WORDLIST")),
+  read_only = TRUE
+)
 
 # success with wordlist
 run_test("spell-check",

tests/testthat/in/spell-check-fail-2.md

@@ -196,6 +196,14 @@ The `lang` arg will be passed to `spelling::spell_check_files()`.
       id: spell-check
       args: [--lang=<language>]
 
+**read only**
+
+The `--read-only` flag will be passed to spell check. This flag makes
+this hook idempotent.
+
+      id: spell-check
+      args: [--read-only]
+
 This hook does not modify input files. It will add all words not found
 in the dictionary to `inst/WORDLIST`, assuming they were spelled
 correctly but were not in the dictionary. An example might be "RStudio".
@@ -205,6 +213,8 @@ them and remove them from `inst/WORDLIST`. If there were not typos, or
 you fixed all, stage `inst/WORDLIST` and this time, the commit should
 pass.
 
+To opt out of updating `inst/WORDLIST` provide the `--read-only` flag.
+
 ## `roxygenize`
 
 A hook to run `roxygen2::roxygenize()`. Makes sure you commit your `.Rd`

tests/testthat/test-hooks.R

@@ -29,7 +29,7 @@ Read only hooks should generally run only after write hooks.
 -   roxygenize - caches
 -   codemeta-description-updated - must be before use-tidy-description
 -   use-tidy-description
--   spell-check - updates `inst/WORDLIST`; should run after roxygenize
+-   spell-check - updates `inst/WORDLIST` (unless `--read-only` arg is specified); should run after roxygenize
 
 ### Read only