Keep "new" snapshots even when terminating early

[DavisVaughan]

This is less a testthat bug, and more of a pain point that a few of us have experienced.

Sometimes I change package code that I know is going to change some snapshot tests. I normally just test the file and then snapshot_accept/review() immediately after. However, if it changes more than 10 tests then testthat fails early and I lose all the changed snapshots.

I know I can change options(testthat.progress.max_fails), but is there any other way that testthat could keep around the “new” snapshots even if it fails early? Or ignore the snapshot failures in the total failure count for determining when to terminate early?

[hadley]

Reprex:

for (i in 1:20) {
  time <- Sys.time()
  test_that(paste0("many snapshot failures ", i), {
    expect_snapshot(time)
  })
}

[hadley]

Not sure how to fix this. ProgressReporter has:

      if (self$is_full()) {
        self$local_user_output()
        self$end_context()
        stop_reporter(paste0(
          "Maximum number of failures exceeded; quitting early.\n",
          "You can increase this number by setting `options(testthat.progress.max_fails)`"
        ))
      }

which invokes the testthat_abort_reporter restart — maybe that needs to call end_file() on every reporter?

[hadley]

Actually this is pretty easy because Reporter is usually a multireporter, so just need:

  withRestarts(
    testthat_abort_reporter = function() {
      reporter$end_file()
      reporter$end_context()
      NULL
    },
    force(code)
  )

(And to remove the end_context() call from ProgressReporter. But I think will need to wait for #1412 in order to keep the output in the desired order)

[hadley]

Hmmmm, that doesn't quite work because it doesn't preserve the snapshot that never get run.

[hadley]

Yeah, in general this is going to need something special so we can distinguish between a file being terminated early because of too many failures vs (e.g.) deleting the last snapshot test in a file.

[hadley]

Maybe progress reporter could call get_snapshotter()$early_end() or similar.

[hadley]

Another idea: what if we always finished out the complete file, even if it meant introducing a few extra failures?

[DavisVaughan]

One more point of clarification that I'm just remembering. If:

• File A has 2 snapshot tests that fail
• File B has 10 snapshot tests that fail
• We run all the tests and we hit the max failure limit somewhere in File B

Then we keep the "new" snapshot file for File A, but we lose it for File B.

i.e. we only lose the snapshot updates for the file that testthat hit the limit in.

---

In light of that, I think what you are suggesting makes sense. Maybe this error could be slightly tweaked to suggest that you have finished out the file:

Maximum number of failures exceeded; quitting early.
You can increase this number by setting `options(testthat.progress.max_fails)` 

Maybe something like:

Quitting early due to exceeding the maximum number of allowed failures.
* 10 failures were allowed
* 14 failures have been detected after finishing `test-bind.R`
* You can increase the maximum allowed failures by setting `options(testthat.progress.max_fails)`