fix checksums() update error (#332)

- ignore the `CHECKSUMS.txt` file when `write = TRUE` - overwrite this file with the checksums of *ALL* files in the module's `data/` directory. - added new tests for `checksums()`
PredictiveEcology · Feb 6, 2017 · bc822cd · bc822cd
1 parent 8a1fb76
commit bc822cd
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 16 deletions.
diff --git a/NEWS b/NEWS
@@ -32,6 +32,7 @@ version 1.3.1.9000
 * improve module template to auto fill module author info using `devtools.desc.author` option if set.
 * `zipModule` now has `data` argument, allowing data to be omitted from zipped module.
 * improved `moduleDiagram` to show `_INPUT_` node in different colour from the other modules
+* `checksums(..., write = TRUE)` ignores the contents of `CHECKSUMS.txt`, overwriting that file with the checksums of all files in the module's `data/` directory. This makes it easier to update the checksum file, *e.g.*, when adding new data (#332).
 * minor bugfixes for unusual cases
 * **Defunct**: `p`. Use `params` or `P` instead.
 

diff --git a/R/module-repository.R b/R/module-repository.R
@@ -575,20 +575,22 @@ setMethod(
     defaultHashAlgo <- "xxhash64"
     defaultWriteHashAlgo <- "xxhash64"
     dots <- list(...)
-
     path <- checkPath(path, create = FALSE) %>% file.path(., module, "data")
-    if (!write) stopifnot(file.exists(file.path(path, "CHECKSUMS.txt")))
+    checksumFile <- file.path(path, "CHECKSUMS.txt")
 
-    files <- list.files(path, full.names = TRUE) %>%
-      grep("CHECKSUMS.txt", ., value = TRUE, invert = TRUE)
+    if (!write) {
+      stopifnot(file.exists(checksumFile))
+    } else if (!file.exists(checksumFile)) {
+      file.create(checksumFile)
+    }
 
-    checksumFile <- file.path(path, "CHECKSUMS.txt")
+    files <- list.files(path, full.names = TRUE) %>%
+      grep(basename(checksumFile), ., value = TRUE, invert = TRUE)
 
-    txt <- if (file.info(checksumFile)$size > 0) {
+    txt <- if (!write && file.info(checksumFile)$size > 0) {
       read.table(checksumFile, header = TRUE, stringsAsFactors = FALSE)
     } else {
-      data.frame(file = character(0), checksum = character(0),
-                 stringsAsFactors = FALSE)
+      data.frame(file = character(0), checksum = character(0), stringsAsFactors = FALSE)
     }
 
     if (is.null(dots$algo)) {
@@ -613,23 +615,21 @@ setMethod(
       }
     }
 
-    message("Checking local files")
+    message("Checking local files...")
     if (length(txt$file) & length(files)) {
       filesToCheck <- files[basename(files) %in% txt$file]
     } else {
-      filesToCheck <- character(0)
+      filesToCheck <- files
     }
     checksums <- do.call(digest, args = append(list(file = filesToCheck), dots))
-    message("Finished checking local files")
+    message("Finished checking local files.")
 
-    if(length(filesToCheck)) {
+    if (length(filesToCheck)) {
       out <- data.frame(file = basename(filesToCheck), checksum = checksums,
-                        algorithm = dots$algo,
-                        stringsAsFactors = FALSE)
+                        algorithm = dots$algo, stringsAsFactors = FALSE)
     } else {
       out <- data.frame(file = character(0), checksum = character(0),
-                        algorithm = character(0),
-                        stringsAsFactors = FALSE)
+                        algorithm = character(0), stringsAsFactors = FALSE)
     }
 
     if (write) {

diff --git a/tests/testthat/test-checksums.R b/tests/testthat/test-checksums.R
@@ -0,0 +1,48 @@
+test_that("checksums read and written correctly", {
+  library(magrittr)
+
+  sampleDir <- system.file("maps", package = "SpaDES")
+  sampleFiles <- list.files(sampleDir, pattern = ".tif", full.names = TRUE)
+  tmpdir <- file.path(tempdir(), "test_checksums", "data") %>%
+    checkPath(create = TRUE)
+  on.exit(unlink(dirname(tmpdir), recursive = TRUE), add = TRUE)
+
+  file.copy(sampleFiles, tmpdir)
+
+  csf <- file.path(tmpdir, "CHECKSUMS.txt")
+  cnames.r <- c("result", "expectedFile", "actualFile", "checksum.x", "checksum.y",
+                "algorithm.x", "algorithm.y")
+  cnames.w <- c("file", "checksum", "algorithm")
+  csums <- c("77c56d42fecac5b1", "8affcdf311555fd6", "e2dd8734d6ed3d05",
+             "f21251dcdf23dde0", "86e342cfc6876b7d")
+
+  # 1. read checksums without CHECKSUMS.txt file
+  expect_error(checksums("test_checksums", dirname(dirname(tmpdir))))
+
+  # 2. read checksums with empty CHECKSUMS.txt file
+  file.create(csf)
+  txt <- checksums("test_checksums", dirname(dirname(tmpdir)))
+  expect_true(all(colnames(txt) == cnames.r))
+  expect_equal(nrow(txt), 0)
+
+  # 3. write checksums without CHECKSUMS.txt
+  file.remove(csf)
+  txt <- checksums("test_checksums", dirname(dirname(tmpdir)), write = TRUE)
+  expect_true(all(colnames(txt) == cnames.w))
+  expect_equal(nrow(txt), 5)
+  expect_true(all(txt$file == basename(sampleFiles)))
+  expect_true(all(txt$checksum == csums))
+
+  # 4. read checksums with non-empty CHECKSUMS.txt file
+  out <- data.frame(file = basename(sampleFiles[-1]),
+                    checksum = csums[-1],
+                    algorithm = c("xxhash64", "xxhash64", "xxhash64", "xxhash64"),
+                    stringsAsFactors = FALSE)
+  write.table(out, csf, eol = "\n", col.names = TRUE, row.names = FALSE)
+
+  txt <- checksums("test_checksums", dirname(dirname(tmpdir)), write = TRUE)
+  expect_true(all(colnames(txt) == cnames.w))
+  expect_equal(nrow(txt), 5)
+  expect_true(all(txt$file == basename(sampleFiles)))
+  expect_true(all(txt$checksum == csums))
+})