Skip to content

Commit

Permalink
resubmission that hopefully fixes sort order of files in cache on sys…
Browse files Browse the repository at this point in the history
…tems with low-accuracy timestamp
  • Loading branch information
Stefan Fleck committed Oct 20, 2020
1 parent 28d1436 commit 6b97bd4
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 22 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
3 changes: 2 additions & 1 deletion R/Cache.R
Original file line number Diff line number Diff line change
Expand Up @@ -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(){
Expand Down
25 changes: 8 additions & 17 deletions cran-comments.md
Original file line number Diff line number Diff line change
Expand Up @@ -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?
3 changes: 0 additions & 3 deletions tests/testthat/test_Cache.R
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,7 @@ test_that("pruning works by number of files works", {
# be added to the cache once
cache <- Cache$new(td, hashfun = function(x) uuid::UUIDgenerate())
k1 <- cache$push(iris)
Sys.sleep(1)
k2 <- cache$push(letters)
Sys.sleep(1)
k3 <- cache$push(cars)
expect_identical(cache$n, 3L)

Expand All @@ -81,7 +79,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()
Expand Down

0 comments on commit 6b97bd4

Please sign in to comment.