Skip test that requires rstudioapi

[MichaelChirico]

Hi @gaborcsardi, I know you're not a fan of skip_if_not_installed(), but the way this test fails on systems without {rstudioapi} is really confusing:

── Failure (test-prettycode.R:190:3): code themes ──────────────────────────────
code_theme_default() (`actual`) not equal to "foo" (`expected`).

`actual` is a list
`expected` is a character vector ('foo')

The test assumes this branch is hit:

cli/R/prettycode.R

Lines 243 to 245 in fbec918

	if (requireNamespace("rstudioapi", quietly = TRUE)) {
	return(code_theme_default_rstudio())
	}

I think a skip is the right approach here -- making the test more flexible seems like overkill. library(rstudioapi) would also make the failure more evident.

[gaborcsardi]

You are probably expecting this question, so here it is. Why don't you install rstudioapi?

[MichaelChirico]

As it happens, I do have it available & was able to fix the test.

However, our build system (https://bazel.build/) requires optional dependencies be specified individually, so I was met with the above failure.

My point is that it requires an expert eye to figure out what's wrong in this case -- there's nothing in the failure report about {rstudioapi} at all. I could easily have spent 30-45 minutes trying to debug the issue -- by pure luck I spotted this requireNamespace() call quickly & could guess the fix.

Having 'rstudioapi' written somewhere in the error log is essential IMO. The proposal here is one way to generate that, and IMO the most canonical.

[MichaelChirico]

Contrast this with the dozen+ tests that require {mockery} (also Suggests); when running without {mockery}, at least we get a transparent error:

── Error (test-ansi-hyperlink.R:289:3): ansi_has_hyperlink_support ─────────────
<packageNotFoundError/error/condition>
Error in `loadNamespace(x)`: there is no package called 'mockery'
Backtrace:
    ▆
 1. └─base::loadNamespace(x) at test-ansi-hyperlink.R:289:3
 2.   └─base::withRestarts(stop(cond), retry_loadNamespace = function() NULL)
 3.     └─base (local) withOneRestart(expr, restarts[[1L]])
 4.       └─base (local) doWithOneRestart(return(expr), restart)

I can see the error log & fix the build in seconds.

[gaborcsardi]

However, our build system (bazel.build) requires optional dependencies be specified individually,

Why not specify the packages in Suggests, that's what Suggests is for? If you are only adding suggested packages if tests fail, then you are potentially missing a lot of tests. Anyway, I obviously know nothing about your build system....

Are you running R CMD check or just the tests? If the former, I assume you set _R_CHECK_FORCE_SUGGESTS_ = false?

[gaborcsardi]

Thanks!