From cca51ed6823c0f224a25052e07b84470f9143250 Mon Sep 17 00:00:00 2001
From: Barret Schloerke <schloerke@gmail.com>
Date: Tue, 13 Sep 2022 15:27:28 -0400
Subject: [PATCH] Add dynamic `AppDriver$stop(timeout=)` support for `{covr}`
 (#259)

---
 NEWS.md             |  2 ++
 R/app-driver-stop.R | 28 ++++++++++++++++++++++++----
 R/app-driver.R      |  8 ++++++--
 man/AppDriver.Rd    | 12 +++++++++++-
 4 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index 84d568d8..72de1639 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -35,6 +35,8 @@
 
 * Fix documentation on on `AppDriver`'s `delay` parameter being in **seconds**, not *milli*seconds (#255)
 
+* Added support for `AppDriver$stop(timeout=)`. The default timeout when sending `SIGINT`, `SIGTERM`, and `SIGKILL` signals to the `{shiny}` process is now 500ms. However, if `{covr}` is running, the default timeout is 20,000 ms to allow time for the report to generate. (#259)
+
 
 # shinytest2 0.1.1
 
diff --git a/R/app-driver-stop.R b/R/app-driver-stop.R
index e39f6fd8..64304056 100644
--- a/R/app-driver-stop.R
+++ b/R/app-driver-stop.R
@@ -1,8 +1,21 @@
 
-app_stop <- function(self, private) {
+app_stop <- function(self, private, timeout = missing_arg()) {
   "!DEBUG AppDriver$stop"
   ckm8_assert_app_driver(self, private)
 
+  timeout <- rlang::maybe_missing(timeout, {
+    # Increased the timeout for packages like covr to upload their results:
+    #   https://github.com/rstudio/shinytest2/issues/250
+    # Taken from `covr::in_covr()`
+    in_covr <- identical(Sys.getenv("R_COVR"), "true")
+    if (in_covr) {
+      20 * 1000
+    } else {
+      500
+    }
+  })
+  ckm8_assert_single_number(timeout, lower = 0, finite = TRUE)
+
   if (private$state == "stopped") {
     return(invisible(private$shiny_proc_value))
   }
@@ -27,10 +40,17 @@ app_stop <- function(self, private) {
           # SIGINT quits the Shiny application, SIGTERM tells R to quit.
           # Unfortunately, SIGTERM isn't quite the same as `q()`, because
           # finalizers with onexit=TRUE don't seem to run.
-          private$shiny_process$signal(tools::SIGINT)
-          private$shiny_process$wait(500)
+
+          # Using private$shiny_process$interrupt() to send SIGNAL 2 (SIGINT) to the process
+          # https://github.com/r-lib/processx/blob/84301784382296217e7f5d11e1116dc4e24da809/R/process.R#L276-L283
+          # https://github.com/r-lib/covr/issues/277#issuecomment-555502769
+          # https://github.com/rstudio/shinytest2/issues/250
+          private$shiny_process$interrupt()
+          private$shiny_process$wait(timeout)
+
           private$shiny_process$signal(tools::SIGTERM)
-          private$shiny_process$wait(250)
+          private$shiny_process$wait(timeout)
+
           private$shiny_process$kill()
         },
         error = function(e) {
diff --git a/R/app-driver.R b/R/app-driver.R
index 9fe8eb17..31e3c016 100644
--- a/R/app-driver.R
+++ b/R/app-driver.R
@@ -1662,6 +1662,10 @@ AppDriver <- R6Class( # nolint
     #' Typically, this can be paired with a button that when clicked will call
     #' `shiny::stopApp(info)` to return `info` from the test app back to the
     #' main R session.
+    #' @param timeout Milliseconds to wait between sending a SIGINT, SIGTERM,
+    #'   and SIGKILL to the Shiny process. Defaults to 500ms. However, if
+    #'   \pkg{covr} is currently executing, then the `timeout` is set to
+    #'   20,000ms to allow for the coverage report to be generated.
     #' @return The result of the background process if the Shiny application has
     #'   already been terminated.
     #' @examples
@@ -1711,8 +1715,8 @@ AppDriver <- R6Class( # nolint
     #' #> ..$ session: chr "bdc7417f2fc8c84fc05c9518e36fdc44"
     #' #> ..$ time   : num 1.65e+09
     #' }
-    stop = function() {
-      app_stop(self, private)
+    stop = function(timeout = missing_arg()) {
+      app_stop(self, private, timeout = timeout)
     }
   )
 )
diff --git a/man/AppDriver.Rd b/man/AppDriver.Rd
index 8b98e94e..182f0afb 100644
--- a/man/AppDriver.Rd
+++ b/man/AppDriver.Rd
@@ -3068,9 +3068,19 @@ Typically, this can be paired with a button that when clicked will call
 \code{shiny::stopApp(info)} to return \code{info} from the test app back to the
 main R session.
 \subsection{Usage}{
-\if{html}{\out{<div class="r">}}\preformatted{AppDriver$stop()}\if{html}{\out{</div>}}
+\if{html}{\out{<div class="r">}}\preformatted{AppDriver$stop(timeout = missing_arg())}\if{html}{\out{</div>}}
 }
 
+\subsection{Arguments}{
+\if{html}{\out{<div class="arguments">}}
+\describe{
+\item{\code{timeout}}{Milliseconds to wait between sending a SIGINT, SIGTERM,
+and SIGKILL to the Shiny process. Defaults to 500ms. However, if
+\pkg{covr} is currently executing, then the \code{timeout} is set to
+20,000ms to allow for the coverage report to be generated.}
+}
+\if{html}{\out{</div>}}
+}
 \subsection{Returns}{
 The result of the background process if the Shiny application has
 already been terminated.