Appsilon / rhino

Build high quality, enterprise-grade Shiny apps at speed
https://appsilon.github.io/rhino
287 stars 24 forks source link

Making it more convenient to set up R process before running rhino::test_e2e #569

Open jakubsob opened 7 months ago

jakubsob commented 7 months ago

Motivation

I want to be able to run Cypress tests with different options or environment variables. Currently if I want to run E2E tests with different config (set via R_CONFIG_ACTIVE) I need to do it manually, before running the command.

Feature description

Options could be passed explicitly to the test command

rhino::test_e2e(
  interactive = TRUE,
  envvar = list(R_CONFIG_ACTIVE = "cypress"),
  options = list()
)

or it could be a hook that allows you to do arbitrary setup in R process that will run the Shiny app

rhino::test_e2e(setup = \() Sys.setenv(R_CONFIG_ACTIVE = "cypress"))

or it could be a special file in tests/ directory, alike testthat setup files, e.g.,:

#' tests/setup-cypress.R

Sys.setenv(R_CONFIG_ACTIVE="cypress")

or rhino could set itself an env var that indicates that E2E tests are running, allowing you to dispatch a different config, similarly to how testthat allows you to do with testthat::is_testing().

Implementation ideas

No response

Impact

No response

Comments

No response

kamilzyla commented 7 months ago

Hey @jakubsob, thanks for this feature suggestion! :smiley:

Have you considered using withr?

withr::with_envvar(c(R_CONFIG_ACTIVE = "cypress"), rhino::test_e2e())

If this approach does not satisfy you, could you please elaborate on your need? Do you need to set env variables only when running rhino::test_e2e() locally, or also in CI? Which of your suggested options seem most convenient for you and why?

jakubsob commented 7 months ago

Yes, withr is the way to go to temporarily change env var. But from a DX perspective, this might be quite inconvenient.

In CI it's easy because we can just configure env var for E2E step – do it once, use it each time.

But to run the command locally, I need to:

If we allow some options to be passed to rhino::test_e2e, we probably won't be able to cover all use cases and needs, so this would never be flexible enough.

Now, the most enticing option to me would be to have an env var set when rhino::test_e2e() is run so that the app can dispatch a proper config – then there's no need to alter the command. As in testthat::is_testing:

> testthat::is_testing
function () 
{
    identical(Sys.getenv("TESTTHAT"), "true")
}
> rhino::test_e2e
function (interactive = FALSE) 
{
    withr::with_envvar(c(RHINO_E2E = "true"), {
        if (interactive) {
            npm("run", "test-e2e-interactive")
        }
        else {
            npm("run", "test-e2e")
        }
    })
}

and

> rhino::is_testing_e2e
function () 
{
    identical(Sys.getenv("RHINO_E2E"), "true")
}

It doesn't mess up with the interface of rhino::test_e2e function and allows developers to alter the behavior/setup of the app however they want in E2E tests based just on that one flag. WDYT?