Merge pull request #114 from hubverse-org/ak/change-check-fail-class-…

…and-print/111

Change check fail class and print method

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: hubValidations
 Title: Testing framework for hubverse hub validations
-Version: 0.5.1
+Version: 0.6.0
 Authors@R: c(
     person(
         given = "Anna", 
@@ -44,7 +44,6 @@ Imports:
     jsonvalidate,
     lubridate,
     magrittr,
-    octolog,
     purrr,
     rlang,
     stringr,
@@ -63,13 +62,12 @@ Suggests:
 Remotes:
     hubverse-org/hubUtils,
     hubverse-org/hubData,
-    hubverse-org/hubAdmin,
-    assignUser/octolog
+    hubverse-org/hubAdmin
 Config/testthat/edition: 3
 Config/Needs/website: pkgdown, hubverse-org/hubStyle
 Encoding: UTF-8
 Roxygen: list(markdown = TRUE)
-RoxygenNote: 7.3.1
+RoxygenNote: 7.3.2
 URL: https://github.com/hubverse-org/hubValidations,
     https://hubverse-org.github.io/hubValidations/
 BugReports: https://github.com/hubverse-org/hubValidations/issues

NEWS.md

@@ -1,3 +1,15 @@
+# hubValidations 0.6.0
+
+* To make clearer that all checks resulting in `check_failure` are required to pass for files to be considered valid, `check_failure` class objects are elevated to errors (#111). Also, to make it easier for users to identify errors from visually scanning the printed output, the following custom bullets have been assigned.
+  - `✖` : `check_failure` class object.  This indicates an error that does not impact the validation process. 
+  - `ⓧ` : `check_error` class object. This also indicates early termination of the validation process.
+  - `☒` : `check_exec_error` class object. This indicates an error in the execution of a check function.
+* `hub_validations` class object `combine()` method now ensures that check names are made unique across all `hub_validations` objects being combined.
+* Additional improvements to `hub_validations` class object `print()` method. 
+  - Check results for each file validated are now split and printed under file name header. 
+  - The check name that can be used to access the check result from the `hub_validations` object is now included as the prefix to the check result message instead of the file name (#76).
+* `octolog` dependency removed. This removes the annotation of validation results onto GitHub Action workflow logs (#113).
+
 # hubValidations 0.5.1
 
 * Remove dependency on development version of `arrow` package and bump required version to 17.0.0.

R/capture_check_cnd.R

@@ -11,7 +11,7 @@
 #' the state when validation succeeds, the second element, when validation fails.
 #' @param error logical. In the case of validation failure, whether the function
 #' should return an object of class `<error/check_error>` (`TRUE`) or
-#' `<warning/check_failure>` (`FALSE`, default).
+#' `<error/check_failure>` (`FALSE`, default).
 #' @param details further details to be appended to the output message.
 #' @inheritParams rlang::error_cnd
 #'
@@ -22,7 +22,7 @@
 #' @return Depending on whether validation has succeeded and the value
 #' of the `error` argument, one of:
 #' - `<message/check_success>` condition class object.
-#' - `<warning/check_failure>` condition class object.
+#' - `<error/check_failure>` condition class object.
 #' - `<error/check_error>` condition class object.
 #'
 #' Returned object also inherits from subclass `<hub_check>`.
@@ -82,7 +82,7 @@ capture_check_cnd <- function(check, file_path, msg_subject, msg_attribute,
         use_cli_format = TRUE
       )
     } else {
-      res <- rlang::warning_cnd(
+      res <- rlang::error_cnd(
         c("check_failure", "hub_check"),
         where = file_path,
         ...,

R/check_tbl_col_types.R

@@ -7,7 +7,7 @@
 #' @return
 #' Depending on whether validation has succeeded, one of:
 #' - `<message/check_success>` condition class object.
-#' - `<warning/check_failure>` condition class object.
+#' - `<error/check_failure>` condition class object.
 #'
 #' Returned object also inherits from subclass `<hub_check>`.
 #' @export

R/check_tbl_unique_round_id.R

@@ -7,7 +7,7 @@
 #' @return
 #' Depending on whether validation has succeeded, one of:
 #' - `<message/check_success>` condition class object.
-#' - `<warning/check_error>` condition class object.
+#' - `<error/check_error>` condition class object.
 #'
 #' If `round_id_from_variable: false` and no `round_id_col` name is provided,
 #' check is skipped and a `<message/check_info>` condition class object is

R/hub_validations_methods.R

@@ -8,26 +8,39 @@
 print.hub_validations <- function(x, ...) {
   if (length(x) == 0L) {
     msg <- cli::format_inline("Empty {.cls hub_validations}")
+    cli::cli_inform(msg)
   } else {
-    msg <- stats::setNames(
-      paste(
-        fs::path_file(purrr::map_chr(x, "where")),
-        purrr::map_chr(x, "message"),
-        sep = ": "
-      ),
-      dplyr::case_when(
-        purrr::map_lgl(x, ~ rlang::inherits_any(.x, "check_success")) ~ "v",
-        purrr::map_lgl(x, ~ rlang::inherits_any(.x, "check_failure")) ~ "!",
-        purrr::map_lgl(x, ~ rlang::inherits_any(.x, "check_exec_warn")) ~ "!",
-        purrr::map_lgl(x, ~ rlang::inherits_any(.x, "check_error")) ~ "x",
-        purrr::map_lgl(x, ~ rlang::inherits_any(.x, "check_exec_error")) ~ "x",
-        purrr::map_lgl(x, ~ rlang::inherits_any(.x, "check_info")) ~ "i",
-        TRUE ~ "*"
+    print_file <- function(file_name, x) {
+      x <- x[get_filenames(x) == file_name]
+      msg <- stats::setNames(
+        paste(
+          apply_cli_span_class(names(x), class = "check_name"),
+          purrr::map_chr(x, "message"),
+          sep = ": "
+        ),
+        dplyr::case_when(
+          is_check_class(x, "check_success") ~ "v",
+          is_check_class(x, "check_failure") ~ "x",
+          is_check_class(x, "check_exec_warn") ~ "!",
+          is_check_class(x, "check_error") ~ "circle_cross",
+          is_check_class(x, "check_exec_error") ~ "lower_block_8",
+          is_check_class(x, "check_info") ~ "i",
+          TRUE ~ "*"
+        )
       )
+
+      cli::cli_div(class = "hub_validations", theme = hub_validation_theme)
+      cli::cli_h2(file_name)
+      cli::cli_inform(msg)
+      cli::cli_end()
+    }
+
+    purrr::walk(
+      .x = get_filenames(x, unique = TRUE),
+      .f = function(file_name, x) print_file(file_name, x),
+      x = x
     )
   }
-
-  octolog::octo_inform(msg)
 }
 
 
@@ -47,8 +60,16 @@ combine.hub_validations <- function(...) {
     purrr::compact() %>%
     validate_internal_class(class = "hub_validations")
 
-  structure(c(...),
-    class = c("hub_validations", "list")
+  combined <- c(...)
+  if (is.null(names(combined))) {
+    combined_names <- NULL
+  } else {
+    combined_names <- make.unique(names(combined), sep = "_")
+  }
+  structure(
+    combined,
+    class = c("hub_validations", "list"),
+    names = combined_names
   )
 }
 
@@ -85,3 +106,61 @@ validate_internal_class <- function(x, class = c(
 print.pr_hub_validations <- function(x, ...) {
   purrr::map(x, print)
 }
+
+
+# cli theme for hub_validations objects that add a circle cross to be applied
+# to check_error objects
+hub_validation_theme <- list(
+  ".bullets .bullet-circle_cross" = list(
+    "before" = function(x) {
+      paste0(cli::col_red(cli::symbol$circle_cross), " ")
+    },
+    "text-exdent" = 2L
+  ),
+  ".bullets .bullet-checkbox_on" = list(
+    "before" = function(x) {
+      paste0(cli::col_red(cli::symbol$checkbox_on), " ")
+    },
+    "text-exdent" = 2L
+  ),
+  "span.check_name" = list(
+    "before" = "[",
+    "after" = "]",
+    color = "grey"
+  ),
+  "h2" = list(
+    fmt = function(x) {
+      cli::col_br_cyan(
+        paste0(
+          cli::symbol$line, cli::symbol$line,
+          " ", cli::style_underline(x), " ",
+          cli::symbol$line, cli::symbol$line,
+          cli::symbol$line, cli::symbol$line
+        )
+      )
+    }
+  )
+)
+
+apply_cli_span_class <- function(x, class = "check_name") {
+  paste0("{.", class, " ", x, "}")
+}
+
+is_check_class <- function(x,
+                           class = c(
+                             "check_success", "check_failure",
+                             "check_exec_warn", "check_error",
+                             "check_exec_error", "check_info"
+                           )) {
+  class <- rlang::arg_match(class)
+  purrr::map_lgl(x, ~ rlang::inherits_any(.x, class))
+}
+
+get_filenames <- function(x, unique = FALSE) {
+  filenames <- fs::path_file(purrr::map_chr(x, "where"))
+  if (unique) {
+    unique(filenames)
+  } else {
+    filenames
+  }
+}

R/validate_pr.R

@@ -285,7 +285,7 @@ check_pr_modf_del_files <- function(pr_df, file_type = c(
     purrr::compact()
 
   as_hub_validations(out) %>%
-    purrr::set_names(sprintf("%s_mod_%i", file_type, seq_along(out)))
+    purrr::set_names(sprintf("%s_mod", file_type))
 }