From 7b235396b42ad4dfcd22fb177ebdb08c70468363 Mon Sep 17 00:00:00 2001 From: chainsawriot Date: Sat, 11 Feb 2023 11:22:16 +0100 Subject: [PATCH 1/3] add minor code clean up and documentation clarification --- R/installation.R | 31 +++++++++++++++---------------- man/dockerize.Rd | 6 +++--- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/R/installation.R b/R/installation.R index 730368f..d1ddfbe 100644 --- a/R/installation.R +++ b/R/installation.R @@ -192,10 +192,10 @@ gran_line <- which(basic_docker == "COPY gran.R ./gran.R") c(basic_docker[1:gran_line], "COPY materials/ ./materials/", - basic_docker[(gran_line+1):length(basic_docker)]) + basic_docker[(gran_line + 1):length(basic_docker)]) } -.generate_pre310_docker <- function(materials_dir,r_version, debian_version = "lenny", lib, sysreqs_cmd, cache) { +.generate_pre310_docker <- function(r_version, debian_version = "lenny", lib, sysreqs_cmd, cache) { basic_docker <- c( paste0("FROM debian/eol:", debian_version), "ENV TZ UTC", @@ -283,9 +283,9 @@ export_granlist <- function(granlist, path, granlist_as_comment = TRUE, verbose #' #' This function exports the result from [resolve()] to a Docker file. For R version >= 3.1.0, the Dockerfile is based on the versioned Rocker image. #' For R version < 3.1.0, the Dockerfile is based on Debian and it compiles R from source. -#' @param output_dir where to put the Docker file -#' @param materials_dir additional resources (e.g. analysis scripts) to be copied into `output_dir` -#' @param image character, which versioned Rocker image to use. Can only be "r-ver", "rstudio", "tidyverse", "verse", "geospatial". +#' @param output_dir character, where to put the Docker file and associated content +#' @param materials_dir character, path to the directiry containing dditional resources (e.g. analysis scripts) to be copied into `output_dir` and in turn into the Docker container +#' @param image character, which versioned Rocker image to use. Can only be "r-ver", "rstudio", "tidyverse", "verse", "geospatial" #' This applies only to R version <= 3.1 #' @param cache logical, whether to cache the content from CRAN now. Please note that the system requirements are not cached #' @param ... arguments to be passed to `dockerize` @@ -309,13 +309,16 @@ dockerize <- function(granlist, output_dir, materials_dir = NULL, image = c("r-v granlist_as_comment = TRUE, cache = FALSE, verbose = TRUE, lib = NA, cran_mirror = "https://cran.r-project.org/", check_cran_mirror = TRUE) { if (missing(output_dir)) { - stop("You must provide `output_dir`.") + stop("You must provide `output_dir`.", call. = FALSE) } if (!grepl("^ubuntu", granlist$os)) { - stop("System dependencies of ", granlist$os, " can't be dockerized.") + stop("System dependencies of ", granlist$os, " can't be dockerized.", call. = FALSE) } if (utils::compareVersion(granlist$r_version, "2.1") == -1) { - stop("`dockerize` doesn't support this R version (yet).") + stop("`dockerize` doesn't support this R version (yet):", granlist$r_version, call. = FALSE) + } + if (!is.null(materials_dir) && !(dir.exists(materials_dir))) { + stop(paste0("The folder ", materials_dir, " does not exist"), call. = FALSE) } image <- match.arg(image) sysreqs_cmd <- .consolidate_sysreqs(granlist) @@ -350,19 +353,15 @@ dockerize <- function(granlist, output_dir, materials_dir = NULL, image = c("r-v basic_docker <- .insert_cache_dir(basic_docker) } } - if(!is.null(materials_dir)){ - if(!dir.exists(materials_dir)){ - stop(paste0("The folder ",materials_dir," does not exist"),call. = FALSE) - } else{ - out_mat_dir <- paste0(output_dir,"/materials") - if (!dir.exists(out_mat_dir)) { - dir.create(out_mat_dir) + if (!(is.null(materials_dir))) { + out_mat_dir <- file.path(output_dir, "materials") + if (isFALSE(dir.exists(out_mat_dir))) { + dir.create(out_mat_dir) } file.copy(list.files(materials_dir, full.names = TRUE), out_mat_dir, recursive = TRUE) basic_docker <- .insert_materials_dir(basic_docker) - } } writeLines(basic_docker, file.path(output_dir, "Dockerfile")) invisible(output_dir) diff --git a/man/dockerize.Rd b/man/dockerize.Rd index 230af0c..2055cbc 100644 --- a/man/dockerize.Rd +++ b/man/dockerize.Rd @@ -29,11 +29,11 @@ dockerise_granlist(...) \arguments{ \item{granlist}{output from \code{\link[=resolve]{resolve()}}} -\item{output_dir}{where to put the Docker file} +\item{output_dir}{character, where to put the Docker file and associated content} -\item{materials_dir}{additional resources (e.g. analysis scripts) to be copied into \code{output_dir}} +\item{materials_dir}{character, path to the directiry containing dditional resources (e.g. analysis scripts) to be copied into \code{output_dir} and in turn into the Docker container} -\item{image}{character, which versioned Rocker image to use. Can only be "r-ver", "rstudio", "tidyverse", "verse", "geospatial". +\item{image}{character, which versioned Rocker image to use. Can only be "r-ver", "rstudio", "tidyverse", "verse", "geospatial" This applies only to R version <= 3.1} \item{granlist_as_comment}{logical, whether to write granlist and the steps to reproduce From 5bf4fea53d6a4ec7262f785c7653dd00aa5de134 Mon Sep 17 00:00:00 2001 From: chainsawriot Date: Sat, 11 Feb 2023 11:31:30 +0100 Subject: [PATCH 2/3] Modify test cases to prevent collision of directories (without cleaning up) --- tests/testthat/test_dockerize.R | 19 ++++++++++++------- tests/testthat/test_resolve.R | 8 ++++++-- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/tests/testthat/test_dockerize.R b/tests/testthat/test_dockerize.R index c715b5d..c9c70a6 100644 --- a/tests/testthat/test_dockerize.R +++ b/tests/testthat/test_dockerize.R @@ -1,3 +1,7 @@ +.gen_temp_dir <- function() { + file.path(tempdir(), paste(sample(c(LETTERS, letters), 20, replace = TRUE), collapse = "")) +} + test_that("defensive programming", { graph <- readRDS("../testdata/sle_graph.RDS") expect_error(dockerize(graph, output_dir = tempdir())) @@ -5,7 +9,7 @@ test_that("defensive programming", { test_that("integration of #13 in dockerize()", { gran_ok <- readRDS("../testdata/gran_ok.RDS") - temp_dir <- file.path(tempdir(), sample(1:10000, size = 1)) + temp_dir <- .gen_temp_dir() dockerize(granlist = gran_ok, output_dir = temp_dir) ## granlist_as_comment = TRUE x <- readLines(file.path(temp_dir, "gran.R")) expect_true(any(grepl("^## ## To reconstruct this file", x))) @@ -17,7 +21,7 @@ test_that("integration of #13 in dockerize()", { test_that("integration of #16 in dockerize()", { ## verbose gran_ok <- readRDS("../testdata/gran_ok.RDS") - temp_dir <- file.path(tempdir(), sample(1:10000, size = 1)) + temp_dir <- .gen_temp_dir() dockerize(granlist = gran_ok, output_dir = temp_dir) ## verbose = TRUE x <- readLines(file.path(temp_dir, "gran.R")) expect_true(any(grepl("^verbose <- TRUE", x))) @@ -39,7 +43,7 @@ test_that("integration of #16 in dockerize()", { test_that("integration of #18 in dockerize()", { gran_ok <- readRDS("../testdata/gran_ok.RDS") - temp_dir <- file.path(tempdir(), sample(1:10000, size = 1)) + temp_dir <- .gen_temp_dir() dockerize(granlist = gran_ok, output_dir = temp_dir) ## cran_mirror = "https://cran.r-project.org/" x <- readLines(file.path(temp_dir, "gran.R")) expect_true(any(grepl("^cran_mirror <- \"https://cran\\.r\\-project\\.org/\"", x))) @@ -58,7 +62,7 @@ test_that("integration of #18 in dockerize()", { test_that("integration of #20 to dockerize()", { gran_ok <- readRDS("../testdata/gran_ok.RDS") expect_equal(gran_ok$r_version, "4.2.2") - temp_dir <- file.path(tempdir(), sample(1:10000, size = 1)) + temp_dir <- .gen_temp_dir() dockerize(gran_ok, output_dir = temp_dir) ## cran_mirror = "https://cran.r-project.org/" x <- readLines(file.path(temp_dir, "gran.R")) expect_true(any(grepl("^cran_mirror <- \"https://cran\\.r\\-project\\.org/\"", x))) @@ -78,7 +82,7 @@ test_that("integration of #20 to dockerize()", { test_that("Dockerize R < 3.1 and >= 2.1", { gran_rio <- readRDS("../testdata/gran_rio_old.RDS") expect_equal(gran_rio$r_version, "3.0.1") - temp_dir <- file.path(tempdir(), sample(1:10000, size = 1)) + temp_dir <- .gen_temp_dir() dockerize(gran_rio, output_dir = temp_dir) expect_true(file.exists(file.path(temp_dir, "compile_r.sh"))) Dockerfile <- readLines(file.path(temp_dir, "Dockerfile")) @@ -92,7 +96,7 @@ test_that("Dockerize R < 3.1 and >= 2.1", { test_that("Docker R < 2.1", { gran_rio <- readRDS("../testdata/gran_rio_old.RDS") gran_rio$r_version <- "2.1.0" ## exactly 2.1.0, no error - temp_dir <- file.path(tempdir(), sample(1:10000, size = 1)) + temp_dir <- .gen_temp_dir() expect_error(dockerize(gran_rio, output_dir = temp_dir), NA) gran_rio <- readRDS("../testdata/gran_rio_old.RDS") gran_rio$r_version <- "2.0.0" @@ -115,6 +119,7 @@ test_that(".consolidate_sysreqs and issue #21", { test_that("Dockerize warning, issue #21", { graph <- readRDS("../testdata/issue21.RDS") - temp_dir <- file.path(tempdir(), sample(1:10000, size = 1)) + temp_dir <- .gen_temp_dir() expect_warning(dockerize(graph, output_dir = temp_dir)) }) + diff --git a/tests/testthat/test_resolve.R b/tests/testthat/test_resolve.R index c25c452..61278ad 100644 --- a/tests/testthat/test_resolve.R +++ b/tests/testthat/test_resolve.R @@ -1,3 +1,7 @@ +.gen_temp_dir <- function() { + file.path(tempdir(), paste(sample(c(LETTERS, letters), 20, replace = TRUE), collapse = "")) +} + test_that("defensive programming", { expect_error(resolve("LDAvis", os = "windows")) }) @@ -48,7 +52,7 @@ test_that("cache #17", { skip_if_offline() skip_on_cran() gran_ok <- readRDS("../testdata/gran_ok.RDS") - temp_dir <- file.path(tempdir(), sample(1:10000, size = 1)) + temp_dir <- .gen_temp_dir() dockerize(gran_ok, output_dir = temp_dir) ## cache = FALSE x <- readLines(file.path(temp_dir, "Dockerfile")) expect_false(any(grepl("^COPY cache", x))) @@ -71,7 +75,7 @@ test_that("cache for R < 3.1 and R >= 2.1", { skip_on_cran() gran_rio <- readRDS("../testdata/gran_rio_old.RDS") expect_equal(gran_rio$r_version, "3.0.1") - temp_dir <- file.path(tempdir(), sample(1:10000, size = 1)) + temp_dir <- .gen_temp_dir() dockerize(gran_rio, output_dir = temp_dir, cache = TRUE, verbose = FALSE) x <- readLines(file.path(temp_dir, "Dockerfile")) expect_true(any(grepl("^COPY cache", x))) From 0c3455f30cb204080b9a54e9faa0254c598d71ac Mon Sep 17 00:00:00 2001 From: chainsawriot Date: Sat, 11 Feb 2023 20:28:43 +0100 Subject: [PATCH 3/3] Add tests for #23 close #23 --- tests/testthat/test_dockerize.R | 71 ++++++++++++++++++++++++++++++++- 1 file changed, 69 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test_dockerize.R b/tests/testthat/test_dockerize.R index c9c70a6..511a5b6 100644 --- a/tests/testthat/test_dockerize.R +++ b/tests/testthat/test_dockerize.R @@ -74,7 +74,7 @@ test_that("integration of #20 to dockerize()", { gran_ok <- readRDS("../testdata/gran_ok.RDS") gran_ok$r_version <- "3.2.0" dockerize(gran_ok, output_dir = temp_dir) ## cran_mirror = "https://cran.r-project.org/" - x <- readLines(file.path(temp_dir, "gran.R")) + x <- readLines(file.path(temp_dir, "gran.R")) expect_false(any(grepl("^cran_mirror <- \"https://cran\\.r\\-project\\.org/\"", x))) expect_true(any(grepl("^cran_mirror <- \"http://cran\\.r\\-project\\.org/\"", x))) }) @@ -100,7 +100,7 @@ test_that("Docker R < 2.1", { expect_error(dockerize(gran_rio, output_dir = temp_dir), NA) gran_rio <- readRDS("../testdata/gran_rio_old.RDS") gran_rio$r_version <- "2.0.0" - expect_error(dockerize(gran_rio, output_dir = temp_dir)) + expect_error(dockerize(gran_rio, output_dir = temp_dir)) }) test_that(".consolidate_sysreqs and issue #21", { @@ -123,3 +123,70 @@ test_that("Dockerize warning, issue #21", { expect_warning(dockerize(graph, output_dir = temp_dir)) }) +test_that("material_dir, non-existing, #23", { + ## normal case + gran_rio <- readRDS("../testdata/gran_rio_old.RDS") + temp_dir <- .gen_temp_dir() + expect_error(dockerize(gran_rio, output_dir = temp_dir, materials_dir = NULL), NA) + ## non-existing + fake_material_dir <- .gen_temp_dir() + expect_false(dir.exists(fake_material_dir)) + expect_error(dockerize(gran_rio, output_dir = temp_dir, materials_dir = fake_material_dir)) +}) + +test_that("material_dir, existing, no subdir, #23", { + ## exist, but empty dir + ## Pre R 3.1.0 + gran_rio <- readRDS("../testdata/gran_rio_old.RDS") + temp_dir <- .gen_temp_dir() + fake_material_dir <- .gen_temp_dir() + dir.create(fake_material_dir) + dockerize(gran_rio, output_dir = temp_dir, materials_dir = fake_material_dir) + expect_true(dir.exists(file.path(temp_dir, "materials"))) + expect_equal(list.files(file.path(temp_dir, "materials")), character(0)) + expect_true(any(readLines(file.path(temp_dir, "Dockerfile")) == "COPY materials/ ./materials/")) + ## Post R 3.1.0 + graph <- readRDS("../testdata/graph.RDS") + temp_dir <- .gen_temp_dir() + fake_material_dir <- .gen_temp_dir() + dir.create(fake_material_dir) + dockerize(graph, output_dir = temp_dir, materials_dir = fake_material_dir) + expect_true(dir.exists(file.path(temp_dir, "materials"))) + expect_equal(list.files(file.path(temp_dir, "materials")), character(0)) + expect_true(any(readLines(file.path(temp_dir, "Dockerfile")) == "COPY materials/ ./materials/")) + ## Will only test post 3.1.0 from now on + ## some files in fake_material_dir + temp_dir <- .gen_temp_dir() + fake_material_dir <- .gen_temp_dir() + dir.create(fake_material_dir) + file.copy("../testdata/graph.RDS", file.path(fake_material_dir, "graph.RDS")) + writeLines(c("831721", "GESIS"), file.path(fake_material_dir, "test.R")) + dockerize(graph, output_dir = temp_dir, materials_dir = fake_material_dir) + expect_true(dir.exists(file.path(temp_dir, "materials"))) + expect_equal(list.files(file.path(temp_dir, "materials")), c("graph.RDS", "test.R")) + expect_true(any(readLines(file.path(temp_dir, "Dockerfile")) == "COPY materials/ ./materials/")) + expect_true(file.exists(file.path(temp_dir, "materials", "graph.RDS"))) + expect_true(file.exists(file.path(temp_dir, "materials", "test.R"))) + content <- readLines(file.path(temp_dir, "materials", "test.R")) + expect_equal(content[1], "831721") + expect_equal(content[2], "GESIS") +}) + +test_that("material_dir, existing, with 1 subdir, #23", { + temp_dir <- .gen_temp_dir() + fake_material_dir <- .gen_temp_dir() + dir.create(fake_material_dir) + dir.create(file.path(fake_material_dir, "data")) + file.copy("../testdata/graph.RDS", file.path(fake_material_dir, "data", "graph.RDS")) + writeLines(c("831721", "GESIS"), file.path(fake_material_dir, "test.R")) + graph <- readRDS("../testdata/graph.RDS") + dockerize(graph, output_dir = temp_dir, materials_dir = fake_material_dir) + expect_true(dir.exists(file.path(temp_dir, "materials"))) + expect_true(any(readLines(file.path(temp_dir, "Dockerfile")) == "COPY materials/ ./materials/")) + expect_true(dir.exists(file.path(temp_dir, "materials", "data"))) + expect_true(file.exists(file.path(temp_dir, "materials", "data", "graph.RDS"))) + expect_true(file.exists(file.path(temp_dir, "materials", "test.R"))) + content <- readLines(file.path(temp_dir, "materials", "test.R")) + expect_equal(content[1], "831721") + expect_equal(content[2], "GESIS") +})