diff --git a/NEWS.md b/NEWS.md index 8c671569..cc16509f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,13 @@ +# shinytest2 (development version) + +## Bug fixes and minor improvements + +* `$get_download()` and `$expect_download()` now wait for the download + element's `href` attribute to be populated before fetching. Previously, + if the element wasn't in the DOM yet (e.g., inside a modal) or Shiny's + output binding hadn't set the `href`, these methods would silently + return the app's HTML page instead of the intended file (#445). + # shinytest2 0.5.1 ## New features diff --git a/R/app-driver-expect-download.R b/R/app-driver-expect-download.R index 1ae8709f..4daef3b2 100644 --- a/R/app-driver-expect-download.R +++ b/R/app-driver-expect-download.R @@ -9,14 +9,37 @@ app_download <- function( ckm8_assert_app_driver(self, private) self$log_message(paste0("Downloading file for output id: ", output)) - # Find the URL to download from (the href of the tag) + # Wait for the download element to exist and have a non-empty href. + # This handles cases where the element is dynamically rendered (e.g., in a + # modal) or Shiny's output binding hasn't set the href yet. (#445) + href_js <- paste0( + "var el = $('#", output, "'); ", + "el.length > 0 && el.attr('href') !== undefined && el.attr('href') !== ''" + ) + tryCatch( + chromote_wait_for_condition( + self$get_chromote_session(), + href_js, + timeout = private$timeout + ), + error = function(e) { + app_abort( + self, private, + c( + paste0("Download from '#", output, "' failed."), + "i" = "Element not found or its `href` attribute was not set within the timeout.", + "i" = "If the download button is rendered dynamically (e.g., in a modal), make sure the UI is visible before calling `$get_download()` or `$expect_download()`." + ), + parent = e + ) + } + ) + + # Now fetch the href value sub_url <- chromote_eval( self$get_chromote_session(), paste0("$('#", output, "').attr('href')") )$result$value - if (identical(sub_url, "")) { - app_abort(self, private, paste0("Download from '#", output, "' failed")) - } # Add the base location to the URL full_url <- paste0(private$shiny_url$get(), sub_url) diff --git a/tests/testthat/test-app-download.R b/tests/testthat/test-app-download.R index 909d6e93..e8e07d91 100644 --- a/tests/testthat/test-app-download.R +++ b/tests/testthat/test-app-download.R @@ -72,13 +72,6 @@ test_that("download files work from link and button", { app <- AppDriver$new(shiny_app, variant = NULL) - app$wait_for_js("$('#download_link_csv').attr('href') != ''") - app$wait_for_js("$('#download_button_csv').attr('href') != ''") - app$wait_for_js("$('#download_link_txt').attr('href') != ''") - app$wait_for_js("$('#download_button_txt').attr('href') != ''") - app$wait_for_js("$('#download_link_binary').attr('href') != ''") - app$wait_for_js("$('#download_button_binary').attr('href') != ''") - app$expect_download("download_link_txt") app$expect_download("download_button_txt") @@ -105,9 +98,6 @@ test_that("download files can be retrieved", { app <- AppDriver$new(shiny_app, variant = NULL) - app$wait_for_js("$('#download_link_csv').attr('href') != ''") - app$wait_for_js("$('#download_button_csv').attr('href') != ''") - link_file <- app$get_download("download_link_csv") button_file <- app$get_download("download_button_csv", "barret.test") @@ -117,3 +107,47 @@ test_that("download files can be retrieved", { expect_gt(file.info(link_file)$size, 0) expect_gt(file.info(button_file)$size, 0) }) + + +test_that("get_download works for download button inside a modal (#445)", { + modal_app <- shinyApp( + ui = basicPage( + actionButton("show_modal", "Show Modal") + ), + server = function(input, output, session) { + observeEvent(input$show_modal, { + showModal(modalDialog( + downloadButton("modal_download", "Download CSV"), + easyClose = TRUE, + title = "Download" + )) + }) + + output$modal_download <- downloadHandler( + filename = function() { + "modal-data.csv" + }, + content = function(file) { + write.csv(head(iris, 5), file, row.names = FALSE) + } + ) + } + ) + + app <- AppDriver$new(modal_app, variant = NULL) + + # Open the modal + app$click("show_modal") + + # get_download should wait for the href to be ready — no manual wait needed + file_path <- app$get_download("modal_download") + + # Verify we got actual CSV content, not HTML + content <- readLines(file_path, n = 1) + expect_false(grepl("