Skip to content

Commit

Permalink
fix #135 : ensure || receives scalar logicals
Browse files Browse the repository at this point in the history
A bug in `check_passed` was revealed while running
goodpractice tests with the experimental env variable
"_R_CHECK_LENGTH_1_LOGIC2_" = TRUE in R 3.6 (or during
devtools::check()).

The `chk` variable passed to `check_passed` can be of two
forms:

- a scalar (logical or NA); as returned in `chk_cyclocomp.R`;

- or a list(status = (logical or NA), positions = list(...))
  as returned in `chk_tnf.R`.

The scalar (first case) or the `status` entry indicates
whether the check passed

```
is.na(chk) || ("status" %in% names(chk) && is.na(chk[["status"]]))
```
caused the list-form of `chk` to throw the above error.

We now extract the `status` indicator from `chk` before
running the logical tests to see if the corresponding
checking function failed. This cleaned up the stream of
logical tests.
  • Loading branch information
russHyde authored and gaborcsardi committed Nov 14, 2019
1 parent f315d32 commit 09dd119
Showing 1 changed file with 11 additions and 6 deletions.
17 changes: 11 additions & 6 deletions R/gp.R
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,18 @@ gp <- function(path = ".", checks = all_checks(), extra_preps = NULL,
}

check_passed <- function(chk, na_as_passed = FALSE) {
if(na_as_passed){
isTRUE(chk) || ("status" %in% names(chk) && isTRUE(chk[["status"]])) ||
is.na(chk) || ("status" %in% names(chk) && is.na(chk[["status"]]))
status <- if ("status" %in% names(chk)) {
chk$status
} else {
if (is.na(chk) || ("status" %in% names(chk) && is.na(chk[["status"]])))
return(NA)
isTRUE(chk) || ("status" %in% names(chk) && isTRUE(chk[["status"]]))
chk
}

if (na_as_passed) {
isTRUE(status) || is.na(status)
} else if (is.na(status)) {
NA
} else {
isTRUE(status)
}
}

Expand Down

0 comments on commit 09dd119

Please sign in to comment.