More fixes for occasional CI and older versions of R (#6387)

* Update R-CMD-check-occasional.yaml

* Error robust to rep.int() vs. rep_len()

* Only positional arguments to on.exit()

* Add some logging

.github/workflows/R-CMD-check-occasional.yaml

@@ -1,6 +1,6 @@
 on:
   schedule:
-   - cron: '17 13 21 * *' # 21st of month at 13:17 UTC
+   - cron: '17 13 22 * *' # 22nd of month at 13:17 UTC
 
 # A more complete suite of checks to run monthly; each PR/merge need not pass all these, but they should pass before CRAN release
 name: R-CMD-check-occasional
@@ -91,21 +91,14 @@ jobs:
         run: |
           options(crayon.enabled = TRUE)
           install.packages("remotes")
+          message("*** DONE remotes, now installing requirements ***")
           remotes::install_deps(dependencies=TRUE, force=TRUE)
 
-          # we define this in data.table namespace, but it appears to be exec
-          if (!exists("isFALSE", "package:base")) { # R>=3.5.0
-            if (!exists("isFALSE", asNamespace("data.table"))) {
-              message("isFALSE not found in base, but data.table did not define it either!\n")
-            }
-            # attempt defining it here as a workaround...
-            isFALSE = function(x) is.logical(x) && length(x) == 1L && !is.na(x) && !x
-          }
-
           other_deps_expr = parse('inst/tests/other.Rraw', n=1L)
           eval(other_deps_expr)
           other_pkgs = get(as.character(other_deps_expr[[1L]][[2L]]))
           # Many will not install on oldest R versions
+          message("*** DONE requirements, now installing optional ***")
           try(remotes::install_cran(c(other_pkgs, "rcmdcheck"), force=TRUE))
 
           has_other_pkg = sapply(other_pkgs, requireNamespace, quietly=TRUE)

R/test.data.table.R

@@ -325,11 +325,12 @@ test = function(num,x,y=TRUE,error=NULL,warning=NULL,message=NULL,output=NULL,no
     # NB: Sys.setenv() (no arguments) errors
     if (!all(to_unset)) do.call(Sys.setenv, as.list(env[!to_unset]))
     Sys.unsetenv(names(env)[to_unset])
-    on.exit(add=TRUE, {
+    # TODO(R>=4.0.2): Use add=TRUE up-front in on.exit() once non-positional arguments are supported.
+    on.exit({
       is_preset = !is.na(old)
       if (any(is_preset)) do.call(Sys.setenv, as.list(old[is_preset]))
       Sys.unsetenv(names(old)[!is_preset])
-    })
+    }, add=TRUE)
   }
   if (!is.null(options)) {
     old_options <- do.call(base::options, as.list(options)) # as.list(): allow passing named character vector for convenience
@@ -365,7 +366,8 @@ test = function(num,x,y=TRUE,error=NULL,warning=NULL,message=NULL,output=NULL,no
     foreign = get("foreign", parent.frame())
     showProgress = get("showProgress", parent.frame())
     time = nTest = RSS = NULL  # to avoid 'no visible binding' note
-    if (num>0) on.exit( add=TRUE, {
+    # TODO(R>=4.0.2): Use add=TRUE up-front in on.exit() once non-positional arguments are supported.
+    if (num>0) on.exit({
        took = proc.time()[3L]-lasttime  # so that prep time between tests is attributed to the following test
        timings[as.integer(num), `:=`(time=time+took, nTest=nTest+1L), verbose=FALSE]
        if (memtest) {
@@ -376,7 +378,7 @@ test = function(num,x,y=TRUE,error=NULL,warning=NULL,message=NULL,output=NULL,no
          if (memtest==2L) gc()
        }
        assign("lasttime", proc.time()[3L], parent.frame(), inherits=TRUE)  # after gc() to exclude gc() time from next test when memtest
-    } )
+    }, add=TRUE )
     if (showProgress)
       # \r can't be in gettextf msg
       cat("\rRunning test id", numStr, "         ")   # nocov.

R/utils.R

@@ -23,6 +23,7 @@ nan_is_na = function(x) {
 
 # TODO(R>=4.0.0): Remove this workaround. From R 4.0.0, rep_len() dispatches rep.Date(), which we need.
 #   Before that, rep_len() strips attributes --> breaks data.table()'s internal recycle() helper.
+#   This also impacts test 2 in S4.Rraw, because the error message differs for rep.int() vs. rep_len().
 if (inherits(rep_len(Sys.Date(), 1L), "Date")) {
   # NB: safe_rep_len=rep_len throws an R CMD check error because it _appears_ to the AST
   #   walker that we've used .Internal ourselves (which is not true, but codetools can't tell:

inst/tests/S4.Rraw

@@ -61,7 +61,7 @@ removeClass("S4Composition")
 # miscellaneous missing tests uncovered by CodeCov difference in the process of PR #2573 [S4 portion, c.f. 1872.* in tests.Rraw]
 ## data.table cannot recycle complicated types
 short_s4_col = getClass("MethodDefinition")
-test(2, data.table(a = 1:4, short_s4_col), error="attempt to replicate non-vector")
+test(2, data.table(a = 1:4, short_s4_col), error="attempt to replicate.*(non-vector|S4)")
 
 # print dims in list-columns, #3671, c.f. 2130.* in tests.Rraw
 s4class = setClass("ex_class", slots = list(x="integer", y="character", z="numeric"))