From 7b3647b7e07b36653bdd984f582d6c7b10f40684 Mon Sep 17 00:00:00 2001 From: Rob Ashton Date: Thu, 22 Feb 2024 17:35:31 +0000 Subject: [PATCH 01/10] Add endpoint to list reports on a branch --- .Rbuildignore | 2 + DESCRIPTION | 3 +- R/api.R | 15 +++++- R/git.R | 37 +++++++++++++ R/porcelain.R | 10 ++++ R/util.R | 24 +++++++++ R/util_assert.R | 23 ++++++++ inst/schema/report_list.json | 10 ++++ orderly.runner.Rproj | 17 ++++++ tests/testthat/examples/data/orderly.R | 3 ++ tests/testthat/examples/parameters/orderly.R | 2 + tests/testthat/helper-orderly-runner.R | 57 ++++++++++++++++++++ tests/testthat/test-api.R | 31 +++++++++++ tests/testthat/test-git.R | 48 +++++++++++++++++ tests/testthat/test-util-assert.R | 12 +++++ 15 files changed, 292 insertions(+), 2 deletions(-) create mode 100644 R/git.R create mode 100644 R/util_assert.R create mode 100644 inst/schema/report_list.json create mode 100644 orderly.runner.Rproj create mode 100644 tests/testthat/examples/data/orderly.R create mode 100644 tests/testthat/examples/parameters/orderly.R create mode 100644 tests/testthat/test-git.R create mode 100644 tests/testthat/test-util-assert.R diff --git a/.Rbuildignore b/.Rbuildignore index 1212a32..baae540 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -21,3 +21,5 @@ ^\.covrignore$ ^\.github$ \.*gcov$ +^.*\.Rproj$ +^\.Rproj\.user$ diff --git a/DESCRIPTION b/DESCRIPTION index 59530b5..10b2341 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -8,12 +8,13 @@ Authors@R: c(person("Rich", "FitzJohn", role = c("aut", "cre"), Description: Small HTTP server for running orderly reports. License: MIT + file LICENSE Encoding: UTF-8 -RoxygenNote: 7.2.3 +RoxygenNote: 7.3.1 Roxygen: list(markdown = TRUE, roclets = c("rd", "namespace", "porcelain::porcelain_roclet")) URL: https://github.com/mrc-ide/orderly.runner BugReports: https://github.com/mrc-ide/orderly.runner/issues Imports: docopt, + gert, jsonlite, orderly2, porcelain, diff --git a/R/api.R b/R/api.R index 14d3fb7..382264e 100644 --- a/R/api.R +++ b/R/api.R @@ -16,7 +16,6 @@ ##' ##' @export api <- function(root, validate = NULL, log_level = "info") { - orderly2::orderly_list_src(root, locate = FALSE) logger <- porcelain::porcelain_logger(log_level) api <- porcelain::porcelain$new(validate = validate, logger = logger) api$include_package_endpoints(state = list(root = root)) @@ -30,3 +29,17 @@ root <- function() { orderly.runner = package_version_string("orderly.runner")) lapply(versions, scalar) } + +##' @porcelain +##' GET /report/list => json(report_list) +##' query hash :: string +##' state root :: root +report_list <- function(root, hash) { + temp_root <- git_clone_depth_1(root, hash) + src_reports <- orderly2::orderly_list_src(temp_root, locate = FALSE) + lapply(src_reports, function(report_name) { + list( + name = scalar(report_name) + ) + }) +} diff --git a/R/git.R b/R/git.R new file mode 100644 index 0000000..29d0971 --- /dev/null +++ b/R/git.R @@ -0,0 +1,37 @@ +git_run <- function(args, repo = NULL, check = FALSE) { + git <- sys_which("git") + if (!is.null(repo)) { + args <- c("-C", repo, args) + } + res <- system3(git, args) + if (check && !res$success) { + stop(sprintf("Error code %d running command:\n%s", + res$code, paste0(" > ", res$output, collapse = "\n"))) + } + res +} + + +git_ref_to_sha <- function(ref, repo = NULL, check = FALSE) { + assert_scalar_character(ref) + res <- git_run(c("rev-parse", ref), repo = repo, check = FALSE) + if (res$success) { + res$output + } else if (check) { + stop(sprintf("Git reference '%s' not found", ref), call. = FALSE) + } else { + NA_character_ + } +} + + +git_clone_depth_1 <- function(origin, ref) { + t <- tempfile() + dir.create(t) + gert::git_init(t) + gert::git_remote_add(origin, repo = t) + sha <- git_ref_to_sha(ref, repo = origin, check = TRUE) + git_run(c("fetch", "--depth", "1", "origin", sha), repo = t) + git_run(c("checkout", "FETCH_HEAD"), repo = t) + t +} diff --git a/R/porcelain.R b/R/porcelain.R index 4381491..e290b13 100644 --- a/R/porcelain.R +++ b/R/porcelain.R @@ -8,5 +8,15 @@ root, returning = porcelain::porcelain_returning_json("root"), validate = validate) + }, + "GET /report/list" = function(state, validate) { + porcelain::porcelain_endpoint$new( + "GET", + "/report/list", + report_list, + porcelain::porcelain_input_query(hash = "string"), + porcelain::porcelain_state(root = state$root), + returning = porcelain::porcelain_returning_json("report_list"), + validate = validate) }) } diff --git a/R/util.R b/R/util.R index e376947..d04efe5 100644 --- a/R/util.R +++ b/R/util.R @@ -11,3 +11,27 @@ scalar <- function(x) { package_version_string <- function(name) { as.character(utils::packageVersion(name)) } + + +system3 <- function(command, args) { + res <- suppressWarnings(system2(command, args, stdout = TRUE, stderr = TRUE)) + code <- attr(res, "status") %||% 0 + attr(res, "status") <- NULL + list(success = code == 0, + code = code, + output = res) +} + + +sys_which <- function(name) { + path <- Sys.which(name) + if (!nzchar(path)) { + stop(sprintf("Did not find '%s'", name), call. = FALSE) + } + unname(path) +} + + +vcapply <- function(X, FUN, ...) { # nolint + vapply(X, FUN, character(1), ...) +} diff --git a/R/util_assert.R b/R/util_assert.R new file mode 100644 index 0000000..93ec841 --- /dev/null +++ b/R/util_assert.R @@ -0,0 +1,23 @@ +assert_scalar <- function(x, name = deparse(substitute(x)), arg = name, + call = NULL) { + if (length(x) != 1) { + cli::cli_abort(c("'{name}' must be a scalar", + i = "{name} has length {length(x)}"), + call = call, arg = arg) + } +} + + +assert_character <- function(x, name = deparse(substitute(x)), + arg = name, call = NULL) { + if (!is.character(x)) { + cli::cli_abort("'{name}' must be character", call = call, arg = arg) + } +} + + +assert_scalar_character <- function(x, name = deparse(substitute(x)), + arg = name, call = NULL) { + assert_scalar(x, name, arg = arg, call = call) + assert_character(x, name, arg = arg, call = call) +} diff --git a/inst/schema/report_list.json b/inst/schema/report_list.json new file mode 100644 index 0000000..9b9bdbc --- /dev/null +++ b/inst/schema/report_list.json @@ -0,0 +1,10 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "array", + "items": { + "type": "object", + "properties": { + "name": { "type": "string" } + } + } +} diff --git a/orderly.runner.Rproj b/orderly.runner.Rproj new file mode 100644 index 0000000..21a4da0 --- /dev/null +++ b/orderly.runner.Rproj @@ -0,0 +1,17 @@ +Version: 1.0 + +RestoreWorkspace: Default +SaveWorkspace: Default +AlwaysSaveHistory: Default + +EnableCodeIndexing: Yes +UseSpacesForTab: Yes +NumSpacesForTab: 2 +Encoding: UTF-8 + +RnwWeave: Sweave +LaTeX: pdfLaTeX + +BuildType: Package +PackageUseDevtools: Yes +PackageInstallArgs: --no-multiarch --with-keep.source diff --git a/tests/testthat/examples/data/orderly.R b/tests/testthat/examples/data/orderly.R new file mode 100644 index 0000000..c7a6add --- /dev/null +++ b/tests/testthat/examples/data/orderly.R @@ -0,0 +1,3 @@ +orderly2::orderly_artefact("Some data", "data.rds") +d <- data.frame(a = 1:10, x = runif(10), y = 1:10 + runif(10)) +saveRDS(d, "data.rds") diff --git a/tests/testthat/examples/parameters/orderly.R b/tests/testthat/examples/parameters/orderly.R new file mode 100644 index 0000000..ecdcd5b --- /dev/null +++ b/tests/testthat/examples/parameters/orderly.R @@ -0,0 +1,2 @@ +orderly2::orderly_parameters(a = NULL, b = 2, c = NULL) +saveRDS(list(a = a, b = b, c = c), "data.rds") diff --git a/tests/testthat/helper-orderly-runner.R b/tests/testthat/helper-orderly-runner.R index 9495802..c2899d2 100644 --- a/tests/testthat/helper-orderly-runner.R +++ b/tests/testthat/helper-orderly-runner.R @@ -10,3 +10,60 @@ create_temporary_root <- function(...) { withr::defer_parent(unlink(path, recursive = TRUE)) suppressMessages(orderly2::orderly_init(path, ...)) } + + +test_prepare_orderly_example <- function(examples, ...) { + tmp <- tempfile() + withr::defer_parent(unlink(tmp, recursive = TRUE)) + suppressMessages(orderly2::orderly_init(tmp, ...)) + copy_examples(examples, tmp) + as.character(fs::path_norm(tmp)) +} + + +copy_examples <- function(examples, path_src) { + fs::dir_create(path_src) + + fs::dir_create(file.path(path_src, "src")) + for (i in examples) { + fs::dir_copy(test_path("examples", i), file.path(path_src, "src")) + } +} + + +helper_add_git <- function(path) { + gert::git_init(path) + gert::git_add(".", repo = path) + user <- "author " + sha <- gert::git_commit("initial", author = user, committer = user, + repo = path) + branch <- gert::git_branch(repo = path) + url <- "https://example.com/git" + gert::git_remote_add(url, repo = path) + list(path = path, user = user, branch = branch, sha = sha, url = url) +} + + +initialise_git_repo <- function() { + t <- tempfile() + dir.create(t) + writeLines(c("# Example", "", "example repo"), file.path(t, "README.md")) + helper_add_git(t) +} + + +create_new_commit <- function(path, new_file = "new", message = "new message") { + writeLines("new file", file.path(path, new_file)) + gert::git_add(".", repo = path) + user <- "author " + gert::git_commit(message, author = user, committer = user, repo = path) +} + + +create_new_branch <- function(path, branch_name = "other") { + initial_branch <- gert::git_branch(repo = path) + gert::git_branch_create(branch_name, repo = path) + commit_sha <- create_new_commit(path, branch_name) + gert::git_branch_checkout(initial_branch, repo = path) + list(branch = branch_name, sha = commit_sha) +} diff --git a/tests/testthat/test-api.R b/tests/testthat/test-api.R index f2d9155..28c8410 100644 --- a/tests/testthat/test-api.R +++ b/tests/testthat/test-api.R @@ -18,3 +18,34 @@ test_that("Can construct the api", { expect_length(logs, 2) expect_equal(logs[[1]]$logger, "orderly.runner") }) + + +test_that("can list orderly reports", { + path <- test_prepare_orderly_example(c("data", "parameters")) + repo <- helper_add_git(path) + endpoint <- orderly_runner_endpoint("GET", "/report/list", path) + get_reports_from_response <- function(res) { + vcapply(res$data, function(item) as.character(item[["name"]])) + } + + res <- endpoint$run("main") + expect_equal(res$status_code, 200) + expect_setequal(get_reports_from_response(res), c("data", "parameters")) + + ## Delete a report on a 2nd branch + gert::git_branch_create("other", repo = path) + unlink(file.path(path, "src", "data"), recursive = TRUE) + gert::git_add(".", repo = path) + sha <- gert::git_commit("Remove data report", repo = path) + + ## Can list items from this sha + other_res <- endpoint$run(sha) + expect_equal(other_res$status_code, 200) + expect_equal(get_reports_from_response(other_res), "parameters") + + ## We can still see all reports on main branch + first_commit_res <- endpoint$run(repo$sha) + expect_equal(first_commit_res$status_code, 200) + expect_setequal(get_reports_from_response(first_commit_res), + c("data", "parameters")) +}) diff --git a/tests/testthat/test-git.R b/tests/testthat/test-git.R new file mode 100644 index 0000000..3e7f3f6 --- /dev/null +++ b/tests/testthat/test-git.R @@ -0,0 +1,48 @@ +test_that("can do depth 1 clone", { + repo <- initialise_git_repo() + + first_commit <- repo$sha + second_commit <- create_new_commit(repo$path) + + first_clone <- git_clone_depth_1(repo$path, first_commit) + log <- gert::git_log(repo = first_clone) + expect_true(nrow(log) == 1) + expect_equal(log$commit, first_commit) + + second_clone <- git_clone_depth_1(repo$path, second_commit) + log <- gert::git_log(repo = second_clone) + expect_true(nrow(log) == 1) + expect_equal(log$commit, second_commit) +}) + + +test_that("git ref to sha", { + testthat::skip_on_cran() + repo <- initialise_git_repo() + other <- create_new_branch(repo$path, "other") + sha1 <- git_ref_to_sha("main", repo$path) + sha2 <- git_ref_to_sha("other", repo$path) + + expect_match(sha1, "^[[:xdigit:]]{40}$") + expect_match(sha2, "^[[:xdigit:]]{40}$") + expect_true(sha1 != sha2) + + expect_equal(git_ref_to_sha("HEAD", repo$path), sha1) + expect_equal(git_ref_to_sha(substr(sha1, 1, 7), repo$path), sha1) + + expect_identical(git_ref_to_sha("unknown", repo$path), + NA_character_) + expect_error(git_ref_to_sha("unknown", repo$path, TRUE), + "Git reference 'unknown' not found") +}) + + +test_that("handle failure", { + testthat::skip_on_cran() + repo <- initialise_git_repo() + r <- git_run("unknown-command", repo = repo$path) + expect_false(r$success) + expect_error( + git_run("unknown-command", repo = repo$path, check = TRUE), + r$output, fixed = TRUE) +}) diff --git a/tests/testthat/test-util-assert.R b/tests/testthat/test-util-assert.R new file mode 100644 index 0000000..73d9105 --- /dev/null +++ b/tests/testthat/test-util-assert.R @@ -0,0 +1,12 @@ +test_that("assert_scalar", { + expect_error(assert_scalar(NULL), "must be a scalar") + expect_error(assert_scalar(numeric(0)), "must be a scalar") + expect_error(assert_scalar(1:2), "must be a scalar") +}) + + +test_that("assert_character", { + expect_silent(assert_character("a")) + expect_error(assert_character(1), "must be character") + expect_error(assert_character(TRUE), "must be character") +}) From efa71bf05e136d3725a72590bc1caae122241de2 Mon Sep 17 00:00:00 2001 From: Rob Ashton Date: Thu, 22 Feb 2024 17:43:52 +0000 Subject: [PATCH 02/10] Add missing imports --- DESCRIPTION | 2 ++ 1 file changed, 2 insertions(+) diff --git a/DESCRIPTION b/DESCRIPTION index 10b2341..b726445 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -13,12 +13,14 @@ Roxygen: list(markdown = TRUE, roclets = c("rd", "namespace", "porcelain::porcel URL: https://github.com/mrc-ide/orderly.runner BugReports: https://github.com/mrc-ide/orderly.runner/issues Imports: + cli, docopt, gert, jsonlite, orderly2, porcelain, Suggests: + fs, httr, lgr, mockery, From d1e0a042b4281675911687561c73c4fef355f199 Mon Sep 17 00:00:00 2001 From: Rob Ashton Date: Thu, 22 Feb 2024 17:46:06 +0000 Subject: [PATCH 03/10] Add author for git commit --- tests/testthat/test-api.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-api.R b/tests/testthat/test-api.R index 28c8410..e85d893 100644 --- a/tests/testthat/test-api.R +++ b/tests/testthat/test-api.R @@ -36,7 +36,8 @@ test_that("can list orderly reports", { gert::git_branch_create("other", repo = path) unlink(file.path(path, "src", "data"), recursive = TRUE) gert::git_add(".", repo = path) - sha <- gert::git_commit("Remove data report", repo = path) + sha <- gert::git_commit("Remove data report", repo = path, + author = "Test User ") ## Can list items from this sha other_res <- endpoint$run(sha) From 1ca6a3f0652857419b0ff7e11a0e5f90b6babebf Mon Sep 17 00:00:00 2001 From: Rob Ashton Date: Thu, 22 Feb 2024 17:55:45 +0000 Subject: [PATCH 04/10] Don't explicitly refer to default branch Use value returned from test setup to avoid issues with master/main from config differences between local and CI --- tests/testthat/test-api.R | 2 +- tests/testthat/test-git.R | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-api.R b/tests/testthat/test-api.R index e85d893..fcc69a6 100644 --- a/tests/testthat/test-api.R +++ b/tests/testthat/test-api.R @@ -28,7 +28,7 @@ test_that("can list orderly reports", { vcapply(res$data, function(item) as.character(item[["name"]])) } - res <- endpoint$run("main") + res <- endpoint$run(repo$branch) expect_equal(res$status_code, 200) expect_setequal(get_reports_from_response(res), c("data", "parameters")) diff --git a/tests/testthat/test-git.R b/tests/testthat/test-git.R index 3e7f3f6..eda36b3 100644 --- a/tests/testthat/test-git.R +++ b/tests/testthat/test-git.R @@ -20,7 +20,7 @@ test_that("git ref to sha", { testthat::skip_on_cran() repo <- initialise_git_repo() other <- create_new_branch(repo$path, "other") - sha1 <- git_ref_to_sha("main", repo$path) + sha1 <- git_ref_to_sha(repo$branch, repo$path) sha2 <- git_ref_to_sha("other", repo$path) expect_match(sha1, "^[[:xdigit:]]{40}$") From c4f1ddb3257b50411e94b2f383b551b4b1e1f2ff Mon Sep 17 00:00:00 2001 From: Rob Ashton Date: Fri, 23 Feb 2024 14:38:00 +0000 Subject: [PATCH 05/10] Remove git shallow checkout and unused utils and helpers --- R/api.R | 17 ++++--- R/git.R | 37 -------------- R/util.R | 22 ++------- R/util_assert.R | 23 --------- inst/schema/report_list.json | 3 +- .../examples/data/{orderly.R => data.R} | 0 tests/testthat/helper-orderly-runner.R | 25 ---------- tests/testthat/test-api.R | 12 ++--- tests/testthat/test-git.R | 48 ------------------- tests/testthat/test-util-assert.R | 12 ----- 10 files changed, 22 insertions(+), 177 deletions(-) delete mode 100644 R/git.R delete mode 100644 R/util_assert.R rename tests/testthat/examples/data/{orderly.R => data.R} (100%) delete mode 100644 tests/testthat/test-git.R delete mode 100644 tests/testthat/test-util-assert.R diff --git a/R/api.R b/R/api.R index 382264e..660c590 100644 --- a/R/api.R +++ b/R/api.R @@ -35,11 +35,14 @@ root <- function() { ##' query hash :: string ##' state root :: root report_list <- function(root, hash) { - temp_root <- git_clone_depth_1(root, hash) - src_reports <- orderly2::orderly_list_src(temp_root, locate = FALSE) - lapply(src_reports, function(report_name) { - list( - name = scalar(report_name) - ) - }) + contents <- gert::git_ls(root, ref = hash) + re <- "^src/([^/]+)/(\\1|orderly)\\.(yml|R)$" + nms <- sub(re, "\\1", + grep(re, contents$path, value = TRUE, perl = TRUE), + perl = TRUE) + last_changed <- function(nm) { + max(contents$modified[startsWith(contents$path, sprintf("src/%s", nm))]) + } + data.frame(name = nms, + updated = vnapply(nms, last_changed, USE.NAMES = FALSE)) } diff --git a/R/git.R b/R/git.R deleted file mode 100644 index 29d0971..0000000 --- a/R/git.R +++ /dev/null @@ -1,37 +0,0 @@ -git_run <- function(args, repo = NULL, check = FALSE) { - git <- sys_which("git") - if (!is.null(repo)) { - args <- c("-C", repo, args) - } - res <- system3(git, args) - if (check && !res$success) { - stop(sprintf("Error code %d running command:\n%s", - res$code, paste0(" > ", res$output, collapse = "\n"))) - } - res -} - - -git_ref_to_sha <- function(ref, repo = NULL, check = FALSE) { - assert_scalar_character(ref) - res <- git_run(c("rev-parse", ref), repo = repo, check = FALSE) - if (res$success) { - res$output - } else if (check) { - stop(sprintf("Git reference '%s' not found", ref), call. = FALSE) - } else { - NA_character_ - } -} - - -git_clone_depth_1 <- function(origin, ref) { - t <- tempfile() - dir.create(t) - gert::git_init(t) - gert::git_remote_add(origin, repo = t) - sha <- git_ref_to_sha(ref, repo = origin, check = TRUE) - git_run(c("fetch", "--depth", "1", "origin", sha), repo = t) - git_run(c("checkout", "FETCH_HEAD"), repo = t) - t -} diff --git a/R/util.R b/R/util.R index d04efe5..9919c03 100644 --- a/R/util.R +++ b/R/util.R @@ -13,25 +13,11 @@ package_version_string <- function(name) { } -system3 <- function(command, args) { - res <- suppressWarnings(system2(command, args, stdout = TRUE, stderr = TRUE)) - code <- attr(res, "status") %||% 0 - attr(res, "status") <- NULL - list(success = code == 0, - code = code, - output = res) -} - - -sys_which <- function(name) { - path <- Sys.which(name) - if (!nzchar(path)) { - stop(sprintf("Did not find '%s'", name), call. = FALSE) - } - unname(path) +vcapply <- function(X, FUN, ...) { # nolint + vapply(X, FUN, character(1), ...) } -vcapply <- function(X, FUN, ...) { # nolint - vapply(X, FUN, character(1), ...) +vnapply <- function(X, FUN, ...) { # nolint + vapply(X, FUN, numeric(1), ...) } diff --git a/R/util_assert.R b/R/util_assert.R deleted file mode 100644 index 93ec841..0000000 --- a/R/util_assert.R +++ /dev/null @@ -1,23 +0,0 @@ -assert_scalar <- function(x, name = deparse(substitute(x)), arg = name, - call = NULL) { - if (length(x) != 1) { - cli::cli_abort(c("'{name}' must be a scalar", - i = "{name} has length {length(x)}"), - call = call, arg = arg) - } -} - - -assert_character <- function(x, name = deparse(substitute(x)), - arg = name, call = NULL) { - if (!is.character(x)) { - cli::cli_abort("'{name}' must be character", call = call, arg = arg) - } -} - - -assert_scalar_character <- function(x, name = deparse(substitute(x)), - arg = name, call = NULL) { - assert_scalar(x, name, arg = arg, call = call) - assert_character(x, name, arg = arg, call = call) -} diff --git a/inst/schema/report_list.json b/inst/schema/report_list.json index 9b9bdbc..40d76bb 100644 --- a/inst/schema/report_list.json +++ b/inst/schema/report_list.json @@ -4,7 +4,8 @@ "items": { "type": "object", "properties": { - "name": { "type": "string" } + "name": { "type": "string" }, + "updated": { "type": "number" } } } } diff --git a/tests/testthat/examples/data/orderly.R b/tests/testthat/examples/data/data.R similarity index 100% rename from tests/testthat/examples/data/orderly.R rename to tests/testthat/examples/data/data.R diff --git a/tests/testthat/helper-orderly-runner.R b/tests/testthat/helper-orderly-runner.R index c2899d2..c3da15d 100644 --- a/tests/testthat/helper-orderly-runner.R +++ b/tests/testthat/helper-orderly-runner.R @@ -42,28 +42,3 @@ helper_add_git <- function(path) { gert::git_remote_add(url, repo = path) list(path = path, user = user, branch = branch, sha = sha, url = url) } - - -initialise_git_repo <- function() { - t <- tempfile() - dir.create(t) - writeLines(c("# Example", "", "example repo"), file.path(t, "README.md")) - helper_add_git(t) -} - - -create_new_commit <- function(path, new_file = "new", message = "new message") { - writeLines("new file", file.path(path, new_file)) - gert::git_add(".", repo = path) - user <- "author " - gert::git_commit(message, author = user, committer = user, repo = path) -} - - -create_new_branch <- function(path, branch_name = "other") { - initial_branch <- gert::git_branch(repo = path) - gert::git_branch_create(branch_name, repo = path) - commit_sha <- create_new_commit(path, branch_name) - gert::git_branch_checkout(initial_branch, repo = path) - list(branch = branch_name, sha = commit_sha) -} diff --git a/tests/testthat/test-api.R b/tests/testthat/test-api.R index fcc69a6..f625c01 100644 --- a/tests/testthat/test-api.R +++ b/tests/testthat/test-api.R @@ -24,13 +24,13 @@ test_that("can list orderly reports", { path <- test_prepare_orderly_example(c("data", "parameters")) repo <- helper_add_git(path) endpoint <- orderly_runner_endpoint("GET", "/report/list", path) - get_reports_from_response <- function(res) { - vcapply(res$data, function(item) as.character(item[["name"]])) - } res <- endpoint$run(repo$branch) expect_equal(res$status_code, 200) - expect_setequal(get_reports_from_response(res), c("data", "parameters")) + expect_setequal(res$data$name, c("data", "parameters")) + # TODO: actually get modified time, looks like some issue with value + # coming back from gert::git_ls + expect_equal(res$data$update, c(0, 0)) ## Delete a report on a 2nd branch gert::git_branch_create("other", repo = path) @@ -42,11 +42,11 @@ test_that("can list orderly reports", { ## Can list items from this sha other_res <- endpoint$run(sha) expect_equal(other_res$status_code, 200) - expect_equal(get_reports_from_response(other_res), "parameters") + expect_equal(other_res$data$name, "parameters") ## We can still see all reports on main branch first_commit_res <- endpoint$run(repo$sha) expect_equal(first_commit_res$status_code, 200) - expect_setequal(get_reports_from_response(first_commit_res), + expect_setequal(first_commit_res$data$name, c("data", "parameters")) }) diff --git a/tests/testthat/test-git.R b/tests/testthat/test-git.R deleted file mode 100644 index eda36b3..0000000 --- a/tests/testthat/test-git.R +++ /dev/null @@ -1,48 +0,0 @@ -test_that("can do depth 1 clone", { - repo <- initialise_git_repo() - - first_commit <- repo$sha - second_commit <- create_new_commit(repo$path) - - first_clone <- git_clone_depth_1(repo$path, first_commit) - log <- gert::git_log(repo = first_clone) - expect_true(nrow(log) == 1) - expect_equal(log$commit, first_commit) - - second_clone <- git_clone_depth_1(repo$path, second_commit) - log <- gert::git_log(repo = second_clone) - expect_true(nrow(log) == 1) - expect_equal(log$commit, second_commit) -}) - - -test_that("git ref to sha", { - testthat::skip_on_cran() - repo <- initialise_git_repo() - other <- create_new_branch(repo$path, "other") - sha1 <- git_ref_to_sha(repo$branch, repo$path) - sha2 <- git_ref_to_sha("other", repo$path) - - expect_match(sha1, "^[[:xdigit:]]{40}$") - expect_match(sha2, "^[[:xdigit:]]{40}$") - expect_true(sha1 != sha2) - - expect_equal(git_ref_to_sha("HEAD", repo$path), sha1) - expect_equal(git_ref_to_sha(substr(sha1, 1, 7), repo$path), sha1) - - expect_identical(git_ref_to_sha("unknown", repo$path), - NA_character_) - expect_error(git_ref_to_sha("unknown", repo$path, TRUE), - "Git reference 'unknown' not found") -}) - - -test_that("handle failure", { - testthat::skip_on_cran() - repo <- initialise_git_repo() - r <- git_run("unknown-command", repo = repo$path) - expect_false(r$success) - expect_error( - git_run("unknown-command", repo = repo$path, check = TRUE), - r$output, fixed = TRUE) -}) diff --git a/tests/testthat/test-util-assert.R b/tests/testthat/test-util-assert.R deleted file mode 100644 index 73d9105..0000000 --- a/tests/testthat/test-util-assert.R +++ /dev/null @@ -1,12 +0,0 @@ -test_that("assert_scalar", { - expect_error(assert_scalar(NULL), "must be a scalar") - expect_error(assert_scalar(numeric(0)), "must be a scalar") - expect_error(assert_scalar(1:2), "must be a scalar") -}) - - -test_that("assert_character", { - expect_silent(assert_character("a")) - expect_error(assert_character(1), "must be character") - expect_error(assert_character(TRUE), "must be character") -}) From caf41ca20015516097a2922e6d3bc402cc856057 Mon Sep 17 00:00:00 2001 From: Rob Ashton Date: Fri, 23 Feb 2024 14:45:18 +0000 Subject: [PATCH 06/10] Add end to end test --- DESCRIPTION | 1 - tests/testthat/test-zzz-e2e.R | 22 ++++++++++++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index b726445..0f62af4 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -13,7 +13,6 @@ Roxygen: list(markdown = TRUE, roclets = c("rd", "namespace", "porcelain::porcel URL: https://github.com/mrc-ide/orderly.runner BugReports: https://github.com/mrc-ide/orderly.runner/issues Imports: - cli, docopt, gert, jsonlite, diff --git a/tests/testthat/test-zzz-e2e.R b/tests/testthat/test-zzz-e2e.R index 5df394e..7cc6ee9 100644 --- a/tests/testthat/test-zzz-e2e.R +++ b/tests/testthat/test-zzz-e2e.R @@ -1,8 +1,11 @@ +skip_if_not_installed("httr") +root <- test_prepare_orderly_example(c("data", "parameters")) +repo <- helper_add_git(root) +bg <- porcelain::porcelain_background$new(api, list(root)) +bg$start() +on.exit(bg$stop()) + test_that("can run server", { - skip_if_not_installed("httr") - root <- create_temporary_root(use_file_store = TRUE) - bg <- porcelain::porcelain_background$new(api, list(root)) - bg$start() r <- bg$request("GET", "/") expect_equal(httr::status_code(r), 200) @@ -14,3 +17,14 @@ test_that("can run server", { expect_equal(dat$data$orderly.runner, package_version_string("orderly.runner")) }) + +test_that("can list reports", { + r <- bg$request("GET", "/report/list?hash=HEAD") + expect_equal(httr::status_code(r), 200) + + dat <- httr::content(r) + expect_equal(dat$status, "success") + expect_null(dat$errors) + reports <- vcapply(dat$data, "[[", "name") + expect_true(all(c("data", "parameters") %in% reports)) +}) From ac474f8d4e4a53b05c32f1fc73e099f26ae98c58 Mon Sep 17 00:00:00 2001 From: Rob Ashton Date: Fri, 23 Feb 2024 17:24:39 +0000 Subject: [PATCH 07/10] Return the actual updated time --- R/api.R | 11 ++++++++--- inst/schema/report_list.json | 2 +- tests/testthat/test-api.R | 4 +--- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/R/api.R b/R/api.R index 660c590..8825095 100644 --- a/R/api.R +++ b/R/api.R @@ -30,19 +30,24 @@ root <- function() { lapply(versions, scalar) } + ##' @porcelain ##' GET /report/list => json(report_list) ##' query hash :: string ##' state root :: root report_list <- function(root, hash) { contents <- gert::git_ls(root, ref = hash) + ## Note there is a bug in current gert, and git_ls returns 0 for modified + ## and created time whenever called with a "ref". Get them via stat_files + ## for now + contents <- gert::git_stat_files(contents$path, ref = hash, repo = root) re <- "^src/([^/]+)/(\\1|orderly)\\.(yml|R)$" nms <- sub(re, "\\1", - grep(re, contents$path, value = TRUE, perl = TRUE), + grep(re, contents$file, value = TRUE, perl = TRUE), perl = TRUE) last_changed <- function(nm) { - max(contents$modified[startsWith(contents$path, sprintf("src/%s", nm))]) + max(contents$modified[startsWith(contents$file, sprintf("src/%s", nm))]) } data.frame(name = nms, - updated = vnapply(nms, last_changed, USE.NAMES = FALSE)) + updated_time = vnapply(nms, last_changed, USE.NAMES = FALSE)) } diff --git a/inst/schema/report_list.json b/inst/schema/report_list.json index 40d76bb..130b0de 100644 --- a/inst/schema/report_list.json +++ b/inst/schema/report_list.json @@ -5,7 +5,7 @@ "type": "object", "properties": { "name": { "type": "string" }, - "updated": { "type": "number" } + "updated_time": { "type": "number" } } } } diff --git a/tests/testthat/test-api.R b/tests/testthat/test-api.R index f625c01..ecd24f5 100644 --- a/tests/testthat/test-api.R +++ b/tests/testthat/test-api.R @@ -28,9 +28,7 @@ test_that("can list orderly reports", { res <- endpoint$run(repo$branch) expect_equal(res$status_code, 200) expect_setequal(res$data$name, c("data", "parameters")) - # TODO: actually get modified time, looks like some issue with value - # coming back from gert::git_ls - expect_equal(res$data$update, c(0, 0)) + expect_true(all(res$data$updated_time > (Sys.time() - 100))) ## Delete a report on a 2nd branch gert::git_branch_create("other", repo = path) From bcdfd9db4a6e7cfb791e4d81bae40941c805fc73 Mon Sep 17 00:00:00 2001 From: Rob Ashton Date: Mon, 26 Feb 2024 17:06:37 +0000 Subject: [PATCH 08/10] Use development version of gert from GitHub Current CRAN version contains an issue where if using git_ls() with a ref then filesize, modified and created times are all set to 0. This is fixed on GitHub --- DESCRIPTION | 3 ++- R/api.R | 8 ++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 0f62af4..4dbfe2c 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -28,4 +28,5 @@ Suggests: Config/testthat/edition: 3 Remotes: mrc-ide/orderly2, - reside-ic/porcelain + reside-ic/porcelain, + r-lib/gert diff --git a/R/api.R b/R/api.R index 8825095..5520ae5 100644 --- a/R/api.R +++ b/R/api.R @@ -37,16 +37,12 @@ root <- function() { ##' state root :: root report_list <- function(root, hash) { contents <- gert::git_ls(root, ref = hash) - ## Note there is a bug in current gert, and git_ls returns 0 for modified - ## and created time whenever called with a "ref". Get them via stat_files - ## for now - contents <- gert::git_stat_files(contents$path, ref = hash, repo = root) re <- "^src/([^/]+)/(\\1|orderly)\\.(yml|R)$" nms <- sub(re, "\\1", - grep(re, contents$file, value = TRUE, perl = TRUE), + grep(re, contents$path, value = TRUE, perl = TRUE), perl = TRUE) last_changed <- function(nm) { - max(contents$modified[startsWith(contents$file, sprintf("src/%s", nm))]) + max(contents$modified[startsWith(contents$path, sprintf("src/%s", nm))]) } data.frame(name = nms, updated_time = vnapply(nms, last_changed, USE.NAMES = FALSE)) From d2307d7a5fa5dcb297921fc4103da08bb538e56c Mon Sep 17 00:00:00 2001 From: Rob <39248272+r-ash@users.noreply.github.com> Date: Tue, 27 Feb 2024 14:01:39 +0000 Subject: [PATCH 09/10] Update R/util.R Co-authored-by: Rich FitzJohn --- R/util.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/util.R b/R/util.R index 9919c03..1cc2dd9 100644 --- a/R/util.R +++ b/R/util.R @@ -13,8 +13,8 @@ package_version_string <- function(name) { } -vcapply <- function(X, FUN, ...) { # nolint - vapply(X, FUN, character(1), ...) +vcapply <- function(...) { + vapply(FUN.VALUE = character(1), ...) } From 0808bb079e0b75ab06ccb80df2a04ab76cd43d3c Mon Sep 17 00:00:00 2001 From: Rob Ashton Date: Tue, 27 Feb 2024 15:16:07 +0000 Subject: [PATCH 10/10] Add minimum version number to gert --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 4dbfe2c..c1e7bd1 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -14,7 +14,7 @@ URL: https://github.com/mrc-ide/orderly.runner BugReports: https://github.com/mrc-ide/orderly.runner/issues Imports: docopt, - gert, + gert (>= 2.0.1), jsonlite, orderly2, porcelain,