search

r-lib / usethis

Set up commonly used 📦 components
https://usethis.r-lib.org/
Other
861 stars 285 forks source link

use_testthat() fails for Shiny apps #1017

Closed alandipert closed 4 years ago

alandipert commented 4 years ago

In the next version of Shiny we're planning on releasing a new function for running tests in the application environment, shiny::runTests() https://github.com/rstudio/shiny/pull/2585

This will promote the use of testthat in Shiny apps.

I thought to start a discussion here on usethis::use_testthat()'s behavior with respect to this change. Currently, running it in a Shiny app results in the following error message:

Error: `use_testthat()` is designed to work with packages.

We hope to promote unit testing in Shiny apps moving forward, and I thought it might be good for use_testthat() to work in this case.

Unfortunately, Shiny doesn't have a predicate function to determine whether or not the current directory is plausibly a Shiny app. However, I think the logic's pretty simple: at a minimum, there has to be either an app.R or both a server.R and ui.R in the current directory.

So, one way forward would be to check for package and Shiny app plausibility, and bail otherwise, but as a non-expert on all things usethis I'm looking forward to hearing how other people think it should work.

Thanks in advance for your feedback, and consider me on the hook for whatever implementation is needed 👍

alandipert commented 4 years ago

Note that the lack of this in usethis would be mitigated by forthcoming "Shiny app scaffold/template" feature that @wch is working on very soon.

hadley commented 4 years ago

Would you consider requiring a DESCRIPTION for shiny apps in this situation?

alandipert commented 4 years ago

I am, though I think we would still need an additional convention to declare the app entrypoint file/function(s). The golem package is a related work that suggests a DESCRIPTION file along with a convention for naming entrypoint files: https://thinkr-open.github.io/building-shiny-apps-workflow/golem.html#understanding-golem-app-structure

~~ 2 minutes elapse ~~

On further reflection the DESCRIPTION/golem way and the autoload way maybe aren't mutually exclusive? I'll have to think about this more but I'd love to hear other thoughts.

hadley commented 4 years ago

I was sort of imagining that this could be a "partial" package so that app.R would still live in the root directory (although maybe that would be too confusing)

hadley commented 4 years ago

OTOH, we could just make use_test() work outside of a package — it would just require a couple of conditional to avoid touching the DESCRIPTION if it doesn't exist.

alandipert commented 4 years ago

@hadley thanks, I like that suggestion about making use_test() more flexible, would you care for a PR? I could give it a whirl tomorrow.

hadley commented 4 years ago

Yeah, that’d be great