Introduce shinytest2

[vedhav]

Part of https://github.com/insightsengineering/coredev-tasks/issues/503

[gogonzo]

Imprecise tests and too long test_that. Please revisit tests and make as short test_that as possible. What is the overhead of starting headless browser here? Is it seconds or fractions of seconds?

[vedhav]

What is the overhead of starting headless browser here? Is it seconds or fractions of seconds?

It takes about 2 seconds to initialize theAppDriver object. I see your point in splitting the tests into smaller and more precise test desc. I'm hesitant about starting them outside the test_that scope as we will add a skip_on_cran in every shinytest2 tests. I think ~2 seconds of app start is not that bad.

[gogonzo]

It takes about 2 seconds to initialize theAppDriver object ... I'm hesitant about starting them outside the test_that scope as we will add a skip_on_cran in every shinytest2 tests. I think ~2 seconds of app start is not that bad.

Let's do this inside test_that - I think it is more readable.

I see your point in splitting the tests into smaller and more precise test desc.

The point is to minimize time to detect failure and understand what is tested and what not. This file will soon have thousands of lines and imprecise test_that description (which doesn't describe expectation) will make it very hard for us developers to track the cause of the bug and monitor true-coverage. I suggest to be as precise as possible even if it affects performance (but in the same time shinytest2 can't take 10 minutes - or can?)

[gogonzo]

Please see another round of comments. I think at least one test of the filter panel is missing. We need to test if <teal_module>$server receives filtered data. It means you need to test teal_data object in the <teal_module>$server namespace and see if for example mtcars is filtered properly.

testthat::test_that("teal_module server receives reactive teal_data object with data filtered by active filters", { 
  ...
})

[averissimo]

I've found it helpful to create a custom helper to define namespaces with an arbitrary number of arguments. Instead of building it manually every time.

This could be integrated with get_active_ns so that it returns a function instead of a string.

WDYT? @vedhav

helper_ns <- function(id) {
  function(..., .css_prefix = "") {
    args <- rlang::list2(...)
    checkmate::assert_list(args, types = "character")
    (shiny::NS(sprintf("%s%s", .css_prefix, id)))(paste(args, collapse = "-"))
  }
}

ns <- helper_ns("teal-main_ui-filter_manager")

a_name <- "snapshot_manager"
output_id <- ns("filter_manager", a_name, "snapshot_add")
output_id
#> [1] "teal-main_ui-filter_manager-filter_manager-snapshot_manager-snapshot_add"

selector <- ns("filter_manager", "snapshot_manager", "snapshot_add", .css_prefix = "#")
selector
#> [1] "#teal-main_ui-filter_manager-filter_manager-snapshot_manager-snapshot_add"

[vedhav]

This could be integrated with get_active_ns so that it returns a function instead of a string.

Thanks @averissimo it can be a new component to that function. Do you want to do the honors or shall I add it?

[averissimo]

@vedhav I can take the honors 😆

[averissimo]

Nice work everyone!

Looks good!

[m7pr]

Whoa. Great job