fix: Wait for download href before fetching (#445)

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

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 <a> 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)

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("<!DOCTYPE|<html", content, ignore.case = TRUE))
+
+  # Verify the CSV is parseable and has expected data
+  df <- read.csv(file_path)
+  expect_equal(nrow(df), 5)
+  expect_equal(names(df), names(iris))
+})