testthat icon indicating copy to clipboard operation
testthat copied to clipboard

disallow empty strings for test names

Open kevinushey opened this issue 1 year ago • 3 comments

Currently, testthat accepts empty strings as test names, but this causes issues with snapshot tests. For example:

test_that("", {
  expect_snapshot(1 + 1)
})

But every time this test file is run, testthat doesn't recognize the existing snapshot file, and so always prints:

ℹ Testing example
✔ | F W  S  OK | Context
✔ |   1      1 | hello                                                        
──────────────────────────────────────────────────────────────────────────────
Warning (test-hello.R:3:4): 
Adding new snapshot:
Code
  1 + 1
Output
  [1] 2
──────────────────────────────────────────────────────────────────────────────

══ Results ═══════════════════════════════════════════════════════════════════
[ FAIL 0 | WARN 1 | SKIP 0 | PASS 1 ]

One user encountered this during the package development workshop at posit::conf, so learners might unintentionally fall into this trap.

kevinushey avatar Aug 23 '24 18:08 kevinushey

Could/should we just make an empty test name an error unconditionally? (or maybe make it a warning and then upgrade it to an error in a couple of years)

hadley avatar Aug 23 '24 19:08 hadley

We could, but there's a lot of tests in testthat itself which also use empty names, so I wasn't sure if that was intentionally allowed.

kevinushey avatar Aug 23 '24 20:08 kevinushey

Hmmm yeah, mostly used for testing though. I think we could fix those.

hadley avatar Aug 23 '24 21:08 hadley