several test enhancements (#256)

A few enhancements including: - consistent usage of `grDevices::pdf(nullfile())` replacing `grDevices::pdf()` - a follow-up on #203 - better catch errors in the test `"type_download_srv_table: pagination, lpp to small"` - DRY for `plot_types` variable - remove `expect_download`. This is not _only_ expectation that the downloadable link exists but it also downloads the file and run snapshot testing on it. The problem is that this file is named from sys time on the moment of test run (e.g. `tests/testthat/_snaps/app_driver_pws_ui/pws-001-plot_20240805_034140.png`) which creates a new file every time you run the test (that's why our integration test fails). Moreover, I believe that this is not the intend of that unit test to test regression of a dummy plot. --------- Signed-off-by: Pawel Rucki <[email protected]>
insightsengineering · Aug 6, 2024 · f7402bc · f7402bc
1 parent 761b2d7
commit f7402bc
Show file tree

Hide file tree

Showing 7 changed files with 22 additions and 32 deletions.
diff --git a/R/plot_with_settings.R b/R/plot_with_settings.R
@@ -413,7 +413,7 @@ plot_with_settings_srv <- function(id,
     })
 
     output$width_warning <- renderUI({
-      grDevices::pdf(NULL) # reset Rplots.pdf for shiny server
+      grDevices::pdf(nullfile()) # reset Rplots.pdf for shiny server
       w <- grDevices::dev.size("px")[1]
       grDevices::dev.off()
       if (p_width() < w) {

diff --git a/R/table_with_settings.R b/R/table_with_settings.R
@@ -202,7 +202,7 @@ type_download_srv_table <- function(id, table_reactive) {
           try(rtables::paginate_table(
             tt = table_reactive(),
             lpp = as.numeric(input$lpp)
-          ))
+          ), silent = TRUE)
         }
 
         if (inherits(catch_warning, "try-error")) {

diff --git a/tests/testthat/helpers-utils.R b/tests/testthat/helpers-utils.R
@@ -7,6 +7,7 @@ is_draw <- function(plot_fun) {
   checkmate::assert_function(plot_fun)
   grDevices::graphics.off() # close any current graphics devices
   cdev <- grDevices::dev.cur()
+  grDevices::pdf(nullfile())
   plot_fun()
   if (cdev != grDevices::dev.cur()) {
     on.exit(grDevices::dev.off())

diff --git a/tests/testthat/test-plot_with_settings.R b/tests/testthat/test-plot_with_settings.R
@@ -75,17 +75,16 @@ plot_funs <- list(
   function() boxplot(2),
   function() 2
 )
+plot_types <- list(
+  function() "gg",
+  function() "trel",
+  function() "grob",
+  function() "base",
+  function() "base",
+  function() "other"
+)
 
 testthat::test_that("print_plot is able to plot different types of graphics", {
-  plot_types <- list(
-    function() "ANYTHING",
-    function() "trel",
-    function() "grob",
-    function() "base",
-    function() "base",
-    function() "other"
-  )
-
   for (p in seq_along(plot_funs)) {
     testthat::expect_true(
       is_draw(function() print_plot(plot_funs[[p]], plot_types[[p]]))
@@ -484,14 +483,13 @@ testthat::test_that("plot_with_settings_srv returns the click ggplot2 functional
 })
 
 testthat::test_that("plot_with_settings_srv and plot_type reactive types", {
-  plot_types <- c("gg", "trel", "grob", "base", "base", "other")
   for (p in seq_along(plot_funs)) {
     plot_with_settings_args[["plot_r"]] <- plot_funs[[p]]
     shiny::testServer(
       teal.widgets:::plot_with_settings_srv,
       args = plot_with_settings_args,
       expr = {
-        testthat::expect_identical(plot_type(), plot_types[p])
+        plot_suppress(testthat::expect_identical(plot_type(), plot_types[[p]]()))
       }
     )
   }

diff --git a/tests/testthat/test-plot_with_settings_ui.R b/tests/testthat/test-plot_with_settings_ui.R
@@ -388,7 +388,6 @@ testthat::test_that(
 
     app_driver$click(selector = "#plot_with_settings-downbutton-downl")
     app_driver$wait_for_idle(timeout = default_idle_timeout)
-    app_driver$expect_download("plot_with_settings-downbutton-data_download")
 
     filename <- app_driver$get_download("plot_with_settings-downbutton-data_download")
     testthat::expect_match(filename, "png$", fixed = FALSE)
@@ -454,8 +453,6 @@ testthat::test_that("e2e teal.widgets::plot_with_settings: expanded image can be
   app_driver$click(selector = "#plot_with_settings-modal_downbutton-downl")
   app_driver$wait_for_idle(timeout = default_idle_timeout)
 
-  app_driver$expect_download("plot_with_settings-modal_downbutton-data_download")
-
   filename <- app_driver$get_download("plot_with_settings-modal_downbutton-data_download")
   testthat::expect_match(filename, "png$", fixed = FALSE)
 

diff --git a/tests/testthat/test-table_with_settings.R b/tests/testthat/test-table_with_settings.R
@@ -110,20 +110,17 @@ testthat::test_that("type_download_srv_table: downloading different output types
 })
 
 testthat::test_that("type_download_srv_table: pagination, lpp to small", {
-  testthat::expect_error(
-    shiny::testServer(
-      teal.widgets:::type_download_srv_table,
-      args = list(id = "tws", table_reactive = table_r),
-      expr = {
-        for (down_type in c(".txt", ".pdf")) {
-          session$setInputs(`pagination_switch` = TRUE)
-          session$setInputs(`lpp` = 1)
-          session$setInputs(`file_format` = down_type)
-          testthat::expect_true(file.exists(output$data_download))
-        }
+  shiny::testServer(
+    teal.widgets:::type_download_srv_table,
+    args = list(id = "tws", table_reactive = table_r),
+    expr = {
+      for (down_type in c(".txt", ".pdf")) {
+        session$setInputs(`pagination_switch` = TRUE)
+        session$setInputs(`lpp` = 1)
+        session$setInputs(`file_format` = down_type)
+        testthat::expect_error(output$data_download, "Lines of repeated context")
       }
-    ),
-    "Lines of repeated context"
+    }
   )
 })
 

diff --git a/tests/testthat/test-table_with_settings_ui.R b/tests/testthat/test-table_with_settings_ui.R
@@ -343,7 +343,6 @@ testthat::test_that(
 
     app_driver$click(selector = "#table_with_settings-downbutton-dwnl")
     app_driver$wait_for_idle(timeout = default_idle_timeout)
-    app_driver$expect_download("table_with_settings-downbutton-data_download")
 
     filename <- app_driver$get_download("table_with_settings-downbutton-data_download")
     testthat::expect_match(filename, "txt$", fixed = FALSE)
@@ -371,8 +370,6 @@ testthat::test_that("e2e teal.widgets::table_with_settings: expanded table can b
   app_driver$click(selector = "#table_with_settings-modal_downbutton-dwnl")
   app_driver$wait_for_idle(timeout = default_idle_timeout)
 
-  app_driver$expect_download("table_with_settings-modal_downbutton-data_download")
-
   filename <- app_driver$get_download("table_with_settings-modal_downbutton-data_download")
   testthat::expect_match(filename, "txt$", fixed = FALSE)