Compound expectations

[hadley]

We currently lack a good way to create compound expectations. The simplest approach has two problems:

expect_numeric_length_10 <- function(x) {
   expect_type(x, "numeric")
   expect_length(x, 10)
}

1. It can generate two successes or two failures
2. The trace_env is incorrect so you get an unnecessary backtrace

We don't suffer too much from 1. in testthat itself but we do have a lot of 2. Mostly in helpers (where we do set trace_env correctly:

• expect_true() → calls expect_waldo_constant()
• expect_false() → calls expect_waldo_constant()
• expect_null() → calls expect_waldo_constant()
• expect_equal() → calls expect_waldo_equal()
• expect_identical() → calls expect_waldo_equal()
• expect_error() → calls expect_condition_matching()
• expect_warning() → calls expect_condition_matching()
• expect_message() → calls expect_condition_matching()
• expect_condition() → calls expect_condition_matching()
• expect_no_error() → calls expect_no_()
• expect_no_warning() → calls expect_no_()
• expect_no_message() → calls expect_no_()
• expect_no_condition() → calls expect_no_()

But there are a few places where we call exported expect_ functions and hence can't control the trace_env

• expect_mapequal() → calls expect_equal()
• expect_output() → calls expect_match()
• expect_snapshot_failure() → calls expect_snapshot_error()
• expect_snapshot_skip() → calls expect_snapshot_error()
• expect_skip() → calls expect_condition()
• expect_no_skip() → calls expect_no_condition()

It would be nice to have general solution to using an existing expectation inside a new expectation.

[hadley]

Here's one option to handle compound expectations. It handles successes/failures but not the trace_env.

expect_integer_length_10 <- function(x) {
  compound({
    expect_type(x, "integer")
    expect_length(x, 10)
  })
}

compound <- function(expr) {
  done <- withCallingHandlers(
    tryCatch(
      {
        expr
        NULL
      },
      expectation_failed = function(cnd) {
        cnd
      }
    ),
    expectation_success = function(cnd) {
      # don't bubble up
      invokeRestart("continue_test")
    }
  )

  if (!is.null(done)) {
    return(exp_signal(done))
  }
  succeed()
}

expect_failure(expect_integer_length_10(1))
expect_failure(expect_integer_length_10(rep("x", 10)))
expect_success(expect_integer_length_10(1:10))

I think the best case I can imagine is something like this:

expect_integer_length_10 <- function(x) {
  local_expectation()

  exp <- wrap(expect_type(x, "integer"))
  if (is.failure(exp)) {
    return(fail(exp))
  }

  exp <- wrap(expect_length(x, 10))
  if (is.failure(exp)) {
    return(fail(exp))
  }
  
  succeed()
}

But that feels pretty clunky, we end up doing some repeated work in each expectation.

I think this is suggesting to me that expectations are just fundamentally not composable.

[hadley]

Hmmm, looking at the current problematic cases:

• expect_mapequal() could call expect_waldo_equal()
• expect_output() could call expect_match_()
• expect_snapshot_failure() could call expect_snapshot_condition()
• expect_snapshot_skip() could call expect_snapshot_condition()
• expect_skip() could call expect_condition_matching()
• expect_no_skip() could call expect_no_()

So at least internal we could resolve a lot of issues by having a naming convention for expectation helpers that can be called from other expectations (probably including an explicit trace_env argument).

Then we just need to document that in the custom expectations vignette — you can't create your own expectations by wrapping existing (but you can create your own helpers).