diff --git a/NAMESPACE b/NAMESPACE index ff40220..92b7aef 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -94,12 +94,15 @@ S3method(report_finalize,reporter_basic_tty) S3method(report_sleep,default) S3method(report_sleep,reporter_ansi_tty) S3method(report_sleep,reporter_basic_tty) +S3method(report_start_checks,"NULL") S3method(report_start_checks,reporter_ansi_tty) +S3method(report_start_setup,"NULL") S3method(report_start_setup,reporter_ansi_tty) S3method(report_start_setup,reporter_basic_tty) S3method(report_status,"NULL") S3method(report_status,reporter_ansi_tty) S3method(report_status,reporter_basic_tty) +S3method(report_step,"NULL") S3method(report_step,reporter_ansi_tty) S3method(report_step,reporter_basic_tty) S3method(report_task,reporter_ansi_tty) @@ -119,7 +122,7 @@ S3method(start_task,igraph.vs) S3method(start_task,install_task) S3method(task_graph,task) S3method(task_graph,task_graph) -export(check_dev_rev_deps) +export(check_pkgs) export(check_rev_deps) export(check_task) export(checker) diff --git a/R/check.R b/R/check.R index 12f1111..a5a19ab 100644 --- a/R/check.R +++ b/R/check.R @@ -70,29 +70,29 @@ check_rev_deps <- function( output = output, lib.loc = lib.loc, repos = repos, + restore = restore ) run(checks, ...) checks } -#' Run reverse dependency checks against a development version only +#' Check packages #' -#' [`check_dev_rev_deps()`] works similarly to [`check_rev_deps()`] but it runs -#' R CMD check only once for each package, with the development version of the -#' package installed. It is advantageous to check whether adding a new package -#' into a repository breaks existing packages that possibly take said package -#' as a `Suggests` dependency. +#' Runs classical `R CMD check` for the given source package. It +#' first identifies and installs, in parallel, all dependencies required +#' to check the package. Then, it runs `R CMD check` for each specified package. #' #' @inheritParams check_functions #' @inheritParams run +#' @inheritParams plan_local_checks #' #' @inherit check_functions return #' #' @family checks #' @export -check_dev_rev_deps <- function( - path, +check_pkgs <- function( + package, n = 2L, output = tempfile(paste(utils::packageName(), Sys.Date(), sep = "-")), lib.loc = .libPaths(), @@ -101,7 +101,7 @@ check_dev_rev_deps <- function( ... ) { checks <- checker$new( - plan_rev_dep_checks(path = path, repos = repos, versions = "dev"), + plan_local_checks(package = package, repos = repos), n = n, output = output, lib.loc = lib.loc, diff --git a/R/check_process.R b/R/check_process.R index 00603a3..30c6c02 100644 --- a/R/check_process.R +++ b/R/check_process.R @@ -1,17 +1,35 @@ # Regular Expression for Parsing R CMD check checks # nolint start, styler: off RE_CHECK <- paste0( - "(?<=^|\n)", # starts on a new line (or start of string) - "\\* checking ", # literal "* checking " - "(?.*?)", # capture any check content as "check" - " \\.\\.\\.", # until literal "..." - "(?:", # ignore additional check details: - "[\\s\\*]{2,}", # any content starting with two or more of spaces or "*" - ".*?(?:\n|$)", # until a newline (or end of string) - ")*", # repeating until - "(?.*?)", # capturing a status as "status" - "(?=\n|$)" # terminated by a new line (or end of string) + "(?<=^|\\n)", # must start at beginning of string OR right after a newline + "\\* checking ", # literal "* checking " + "(?.*?)", # capture check name/content (non-greedy) as "check" + " \\.\\.\\.", # followed by literal " ..." + "(?:", # zero or more "detail" lines that belong to this check + "\\n[ \\t]*(?=\\n)", # a blank line (newline + optional spaces/tabs + next newline) + "|", + "\\n", # or: a normal detail line starting on the next line + "(?:[ \\t]{2,}", # either indented (2+ spaces/tabs) + "|\\*(?! (?:DONE|checking )))", # or a '*' line that is NOT "* DONE" and NOT "* checking ..." + "[^\\n]*(?:\\n|$)", # consume the rest of that detail line (to newline/end) + ")*", + "[ \\t]*", # allow extra spaces/tabs after "..." on the SAME line + "(?:", # position the engine right before a status token if one exists + # Case 1: status token is on the current line (possibly preceded by comment text) + "(?:[^\\n]*[ \\t]+)?(?=(?:[A-Z]{2}[A-Z0-9_-]*)\\s*(?:\\n|$))", + "|", + # Case 2: status token is on the next line: + # consume remainder of current line + newline + optional indent, + # but only if the next thing is a status token at end-of-line + "[^\\n]*\\n[ \\t]*(?=(?:[A-Z]{2}[A-Z0-9_-]*)\\s*(?:\\n|$))", + "|", + # Case 3: no status token (eat remainder/comment and stop here) + "[^\\n]*", + ")", + "(?(?:[A-Z]{2}[A-Z0-9_-]*)|)", # capture status token, or capture empty string if absent + "(?=\\s*(?:\\n|$))" # must end at newline/end (allow trailing whitespace) ) + # nolint end, styler: on #' @importFrom R6 R6Class @@ -46,9 +64,17 @@ check_process <- R6::R6Class( if (!self$is_alive()) callback(self) }, finish = function() { - self$poll_output() - self$save_results() - if (is.function(f <- private$finish_callback)) f(self) + # self$checks active binding calls poll_output so there is not need + # to call it explicitly + checks <- self$checks + # In some cases, check subprocess might suffer from a race condition, when + # process itself finished, but the final results of the last subcheck + # are not yet available to parse. Therefore we allow the process to + # finalize only if the last subcheck has reported status. + if (checks[length(checks)] != "") { + self$save_results() + if (is.function(f <- private$finish_callback)) f(self) + } }, get_time_last_check_start = function() { private$time_last_check_start diff --git a/R/checker.R b/R/checker.R index b87e95e..16adfb5 100644 --- a/R/checker.R +++ b/R/checker.R @@ -151,7 +151,6 @@ checker <- R6::R6Class( #' @return A integer value, coercible to logical to indicate whether a new #' process was spawned, or `-1` if all tasks have finished. start_next_task = function() { - # finish any finished processes for (process in private$active) { if (!process$is_alive()) { process$finish() diff --git a/R/pkg_origin.R b/R/pkg_origin.R index 98f1106..bbbf1c4 100644 --- a/R/pkg_origin.R +++ b/R/pkg_origin.R @@ -212,14 +212,14 @@ pkg_deps.pkg_origin_local <- function( dependencies = dependencies, db = row ) - direct_deps$depth <- "direct" + direct_deps$depth <- rep.int("direct", NROW(direct_deps)) indirect_deps <- pkg_dependencies( packages = direct_deps$name, dependencies = dependencies, db = db ) - indirect_deps$depth <- "indirect" + indirect_deps$depth <- rep.int("indirect", NROW(indirect_deps)) rbind(direct_deps, indirect_deps) } diff --git a/R/plan.R b/R/plan.R index 7db8a8f..eda9c32 100644 --- a/R/plan.R +++ b/R/plan.R @@ -7,22 +7,13 @@ #' #' @param path path to the package source. #' @param repos repository used to identify reverse dependencies. -#' @param versions character vector indicating against which versions of the -#' package reverse dependency should be checked. `c("dev", "release")` -#' (default) stands for the classical reverse dependency check. `"dev"` -#' checks only against development version of the package which is applicable -#' mostly when checking whether adding new package would break tests of -#' packages already in the repository and take the package as suggests -#' dependency. #' #' @family plan #' @export plan_rev_dep_checks <- function( path, - repos = getOption("repos"), - versions = c("dev", "release") + repos = getOption("repos") ) { - version_types <- match.arg(versions, c("dev", "release"), several.ok = TRUE) path <- check_path_is_pkg_source(path) ap <- available_packages(repos = repos) @@ -35,22 +26,7 @@ plan_rev_dep_checks <- function( )[[1]] if (length(revdeps) == 0) { - return(igraph::make_empty_graph()) - } - - if ("release" %in% version_types && !package %in% ap[, "Package"]) { - msg <- sprintf( - "Skipping 'release' checks. Package `%s` not found in repositories: \n", - package, - paste0(" * ", repos, collapse = "\n") - ) - - warning(msg, immediate. = TRUE) - version_types <- setdiff(version_types, "release") - } - - if (length(version_types) == 0) { - return(igraph::make_empty_graph()) + return(task_graph_class(igraph::make_empty_graph())) } # root meta task, indicating a reverse-dependency check plan diff --git a/R/reporter_ansi_tty.R b/R/reporter_ansi_tty.R index a23dc43..db24e77 100644 --- a/R/reporter_ansi_tty.R +++ b/R/reporter_ansi_tty.R @@ -499,15 +499,24 @@ report_finalize.reporter_ansi_tty <- function(reporter, checker) { failed_tasks[vlapply(failed_tasks, function(x) is_install(x$task))] failures_output <- vcapply(failed_packages, function(x) { - paste0(cli_wrap_lines(cli::cli_fmt(cli::cli_alert_danger( - sprintf( - "%s package installation had non-zero exit status", - package(x$task[[1]]) - ), - ))), "\n", - cli_wrap_lines(as.character(cli::style_dim( - sprintf("log: %s", x$process[[1]]$log) - )))) + msg <- paste0( + cli_wrap_lines(cli::cli_fmt(cli::cli_alert_danger( + sprintf( + "%s package installation had non-zero exit status", + package(x$task[[1]]) + ) + ))), + collapse = "\n" + ) + log <- paste0( + cli_wrap_lines(cli::cli_fmt(cli::cli_alert_danger( + sprintf("log: %s", x$process[[1]]$log) + ))), + collapse = "\n" + ) + paste0( + msg, "\n", log + ) }) cat(failures_output, "\n") diff --git a/R/reporter_null.R b/R/reporter_null.R index 301bf78..038e3b0 100644 --- a/R/reporter_null.R +++ b/R/reporter_null.R @@ -1,5 +1,19 @@ +#' @export +report_start_setup.NULL <- function(...) {} + +#' @export +report_start_checks.NULL <- function(...) {} + +#' @export +report_finalize.NULL <- function(...) {} + #' @export report_status.NULL <- function(...) {} #' @export report_finalize.NULL <- function(...) {} + +#' @export +report_step.NULL <- function(reporter, checker) { + checker$start_next_task() >= 0 +} diff --git a/R/results.R b/R/results.R index 05c33f4..eb61cc1 100644 --- a/R/results.R +++ b/R/results.R @@ -227,7 +227,6 @@ filter_results <- function(x, keep, ...) { } else { x } - } count <- function(d, ...) { diff --git a/R/task_graph.R b/R/task_graph.R index 7f383b0..3bf9d6a 100644 --- a/R/task_graph.R +++ b/R/task_graph.R @@ -201,7 +201,11 @@ task_graph_sort <- function(g) { # use only strong dependencies to prioritize by topology (leafs first) strong_edges <- dep_edges(E(g), check_dependencies("strong")) - g_strong <- igraph::subgraph.edges(g, strong_edges, delete.vertices = FALSE) + g_strong <- igraph::subgraph_from_edges( + g, + strong_edges, + delete.vertices = FALSE + ) topo <- igraph::topo_sort(g_strong, mode = "out") priority_topo <- integer(length(g)) priority_topo[match(topo$name, V(g)$name)] <- rev(seq_along(topo)) diff --git a/R/utils-deps.R b/R/utils-deps.R index 7881450..a2b4a4e 100644 --- a/R/utils-deps.R +++ b/R/utils-deps.R @@ -68,6 +68,7 @@ pkg_dependencies <- function( verbose = FALSE ) { dependencies <- as_pkg_dependencies(dependencies) + na_version <- package_version(NA_character_, strict = FALSE) proto_df <- data.frame( package = character(0L), @@ -78,9 +79,9 @@ pkg_dependencies <- function( ) depth <- 0L - out <- list() + out <- list(list(proto_df)) while (length(packages) > 0L) { - depth <- depth + 1L + depth <- depth + 2L deptypes <- if (depth == 1L) dependencies$direct else dependencies$indirect # db does not need to have all the dependencies types. deptypes <- intersect(colnames(db), deptypes) diff --git a/R/utils-igraph.R b/R/utils-igraph.R index 8162763..e199a4f 100644 --- a/R/utils-igraph.R +++ b/R/utils-igraph.R @@ -59,11 +59,11 @@ graph_dedup_attrs <- function(g) { for (i in seq_along(v_dup_attrs)) { attr_name <- names(v_dup_attrs[i]) attr_value <- igraph::vertex_attr(g, v_dup_attrs[[i]][[1L]]) - g <- igraph::remove.vertex.attribute(g, v_dup_attrs[[i]][[1L]]) + g <- igraph::delete_vertex_attr(g, v_dup_attrs[[i]][[1L]]) for (attr_dup_name in v_dup_attrs[[i]][-1L]) { is_na <- is.na(attr_value) attr_value[is_na] <- igraph::vertex_attr(g, attr_dup_name)[is_na] - g <- igraph::remove.vertex.attribute(g, attr_dup_name) + g <- igraph::delete_vertex_attr(g, attr_dup_name) } g <- igraph::set_vertex_attr(g, attr_name, value = attr_value) } @@ -76,11 +76,11 @@ graph_dedup_attrs <- function(g) { for (i in seq_along(e_dup_attrs)) { attr_name <- names(e_dup_attrs[i]) attr_value <- igraph::edge_attr(g, e_dup_attrs[[i]][[1L]]) - g <- igraph::remove.edge.attribute(g, e_dup_attrs[[i]][[1L]]) + g <- igraph::delete_edge_attr(g, e_dup_attrs[[i]][[1L]]) for (attr_dup_name in e_dup_attrs[[i]][-1L]) { is_na <- is.na(attr_value) attr_value[is_na] <- igraph::edge_attr(g, attr_dup_name)[is_na] - g <- igraph::remove.edge.attribute(g, attr_dup_name) + g <- igraph::delete_edge_attr(g, attr_dup_name) } g <- igraph::set_edge_attr(g, attr_name, value = attr_value) } diff --git a/R/utils-paths.R b/R/utils-paths.R index 586b82c..1ff40a1 100644 --- a/R/utils-paths.R +++ b/R/utils-paths.R @@ -25,9 +25,7 @@ format_simplify_path <- function(x, ..., full.path = FALSE) { parts <- "." } do.call(file.path, as.list(parts)) - } - - if (first_diff > min_len) { + } else if (first_diff > min_len) { parts <- utils::tail(xp, -first_diff + 1) do.call(file.path, as.list(parts)) } else if (first_diff <= min_len) { diff --git a/man/check_dev_rev_deps.Rd b/man/check_pkgs.Rd similarity index 70% rename from man/check_dev_rev_deps.Rd rename to man/check_pkgs.Rd index 5c0ff93..3f3fd6c 100644 --- a/man/check_dev_rev_deps.Rd +++ b/man/check_pkgs.Rd @@ -1,11 +1,11 @@ % Generated by roxygen2: do not edit by hand % Please edit documentation in R/check.R -\name{check_dev_rev_deps} -\alias{check_dev_rev_deps} -\title{Run reverse dependency checks against a development version only} +\name{check_pkgs} +\alias{check_pkgs} +\title{Check packages} \usage{ -check_dev_rev_deps( - path, +check_pkgs( + package, n = 2L, output = tempfile(paste(utils::packageName(), Sys.Date(), sep = "-")), lib.loc = .libPaths(), @@ -15,7 +15,8 @@ check_dev_rev_deps( ) } \arguments{ -\item{path}{file path to the package source directory} +\item{package}{A path to either package, directory with packages or name +of the package (details)} \item{n}{\code{integer} value indicating maximum number of subprocesses that can be simultaneously spawned when executing tasks.} @@ -41,11 +42,9 @@ regarding checks that run. Can be combined with \code{\link{results}} and \code{\link[=summary]{summary()}} methods to generate results. } \description{ -\code{\link[=check_dev_rev_deps]{check_dev_rev_deps()}} works similarly to \code{\link[=check_rev_deps]{check_rev_deps()}} but it runs -R CMD check only once for each package, with the development version of the -package installed. It is advantageous to check whether adding a new package -into a repository breaks existing packages that possibly take said package -as a \code{Suggests} dependency. +Runs classical \verb{R CMD check} for the given source package. It +first identifies and installs, in parallel, all dependencies required +to check the package. Then, it runs \verb{R CMD check} for each specified package. } \seealso{ Other checks: diff --git a/man/check_rev_deps.Rd b/man/check_rev_deps.Rd index 80d6afd..e729aee 100644 --- a/man/check_rev_deps.Rd +++ b/man/check_rev_deps.Rd @@ -62,7 +62,7 @@ identify changes in reverse dependency behaviors. } \seealso{ Other checks: -\code{\link{check_dev_rev_deps}()}, +\code{\link{check_pkgs}()}, \code{\link{checker}}, \code{\link{new_checker}()} } diff --git a/man/checker.Rd b/man/checker.Rd index f874878..1dbab10 100644 --- a/man/checker.Rd +++ b/man/checker.Rd @@ -29,7 +29,7 @@ while (!orchestrator$is_done()) { } \seealso{ Other checks: -\code{\link{check_dev_rev_deps}()}, +\code{\link{check_pkgs}()}, \code{\link{check_rev_deps}()}, \code{\link{new_checker}()} } diff --git a/man/new_checker.Rd b/man/new_checker.Rd index 80ae561..6e8c94d 100644 --- a/man/new_checker.Rd +++ b/man/new_checker.Rd @@ -19,12 +19,12 @@ Instantiate a check design from a path or directory. } \seealso{ Other checks: -\code{\link{check_dev_rev_deps}()}, +\code{\link{check_pkgs}()}, \code{\link{check_rev_deps}()}, \code{\link{checker}} Other checks: -\code{\link{check_dev_rev_deps}()}, +\code{\link{check_pkgs}()}, \code{\link{check_rev_deps}()}, \code{\link{checker}} } diff --git a/man/plan_rev_dep_checks.Rd b/man/plan_rev_dep_checks.Rd index 40bcfb4..a61a5a6 100644 --- a/man/plan_rev_dep_checks.Rd +++ b/man/plan_rev_dep_checks.Rd @@ -4,24 +4,12 @@ \alias{plan_rev_dep_checks} \title{Plan Reverse Dependency Checks} \usage{ -plan_rev_dep_checks( - path, - repos = getOption("repos"), - versions = c("dev", "release") -) +plan_rev_dep_checks(path, repos = getOption("repos")) } \arguments{ \item{path}{path to the package source.} \item{repos}{repository used to identify reverse dependencies.} - -\item{versions}{character vector indicating against which versions of the -package reverse dependency should be checked. \code{c("dev", "release")} -(default) stands for the classical reverse dependency check. \code{"dev"} -checks only against development version of the package which is applicable -mostly when checking whether adding new package would break tests of -packages already in the repository and take the package as suggests -dependency.} } \description{ Generates a plan for running reverse dependency check for certain diff --git a/tests/testthat.R b/tests/testthat.R index 6d04abf..78ec056 100644 --- a/tests/testthat.R +++ b/tests/testthat.R @@ -9,4 +9,4 @@ library(testthat) library(checked) -#test_check("checked") +test_check("checked") diff --git a/tests/testthat/_snaps/reporters.md b/tests/testthat/_snaps/reporters.md new file mode 100644 index 0000000..6806e4f --- /dev/null +++ b/tests/testthat/_snaps/reporters.md @@ -0,0 +1,28 @@ +# reporter_basic_tty works as expected for pkg.none + + Code + run(design, reporter = reporter) + Message + +# reporter_basic_tty works as expected for pkg.ok.error + + Code + run(design, reporter = reporter) + Message + Checks + [][install] rev.both.dependency started + [][install] rev.both.dependency finished () + [][install] pkg.ok.error started + [][install] pkg.ok.error finished () + [][install] pkg.ok.error started + [][check] rev.both.ok started + [][install] pkg.ok.error finished () + [][check] rev.both.ok finished with 1 NOTE () + [][check] rev.both.error started + [][check] rev.both.error finished with 1 NOTE () + [][check] rev.both.ok started + [][check] rev.both.ok finished with 1 NOTE () + [][check] rev.both.error started + [][check] rev.both.error finished with 1 ERROR, 1 WARNING () + Finished in + diff --git a/tests/testthat/fixtures/revdeps/v1/pkg.none_1.0.0.tar.gz b/tests/testthat/fixtures/revdeps/v1/pkg.none_1.0.0.tar.gz index 1822907..6956555 100644 Binary files a/tests/testthat/fixtures/revdeps/v1/pkg.none_1.0.0.tar.gz and b/tests/testthat/fixtures/revdeps/v1/pkg.none_1.0.0.tar.gz differ diff --git a/tests/testthat/test-check-reverse.R b/tests/testthat/test-check-reverse.R index c406e0b..25af09b 100644 --- a/tests/testthat/test-check-reverse.R +++ b/tests/testthat/test-check-reverse.R @@ -48,54 +48,58 @@ test_that("check_rev_deps works for package with one breaking change", { expect_true(is.list(r)) expect_named(r) expect_length(r, 1L) - expect_length(r$revdep_check_task, 2L) + expect_length(r[[1]], 2L) + + expect_s3_class(r[[1]], "rev_dep_dep_results") + expect_s3_class(r[[1]][[1]], "rcmdcheck_rev_dep_results") + expect_s3_class(r[[1]][[2]], "rcmdcheck_rev_dep_results") # rev.both.error - expect_length(r$revdep_check_task$rev.both.error$notes$issues, 0L) - expect_length(r$revdep_check_task$rev.both.error$notes$potential_issues$new, 0L) - expect_length(r$revdep_check_task$rev.both.error$notes$potential_issues$old, 0L) + expect_length(r[[1]][[2]]$notes$issues, 0L) + expect_length(r[[1]][[2]]$notes$potential_issues$new, 0L) + expect_length(r[[1]][[2]]$notes$potential_issues$old, 0L) - expect_length(r$revdep_check_task$rev.both.error$warnings$issues, 1L) + expect_length(r[[1]][[2]]$warnings$issues, 1L) expect_true( grepl("Namespace in Imports field not imported from", - r$revdep_check_task$rev.both.error$warnings$issues), + r[[1]][[2]]$warnings$issues), grepl("Missing or unexported object", - r$revdep_check_task$rev.both.error$warnings$issues) + r[[1]][[2]]$warnings$issues) ) - expect_length(r$revdep_check_task$rev.both.error$warnings$potential_issues$new, 0L) - expect_length(r$revdep_check_task$rev.both.error$warnings$potential_issues$old, 0L) + expect_length(r[[1]][[2]]$warnings$potential_issues$new, 0L) + expect_length(r[[1]][[2]]$warnings$potential_issues$old, 0L) - expect_length(r$revdep_check_task$rev.both.error$errors$issues, 1L) + expect_length(r[[1]][[2]]$errors$issues, 1L) expect_true( grepl("Running the tests in", - r$revdep_check_task$rev.both.error$errors$issues), + r[[1]][[2]]$errors$issues), grepl("is not an exported object from", - r$revdep_check_task$rev.both.error$errors$issues) + r[[1]][[2]]$errors$issues) ) - expect_length(r$revdep_check_task$rev.both.error$errors$potential_issues$new, 0L) - expect_length(r$revdep_check_task$rev.both.error$errors$potential_issues$old, 0L) + expect_length(r[[1]][[2]]$errors$potential_issues$new, 0L) + expect_length(r[[1]][[2]]$errors$potential_issues$old, 0L) # rev.both.ok - expect_length(r$revdep_check_task$rev.both.ok$notes$issues, 0L) - expect_length(r$revdep_check_task$rev.both.ok$notes$potential_issues$new, 0L) - expect_length(r$revdep_check_task$rev.both.ok$notes$potential_issues$old, 0L) + expect_length(r[[1]][[1]]$notes$issues, 0L) + expect_length(r[[1]][[1]]$notes$potential_issues$new, 0L) + expect_length(r[[1]][[1]]$notes$potential_issues$old, 0L) - expect_length(r$revdep_check_task$rev.both.ok$warnings$issues, 0L) - expect_length(r$revdep_check_task$rev.both.ok$warnings$potential_issues$new, 0L) - expect_length(r$revdep_check_task$rev.both.ok$warnings$potential_issues$old, 0L) + expect_length(r[[1]][[1]]$warnings$issues, 0L) + expect_length(r[[1]][[1]]$warnings$potential_issues$new, 0L) + expect_length(r[[1]][[1]]$warnings$potential_issues$old, 0L) - expect_length(r$revdep_check_task$rev.both.ok$errors$issues, 0L) - expect_length(r$revdep_check_task$rev.both.ok$errors$potential_issues$new, 0L) - expect_length(r$revdep_check_task$rev.both.ok$errors$potential_issues$old, 0L) + expect_length(r[[1]][[1]]$errors$issues, 0L) + expect_length(r[[1]][[1]]$errors$potential_issues$new, 0L) + expect_length(r[[1]][[1]]$errors$potential_issues$old, 0L) }) -test_that("check_rev_deps works for a package without release version", { +test_that("check_rev_deps works for a package without a version in repos", { # Ensure source installation to make sure test works also on mac and windows withr::with_options(list(pkgType = "source"), { - expect_warning(design <- check_rev_deps( + expect_no_error(design <- check_rev_deps( file.path(sources_new, "pkg.suggests"), n = 2L, repos = repo, @@ -106,26 +110,33 @@ test_that("check_rev_deps works for a package without release version", { r <- results(design) expect_s3_class(r, "checked_results") expect_true(is.list(r)) + expect_length( + list.dirs(file.path(design$output, "checks"), recursive = FALSE), + 2 + ) expect_named(r) expect_length(r, 1L) - expect_length(r$check_task, 1L) + expect_length(r[[1]], 1L) + + expect_s3_class(r[[1]], "rev_dep_dep_results") + expect_s3_class(r[[1]][[1]], "rcmdcheck_rev_dep_results") - expect_length(r$check_task$`pkg.none (dev)`$notes$issues, 0L) - expect_length(r$check_task$`pkg.none (dev)`$notes$potential_issues$new, 0L) - expect_length(r$check_task$`pkg.none (dev)`$notes$potential_issues$old, 0L) + expect_length(r[[1]][[1]]$notes$issues, 0L) + expect_length(r[[1]][[1]]$notes$potential_issues$new, 0L) + expect_length(r[[1]][[1]]$notes$potential_issues$old, 0L) - expect_length(r$check_task$`pkg.none (dev)`$warnings$issues, 0L) - expect_length(r$check_task$`pkg.none (dev)`$warnings$potential_issues$new, 0L) - expect_length(r$check_task$`pkg.none (dev)`$warnings$potential_issues$old, 0L) + expect_length(r[[1]][[1]]$warnings$issues, 0L) + expect_length(r[[1]][[1]]$warnings$potential_issues$new, 0L) + expect_length(r[[1]][[1]]$warnings$potential_issues$old, 0L) - expect_length(r$check_task$`pkg.none (dev)`$errors$issues, 1L) + expect_length(r[[1]][[1]]$errors$issues, 1L) expect_true( - grepl("Running the tests in", - r$check_task$`pkg.none (dev)`$errors$issues), - grepl("\"hello world\" is not TRUE", - r$check_task$`pkg.none (dev)`$errors$issues) + grepl("Running the tests in", r[[1]][[1]]$errors$issues) + ) + expect_true( + grepl("Reverse suggested deps detected", r[[1]][[1]]$errors$issues) ) - expect_length(r$check_task$`pkg.none (dev)`$errors$potential_issues$new, 0L) - expect_length(r$check_task$`pkg.none (dev)`$errors$potential_issues$old, 0L) + expect_length(r[[1]][[1]]$errors$potential_issues$new, 0L) + expect_length(r[[1]][[1]]$errors$potential_issues$old, 0L) }) diff --git a/tests/testthat/test-check.R b/tests/testthat/test-check.R index 3af567d..210d8bc 100644 --- a/tests/testthat/test-check.R +++ b/tests/testthat/test-check.R @@ -1,23 +1,33 @@ test_that("check_pkgs works as expected", { examples_path <- system.file("example_packages", package = "checked") - # WIP - expect_no_error(check_pkgs( - file.path(examples_path, c("exampleGood", "exampleBad")), - n = 2L, - repos = "https://cran.r-project.org/", - reporter = NULL - )) -}) + expect_no_error( + plan <- check_pkgs( + file.path(examples_path, c("exampleGood", "exampleBad")), + n = 2L, + repos = "https://cran.r-project.org/", + reporter = NULL + ) + ) + + r <- results(plan) + expect_s3_class(r, "checked_results") + expect_true(is.list(r)) + expect_length( + list.dirs(file.path(plan$output, "checks"), recursive = FALSE), + 2 + ) + expect_named(r) + expect_length(r, 1L) + expect_length(r[[1]], 2L) -test_that("check_pkgs works as expected", { - examples_path <- system.file("example_packages", package = "checked") + expect_s3_class(r[[1]], "local_check_results") - # WIP - expect_no_error(check_pkgs( - file.path(examples_path, c("exampleGood", "exampleBad")), - n = 2L, - repos = "https://cran.r-project.org/", - reporter = NULL - )) + expect_length(r[[1]][[1]]$notes$issues, 1L) + expect_length(r[[1]][[1]]$notes$potential_issues$new, 0L) + expect_length(r[[1]][[1]]$notes$potential_issues$old, 0L) + + expect_length(r[[1]][[1]]$warnings$issues, 3L) + expect_length(r[[1]][[1]]$warnings$potential_issues$new, 0L) + expect_length(r[[1]][[1]]$warnings$potential_issues$old, 0L) }) diff --git a/tests/testthat/test-dep-graph-next-package.R b/tests/testthat/test-dep-graph-next-package.R index 3a95b6a..8375f1e 100644 --- a/tests/testthat/test-dep-graph-next-package.R +++ b/tests/testthat/test-dep-graph-next-package.R @@ -1,10 +1,10 @@ test_that("dep_graph_next_package finds next installable package", { # nolint start, styler: off g <- igraph::make_graph(~ - A +- B +- C, - A +------ D, - A +- E +- D, - A +- F +- D + A -+ B -+ C, + A ------+ D, + A -+ E -+ D, + A -+ F -+ D ) # nolint end, styler: on @@ -21,12 +21,14 @@ test_that("dep_graph_next_package finds next installable package", { V(g)["D"]$status <- STATUS[["in progress"]] V(g)["C"]$status <- STATUS[["done"]] g <- task_graph_update_ready(g) - expect_equal(names(task_graph_which_ready(g)), "B") + ready_nodes <- V(g)[V(g)$status == STATUS$ready] + expect_equal(names(ready_nodes), "B") # if the order is reversed, now "F" and "E" should be next V(g)$status <- STATUS[["pending"]] V(g)["D"]$status <- STATUS[["done"]] V(g)["C"]$status <- STATUS[["in progress"]] g <- task_graph_update_ready(g) - expect_equal(names(task_graph_which_ready(g)), c("F", "E")) + ready_nodes <- V(g)[V(g)$status == STATUS$ready] + expect_equal(names(ready_nodes), c("F", "E")) }) diff --git a/tests/testthat/test-plan.R b/tests/testthat/test-plan.R index a4b9d99..d8323ff 100644 --- a/tests/testthat/test-plan.R +++ b/tests/testthat/test-plan.R @@ -1,69 +1,58 @@ +default_env <- options::opt("check_envvars", env = "checked") +default_args <- options::opt("check_args", env = "checked") +default_build_args <- options::opt("check_build_args", env = "checked") + test_that("rev_dep_check_tasks_df works with deafult params", { expect_silent( - df <- plan_rev_dep_checks( + plan <- plan_rev_dep_checks( test_path("fixtures", "DALEXtra"), repos = "https://cran.r-project.org/" ) ) - expect_s3_class(df, "data.frame") - expect_true(NROW(df) >= 8) - expect_named(df, c("alias", "version", "package", "custom")) - - expect_s3_class(df$package, "list_of_task") - task_classes <- vcapply(df$package, function(x) class(x)[[1]]) - expect_equal(unique(task_classes), "revdep_check_task") - pkg_sub_classes <- vcapply(df$package, function(x) class(x$package)[[1]]) - expect_equal(unique(pkg_sub_classes), "pkg_origin_repo") - - expect_s3_class(df$custom, "list_of_task") - task_classes <- vcapply(df$custom, function(x) class(x)[[1]]) - expect_equal(unique(task_classes), "custom_install_task") - pkg_sub_classes <- vcapply(df$custom, function(x) class(x$package)[[1]]) - expect_equal(unique(pkg_sub_classes), c("pkg_origin_local", "pkg_origin_repo")) - - expect_true(all(endsWith(df$alias[seq(1, NROW(df), by = 2)], "(dev)"))) - expect_true(all(endsWith(df$alias[seq(2, NROW(df), by = 2)], "(v2.3.0)"))) + expect_s3_class(plan, "task_graph") + expect_true(length(plan) >= 15) - # Test displayes - expect_no_error(expect_output(print(df))) - expect_no_error(expect_output(print(df$package))) - expect_no_error(expect_output(print(df$custom))) -}) - -test_that("rev_dep_check_tasks_df development_only = TRUE", { - expect_silent( - plan <- plan_rev_dep_checks( - test_path("fixtures", "DALEXtra"), - repos = "https://cran.r-project.org/", - versions = "dev" - ) + expect_all_true( + c( + "rev_dep_dep_meta_task", + "rev_dep_check_meta_task", + "check_task", + "install_task" + ) %in% vcapply(V(plan)$task, function(x) + class(x)[[1]]) ) - - expect_s3_class(plan, "data.frame") - expect_true(NROW(plan) >= 4) - expect_named(plan, c("alias", "version", "package", "custom")) - - expect_s3_class(plan$package, "list_of_task") - task_classes <- vcapply(plan$package, function(x) class(x)[[1]]) - expect_equal(unique(task_classes), "check_task") - pkg_sub_classes <- vcapply(plan$package, function(x) class(x$package)[[1]]) - expect_equal(unique(pkg_sub_classes), "pkg_origin_repo") - - expect_s3_class(plan$custom, "list_of_task") - task_classes <- vcapply(plan$custom, function(x) class(x)[[1]]) - expect_equal(unique(task_classes), "custom_install_task") - package_sub_classes <- vcapply(plan$custom, function(x) class(x$package)[[1]]) - expect_equal(unique(package_sub_classes), c("pkg_origin_local")) - - expect_true(all(endsWith(plan$alias, "(dev)"))) - expect_true(all(!endsWith(plan$alias, "(v2.3.0)"))) + expect_s3_class(V(plan)$task[[1]], "rev_dep_dep_meta_task") + expect_s3_class(V(plan)$task[[1]]$origin, "pkg_origin_local") + expect_true(V(plan)$task[[1]]$origin$package == "DALEXtra") + expect_s3_class(V(plan)$task[[1]]$origin$version, "package_version") + expect_true(V(plan)$task[[1]]$origin$version == "2.3.0") + + + expect_s3_class(V(plan)$task[[3]], "check_task") + expect_equal( + names(V(plan)$task[[3]]), + c("env", "args", "build_args", "origin", "seed") + ) + expect_s3_class(V(plan)$task[[3]]$origin, "pkg_origin_repo") + expect_equal(V(plan)$task[[3]]$env, default_env) + expect_equal(V(plan)$task[[3]]$args, default_args) + expect_equal(V(plan)$task[[3]]$build_args, default_build_args) + expect_true(V(plan)$task[[3]]$origin$package == "marginaleffects") + expect_s3_class(V(plan)$task[[3]]$origin$version, "package_version") + expect_true(V(plan)$task[[3]]$origin$version == "0.31.0") + + # Test displayes + expect_no_error(expect_output(print(plan))) + expect_no_error(expect_output(print(V(plan)$task[[1]]))) + expect_no_error(expect_output(print(V(plan)$task[[3]]))) + expect_no_error(expect_output(print(V(plan)$task[[3]]$origin))) }) test_that("source_check_tasks_df works as expected", { examples_path <- system.file("example_packages", package = "checked") expect_silent( - df <- plan_checks( + plan <- plan_local_checks( c( test_path("fixtures", "DALEXtra"), test_path("fixtures", "rd2markdown"), @@ -73,61 +62,36 @@ test_that("source_check_tasks_df works as expected", { ) ) ) - expect_s3_class(df, "data.frame") - expect_equal(NROW(df), 5) - expect_named(df, c("alias", "version", "package", "custom")) - - expect_s3_class(df$package, "list_of_task") - expect_equal(unique(vcapply(df$package, function(x) class(x)[[1]])), "check_task") - expect_equal(unique(vcapply(df$package, function(x) class(x$package)[[1]])), "pkg_origin_local") - - expect_s3_class(df$custom, "list_of_task") - expect_equal(unique(vcapply(df$custom, function(x) class(x)[[1]])), "custom_install_task") - expect_equal(unique(vcapply(df$custom, function(x) class(x$package)[[1]])), "NULL") - - expect_true(all(endsWith(df$alias, "(source)"))) -}) - -test_that("source_check_tasks_df aliases are properly handled", { - examples_path <- system.file("example_packages", package = "checked") - names <- c( - "DALEXtra_new", - "rd2markdown_new", - "exampleGood_new", - "exampleOkay_new", - "exampleBad_new" - ) - - path <- c( - test_path("fixtures", "DALEXtra"), - test_path("fixtures", "rd2markdown"), - file.path(examples_path, "exampleGood"), - file.path(examples_path, "exampleOkay"), - file.path(examples_path, "exampleBad") - ) - names(path) <- names - - expect_silent( - df <- plan_checks(path) - ) - - expect_true(all(endsWith(df$alias, "_new"))) - expect_equal(df$alias, names) - - expect_silent( - df <- plan_checks(c( - file.path(examples_path, "exampleGood"), - file.path(examples_path, "exampleGood"), - file.path(examples_path, "exampleGood") - )) + expect_s3_class(plan, "task_graph") + expect_true(length(plan) == 6) + + expect_all_true( + c( + "local_check_meta_task", + "check_task" + ) %in% vcapply(V(plan)$task, function(x) + class(x)[[1]]) ) + expect_s3_class(V(plan)$task[[1]], "local_check_meta_task") + expect_null(V(plan)$task[[1]]$origin) + + expect_s3_class(V(plan)$task[[2]], "check_task") expect_equal( - df$alias, - c( - "exampleGood (source_1)", - "exampleGood (source_2)", - "exampleGood (source_3)" - ) + names(V(plan)$task[[3]]), + c("env", "args", "build_args", "origin") ) + expect_s3_class(V(plan)$task[[3]]$origin, "pkg_origin_local") + expect_equal(V(plan)$task[[3]]$env, default_env) + expect_equal(V(plan)$task[[3]]$args, default_args) + expect_equal(V(plan)$task[[3]]$build_args, default_build_args) + expect_true(V(plan)$task[[3]]$origin$package == "rd2markdown") + expect_s3_class(V(plan)$task[[3]]$origin$version, "package_version") + expect_true(V(plan)$task[[3]]$origin$version == "0.0.8") + + # Test displayes + expect_no_error(expect_output(print(plan))) + expect_no_error(expect_output(print(V(plan)$task[[1]]))) + expect_no_error(expect_output(print(V(plan)$task[[3]]))) + expect_no_error(expect_output(print(V(plan)$task[[3]]$origin))) }) diff --git a/tests/testthat/test-reporters.R b/tests/testthat/test-reporters.R new file mode 100644 index 0000000..6413c74 --- /dev/null +++ b/tests/testthat/test-reporters.R @@ -0,0 +1,95 @@ +create_temp_repo <- function(dir, repo_path) { + contrib_url <- utils::contrib.url(repo_path, type = "source") + dir.create(contrib_url, recursive = TRUE) + sources <- list.files(dir, pattern = "^.*\\.tar\\.gz$", full.names = TRUE) + vapply(sources, file.copy, FUN.VALUE = logical(1), to = contrib_url) + tools::write_PACKAGES(contrib_url, type = "source") +} + +sources_old <- test_path("fixtures", "revdeps", "v1") +sources_new <- test_path("fixtures", "revdeps", "v2") + +dir.create(repo_dir <- tempfile("repo")) +repo <- paste0("file:///", repo_dir) +create_temp_repo(sources_old, repo_dir) +old <- getOption("pkgType") +options(pkgType = "source") + +test_that("reporter_basic_tty works as expected for pkg.none", { + plan <- plan_rev_dep_checks( + file.path(sources_new, "pkg.none"), + repos = repo + ) + + design <- checker$new( + plan, + n = 1L, + repos = repo, + restore = FALSE + ) + + reporter <- reporter_basic_tty() + + expect_snapshot( + run(design, reporter = reporter), + # We remove the last line as it reports the time which can change + transform = function(lines) { + lines[-length(lines)] + } + ) +}) + +test_that("reporter_basic_tty works as expected for pkg.ok.error", { + + plan <- plan_rev_dep_checks( + file.path(sources_new, "pkg.ok.error"), + repos = repo + ) + + design <- checker$new( + plan, + n = 1L, + repos = repo, + restore = FALSE + ) + + reporter <- reporter_basic_tty() + + expect_snapshot( + run(design, reporter = reporter), + # We remove the last line as it reports the time which can change + transform = function(lines) { + lines <- gsub("\\d+(?:\\.\\d+)?(?:ms|s|m)", "", lines) + lines[!startsWith(lines, "ETA")] + } + ) +}) + +test_that("reporter_ansi_tty works as expected for pkg.ok.error", { + plan <- plan_rev_dep_checks( + file.path(sources_new, "pkg.ok.error"), + repos = repo + ) + + design <- checker$new( + plan, + n = 1L, + repos = repo, + restore = FALSE + ) + reporter <- reporter_ansi_tty() + + expect_no_error(suppressMessages( + capture.output( + run(design, reporter = reporter) + ) + )) + + expect_s3_class(reporter$buffer, "data.frame") + expect_equal(NROW(reporter$buffer), 7) + expect_all_true(reporter$buffer$final) + expect_all_false(reporter$buffer$updated) + expect_all_false(reporter$buffer$new) +}) + +options(pkgType = old) diff --git a/tests/testthat/test-results-utils.R b/tests/testthat/test-results-utils.R new file mode 100644 index 0000000..93c481a --- /dev/null +++ b/tests/testthat/test-results-utils.R @@ -0,0 +1,26 @@ +test_that("results_to_df works as expected", { + examples_path <- system.file("example_packages", package = "checked") + + expect_no_error( + plan <- check_pkgs( + file.path(examples_path, c("exampleGood", "exampleBad")), + n = 2L, + repos = "https://cran.r-project.org/", + reporter = NULL + ) + ) + + r <- results(plan) + df <- results_to_df(r[[1]]) + expect_equal(NROW(df), 2) + expect_equal(names(df), c("notes", "warnings", "errors")) + expect_equal(df$notes, c(1, 0)) + expect_equal(df$warnings, c(3, 0)) + expect_equal(df$errors, c(0, 1)) + expect_true( + all( + endsWith(row.names(df)[[1]], "check-exampleBad"), + endsWith(row.names(df)[[2]], "check-exampleGood") + ) + ) +}) diff --git a/tests/testthat/test-results.R b/tests/testthat/test-results.R deleted file mode 100644 index 1edab35..0000000 --- a/tests/testthat/test-results.R +++ /dev/null @@ -1,32 +0,0 @@ -test_that("results_to_file works as expected", { - examples_path <- system.file("example_packages", package = "checked") - - # WIP - expect_no_error( - plan <- check_pkgs( - file.path(examples_path, c("exampleGood", "exampleBad")), - n = 2L, - repos = "https://cran.r-project.org/", - reporter = NULL - ) - ) - - r <- results(plan) - r_file <- tempfile() - expect_no_error(results_to_file(r, r_file)) - expect_true(!identical(readLines(r_file), "No issues identified.")) - - expect_no_error( - plan <- check_rev_deps( - file.path(examples_path, "exampleBad"), - n = 2L, - repos = "https://cran.r-project.org/", - reporter = NULL - ) - ) - - r <- results(plan) - r_file <- tempfile() - expect_no_error(results_to_file(r, r_file)) - expect_true(identical(readLines(r_file), "No issues identified.")) -})