From f7402bc22155e402bbb92a341c06f7f5105366cb Mon Sep 17 00:00:00 2001 From: Pawel Rucki <12943682+pawelru@users.noreply.github.com> Date: Tue, 6 Aug 2024 08:41:03 +0200 Subject: [PATCH] several test enhancements (#256) A few enhancements including: - consistent usage of `grDevices::pdf(nullfile())` replacing `grDevices::pdf()` - a follow-up on https://github.com/insightsengineering/teal.widgets/pull/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 <12943682+pawelru@users.noreply.github.com> --- R/plot_with_settings.R | 2 +- R/table_with_settings.R | 2 +- tests/testthat/helpers-utils.R | 1 + tests/testthat/test-plot_with_settings.R | 20 ++++++++--------- tests/testthat/test-plot_with_settings_ui.R | 3 --- tests/testthat/test-table_with_settings.R | 23 +++++++++----------- tests/testthat/test-table_with_settings_ui.R | 3 --- 7 files changed, 22 insertions(+), 32 deletions(-) diff --git a/R/plot_with_settings.R b/R/plot_with_settings.R index 65cc8ba1..2a681065 100644 --- 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 index fcfb1a9a..b85e435b 100644 --- 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 index 71e3af76..d3c4a251 100644 --- 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 index cc44b37e..bc572ff0 100644 --- 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 index cb4d48d0..d18b2758 100644 --- 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 index ca43c318..a21a13c4 100644 --- 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 index d6cdca53..a9f3912f 100644 --- 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)