Replace aaa.R (#168)

* Eliminate `run_on_load()` helper
* Use zzz.R
* Add package doc and move imports there
* Add some basic pause tests
* Improve pause error checking

R/aaa.R

@@ -1,22 +0,0 @@
-#' @import rlang
-NULL
-
-.onLoad <- function(...) {
-  run_on_load()
-}
-
-on_load <- function(expr, env = topenv(caller_env())) {
-  callback <- function() eval_bare(expr, env)
-  env$.__rlang_hook__. <- list2(!!!env$.__rlang_hook__., callback)
-}
-
-run_on_load <- function(env = topenv(caller_env())) {
-  hook <- env$.__rlang_hook__.
-  env_unbind(env, ".__rlang_hook__.")
-
-  for (callback in hook) {
-    callback()
-  }
-
-  env$.__rlang_hook__. <- NULL
-}

R/pause.R

@@ -13,15 +13,18 @@
 #' @useDynLib profvis, .registration = TRUE, .fixes = "c_"
 #' @export
 pause <- function(seconds) {
-  .Call(c_profvis_pause, as.numeric(seconds))
+  if (is.integer(seconds)) {
+    seconds <- as.numeric(seconds)
+  }
+  .Call(c_profvis_pause, seconds)
 }
 
 # This guarantees that (1) `pause()` is always compiled, even on
 # `load_all()` and (2) it doesn't include source references. This in
 # turn ensures consistent profile output: if the function is not
 # compiled and doesn't contain srcrefs, `.Call()` is never included in
 # the profiles, even when `line.profiling` is set.
-on_load({
-  pause <- utils::removeSource(pause)
-  pause <- compiler::cmpfun(pause)
-})
+on_load_pause <- function() {
+  pause <<- utils::removeSource(pause)
+  pause <<- compiler::cmpfun(pause)
+}

R/profvis-package.R

@@ -0,0 +1,7 @@
+#' @keywords internal
+"_PACKAGE"
+
+## usethis namespace: start
+#' @import rlang
+## usethis namespace: end
+NULL

R/zzz.R

@@ -0,0 +1,3 @@
+.onLoad <- function(...) {
+  on_load_pause()
+}

src/pause.c

@@ -24,11 +24,11 @@ double get_time_ms(void) {
 }
 
 SEXP profvis_pause (SEXP seconds) {
-  if (TYPEOF(seconds) != REALSXP)
-    error("`seconds` must be a numeric");
+  if (TYPEOF(seconds) != REALSXP || Rf_length(seconds) != 1)
+    Rf_error("`seconds` must be a single number.");
 
   double start = get_time_ms();
-  double sec = asReal(seconds);
+  double sec = Rf_asReal(seconds);
 
   while(get_time_ms() - start < sec) {
     R_CheckUserInterrupt();

tests/testthat/_snaps/pause.md

@@ -0,0 +1,13 @@
+# checks its inputs
+
+    Code
+      pause(c(1, 2))
+    Condition
+      Error in `pause()`:
+      ! `seconds` must be a single number.
+    Code
+      pause("a")
+    Condition
+      Error in `pause()`:
+      ! `seconds` must be a single number.
+

tests/testthat/test-pause.R

@@ -0,0 +1,20 @@
+test_that("pause takes expected time", {
+  time <- system.time(pause(0.2))[[3]]
+  # system.time is a little inaccurate so allow 10% padding
+  expect_true(abs(time - 0.2) < 1e-2)
+})
+
+test_that("works with integers", {
+  expect_no_error(pause(0L))
+})
+
+test_that("pause has no srcrefs", {
+  expect_equal(attr(pause, "srcref"), NULL)
+})
+
+test_that("checks its inputs", {
+  expect_snapshot(error = TRUE, {
+    pause(c(1, 2))
+    pause("a")
+  })
+})