Test relies on {tibble} in an opaque way

[MichaelChirico]

fs/tests/testthat/test-file.R

Line 42 in 8d40528

expect_s3_class(x, "tbl_df")

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}:

fs/R/utils.R

Lines 135 to 139 in 8d40528

	if (getOption("fs.use_tibble", TRUE) && is_installed("tibble")) {
	tibble::as_tibble(x)
	} else {
	x
	}

[gaborcsardi]

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

[MichaelChirico]

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

fs/tests/testthat/test-file.R

Line 45 in 0d261dd

it("can be subset as a tibble", {

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.