Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disallow empty strings for test names #1980

Open
kevinushey opened this issue Aug 23, 2024 · 3 comments · May be fixed by #1981
Open

disallow empty strings for test names #1980

kevinushey opened this issue Aug 23, 2024 · 3 comments · May be fixed by #1981

Comments

@kevinushey
Copy link
Collaborator

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 kevinushey linked a pull request Aug 23, 2024 that will close this issue
@hadley
Copy link
Member

hadley commented Aug 23, 2024

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)

@kevinushey
Copy link
Collaborator Author

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.

@hadley
Copy link
Member

hadley commented Aug 23, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants