Mitigate linebreak issues in snapshot tests

[jennybc]

I periodically struggle with linebreak differences in snapshot tests and I've seen it come up in Slack a few times. Highlights of the situation:

• Snapshot test fails (seemingly) at random, locally, or in CI, for some jobs but not others, because of linebreak differences. The example I'm dealing with today looks like:

  -- Failure ('test-tidyverse.R:22:3'): use_tidy_dependencies() isn't overly informative --
  Snapshot of code has changed:
  old[8:14] vs new[8:13]
      v Adding withr to 'Imports' field in DESCRIPTION.
      v Adding "@import rlang" to 'R/{TESTPKG}-package.R'.
      v Adding "@importFrom glue glue" to 'R/{TESTPKG}-package.R'.
  -   v Adding "@importFrom lifecycle deprecated" to
  -     'R/{TESTPKG}-package.R'.
  +   v Adding "@importFrom lifecycle deprecated" to 'R/{TESTPKG}-package.R'.
      v Writing 'NAMESPACE'.
      v Writing 'R/import-standalone-purrr.R'.
  
  * Run `testthat::snapshot_accept('tidyverse')` to accept the change.
  * Run `testthat::snapshot_review('tidyverse')` to interactively review the change.
  
  [ FAIL 1 | WARN 0 | SKIP 37 | PASS 796 ]
  

• Then I think "I thought local_reproducible_output() was supposed to design this away".
• I am generally snapshotting output created with cli and that is the case for today's example.
• I think there's always a wrinkle which is that I've had to use expect_snapshot(transform =) to replace, e.g. a volatile filepath (as is true above). @gaborcsardi indicates that one factor here is that the wrapping is done before the transform function is applied. Now that I think about it, at least in the cli case, I can see why it's this way.
• I then try to make the nominal width absolutely huge, so that wrapping doesn't introduce any linebreaks with something like:

   test_that("whatever", {
     withr::local_options(width = 200)
     expect_snapshot(
       some_function(),
       transform = scrub_testpkg
     )
  }

  In my current example, this also did not work and it seems I had to specify cli.width instead of just width.

Various ideas that might help:

• Document that expect_snapshot(transform =) is prone to creating linebreak problems and is best used in combination with explicit width setting. Make it wide folks!
• Could expect_snapshot() get smarter about width or gain a width argument?
• I'm not sure what's going on with width vs. cli.width, but I feel like there's something here.

[gaborcsardi]

The problem is the line breaks are calculated before the transform takes effect, so the output potentially still depends on how long the transformed value was.

There isn't much we can do about this, other than set

withr::local_options(cli.width = Inf)

to get rid of the line breaks.

In short, if you use cli and transform, then better use Inf width.

In theory expect_snapshot() could try to detect this situation, but I am not sure if it is worth it. We should definitely document it, though, possibly in the documentation of transform.

I'm not sure what's going on with width vs. cli.width, but I feel like there's something here.

cli.width takes precedence, and since testthat sets that (as well) to a fixed width, that's what you need to set as well.

[jennybc]

cli.width takes precedence, and since testthat sets that (as well) to a fixed width, that's what you need to set as well.

AHA! I have not come across that in the docs I've been reading, so either that's user error or something else that should be documented.

[hadley]

You could also do local_reproducible_output(width = Inf)

[jennybc]

You could also do local_reproducible_output(width = Inf)

Are you saying that local_reproducible_output(width = Inf) sets cli.width? Not width or, at least, not only width?

[hadley]

Yes, it sets both.