Is snapshoting messages to the user a good idea?

[clauswilke]

When running devtools::test() locally, I get a failed test due to small changes in the output of use_extendr():

x |  14 1     | use_extendr [1.0 s]                                        
───────────────────────────────────────────────────────────────────────────
Failure (test-use_extendr.R:5:3): use_extendr() sets up extendr files correctly
Snapshot of code has changed:
old[2:14] vs new[2:24]
  "Code"
  "  use_extendr()"
  "Message <message>"
  "  v Creating 'src/rust/src'."
+ "Message <message>"
  "  v Writing 'src/entrypoint.c'"
+ "Message <message>"
  "  v Writing 'src/Makevars'"
+ "Message <message>"
  "  v Writing 'src/Makevars.win'"
and 13 more ...

Run `snapshot_accept('use_extendr')` if this is a deliberate change

Not sure what causes this, maybe some of my installed packages are outdated. However, this makes me wonder whether this is a useful test or whether it is too brittle. If anything minor changes in how usethis formats messages our tests break. Should we maybe disable this particular snapshot test and only test that the created output files are correct?

I'd like to fix this before submitting the package to CRAN.

cc @malcolmbarrett @Ilia-Kosenkov @yutannihilation

[yutannihilation]

If anything minor changes in how usethis formats messages our tests break.

If you are worrying about the breakage after CRAN release, snapshot tests are disabled on CRAN (I'm not sure if it's generally a good idea, though) except when we explicitly enable it by cran = TRUE.

https://testthat.r-lib.org/reference/expect_snapshot.html

[clauswilke]

I'm not specifically concerned about CRAN. I'm more generally concerned about brittle tests that could fail due to a user-interface change in an upstream dependency.

Also, I can't really submit a package to CRAN when it doesn't even pass all tests locally. :-)

[yutannihilation]

Got it. But, to me, this looks actually the case when we want the test fail, if we really care about the experience of the end users; we probably don't want to show that cryptic Message <message> no matter whether it's by the upstream change or our failure.

That said, honestly, I'm not that passionate on this topic, so I'm not against the idea of getting rid of snapshot tests personally. I have no idea how strict we should be on the outputs... Let's wait for the comments from @Ilia-Kosenkov and @malcolmbarrett.

[malcolmbarrett]

Users don't see Message <message>; I believe that's due to a change in how testthat creates snapshots.

snapshot tests, in general, are definitely more sensitive than specific. I'd be more concerned if they ran on CRAN tests, personally, though. I picked up that pattern from Hadley on usethis, but as you can imagine, much of the UI is being generated by usethis itself.

However, we're using custom UI, so while upstream breakage is possible, I still see these tests as worth it.

[malcolmbarrett]

PS this is definitely not a hill I'm going to die on!

[yutannihilation]

I believe that's due to a change in how testthat creates snapshots.

Oh, thanks, I didn't expect it matters! Confirmed I can reproduce the failure with testthat 3.0.1.

[clauswilke]

Ok, updating testthat fixed this for me. Presumably testthat will not continuously change how the capture snapshots, so let's leave it as is for now and see how it goes.

[yutannihilation]

Yeah, hope we don't see this kind of failure so often...

[Ilia-Kosenkov]

Sorry I am late to the discussion (different time zone), but to me snapshots appeared to be reliable. The only issue I had was when {cli} decided that . file interpolations should be put in quotes, while snapshot had the old text without quotes.

I doubt there will be frequent failures unless it is a major release of one of the packages.

As an alternative, we can have one test comparing rextendr output with cli output. If it passes but snapshot tests fail, then some external package was changed. If all tests fail, it is an issue on our side.