resubmission that hopefully fixes sort order of files in cache on systems with low-accuracy timestamp

Author: Stefan Fleck

DESCRIPTION

@@ -1,7 +1,7 @@
 Type: Package
 Package: rotor
 Title: Log Rotation and Conditional Backups
-Version: 0.3.1
+Version: 0.3.2
 Authors@R: 
     person(given = "Stefan",
            family = "Fleck",

R/Cache.R

@@ -333,7 +333,8 @@ Cache <- R6::R6Class(
       )
       row.names(res) <- NULL
 
-      res[order(res$mtime), ]
+      assert(!is.null(res$mtime))
+      res[order(res$mtime, res$key), ]
     },
 
     size = function(){

cran-comments.md

@@ -10,21 +10,12 @@
 
 0 errors | 0 warnings | 0 notes
 
-* fixes time zone related issue in `Cache$prune()` that causes macOS tests to fail
-* more robust cleanup of temporary files between some tests
-* resubmission that improves one test where 
-  `https://cran.r-project.org/web/checks/check_results_rotor.html` shows an
-  error that I could not reproduce on any machine available on rhub. 
-  Hopefully the test succeedes now, but if not it will give me more useful 
-  debug information for another resubmission.
+Sorry for the many resubmissions. This package currently has an issue that
+only to occur on macOS. It does not occur on the same macOS version available
+on Rhub. 
 
-
-Notes:
-
-There was a timezone related issue that only occured on macos. I fixed it and
-was able to test it on macOS 10.13.6 High Sierra with R-release (but not devel)
-on Rhub.
-
-I could not reproduce the other errors; however, it looks as if they were caused
-by improperly cleaned-up temporary files between failing tests. I made the
-cleanup code a bit more robust and hope the problem is fixed now.
+My current theory is that the error is caused by the accuracy of the
+'mtime' filestamp, which is linked to the file system the machine uses. 
+I implemented a workaround under that assumption, but I cannot be 100% sure 
+that this fixes that error. If the error still persists... could you please
+inform me which file system you use on r-release-macos-x86_64?

tests/testthat/test_Cache.R

@@ -1,6 +1,21 @@
 context("Cache")
 
 
+# generate lexically sortable ids. For equal timestamp, Cache$files is sorted
+# by id, so that the tests do not fail on file systems with low-accuracy
+# timestamps
+
+
+.id_cache <- new.env()
+assign("id", 0L, .id_cache)
+ascending_id <- function(){
+  x <- get("id", .id_cache)
+  x <- as.character(as.integer(x) + 1L)
+  assign("id", x, .id_cache)
+  x
+}
+
+
 test_that("Cache works as expected", {
   td <- file.path(tempdir(), "cache-test")
   on.exit(unlink(td, recursive = TRUE))
@@ -47,14 +62,14 @@ test_that("setting hash functions work", {
 
 
   # To override this behaviour use a generate for unique ids, such as
-  cache_uid <- Cache$new(td, hashfun = function(x) uuid::UUIDgenerate())
+  cache_uid <- Cache$new(td, hashfun = function(x) ascending_id())
   cache_uid$push(iris)
   cache_uid$push(iris)
   expect_identical(cache_hash$n, 2L)
   cache_hash$purge()
 
   # ensure hashfun allways returns a scalar
-  cache_err <- Cache$new(td, hashfun = function(x) uuid::UUIDgenerate(n = 2))
+  cache_err <- Cache$new(td, hashfun = function(x) c(ascending_id(),  ascending_id()))
   expect_error(cache_err$push(iris), class = "ValueError")
 })
 
@@ -66,13 +81,9 @@ test_that("pruning works by number of files works", {
   td <- file.path(tempdir(), "cache-test")
   on.exit(unlink(td, recursive = TRUE))
 
-  # When using a real hash function as hashfun, identical objects will only
-  # be added to the cache once
-  cache <- Cache$new(td, hashfun = function(x) uuid::UUIDgenerate())
+  cache <- Cache$new(td, hashfun = function(x) ascending_id())
   k1 <- cache$push(iris)
-  Sys.sleep(1)
   k2 <- cache$push(letters)
-  Sys.sleep(1)
   k3 <- cache$push(cars)
   expect_identical(cache$n, 3L)
 
@@ -81,7 +92,6 @@ test_that("pruning works by number of files works", {
   expect_identical(cache$files$key[[3]], k3)
 
   cache$prune(max_files = 2)
-  cache$files
   expect_identical(cache$read(cache$files$key[[1]]), letters)
   expect_identical(cache$read(cache$files$key[[2]]), cars)
   cache$purge()
@@ -96,7 +106,7 @@ test_that("pruning by size works", {
 
   # When using a real hash function as hashfun, identical objects will only
   # be added to the cache once
-  cache <- Cache$new(td, hashfun = function(x) uuid::UUIDgenerate())
+  cache <- Cache$new(td, hashfun = function(x) ascending_id())
   cache$push(iris)
   Sys.sleep(0.1)
   cache$push(iris)
@@ -128,7 +138,7 @@ test_that("Inf max_* do not prunes", {
 
   # When using a real hash function as hashfun, identical objects will only
   # be added to the cache once
-  cache <- Cache$new(td, hashfun = function(x) uuid::UUIDgenerate())
+  cache <- Cache$new(td, hashfun = function(x) ascending_id())
   cache$push(iris)
   Sys.sleep(0.1)
   cache$push(iris)
@@ -194,7 +204,7 @@ test_that("pruning by age works", {
     )
   )
 
-  cache <- MockCache$new(dir = td, hashfun = function(x) uuid::UUIDgenerate())
+  cache <- MockCache$new(dir = td, hashfun = function(x) ascending_id())
   on.exit(cache$purge(), add = TRUE)
   cache$push(iris)
   Sys.sleep(0.1)