Suggest a new expectation around "Vector with a constant value"

[MichaelChirico]

Inspiration: r-lib/lintr#2927

I recently found a test that was silently passing with the format:

expect_true(all(x == y))

It should have failed because the "actual" value, x, wound up being NULL (think x <- df$COL where a refactor removed the COL vector from the data.frame).

With that in mind I think this style of test should be avoided at all costs (and there's nothing expect_true() should do because it's all() causing the issue).

That said, my immediate thought was to turn to expect_identical(x, y), with tolerance= added as needed (or expect_equal, pick your favorite).

But a very common style for this test is:

expect_true(all(x == "FOO"))

I.e., testing that a vector has a single value, "FOO".

I don't think there's a neat way to express that in {testthat} today. There are two stricter equivalents I've hit on, neither of which feel particularly satisfying:

# ugly, and all the uglier if 'x' is an expression!
expect_identical(x, rep("FOO", length(x))

# somewhat "tricky", and requires coercion for non-character 'x'
expect_named(table(x), "FOO")
## c.f.
expect_named(table(int_vector), "1")
## even worse
expect_named(table(dbl_vector), "3.14159265358979")

## still not fully satisfying
expect_identical(unique(x), "FOO")
expect_identical(unique(int_vector), 1L)
expect_identical(unique(dbl_vector), pi, tolerance=...)

Is this a good enough use case to generate its own expectation? Possibly #1836 could be related here -- if we remove the all() call, {testthat} itself could catch the silent error case under the hood and offer a useful failure:

expect_true(x == "FOO", all = TRUE)
# internally, testthat finds 'x' to be empty and fails

[MichaelChirico]

And of course expect_identical(x, rep("FOO", length(x)) is also not ideal if, say, x is character() (i.e., a non-NULL empty vector). It's not really a useful test to check all(x == "foo") when x might be empty -- at a minimum, we should first check expect_gt(length(x), 0); the rep("foo", 0) equivalent is equally unappealing & more convoluted. Just another rake to step on when writing this sort of test :)

[hadley]

Hmmmm, I wonder if expect_all_true() would be a useful application of expect_all_equal()

expect_all_equal() would require that both object and expected are vectors of the same type, and additionally that expected is length 1. I think it would use == with the possible exception of when object is a list. Hmmm, although maybe it would be better for it to start as a simple wrapper of expect_equal(object, rep(expected, length(object))?

[hadley]

I think this has to be a wrapper around waldo::compare() since that takes care of comparing lists, comparing numbers ignoring small FP differences, and concisely displaying differences:

expect_all_equal <- function(object, expected) {
  act <- quasi_label(enquo(object))
  exp <- quasi_label(enquo(expected))

  check_vector(act$val, error_arg = "object")
  if (length(act$val) == 0) {
    cli::cli_abort("{.arg object} must not be empty.")
  }

  check_vector(exp$val, error_arg = "expected")
  if (length(exp$val) != 1) {
    cli::cli_abort("{.arg expected} must be length 1.")
  }

  exp$val <- rep(exp$val, length(act$val))
  names(exp$val) <- names(act$val)
  expect_waldo_equal_("Expected every element of %s to equal %s.", act, exp)

  invisible(act$val)
}

I do like the generalisation of expect_all_equal() to expect_all_true() and expect_all_false(). I think it's less obvious that expect_any_*() is useful because you can't generate very informative failure messages.