From 4115c0d554e8201ca2e6462c20e6cd5583209c13 Mon Sep 17 00:00:00 2001 From: Sam Parmar <107635309+parmsam-pfizer@users.noreply.github.com> Date: Tue, 7 Mar 2023 14:25:44 -0500 Subject: [PATCH 1/6] add arg to capture repo urls and ensure unit tests --- R/axecute.R | 10 +++++++-- R/get.R | 12 ++++++++++ R/log.R | 16 +++++++++++-- R/writer.R | 22 ++++++++++++++++++ man/axecute.Rd | 6 ++++- man/log_write.Rd | 6 ++++- tests/testthat/test-axecute.R | 42 +++++++++++++++++++++++++++++++++++ tests/testthat/test-log.R | 2 +- 8 files changed, 109 insertions(+), 7 deletions(-) diff --git a/R/axecute.R b/R/axecute.R index 7af7930..3dd30cf 100644 --- a/R/axecute.R +++ b/R/axecute.R @@ -17,6 +17,8 @@ #' * messages: any messages generated by program execution #' * output: any output generated by program execution #' * result: any result generated by program execution +#' @param show_repo_url Boolean. Should the repo URLs be reported +#' Defaults to FALSE #' #' @importFrom purrr map_chr #' @@ -35,7 +37,8 @@ axecute <- function(file, log_name = NA, log_path = NA, remove_log_object = TRUE, quit_on_error = TRUE, - to_report = c("messages", "output", "result")){ + to_report = c("messages", "output", "result"), + show_repo_url = FALSE){ # lower everything for consistency and check values to_report <- map_chr(to_report, tolower) @@ -51,7 +54,10 @@ axecute <- function(file, log_name = NA, any_errors <- get_log_element("errors") # write log - log_write(file = file, remove_log_object = remove_log_object, to_report = to_report) + log_write(file = file, + remove_log_object = remove_log_object, + to_report = to_report, + show_repo_url = show_repo_url) # if error, quit with status = 1 if not interactive if(!interactive() & !is.null(any_errors) & quit_on_error) { diff --git a/R/get.R b/R/get.R index 9870557..442ba0e 100644 --- a/R/get.R +++ b/R/get.R @@ -289,3 +289,15 @@ get_lint_results <- function(file) { lint(file, getOption('log.rx.lint')) } } + +#' Get repository URLs +#' +#' Obtain repository URLs possibly used to install packages in session +#' +#' @return results from `getOption("repos")` as list +#' +#' @noRd +#' +get_repo_urls <- function() { + as.list(getOption("repos")) +} diff --git a/R/log.R b/R/log.R index 7e10223..5322618 100644 --- a/R/log.R +++ b/R/log.R @@ -83,7 +83,8 @@ log_config <- function(file = NA, log_name = NA, log_path = NA){ "unapproved_packages_functions", "lint_results", "log_name", - "log_path") + "log_path", + "repo_urls") # Add attributes to the log.rx environment, and set them to NA for (key in 1:length(keys)){ @@ -108,6 +109,8 @@ log_config <- function(file = NA, log_name = NA, log_path = NA){ set_log_name_path(log_name, log_path) # lint results set_log_element("lint_results", get_lint_results(file)) + # repo urls + set_log_element("repo_urls", get_repo_urls()) } #' Cleaning-up of log.rx object @@ -154,6 +157,8 @@ log_cleanup <- function() { #' writing the log file? Defaults to TRUE #' @param to_report String vector. Objects to optionally report; additional #' information in \code{\link{axecute}} +#' @param show_repo_url Boolean. Should the repo URLs be reported +#' Defaults to FALSE #' #' @return Nothing #' @export @@ -177,7 +182,8 @@ log_cleanup <- function() { #' log_write(file) log_write <- function(file = NA, remove_log_object = TRUE, - to_report = c("messages", "output", "result")){ + to_report = c("messages", "output", "result"), + show_repo_url = FALSE){ # Set end time and run time set_log_element("end_time", strftime(Sys.time(), usetz = TRUE)) set_log_element("run_time", @@ -210,6 +216,12 @@ log_write <- function(file = NA, write_log_header("Session Information"), write_session_info()) + if (show_repo_url) { + cleaned_log_vec <- c(cleaned_log_vec, + write_log_header("Repo URLs"), + write_repo_urls()) + } + if ("masked_functions" %in% names(log_cleanup())) { cleaned_log_vec <- c(cleaned_log_vec, write_log_header("Masked Functions"), diff --git a/R/writer.R b/R/writer.R index f10ac82..4bde2f2 100644 --- a/R/writer.R +++ b/R/writer.R @@ -62,6 +62,28 @@ write_session_info <- function(){ return(session_info) } +#' Format repo URLs for writing +#' +#' @return A vector of file name and path prefixed +#' +#' @noRd +#' +write_repo_urls <- function(){ + repo_urls <- ifelse(is.na(get_log_element("repo_urls")), + "Repo URLs not able to be determined", + map2( + names(get_log_element("repo_urls")), + get_log_element("repo_urls"), + ~paste(paste0(.x, ": "), + paste0(.y, collapse = ", ")) + ) %>% + unname() %>% + unlist() + ) + + return(repo_urls) +} + #' Format file name and path for writing #' #' @return A vector of file name and path prefixed diff --git a/man/axecute.Rd b/man/axecute.Rd index 89e8e67..8a385c5 100644 --- a/man/axecute.Rd +++ b/man/axecute.Rd @@ -10,7 +10,8 @@ axecute( log_path = NA, remove_log_object = TRUE, quit_on_error = TRUE, - to_report = c("messages", "output", "result") + to_report = c("messages", "output", "result"), + show_repo_url = FALSE ) } \arguments{ @@ -33,6 +34,9 @@ many as necessary: \item output: any output generated by program execution \item result: any result generated by program execution }} + +\item{show_repo_url}{Boolean. Should the repo URLs be reported +Defaults to FALSE} } \value{ 0 if there are no errors or 1 if there are any errors diff --git a/man/log_write.Rd b/man/log_write.Rd index d269420..4ea1713 100644 --- a/man/log_write.Rd +++ b/man/log_write.Rd @@ -7,7 +7,8 @@ log_write( file = NA, remove_log_object = TRUE, - to_report = c("messages", "output", "result") + to_report = c("messages", "output", "result"), + show_repo_url = FALSE ) } \arguments{ @@ -18,6 +19,9 @@ writing the log file? Defaults to TRUE} \item{to_report}{String vector. Objects to optionally report; additional information in \code{\link{axecute}}} + +\item{show_repo_url}{Boolean. Should the repo URLs be reported +Defaults to FALSE} } \value{ Nothing diff --git a/tests/testthat/test-axecute.R b/tests/testthat/test-axecute.R index 8f2dca2..932e01e 100644 --- a/tests/testthat/test-axecute.R +++ b/tests/testthat/test-axecute.R @@ -66,3 +66,45 @@ test_that("to_report works to control log output elements", { rm(flines, con, scriptPath, logDir) log_remove() }) + +test_that("show_repo_url works to show repo url elements", { + options("log.rx" = NULL) + scriptPath <- tempfile() + logDir <- tempdir() + writeLines( + c("message('hello logrx')", + "cat('this is output')", + "data.frame(c(8, 6, 7, 5, 3, 0, 9))"), + con = scriptPath) + + # check no log is currently written out + expect_warning(expect_error(file(file.path(logDir, "log_out_repo_url"), "r"), "cannot open the connection")) + + axecute(scriptPath, log_name = "log_out_repo_url", + log_path = logDir, + remove_log_object = FALSE, + show_repo_url = TRUE + ) + con <- file(file.path(logDir, "log_out_repo_url"), "r") + flines <- readLines(con) + close(con) + + expect_true(grepl(paste(write_log_header("Repo URLs"), collapse = ','), + paste(flines,collapse = ','))) + rm(flines, con) + log_remove() + + axecute(scriptPath, log_name = "log_out_repo_url2", + log_path = logDir, + remove_log_object = FALSE, + show_repo_url = FALSE + ) + con <- file(file.path(logDir, "log_out_repo_url2"), "r") + flines <- readLines(con) + close(con) + + expect_false(grepl(paste(write_log_header("Repo URLs"), collapse = ','), + paste(flines,collapse = ','))) + rm(flines, con, scriptPath, logDir) + log_remove() +}) diff --git a/tests/testthat/test-log.R b/tests/testthat/test-log.R index d8ab347..e26338e 100644 --- a/tests/testthat/test-log.R +++ b/tests/testthat/test-log.R @@ -13,7 +13,7 @@ test_that("log_config configures the log and all the necessary elements", { "result","output","start_time", "end_time", "run_time", "file_name","file_path","user", "hash_sum", "masked_functions", "used_packages_functions", "unapproved_packages_functions", - "lint_results", "log_name","log_path")) + "lint_results", "log_name","log_path", "repo_urls")) expect_identical(getOption("log.rx")[['file_path']], dirname(get_file_path('./test-get.R'))) expect_identical(getOption("log.rx")[['file_name']], basename(get_file_path('./test-get.R'))) From 7c4785a336c401f9f462d36784d4af4a90a6f96d Mon Sep 17 00:00:00 2001 From: Sam Parmar <107635309+parmsam-pfizer@users.noreply.github.com> Date: Tue, 7 Mar 2023 14:35:41 -0500 Subject: [PATCH 2/6] update news md --- NEWS.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS.md b/NEWS.md index 46a8d26..745b583 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,7 @@ +# logrx 0.2.2 + +- Add `show_repo_url` option in `axecute()` to capture repo URL(s) into log file + # logrx 0.2.1 - non-function objects are no longer returned as functions by `get_used_functions` (#154) From 7cdac300793573a8ad9bad2469fd4126d68e86e5 Mon Sep 17 00:00:00 2001 From: Sam Parmar <107635309+parmsam-pfizer@users.noreply.github.com> Date: Mon, 10 Jul 2023 13:46:59 -0400 Subject: [PATCH 3/6] fix unit tests --- tests/testthat/test-axecute.R | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-axecute.R b/tests/testthat/test-axecute.R index 99807fd..ac45188 100644 --- a/tests/testthat/test-axecute.R +++ b/tests/testthat/test-axecute.R @@ -68,7 +68,17 @@ test_that("to_report works to control log output elements", { }) test_that("show_repo_url works to show repo url elements", { - expect_warning(expect_error(file(file.path(logDir, "log_out_repo_url"), "r"), "cannot open the connection")) + options("log.rx" = NULL) + scriptPath <- tempfile() + logDir <- tempdir() + writeLines( + c("message('hello logrx')", + "cat('this is output')", + "data.frame(c(8, 6, 7, 5, 3, 0, 9))"), + con = scriptPath) + + # check no log is currently written out + expect_warning(expect_error(file(file.path(logDir, "log_out_repo_url"), "r"), "cannot open the connection")) axecute(scriptPath, log_name = "log_out_repo_url", log_path = logDir, @@ -99,7 +109,6 @@ test_that("show_repo_url works to show repo url elements", { }) test_that("include_rds works to output log as rds", { - options("log.rx" = NULL) scriptPath <- tempfile() logDir <- tempdir() From 3e0257f2127ec780bdac15b4628e901ca3eda43f Mon Sep 17 00:00:00 2001 From: Sam Parmar <107635309+parmsam-pfizer@users.noreply.github.com> Date: Mon, 10 Jul 2023 13:47:07 -0400 Subject: [PATCH 4/6] update doc --- man/log_write.Rd | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/man/log_write.Rd b/man/log_write.Rd index bff8a7b..906dc09 100644 --- a/man/log_write.Rd +++ b/man/log_write.Rd @@ -7,7 +7,7 @@ log_write( file = NA, remove_log_object = TRUE, - show_repo_url = FALSE + show_repo_url = FALSE, include_rds = FALSE, to_report = c("messages", "output", "result") ) @@ -18,14 +18,14 @@ log_write( \item{remove_log_object}{Boolean. Should the log object be removed after writing the log file? Defaults to TRUE} +\item{show_repo_url}{Boolean. Should the repo URLs be reported +Defaults to FALSE} + \item{include_rds}{Boolean. Option to export log object as Rds file. Defaults to FALSE} \item{to_report}{String vector. Objects to optionally report; additional information in \code{\link{axecute}}} - -\item{show_repo_url}{Boolean. Should the repo URLs be reported -Defaults to FALSE} } \value{ Nothing From 3c3d70103edb7ab28b65f7da8a71e984b645140d Mon Sep 17 00:00:00 2001 From: Sam Parmar <107635309+parmsam-pfizer@users.noreply.github.com> Date: Thu, 27 Jul 2023 09:33:45 -0500 Subject: [PATCH 5/6] Update NEWS.md Co-authored-by: Nicholas Masel <61123199+nicholas-masel@users.noreply.github.com> --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index ad63559..58a8c7e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,6 @@ # logrx 0.3.0 -- Add `show_repo_url` option in `axecute()` to capture repo URL(s) into log file +- Add `show_repo_url` option in `axecute()` to capture repo URL(s) into log file (#167) - Moved website theme to bootstarp 5, enabled search (#179) - Add `include_rds` argument to `axecute()` to export log as rds file From c5809caa254bf99f91dd4abbf758370c36c84713 Mon Sep 17 00:00:00 2001 From: Sam Parmar <107635309+parmsam-pfizer@users.noreply.github.com> Date: Thu, 27 Jul 2023 09:33:55 -0500 Subject: [PATCH 6/6] Update R/axecute.R Co-authored-by: Nicholas Masel <61123199+nicholas-masel@users.noreply.github.com> --- R/axecute.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/axecute.R b/R/axecute.R index aa45f84..48efd02 100644 --- a/R/axecute.R +++ b/R/axecute.R @@ -19,7 +19,7 @@ #' * messages: any messages generated by program execution #' * output: any output generated by program execution #' * result: any result generated by program execution -#' @param show_repo_url Boolean. Should the repo URLs be reported +#' @param show_repo_url Boolean. Should the repository URLs be reported #' Defaults to FALSE #' #' @importFrom purrr map_chr