Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

712 - shinytest2 Run all example apps #721

Merged
merged 23 commits into from
Apr 22, 2024
Merged

Conversation

averissimo
Copy link
Contributor

@averissimo averissimo commented Apr 12, 2024

Pull Request

Part of #712

Changes description

  • Adds tests that iterate on each documentation file and runs the examples apps by mocking interactive and
    shinyApp functions.
    • Checks if there are no errors nor validation errors (with exceptions)
  • Implements run examples using shinytest2 teal.modules.clinical#983 on this repository

Changes from insightsengineering/teal.modules.clinical#983

  • Adds:
    • Regex rules define "accepted" validation errors
  • Fixes:
    • Reverts to use library instead of pkgload::load_all due to problems with system.file call that cannot find package files.
diff -u teal.modules.clinical/tests/testthat/test-examples.R teal.modules.general/tests/testthat/test-examples.R 
--- teal.modules.clinical/tests/testthat/test-examples.R	2024-04-12 10:32:33.100707738 +0200
+++ teal.modules.general/tests/testthat/test-examples.R	2024-04-12 10:26:27.645642183 +0200
@@ -38,12 +38,7 @@ with_mocked_app_bindings <- function(code) {
   # change to `print(shiny__shinyApp(...))` and remove allow warning once fixed
   mocked_shinyApp <- function(ui, server, ...) { # nolint object_name_linter.
     functionBody(server) <- bquote({
-      pkgload::load_all(
-        .(normalizePath(file.path(testthat::test_path(), "..", ".."))),
-        export_all = FALSE,
-        attach_testthat = FALSE,
-        warn_conflicts = FALSE
-      )
+      library(.(testthat::testing_package()), character.only = TRUE)
       .(functionBody(server))
     })
     print(do.call(shiny__shinyApp, append(x = list(ui = ui, server = server), list(...))))
@@ -56,16 +51,34 @@ with_mocked_app_bindings <- function(code) {
     app_driver <- shinytest2::AppDriver$new(
       x,
       shiny_args = args,
+      timeout = 20 * 1000,
+      load_timeout = 30 * 1000,
       check_names = FALSE, # explicit check below
       options = options() # https://github.com/rstudio/shinytest2/issues/377
     )
     on.exit(app_driver$stop(), add = TRUE)
-    app_driver$wait_for_idle(timeout = 20000)
+    app_driver$wait_for_idle()
 
     # Simple testing
     ## warning in the app does not invoke a warning in the test
     ## https://github.com/rstudio/shinytest2/issues/378
     app_logs <- subset(app_driver$get_logs(), location == "shiny")[["message"]]
+
+    # Check if the teal app has content (indicator of a Shiny App fatal error)
+    if (identical(trimws(app_driver$get_text("#teal-main_ui_container")), "")) {
+      tryCatch(
+        app_driver$wait_for_idle(duration = 2000), # wait 2 seconds for session to disconnect
+        error = function(err) {
+          stop(
+            sprintf(
+              "Teal Application is empty. An Error may have occured:\n%s",
+              paste0(subset(app_driver$get_logs(), location == "shiny")[["message"]], collapse = "\n")
+            )
+          )
+        }
+      )
+    }
+
     # allow `Warning in file(con, "r")` warning coming from pkgload::load_all()
     if (any(grepl("Warning in.*", app_logs) & !grepl("Warning in file\\(con, \"r\"\\)", app_logs))) {
       warning(
@@ -79,9 +92,17 @@ with_mocked_app_bindings <- function(code) {
     ## Throw an error instead of a warning (default `AppDriver$new(..., check_names = TRUE)` throws a warning)
     app_driver$expect_unique_names()
 
+    err_el <- Filter(
+      function(x) {
+        allowed_errors <- getOption("test_examples.discard_error_regex", "")
+        identical(allowed_errors, "") || !grepl(allowed_errors, x)
+      },
+      app_driver$get_html(".shiny-output-error")
+    )
+
     ## shinytest2 captures app crash but teal continues on error inside the module
     ## we need to use a different way to check if there are errors
-    if (!is.null(err_el <- app_driver$get_html(".shiny-output-error"))) {
+    if (!is.null(err_el) && length(err_el) > 0) {
       stop(sprintf("Module error is observed:\n%s", err_el))
     }
 
@@ -110,11 +131,14 @@ with_mocked_app_bindings <- function(code) {
 
 strict_exceptions <- c(
   # https://github.com/r-lib/gtable/pull/94
-  "tm_g_barchart_simple.Rd",
-  "tm_g_ci.Rd",
-  "tm_g_ipp.Rd",
-  "tm_g_pp_adverse_events.Rd",
-  "tm_g_pp_vitals.Rd"
+  "tm_outliers.Rd",
+  "tm_g_response.Rd",
+  "tm_a_pca.Rd"
+)
+
+discard_validation_regex <- list(
+  "tm_file_viewer.Rd" = "Please select a file\\.",
+  "tm_g_distribution.Rd" = "Please select a test"
 )
 
 for (i in rd_files()) {
@@ -122,11 +146,18 @@ for (i in rd_files()) {
     paste0("example-", basename(i)),
     {
       testthat::skip_on_cran()
+      skip_if_too_deep(5)
       if (basename(i) %in% strict_exceptions) {
         op <- options()
         withr::local_options(opts_partial_match_old)
         withr::defer(options(op))
       }
+      # Allow for specific validation errors for individual examples
+      withr::local_options(
+        list(
+          "test_examples.discard_error_regex" = discard_validation_regex[[basename(i)]]
+        )
+      )
       with_mocked_app_bindings(
         # suppress warnings coming from saving qenv https://github.com/insightsengineering/teal.code/issues/194
         suppress_warnings(```

@averissimo averissimo mentioned this pull request Apr 12, 2024
19 tasks
@averissimo averissimo marked this pull request as ready for review April 12, 2024 08:24
Copy link
Contributor

github-actions bot commented Apr 12, 2024

Unit Tests Summary

  1 files   22 suites   10m 40s ⏱️
147 tests 147 ✅ 0 💤 0 ❌
475 runs  475 ✅ 0 💤 0 ❌

Results for commit b13ce16.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Apr 12, 2024

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
examples 👶 $+4.95$ $+40$ $0$ $0$ $0$
shinytest2-tm_g_distribution 💚 $56.15$ $-1.01$ $0$ $0$ $0$ $0$
shinytest2-tm_outliers 💚 $87.00$ $-2.35$ $0$ $0$ $0$ $0$
shinytest2-tm_t_crosstable 💚 $27.88$ $-1.26$ $0$ $0$ $0$ $0$
shinytest2-tm_variable_browser 💚 $50.88$ $-1.95$ $0$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
examples 👶 $+1.63$ example_add_facet_labels.Rd
examples 👶 $+0.05$ example_call_fun_dots.Rd
examples 👶 $+0.01$ example_create_sparklines.Rd
examples 👶 $+0.01$ example_establish_updating_selection.Rd
examples 👶 $+0.01$ example_get_scatterplotmatrix_stats.Rd
examples 👶 $+0.01$ example_include_css_files.Rd
examples 👶 $+0.01$ example_is_tab_active_js.Rd
examples 👶 $+0.01$ example_plot_var_summary.Rd
examples 👶 $+0.01$ example_rADAE.Rd
examples 👶 $+0.01$ example_rADLB.Rd
examples 👶 $+0.01$ example_rADRS.Rd
examples 👶 $+0.01$ example_rADSL.Rd
examples 👶 $+0.01$ example_rADTTE.Rd
examples 👶 $+0.01$ example_remove_outliers_from.Rd
examples 👶 $+0.01$ example_render_single_tab.Rd
examples 👶 $+0.01$ example_render_tab_header.Rd
examples 👶 $+0.01$ example_render_tab_table.Rd
examples 👶 $+0.01$ example_render_tabset_panel_content.Rd
examples 👶 $+0.01$ example_shared_params.Rd
examples 👶 $+0.01$ example_teal.modules.general.Rd
examples 👶 $+0.13$ example_tm_a_pca.Rd
examples 👶 $+0.10$ example_tm_a_regression.Rd
examples 👶 $+0.09$ example_tm_data_table.Rd
examples 👶 $+0.15$ example_tm_file_viewer.Rd
examples 👶 $+0.05$ example_tm_front_page.Rd
examples 👶 $+0.10$ example_tm_g_association.Rd
examples 👶 $+0.10$ example_tm_g_bivariate.Rd
examples 👶 $+0.99$ example_tm_g_distribution.Rd
examples 👶 $+0.11$ example_tm_g_response.Rd
examples 👶 $+0.14$ example_tm_g_scatterplot.Rd
examples 👶 $+0.22$ example_tm_g_scatterplotmatrix.Rd
examples 👶 $+0.10$ example_tm_missing_data.Rd
examples 👶 $+0.14$ example_tm_outliers.Rd
examples 👶 $+0.58$ example_tm_t_crosstable.Rd
examples 👶 $+0.12$ example_tm_variable_browser.Rd
examples 👶 $+0.01$ example_validate_input.Rd
examples 👶 $+0.01$ example_var_missings_info.Rd
examples 👶 $+0.01$ example_var_summary_table.Rd
examples 👶 $+0.01$ example_variable_type_icons.Rd
examples 👶 $+0.01$ example_varname_w_label.Rd
shinytest2-tm_g_distribution 💚 $32.48$ $-1.12$ e2e_tm_g_distribution_module_is_initialised_with_the_specified_defaults

Results for commit f8c3396

♻️ This comment has been updated with latest results.

@averissimo averissimo changed the title Run examples using shinytest2 712 - shinytest2 Run all example apps Apr 12, 2024
@averissimo
Copy link
Contributor Author

I find it somehow fishy 🐟 that the test only take around 5s, it takes longer on my local machine.

However, the TESTING_DEPTH env var is defined.

@kartikeyakirar
Copy link
Contributor

@averissimo I have not investigated the issue, but I am receiving a warning message. Can you check this?
image

@averissimo
Copy link
Contributor Author

Very weird @kartikeyakirar

image

Signed-off-by: André Veríssimo <211358+averissimo@users.noreply.github.com>
Copy link
Contributor

@kartikeyakirar kartikeyakirar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic work, @averissimo! The solution is outstanding
image

@averissimo averissimo merged commit 46dc219 into main Apr 22, 2024
24 checks passed
@averissimo averissimo deleted the 477_shinytest2_examples@main branch April 22, 2024 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants