fs icon indicating copy to clipboard operation
fs copied to clipboard

Test relies on {tibble} in an opaque way

Open MichaelChirico opened this issue 1 year ago • 2 comments

https://github.com/r-lib/fs/blob/8d405284484f90b509b8f61cf5c550b7355c5eb8/tests/testthat/test-file.R#L42

Recent update of {waldo} to drop {tibble} as a required package caused this test to break for us (because now {tibble} needs to be included intentionally for testing).

The failure mode is pretty opaque:

══ Failed tests ════════════════════════════════════════════════════════════════
── Failure (test-file.R:42:7): file_info: can be subset as a tibble ────────────
`x` inherits from 'data.frame' not 'tbl_df'.

Suggestions:

  1. skip_if_not_installed("tibble")
  2. Write requireNamespace("tibble") clearly in the test body to make the connection to {tibble} crystal clear
  3. Use withr::local_options(list(fs.use_tibble=FALSE)) to make this test independent of {tibble}:

https://github.com/r-lib/fs/blob/8d405284484f90b509b8f61cf5c550b7355c5eb8/R/utils.R#L135-L139

MichaelChirico avatar Nov 25 '24 21:11 MichaelChirico

I would vote for option 3. Would you like to submit a PR? (No pressure.)

gaborcsardi avatar Apr 25 '25 11:04 gaborcsardi

Ah, I was working on option 3 but then I looked at the previous line:

https://github.com/r-lib/fs/blob/0d261dd965404cb02ab57788f4437fa057a9e0a3/tests/testthat/test-file.R#L45

Since this test is designed quite explicitly to be testing tibble capabilities, I think 1. is most appropriate in this case. WDYT?

Another option to avoid your least favorite skip_if_not_installed() would be to make the tibble requirement clear in the test description:

-it("can be subset as a tibble", {
+it("can be subset as a tibble (requires tibble to be installed)", {

So that it shows up directly in the failure log at least:

══ Failed tests ════════════════════════════════════════════════════════════════
- ── Failure (test-file.R:42:7): file_info: can be subset as a tibble ────────────
+ ── Failure (test-file.R:42:7): file_info: can be subset as a tibble (requires tibble to be installed) ────────────
`x` inherits from 'data.frame' not 'tbl_df'.

That way at least as I glance through test failure logs it's immediately clear what the issue probably is.

MichaelChirico avatar Apr 25 '25 16:04 MichaelChirico