rob's comments

R/runner.R

@@ -1,47 +1,4 @@
 runner_run <- function(orderly_root, reportname, parameters, branch, ref, ...) {
-  # Helper functions
-  # ==================
-  point_head_to_ref <- function(worker_path, branch, ref) {
-    gert::git_fetch(repo = worker_path)
-    gert::git_branch_checkout(branch, repo = worker_path)
-    gert::git_reset_hard(ref, repo = worker_path)
-  }
-
-  add_dir_parent_if_empty <- function(files_to_delete, path) {
-    contained_files <- list.files(path, full.names = TRUE)
-    if (length(setdiff(contained_files, files_to_delete)) > 0) {
-      return(files_to_delete)
-    }
-    add_dir_parent_if_empty(c(files_to_delete, path), dirname(path))
-  }
-
-  get_empty_dirs <- function(worker_path) {
-    dirs <- fs::dir_ls(worker_path, recurse = TRUE, type = "directory")
-    Reduce(add_dir_parent_if_empty, c(list(character()), dirs))
-  }
-
-  git_clean <- function(worker_path) {
-    # gert does not have git clean but this should achieve the same thing
-    res <- tryCatch(
-      gert::git_stash_save(
-        include_untracked = TRUE,
-        include_ignored = TRUE,
-        repo = worker_path
-      ),
-      error = function(e) NULL
-    )
-    if (!is.null(res)) {
-      gert::git_stash_drop(repo = worker_path)
-    }
-    # however git ignores all directories, only cares about files, so we may
-    # have empty directories left
-    unlink(get_empty_dirs(worker_path), recursive = TRUE)
-  }
-  # ==================
-
-
-  # Actual runner code
-  # ==================
   # Setup
   worker_id <- Sys.getenv("RRQ_WORKER_ID")
   worker_path <- file.path(orderly_root, ".packit", "workers", worker_id)
@@ -57,3 +14,40 @@ runner_run <- function(orderly_root, reportname, parameters, branch, ref, ...) {
   # Cleanup
   git_clean(worker_path)
 }
+
+point_head_to_ref <- function(worker_path, branch, ref) {
+  gert::git_fetch(repo = worker_path)
+  gert::git_branch_checkout(branch, repo = worker_path)
+  gert::git_reset_hard(ref, repo = worker_path)
+}
+
+add_dir_parent_if_empty <- function(files_to_delete, path) {
+  contained_files <- list.files(path, full.names = TRUE)
+  if (length(setdiff(contained_files, files_to_delete)) > 0) {
+    return(files_to_delete)
+  }
+  add_dir_parent_if_empty(c(files_to_delete, path), dirname(path))
+}
+
+get_empty_dirs <- function(worker_path) {
+  dirs <- fs::dir_ls(worker_path, recurse = TRUE, type = "directory")
+  Reduce(add_dir_parent_if_empty, c(list(character()), dirs))
+}
+
+git_clean <- function(worker_path) {
+  # gert does not have git clean but this should achieve the same thing
+  res <- tryCatch(
+    gert::git_stash_save(
+      include_untracked = TRUE,
+      include_ignored = TRUE,
+      repo = worker_path
+    ),
+    error = function(e) NULL
+  )
+  if (!is.null(res)) {
+    gert::git_stash_drop(repo = worker_path)
+  }
+  # however git ignores all directories, only cares about files, so we may
+  # have empty directories left
+  unlink(get_empty_dirs(worker_path), recursive = TRUE)
+}

tests/testthat/helper-orderly-runner.R

@@ -105,6 +105,14 @@ start_queue_workers_quietly <- function(n_workers,
   worker_manager
 }
 
+start_queue_with_workers <- function(root, n_workers, env = parent.frame()) {
+  q <- new_queue_quietly(root)
+  worker_manager <- start_queue_workers_quietly(n_workers, q$controller,
+                                                env = env)
+  make_worker_dirs(root, worker_manager$id)
+  q
+}
+
 skip_if_no_redis <- function() {
   available <- redux::redis_available()
   if (!available) {
@@ -114,17 +122,10 @@ skip_if_no_redis <- function() {
 }
 
 expect_worker_task_complete <- function(task_id, controller, n_tries) {
-  is_completed <- FALSE
-  for (i in seq_len(n_tries)) {
-    is_completed <- rrq::rrq_task_status(
-      task_id, controller = controller
-    ) == "COMPLETE"
-    if (is_completed == TRUE) {
-      break
-    }
-    Sys.sleep(1)
-  }
-  expect_equal(is_completed, TRUE)
+  is_task_successful <- rrq::rrq_task_wait(
+    task_id, controller = controller, timeout = n_tries
+  )
+  expect_equal(is_task_successful, TRUE)
 }
 
 initialise_git_repo <- function() {

tests/testthat/test-queue.R

@@ -66,9 +66,7 @@ test_that("Can submit task", {
   root <- test_prepare_orderly_example("data")
   helper_add_git(root, c("src", "orderly_config.yml"))
 
-  q <- new_queue_quietly(root)
-  worker_manager <- start_queue_workers_quietly(1, q$controller)
-  make_worker_dirs(root, worker_manager$id)
+  q <- start_queue_with_workers(root, 1)
 
   task_id <- q$submit("data")
   expect_worker_task_complete(task_id, q$controller, 10)
@@ -85,9 +83,7 @@ test_that("Can submit 2 tasks on different branches", {
   gert::git_branch_checkout("branch", repo = root)
   create_new_commit(root, new_file = "test.txt", add = "test.txt")
 
-  q <- new_queue_quietly(root)
-  worker_manager <- start_queue_workers_quietly(2, q$controller)
-  make_worker_dirs(root, worker_manager$id)
+  q <- start_queue_with_workers(root, 2)
 
   task_id1 <- q$submit("data", branch = "master")
   task_id2 <- q$submit("data", branch = "branch")
@@ -107,9 +103,7 @@ test_that("Can submit 2 tasks on different commit hashes", {
   sha1 <- helper_add_git(root, c("src", "orderly_config.yml"))$sha
   sha2 <- create_new_commit(root, new_file = "test.txt", add = "test.txt")
 
-  q <- new_queue_quietly(root)
-  worker_manager <- start_queue_workers_quietly(2, q$controller)
-  make_worker_dirs(root, worker_manager$id)
+  q <- start_queue_with_workers(root, 2)
 
   task_id1 <- q$submit("data", ref = sha1)
   task_id2 <- q$submit("data", ref = sha2)