From a943d955dd914b6de969aafce72eefd47da175a4 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Thu, 12 Mar 2020 15:58:46 -0400 Subject: [PATCH 01/36] make sure the server is only run if the example is interactive --- R/modules.R | 8 ++++++-- man/moduleServer.Rd | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/R/modules.R b/R/modules.R index 24bea6a346..d4cf3f3915 100644 --- a/R/modules.R +++ b/R/modules.R @@ -91,7 +91,9 @@ createSessionProxy <- function(parentSession, ...) { #' counterServer("counter1") #' counterServer("counter2") #' } -#' shinyApp(ui, server) +#' if (interactive()) { +#' shinyApp(ui, server) +#' } #' #' #' @@ -117,7 +119,9 @@ createSessionProxy <- function(parentSession, ...) { #' server <- function(input, output, session) { #' counterServer2("counter", "The current count is: ") #' } -#' shinyApp(ui, server) +#' if (interactive()) { +#' shinyApp(ui, server) +#' } #' #' @export moduleServer <- function(id, module, session = getDefaultReactiveDomain()) { diff --git a/man/moduleServer.Rd b/man/moduleServer.Rd index 95e57a8833..b2daa7e917 100644 --- a/man/moduleServer.Rd +++ b/man/moduleServer.Rd @@ -67,7 +67,9 @@ server <- function(input, output, session) { counterServer("counter1") counterServer("counter2") } -shinyApp(ui, server) +if (interactive()) { + shinyApp(ui, server) +} @@ -93,7 +95,9 @@ ui <- fluidPage( server <- function(input, output, session) { counterServer2("counter", "The current count is: ") } -shinyApp(ui, server) +if (interactive()) { + shinyApp(ui, server) +} } \seealso{ From 386135788b2c461ddce48d789671cd5d36ad97c3 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Thu, 12 Mar 2020 16:12:05 -0400 Subject: [PATCH 02/36] get travis to pass for now --- inst/_pkgdown.yml | 2 +- tests/testthat/test-test-module.R | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/inst/_pkgdown.yml b/inst/_pkgdown.yml index 49ecfe2b38..8112bf3ca3 100644 --- a/inst/_pkgdown.yml +++ b/inst/_pkgdown.yml @@ -207,7 +207,7 @@ reference: desc: Functions for modularizing Shiny apps contents: - NS - - callModule + - moduleServer - title: Embedding desc: Functions that are intended for third-party packages that embed Shiny applications. contents: diff --git a/tests/testthat/test-test-module.R b/tests/testthat/test-test-module.R index a542b38a07..758d8ff61b 100644 --- a/tests/testthat/test-test-module.R +++ b/tests/testthat/test-test-module.R @@ -519,6 +519,7 @@ test_that("testModule works with nested modules", { reactive(paste("a value:", r())) } + testthat::skip("This fails. I believe it is being removed.") testModule(outerModule, { session$setInputs(x = 1) expect_equal(output$someVar, "a value: 2") From 88374eca74d9a898a19ef0057c32d49b4be1ffd1 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Thu, 12 Mar 2020 13:16:55 -0700 Subject: [PATCH 03/36] Update modules.R --- R/modules.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/modules.R b/R/modules.R index d4cf3f3915..9cf15c3de9 100644 --- a/R/modules.R +++ b/R/modules.R @@ -132,7 +132,7 @@ moduleServer <- function(id, module, session = getDefaultReactiveDomain()) { #' @rdname moduleServer #' @export callModule <- function(module, id, ..., session = getDefaultReactiveDomain()) { - if (!inherits(session, "ShinySession") && !inherits(session, "session_proxy")) { + if (!inherits(session, c("ShinySession", "session_proxy", "MockShinySession"))) { stop("session must be a ShinySession or session_proxy object.") } childScope <- session$makeScope(id) From 2d324c77c15b6843abff7b21153c3c49cdd0e05e Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Thu, 12 Mar 2020 13:18:23 -0700 Subject: [PATCH 04/36] Add S3 class to MockShinySession --- R/mock-session.R | 1 - 1 file changed, 1 deletion(-) diff --git a/R/mock-session.R b/R/mock-session.R index a88871cf30..a90b060f2e 100644 --- a/R/mock-session.R +++ b/R/mock-session.R @@ -80,7 +80,6 @@ extract <- function(promise) { MockShinySession <- R6Class( 'MockShinySession', portable = FALSE, - class = FALSE, public = list( #' @field env The environment associated with the session. env = NULL, From a577b1e22ea8ebf8aab847053e65062b75cdf9e1 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Thu, 12 Mar 2020 16:20:13 -0400 Subject: [PATCH 05/36] un-skip test --- tests/testthat/test-test-module.R | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/testthat/test-test-module.R b/tests/testthat/test-test-module.R index 758d8ff61b..a542b38a07 100644 --- a/tests/testthat/test-test-module.R +++ b/tests/testthat/test-test-module.R @@ -519,7 +519,6 @@ test_that("testModule works with nested modules", { reactive(paste("a value:", r())) } - testthat::skip("This fails. I believe it is being removed.") testModule(outerModule, { session$setInputs(x = 1) expect_equal(output$someVar, "a value: 2") From e17f416bb0449441768d9ab80afb9d1ea332dd60 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Thu, 12 Mar 2020 16:30:18 -0400 Subject: [PATCH 06/36] Update roxygen level --- DESCRIPTION | 2 +- man/MockShinySession.Rd | 37 +++++++++++++++++++++++++++++++++++++ man/NS.Rd | 4 +++- man/Progress.Rd | 8 ++++++++ 4 files changed, 49 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 67fb988d3f..162b750cda 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -173,6 +173,6 @@ Collate: 'test-module.R' 'test.R' 'update-input.R' -RoxygenNote: 7.0.2 +RoxygenNote: 7.1.0 Encoding: UTF-8 Roxygen: list(markdown = TRUE) diff --git a/man/MockShinySession.Rd b/man/MockShinySession.Rd index e0236583d7..b5df4e90e9 100644 --- a/man/MockShinySession.Rd +++ b/man/MockShinySession.Rd @@ -88,6 +88,7 @@ s$setInputs(x=1, y=2) } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-reactlog}{}}} \subsection{Method \code{reactlog()}}{ No-op \subsection{Usage}{ @@ -104,6 +105,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-incrementBusyCount}{}}} \subsection{Method \code{incrementBusyCount()}}{ No-op \subsection{Usage}{ @@ -113,6 +115,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-new}{}}} \subsection{Method \code{new()}}{ Create a new MockShinySession \subsection{Usage}{ @@ -122,6 +125,7 @@ Create a new MockShinySession } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-onFlush}{}}} \subsection{Method \code{onFlush()}}{ Define a callback to be invoked before a reactive flush \subsection{Usage}{ @@ -140,6 +144,7 @@ Define a callback to be invoked before a reactive flush } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-onFlushed}{}}} \subsection{Method \code{onFlushed()}}{ Define a callback to be invoked after a reactive flush \subsection{Usage}{ @@ -158,6 +163,7 @@ Define a callback to be invoked after a reactive flush } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-onEnded}{}}} \subsection{Method \code{onEnded()}}{ Define a callback to be invoked when the session ends \subsection{Usage}{ @@ -174,6 +180,7 @@ Define a callback to be invoked when the session ends } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-isEnded}{}}} \subsection{Method \code{isEnded()}}{ Returns \code{FALSE} if the session has not yet been closed \subsection{Usage}{ @@ -183,6 +190,7 @@ Returns \code{FALSE} if the session has not yet been closed } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-isClosed}{}}} \subsection{Method \code{isClosed()}}{ Returns \code{FALSE} if the session has not yet been closed \subsection{Usage}{ @@ -192,6 +200,7 @@ Returns \code{FALSE} if the session has not yet been closed } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-close}{}}} \subsection{Method \code{close()}}{ Closes the session \subsection{Usage}{ @@ -201,6 +210,7 @@ Closes the session } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-cycleStartAction}{}}} \subsection{Method \code{cycleStartAction()}}{ Unsophisticated mock implementation that merely invokes the given callback immediately. @@ -218,6 +228,7 @@ the given callback immediately. } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-fileUrl}{}}} \subsection{Method \code{fileUrl()}}{ Base64-encode the given file. Needed for image rendering. \subsection{Usage}{ @@ -238,6 +249,7 @@ Base64-encode the given file. Needed for image rendering. } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-setInputs}{}}} \subsection{Method \code{setInputs()}}{ Sets reactive values associated with the \code{session$inputs} object and flushes the reactives. @@ -264,6 +276,7 @@ s$setInputs(x=1, y=2) } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-.scheduleTask}{}}} \subsection{Method \code{.scheduleTask()}}{ An internal method which shouldn't be used by others. \subsection{Usage}{ @@ -282,6 +295,7 @@ An internal method which shouldn't be used by others. } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-elapse}{}}} \subsection{Method \code{elapse()}}{ Simulate the passing of time by the given number of milliseconds. \subsection{Usage}{ @@ -298,6 +312,7 @@ Simulate the passing of time by the given number of milliseconds. } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-.now}{}}} \subsection{Method \code{.now()}}{ An internal method which shouldn't be used by others. \subsection{Usage}{ @@ -307,6 +322,7 @@ An internal method which shouldn't be used by others. } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-defineOutput}{}}} \subsection{Method \code{defineOutput()}}{ An internal method which shouldn't be used by others. \subsection{Usage}{ @@ -327,6 +343,7 @@ An internal method which shouldn't be used by others. } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-getOutput}{}}} \subsection{Method \code{getOutput()}}{ An internal method which shouldn't be used by others. \subsection{Usage}{ @@ -343,6 +360,7 @@ An internal method which shouldn't be used by others. } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-registerDataObj}{}}} \subsection{Method \code{registerDataObj()}}{ No-op \subsection{Usage}{ @@ -363,6 +381,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-allowReconnect}{}}} \subsection{Method \code{allowReconnect()}}{ No-op \subsection{Usage}{ @@ -379,6 +398,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-reload}{}}} \subsection{Method \code{reload()}}{ No-op \subsection{Usage}{ @@ -388,6 +408,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-resetBrush}{}}} \subsection{Method \code{resetBrush()}}{ No-op \subsection{Usage}{ @@ -404,6 +425,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-sendCustomMessage}{}}} \subsection{Method \code{sendCustomMessage()}}{ No-op \subsection{Usage}{ @@ -422,6 +444,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-sendBinaryMessage}{}}} \subsection{Method \code{sendBinaryMessage()}}{ No-op \subsection{Usage}{ @@ -440,6 +463,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-sendInputMessage}{}}} \subsection{Method \code{sendInputMessage()}}{ No-op \subsection{Usage}{ @@ -458,6 +482,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-setBookmarkExclude}{}}} \subsection{Method \code{setBookmarkExclude()}}{ No-op \subsection{Usage}{ @@ -474,6 +499,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-getBookmarkExclude}{}}} \subsection{Method \code{getBookmarkExclude()}}{ No-op \subsection{Usage}{ @@ -483,6 +509,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-onBookmark}{}}} \subsection{Method \code{onBookmark()}}{ No-op \subsection{Usage}{ @@ -499,6 +526,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-onBookmarked}{}}} \subsection{Method \code{onBookmarked()}}{ No-op \subsection{Usage}{ @@ -515,6 +543,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-doBookmark}{}}} \subsection{Method \code{doBookmark()}}{ No-op \subsection{Usage}{ @@ -524,6 +553,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-onRestore}{}}} \subsection{Method \code{onRestore()}}{ No-op \subsection{Usage}{ @@ -540,6 +570,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-onRestored}{}}} \subsection{Method \code{onRestored()}}{ No-op \subsection{Usage}{ @@ -556,6 +587,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-exportTestValues}{}}} \subsection{Method \code{exportTestValues()}}{ No-op \subsection{Usage}{ @@ -565,6 +597,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-getTestSnapshotUrl}{}}} \subsection{Method \code{getTestSnapshotUrl()}}{ No-op \subsection{Usage}{ @@ -592,6 +625,7 @@ No-op } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-ns}{}}} \subsection{Method \code{ns()}}{ Returns the given id prefixed by \verb{mock-session-}. \subsection{Usage}{ @@ -608,6 +642,7 @@ Returns the given id prefixed by \verb{mock-session-}. } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-flushReact}{}}} \subsection{Method \code{flushReact()}}{ Trigger a reactive flush right now. \subsection{Usage}{ @@ -617,6 +652,7 @@ Trigger a reactive flush right now. } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-makeScope}{}}} \subsection{Method \code{makeScope()}}{ Create and return a namespace-specific session proxy. \subsection{Usage}{ @@ -633,6 +669,7 @@ Create and return a namespace-specific session proxy. } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-clone}{}}} \subsection{Method \code{clone()}}{ The objects of this class are cloneable with this method. \subsection{Usage}{ diff --git a/man/NS.Rd b/man/NS.Rd index fff744c3d7..27f0c66ce7 100644 --- a/man/NS.Rd +++ b/man/NS.Rd @@ -5,7 +5,9 @@ \alias{NS} \alias{ns.sep} \title{Namespaced IDs for inputs/outputs} -\format{An object of class \code{character} of length 1.} +\format{ +An object of class \code{character} of length 1. +} \usage{ NS(namespace, id = NULL) diff --git a/man/Progress.Rd b/man/Progress.Rd index 7f165fd1ca..63d6630a9c 100644 --- a/man/Progress.Rd +++ b/man/Progress.Rd @@ -74,6 +74,7 @@ shinyApp(ui, server) } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-new}{}}} \subsection{Method \code{new()}}{ Creates a new progress panel (but does not display it). \subsection{Usage}{ @@ -107,6 +108,7 @@ is for backward-compatibility).} } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-set}{}}} \subsection{Method \code{set()}}{ Updates the progress panel. When called the first time, the progress panel is displayed. @@ -134,6 +136,7 @@ relative to \code{message}.} } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-inc}{}}} \subsection{Method \code{inc()}}{ Like \code{set}, this updates the progress panel. The difference is that \code{inc} increases the progress bar by \code{amount}, instead of @@ -161,6 +164,7 @@ relative to \code{message}.} } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-getMin}{}}} \subsection{Method \code{getMin()}}{ Returns the minimum value. \subsection{Usage}{ @@ -170,6 +174,7 @@ Returns the minimum value. } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-getMax}{}}} \subsection{Method \code{getMax()}}{ Returns the maximum value. \subsection{Usage}{ @@ -179,6 +184,7 @@ Returns the maximum value. } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-getValue}{}}} \subsection{Method \code{getValue()}}{ Returns the current value. \subsection{Usage}{ @@ -188,6 +194,7 @@ Returns the current value. } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-close}{}}} \subsection{Method \code{close()}}{ Removes the progress panel. Future calls to \code{set} and \code{close} will be ignored. @@ -198,6 +205,7 @@ Removes the progress panel. Future calls to \code{set} and } \if{html}{\out{
}} \if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-clone}{}}} \subsection{Method \code{clone()}}{ The objects of this class are cloneable with this method. \subsection{Usage}{ From b005799d92a1d752c4ed12cb4338b8af49427658 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Mon, 23 Mar 2020 23:01:53 +0000 Subject: [PATCH 07/36] Add back many working/converted tests --- R/mock-session.R | 24 +- R/modules.R | 6 +- R/test-module.R | 60 +- tests/testthat/test-test-module.R | 1047 +++++++++++++++-------------- 4 files changed, 594 insertions(+), 543 deletions(-) diff --git a/R/mock-session.R b/R/mock-session.R index a90b060f2e..57ae565d55 100644 --- a/R/mock-session.R +++ b/R/mock-session.R @@ -83,6 +83,10 @@ MockShinySession <- R6Class( public = list( #' @field env The environment associated with the session. env = NULL, + #' @field mask The inner-module environment mask + mask = NULL, + #' @field returned The value returned by the module. + returned = NULL, #' @field singletons Hardcoded as empty. Needed for rendering HTML (i.e. renderUI) singletons = character(0), #' @field clientData Mock client data that always returns a size for plots @@ -390,6 +394,13 @@ MockShinySession <- R6Class( output = structure(.createOutputWriter(self, ns = ns), class = "shinyoutput"), makeScope = function(namespace) self$makeScope(ns(namespace)) ) + }, + # If assigning to `returned`, proactively flush + #' @param value The value returned from the module + setReturned = function(value) { + self$returned <- value + private$flush() + value } ), private = list( @@ -400,7 +411,6 @@ MockShinySession <- R6Class( timer = NULL, closed = FALSE, outs = list(), - returnedVal = NULL, flush = function(){ isolate(private$flushCBs$invoke(..stacktraceon = TRUE)) @@ -410,18 +420,6 @@ MockShinySession <- R6Class( } ), active = list( - # If assigning to `returned`, proactively flush - #' @field returned The value returned from the module - returned = function(value){ - if(missing(value)){ - return(private$returnedVal) - } - # When you assign to returned, that implies that you just ran - # the module. So we should proactively flush. We have to do this - # here since flush is private. - private$returnedVal <- value - private$flush() - }, #' @field request An empty environment where the request should be. The request isn't meaningfully mocked currently. request = function(value) { if (!missing(value)){ diff --git a/R/modules.R b/R/modules.R index f76587fece..455dc6d12d 100644 --- a/R/modules.R +++ b/R/modules.R @@ -132,7 +132,11 @@ createSessionProxy <- function(parentSession, ...) { #' #' @export moduleServer <- function(id, module, session = getDefaultReactiveDomain()) { - callModule(module, id, session = session) + if (inherits(session, "MockShinySession")) { + testCallModule(module, id, session) + } else { + callModule(module, id, session = session) + } } diff --git a/R/test-module.R b/R/test-module.R index 0e17f05a35..21f099483f 100644 --- a/R/test-module.R +++ b/R/test-module.R @@ -110,39 +110,45 @@ testModule <- function(module, expr, ...) { }) } +testCallModule <- function(module, id, session) { + # TODO alan Figure out what to do with id here, necessary for nested usage + body(module) <- rlang::expr({ + session$env <- base::environment() + !!!body(module) + }) + + session$setReturned(do.call(module, list( + input = session$input, + output = session$output, + session = session + ))) +} + #' Test an app's server-side logic #' @param appDir The directory root of the Shiny application. If `NULL`, this function #' will work up the directory hierarchy --- starting with the current directory --- #' looking for a directory that contains an `app.R` or `server.R` file. #' @rdname testModule #' @export -testServer <- function(expr, appDir=NULL) { - if (is.null(appDir)){ - appDir <- findApp() - } - - app <- shinyAppDir(appDir) - message("Testing application found in: ", appDir) - server <- app$serverFuncSource() - - origwd <- getwd() - setwd(appDir) - on.exit({ setwd(origwd) }, add=TRUE) - - # Add `session` argument if not present - fn_formals <- formals(server) - if (! "session" %in% names(fn_formals)) { - fn_formals$session <- bquote() - formals(server) <- fn_formals - } - - # Test the server function almost as if it were a module. `dots` is empty - # because server functions never take additional arguments. - .testModule( - server, - quosure = rlang::enquo(expr), - dots = list(), - env = rlang::caller_env() +testServer <- function(app, expr, ...) { + session <- MockShinySession$new() + on.exit(if (!session$isClosed()) session$close()) + quosure <- rlang::enquo(expr) + isolate( + withReactiveDomain( + session, + withr::with_options(list(`shiny.allowoutputreads` = TRUE), { + rlang::exec(app, ...) + }) + ) + ) + isolate( + withReactiveDomain( + session, + withr::with_options(list(`shiny.allowoutputreads`=TRUE), { + rlang::eval_tidy(quosure, as.list(session$env), rlang::caller_env()) + }) + ) ) } diff --git a/tests/testthat/test-test-module.R b/tests/testthat/test-test-module.R index a542b38a07..d6362c3357 100644 --- a/tests/testthat/test-test-module.R +++ b/tests/testthat/test-test-module.R @@ -1,47 +1,71 @@ context("testModule") -library(promises) +library(shiny) +library(testthat) library(future) -plan(multisession) -test_that("testModule passes dots", { - module <- function(input, output, session, someArg) { +#m <- function(id, multiplier = 2) { +# moduleServer(id, function(input, output, session) { +# multiplied <- reactive(input$x * multiplier) +# output$txt <- renderText(multiplied()) +# }) +#} +# +#test_that("it works", { +# expected <- 4 +# testServer(m, { +# session$setInputs(x = 2) +# expect_equal(multiplied(), expected) +# }, multiplier = 2) +#}) + +#library(promises) +#plan(multisession) + +test_that("testServer passes dots", { + server <- function(id, someArg) { expect_false(missing(someArg)) - expect_equal(someArg, 123) + moduleServer(id, function(input, output, session) { + expect_equal(someArg, 123) + }) } - testModule(module, {}, someArg = 123) + testServer(server, {}, someArg = 123) }) -test_that("testModule passes dynamic dots", { - module <- function(input, output, session, someArg) { +test_that("testServer passes dynamic dots", { + server <- function(id, someArg) { expect_false(missing(someArg)) - expect_equal(someArg, 123) + moduleServer(id, function(input, output, session) { + expect_equal(someArg, 123) + }) } # Test with !!! to splice in a whole named list constructed with base::list() moreArgs <- list(someArg = 123) - testModule(module, {}, !!!moreArgs) + testServer(server, {}, !!!moreArgs) # Test with !!/:= to splice in an argument name argName <- "someArg" - testModule(module, {}, !!argName := 123) + testServer(server, {}, !!argName := 123) }) -test_that("testModule handles observers", { - module <- function(input, output, session) { - rv <- reactiveValues(x = 0, y = 0) - observe({ - rv$x <- input$x * 2 - }) - observe({ - rv$y <- rv$x - }) - output$txt <- renderText({ - paste0("Value: ", rv$x) +test_that("testServer handles observers", { + server <- function(id) { + moduleServer(id, function(input, output, session) { + rv <- reactiveValues(x = 0, y = 0) + observe({ + rv$x <- input$x * 2 + }) + observe({ + rv$y <- rv$x + }) + output$txt <- renderText({ + paste0("Value: ", rv$x) + }) }) } - testModule(module, { + testServer(server, { session$setInputs(x=1) expect_equal(rv$y, 2) expect_equal(rv$x, 2) @@ -55,24 +79,28 @@ test_that("testModule handles observers", { }) test_that("inputs aren't directly assignable", { - module <- function(input, output, session) { + server <- function(id) { + moduleServer(id, function(input, output, session) { + }) } - testModule(module, { + testServer(server, { session$setInputs(x = 0) expect_error({ input$x <- 1 }, "Attempted to assign value to a read-only") expect_error({ input$y <- 1 }, "Attempted to assign value to a read-only") }) }) -test_that("testModule handles more complex expressions", { - module <- function(input, output, session){ - output$txt <- renderText({ - input$x +test_that("testServer handles more complex expressions", { + server <- function(id) { + moduleServer(id, function(input, output, session){ + output$txt <- renderText({ + input$x + }) }) } - testModule(module, { + testServer(server, { for (i in 1:5){ session$setInputs(x=i) expect_equal(output$txt, as.character(i)) @@ -86,7 +114,7 @@ test_that("testModule handles more complex expressions", { }) }) -test_that("testModule handles reactiveVal", { +test_that("testServer handles reactiveVal", { module <- function(input, output, session) { x <- reactiveVal(0) observe({ @@ -107,18 +135,20 @@ test_that("testModule handles reactiveVal", { }) }) -test_that("testModule handles reactives with complex dependency tree", { - module <- function(input, output, session) { - x <- reactiveValues(x=1) - r <- reactive({ - x$x + input$a + input$b - }) - r2 <- reactive({ - r() + input$c +test_that("testServer handles reactives with complex dependency tree", { + server <- function(id) { + moduleServer(id, function(input, output, session) { + x <- reactiveValues(x=1) + r <- reactive({ + x$x + input$a + input$b + }) + r2 <- reactive({ + r() + input$c + }) }) } - testModule(module, { + testServer(server, { session$setInputs(a=1, b=2, c=3) expect_equal(r(), 4) expect_equal(r2(), 7) @@ -137,18 +167,20 @@ test_that("testModule handles reactives with complex dependency tree", { }) }) -test_that("testModule handles reactivePoll", { - module <- function(input, output, session) { - rv <- reactiveValues(x = 0) - rp <- reactivePoll(50, session, function(){ rnorm(1) }, function(){ - isolate(rv$x <- rv$x + 1) - rnorm(1) - }) +test_that("testServer handles reactivePoll", { + server <- function(id) { + moduleServer(id, function(input, output, session) { + rv <- reactiveValues(x = 0) + rp <- reactivePoll(50, session, function(){ rnorm(1) }, function(){ + isolate(rv$x <- rv$x + 1) + rnorm(1) + }) - observe({rp()}) + observe({rp()}) + }) } - testModule(module, { + testServer(server, { expect_equal(rv$x, 1) for (i in 1:4){ @@ -159,18 +191,20 @@ test_that("testModule handles reactivePoll", { }) }) -test_that("testModule handles reactiveTimer", { - module <- function(input, output, session) { - rv <- reactiveValues(x = 0) +test_that("testServer handles reactiveTimer", { + server <- function(id) { + moduleServer(id, function(input, output, session) { + rv <- reactiveValues(x = 0) - rp <- reactiveTimer(50) - observe({ - rp() - isolate(rv$x <- rv$x + 1) + rp <- reactiveTimer(50) + observe({ + rp() + isolate(rv$x <- rv$x + 1) + }) }) } - testModule(module, { + testServer(server, { expect_equal(rv$x, 1) session$elapse(200) @@ -179,27 +213,29 @@ test_that("testModule handles reactiveTimer", { }) }) -test_that("testModule handles debounce/throttle", { - module <- function(input, output, session) { - rv <- reactiveValues(t = 0, d = 0) - react <- reactive({ - input$y - }) - rt <- throttle(react, 100) - rd <- debounce(react, 100) +test_that("testServer handles debounce/throttle", { + server <- function(id) { + moduleServer(id, function(input, output, session) { + rv <- reactiveValues(t = 0, d = 0) + react <- reactive({ + input$y + }) + rt <- throttle(react, 100) + rd <- debounce(react, 100) - observe({ - rt() # Invalidate this block on the timer - isolate(rv$t <- rv$t + 1) - }) + observe({ + rt() # Invalidate this block on the timer + isolate(rv$t <- rv$t + 1) + }) - observe({ - rd() - isolate(rv$d <- rv$d + 1) + observe({ + rd() + isolate(rv$d <- rv$d + 1) + }) }) } - testModule(module, { + testServer(server, { session$setInputs(y = TRUE) expect_equal(rv$d, 1) for (i in 2:5){ @@ -219,21 +255,22 @@ test_that("testModule handles debounce/throttle", { }) }) -test_that("testModule wraps output in an observer", { +test_that("testServer wraps output in an observer", { testthat::skip("I'm not sure of a great way to test this without timers.") # And honestly it's so foundational in what we're doing now that it might not be necessary to test? - - module <- function(input, output, session) { - rv <- reactiveValues(x=0) - rp <- reactiveTimer(50) - output$txt <- renderText({ - rp() - isolate(rv$x <- rv$x + 1) + server <- function(id) { + moduleServer(id, function(input, output, session) { + rv <- reactiveValues(x=0) + rp <- reactiveTimer(50) + output$txt <- renderText({ + rp() + isolate(rv$x <- rv$x + 1) + }) }) } - testModule(module, { + testServer(server, { session$setInputs(x=1) # Timers only tick if they're being observed. If the output weren't being # wrapped in an observer, we'd see the value of rv$x initialize to zero and @@ -259,24 +296,26 @@ test_that("testModule wraps output in an observer", { # - plots and such? }) -test_that("testModule works with async", { - module <- function(input, output, session) { - output$txt <- renderText({ - val <- input$x - future({ val }) - }) +test_that("testServer works with async", { + server <- function(id) { + moduleServer(id, function(input, output, session) { + output$txt <- renderText({ + val <- input$x + future({ val }) + }) - output$error <- renderText({ - future({ stop("error here") }) - }) + output$error <- renderText({ + future({ stop("error here") }) + }) - output$sync <- renderText({ - # No promises here - "abc" + output$sync <- renderText({ + # No promises here + "abc" + }) }) } - testModule(module, { + testServer(server, { session$setInputs(x=1) expect_equal(output$txt, "1") expect_equal(output$sync, "abc") @@ -294,23 +333,25 @@ test_that("testModule works with async", { }) test_that("testModule works with multiple promises in parallel", { - module <- function(input, output, session) { - output$txt1 <- renderText({ - future({ - Sys.sleep(1) - 1 + server <- function(id) { + moduleServer(id, function(input, output, session) { + output$txt1 <- renderText({ + future({ + Sys.sleep(1) + 1 + }) }) - }) - output$txt2 <- renderText({ - future({ - Sys.sleep(1) - 2 + output$txt2 <- renderText({ + future({ + Sys.sleep(1) + 2 + }) }) }) } - testModule(module, { + testServer(server, { # As we enter this test code, the promises will still be running in the background. # We'll need to give them ~2s (plus overhead) to complete startMS <- as.numeric(Sys.time()) * 1000 @@ -330,409 +371,411 @@ test_that("testModule works with multiple promises in parallel", { }) test_that("testModule handles async errors", { - module <- function(input, output, session, arg1, arg2){ - output$err <- renderText({ - future({ "my error"}) %...>% - stop() %...>% - print() # Extra steps after the error - }) - - output$safe <- renderText({ - future({ safeError("my safe error") }) %...>% - stop() - }) - } - - testModule(module, { - expect_error(output$err, "my error") - # TODO: helper for safe errors so users don't have to learn "shiny.custom.error"? - expect_error(output$safe, "my safe error", class="shiny.custom.error") - }) -}) - -test_that("testModule handles modules with additional arguments", { - module <- function(input, output, session, arg1, arg2){ - output$txt1 <- renderText({ - arg1 - }) - - output$txt2 <- renderText({ - arg2 - }) - - output$inp <- renderText({ - input$x - }) - } - - testModule(module, { - expect_equal(output$txt1, "val1") - expect_equal(output$txt2, "val2") - }, arg1="val1", arg2="val2") -}) - -test_that("testModule captures htmlwidgets", { - # TODO: use a simple built-in htmlwidget instead of something complex like dygraph - if (!requireNamespace("dygraphs")){ - testthat::skip("dygraphs not available to test htmlwidgets") - } - - if (!requireNamespace("jsonlite")){ - testthat::skip("jsonlite not available to test htmlwidgets") - } - - module <- function(input, output, session){ - output$dy <- dygraphs::renderDygraph({ - dygraphs::dygraph(data.frame(outcome=0:5, year=2000:2005)) - }) - } - - testModule(module, { - # Really, this test should be specific to each htmlwidget. Here, we don't want to bind ourselves - # to the current JSON structure of dygraphs, so we'll just check one element to see that the raw - # JSON was exposed and is accessible in tests. - d <- jsonlite::fromJSON(output$dy)$x$data - expect_equal(d[1,], 0:5) - expect_equal(d[2,], 2000:2005) - }) -}) - -test_that("testModule captures renderUI", { - module <- function(input, output, session){ - output$ui <- renderUI({ - tags$a(href="https://rstudio.com", "hello!") - }) - } - - testModule(module, { - expect_equal(output$ui$deps, list()) - expect_equal(as.character(output$ui$html), "hello!") - }) -}) - -test_that("testModule captures base graphics outputs", { - module <- function(input, output, session){ - output$fixed <- renderPlot({ - plot(1,1) - }, width=300, height=350) - - output$dynamic <- renderPlot({ - plot(1,1) - }) - } - - testModule(module, { - # We aren't yet able to create reproducible graphics, so this test is intentionally pretty - # limited. - expect_equal(output$fixed$width, 300) - expect_equal(output$fixed$height, 350) - expect_match(output$fixed$src, "^data:image/png;base64,") - - # Ensure that the plot defaults to a reasonable size. - expect_equal(output$dynamic$width, 600) - expect_equal(output$dynamic$height, 400) - expect_match(output$dynamic$src, "^data:image/png;base64,") - - # TODO: how do you customize automatically inferred plot sizes? - # session$setPlotMeta("dynamic", width=600, height=300) ? - }) -}) - -test_that("testModule captures ggplot2 outputs", { - if (!requireNamespace("ggplot2")){ - testthat::skip("ggplot2 not available") - } - - module <- function(input, output, session){ - output$fixed <- renderPlot({ - ggplot2::qplot(iris$Sepal.Length, iris$Sepal.Width) - }, width=300, height=350) - - output$dynamic <- renderPlot({ - ggplot2::qplot(iris$Sepal.Length, iris$Sepal.Width) - }) - } - - testModule(module, { - expect_equal(output$fixed$width, 300) - expect_equal(output$fixed$height, 350) - expect_match(output$fixed$src, "^data:image/png;base64,") - - # Ensure that the plot defaults to a reasonable size. - expect_equal(output$dynamic$width, 600) - expect_equal(output$dynamic$height, 400) - expect_match(output$dynamic$src, "^data:image/png;base64,") - }) -}) - -test_that("testModule exposes the returned value from the module", { - module <- function(input, output, session){ - reactive({ - return(input$a + input$b) - }) - } - - testModule(module, { - session$setInputs(a=1, b=2) - expect_equal(session$returned(), 3) - - # And retains reactivity - session$setInputs(a=2) - expect_equal(session$returned(), 4) - }) -}) - -test_that("testModule handles synchronous errors", { - module <- function(input, output, session, arg1, arg2){ - output$err <- renderText({ - stop("my error") - }) + server <- function(id) { + moduleServer(id, function(input, output, session, arg1, arg2){ + output$err <- renderText({ + future({ "my error"}) %...>% + stop() %...>% + print() # Extra steps after the error + }) - output$safe <- renderText({ - stop(safeError("my safe error")) + output$safe <- renderText({ + future({ safeError("my safe error") }) %...>% + stop() + }) }) } - testModule(module, { + testServer(server, { expect_error(output$err, "my error") # TODO: helper for safe errors so users don't have to learn "shiny.custom.error"? expect_error(output$safe, "my safe error", class="shiny.custom.error") }) }) -test_that("accessing a non-existant output gives an informative message", { - module <- function(input, output, session){} - - testModule(module, { - expect_error(output$dontexist, "hasn't been defined yet: output\\$dontexist") - }) -}) - -test_that("testModule works with nested modules", { - outerModule <- function(input, output, session) { - r1 <- reactive({ input$x + 1}) - r2 <- callModule(innerModule, "innerModule", r1) - output$someVar <- renderText(r2()) - } - - innerModule <- function(input, output, session, r) { - reactive(paste("a value:", r())) - } - - testModule(outerModule, { - session$setInputs(x = 1) - expect_equal(output$someVar, "a value: 2") - }) -}) - -test_that("testModule calls can be nested", { - outerModule <- function(input, output, session) { - doubled <- reactive({ input$x * 2 }) - innerModule <- function(input, output, session) { - quadrupled <- reactive({ doubled() * 2 }) - } - } - - testModule(outerModule, { - session$setInputs(x = 1) - expect_equal(doubled(), 2) - testModule(innerModule, { - expect_equal(quadrupled(), 4) - }) - }) -}) - -test_that("testModule returns a meaningful result", { - result <- testModule(function(input, output, session) { - reactive({ input$x * 2 }) - }, { - session$setInputs(x = 2) - session$returned() - }) - expect_equal(result, 4) -}) - -test_that("assigning an output in a module function with a non-function errors", { - module <- function(input, output, session) { - output$someVar <- 123 - } - - expect_error(testModule(module, {}), "^Unexpected") -}) - -test_that("testServer works", { - # app.R - testServer({ - session$setInputs(dist="norm", n=5) - expect_length(d(), 5) - - session$setInputs(dist="unif", n=6) - expect_length(d(), 6) - }, appDir=test_path("..", "test-modules", "06_tabsets")) - - # server.R - testServer({ - session$setInputs(dist="norm", n=5) - expect_length(d(), 5) - - session$setInputs(dist="unif", n=6) - expect_length(d(), 6) - }, appDir=test_path("..", "test-modules", "server_r")) -}) - -test_that("testServer works when referencing external globals", { - # If global is defined at the top of app.R outside of the server function. - testServer({ - expect_equal(get("global", session$env), 123) - }, appDir=test_path("..", "test-modules", "06_tabsets")) -}) - -test_that("testModule allows lexical environment access through session$env", { - m <- local({ - a_var <- 123 - function(input, output, session) { - b_var <- 321 - } - }) - expect_false(exists("a_var", inherits = FALSE)) - testModule(m, { - expect_equal(b_var, 321) - expect_equal(get("a_var", session$env), 123) - }) -}) - -test_that("Module shadowing can be mitigated with unquote", { - i <- 0 - inc <- function() i <<- i+1 - - m <- local({ - function(input, output, session) { - inc <- function() stop("I should never be called") - } - }) - - testModule(m, { - expect_is(inc, "function") - expect_false(identical(inc, !!inc)) - !!inc() - }) - - expect_equal(i, 1) -}) - -test_that("testModule handles invalidateLater", { - module <- function(input, output, session) { - rv <- reactiveValues(x = 0) - observe({ - isolate(rv$x <- rv$x + 1) - # We're only testing one invalidation - if (isolate(rv$x) <= 1){ - invalidateLater(50) - } - }) - } - - testModule(module, { - # Should have run once - expect_equal(rv$x, 1) - - session$elapse(49) - expect_equal(rv$x, 1) - - session$elapse(1) - # Should have been incremented now - expect_equal(rv$x, 2) - }) -}) - -test_that("session ended handlers work", { - module <- function(input, output, session){} - - testModule(module, { - rv <- reactiveValues(closed = FALSE) - session$onEnded(function(){ - rv$closed <- TRUE - }) - - expect_equal(session$isEnded(), FALSE) - expect_equal(session$isClosed(), FALSE) - expect_false(rv$closed, FALSE) - - session$close() - - expect_equal(session$isEnded(), TRUE) - expect_equal(session$isClosed(), TRUE) - expect_false(rv$closed, TRUE) - }) -}) - -test_that("session flush handlers work", { - module <- function(input, output, session) { - rv <- reactiveValues(x = 0, flushCounter = 0, flushedCounter = 0, - flushOnceCounter = 0, flushedOnceCounter = 0) - - onFlush(function(){rv$flushCounter <- rv$flushCounter + 1}, once=FALSE) - onFlushed(function(){rv$flushedCounter <- rv$flushedCounter + 1}, once=FALSE) - onFlushed(function(){rv$flushOnceCounter <- rv$flushOnceCounter + 1}, once=TRUE) - onFlushed(function(){rv$flushedOnceCounter <- rv$flushedOnceCounter + 1}, once=TRUE) - - observe({ - rv$x <- input$x * 2 - }) - } - - testModule(module, { - session$setInputs(x=1) - expect_equal(rv$x, 2) - # We're not concerned with the exact values here -- only that they increase - fc <- rv$flushCounter - fdc <- rv$flushedCounter - - session$setInputs(x=2) - expect_gt(rv$flushCounter, fc) - expect_gt(rv$flushedCounter, fdc) - - # These should have only run once - expect_equal(rv$flushOnceCounter, 1) - expect_equal(rv$flushedOnceCounter, 1) - - }) -}) - -test_that("findApp errors with no app", { - calls <- 0 - nothingExists <- function(path){ - calls <<- calls + 1 - FALSE - } - fa <- rewire(findApp, file.exists.ci=nothingExists) - expect_error( - expect_warning(fa("/some/path/here"), "No such file or directory"), # since we just made up a path - "No shiny app was found in ") - expect_equal(calls, 4 * 2) # Checks here, path, some, and / -- looking for app.R and server.R for each -}) - -test_that("findApp works with app in current or parent dir", { - calls <- 0 - cd <- normalizePath(".") - mockExists <- function(path){ - # Only TRUE if looking for server.R or app.R in current Dir - calls <<- calls + 1 - - path <- normalizePath(path, mustWork = FALSE) - - appPath <- normalizePath(file.path(cd, "app.R"), mustWork = FALSE) - serverPath <- normalizePath(file.path(cd, "server.R"), mustWork = FALSE) - return(path %in% c(appPath, serverPath)) - } - fa <- rewire(findApp, file.exists.ci=mockExists) - expect_equal(fa(), cd) - expect_equal(calls, 1) # Should get a hit on the first call and stop - - # Reset and point to the parent dir - calls <- 0 - cd <- normalizePath("..") # TODO: won't work if running tests in the root dir. - f <- fa() - expect_equal(normalizePath(f, mustWork = FALSE), cd) - expect_equal(calls, 3) # Two for current dir and hit on the first in the parent -}) +#test_that("testModule handles modules with additional arguments", { +# module <- function(input, output, session, arg1, arg2){ +# output$txt1 <- renderText({ +# arg1 +# }) + +# output$txt2 <- renderText({ +# arg2 +# }) + +# output$inp <- renderText({ +# input$x +# }) +# } + +# testModule(module, { +# expect_equal(output$txt1, "val1") +# expect_equal(output$txt2, "val2") +# }, arg1="val1", arg2="val2") +#}) + +#test_that("testModule captures htmlwidgets", { +# # TODO: use a simple built-in htmlwidget instead of something complex like dygraph +# if (!requireNamespace("dygraphs")){ +# testthat::skip("dygraphs not available to test htmlwidgets") +# } + +# if (!requireNamespace("jsonlite")){ +# testthat::skip("jsonlite not available to test htmlwidgets") +# } + +# module <- function(input, output, session){ +# output$dy <- dygraphs::renderDygraph({ +# dygraphs::dygraph(data.frame(outcome=0:5, year=2000:2005)) +# }) +# } + +# testModule(module, { +# # Really, this test should be specific to each htmlwidget. Here, we don't want to bind ourselves +# # to the current JSON structure of dygraphs, so we'll just check one element to see that the raw +# # JSON was exposed and is accessible in tests. +# d <- jsonlite::fromJSON(output$dy)$x$data +# expect_equal(d[1,], 0:5) +# expect_equal(d[2,], 2000:2005) +# }) +#}) + +#test_that("testModule captures renderUI", { +# module <- function(input, output, session){ +# output$ui <- renderUI({ +# tags$a(href="https://rstudio.com", "hello!") +# }) +# } + +# testModule(module, { +# expect_equal(output$ui$deps, list()) +# expect_equal(as.character(output$ui$html), "hello!") +# }) +#}) + +#test_that("testModule captures base graphics outputs", { +# module <- function(input, output, session){ +# output$fixed <- renderPlot({ +# plot(1,1) +# }, width=300, height=350) + +# output$dynamic <- renderPlot({ +# plot(1,1) +# }) +# } + +# testModule(module, { +# # We aren't yet able to create reproducible graphics, so this test is intentionally pretty +# # limited. +# expect_equal(output$fixed$width, 300) +# expect_equal(output$fixed$height, 350) +# expect_match(output$fixed$src, "^data:image/png;base64,") + +# # Ensure that the plot defaults to a reasonable size. +# expect_equal(output$dynamic$width, 600) +# expect_equal(output$dynamic$height, 400) +# expect_match(output$dynamic$src, "^data:image/png;base64,") + +# # TODO: how do you customize automatically inferred plot sizes? +# # session$setPlotMeta("dynamic", width=600, height=300) ? +# }) +#}) + +#test_that("testModule captures ggplot2 outputs", { +# if (!requireNamespace("ggplot2")){ +# testthat::skip("ggplot2 not available") +# } + +# module <- function(input, output, session){ +# output$fixed <- renderPlot({ +# ggplot2::qplot(iris$Sepal.Length, iris$Sepal.Width) +# }, width=300, height=350) + +# output$dynamic <- renderPlot({ +# ggplot2::qplot(iris$Sepal.Length, iris$Sepal.Width) +# }) +# } + +# testModule(module, { +# expect_equal(output$fixed$width, 300) +# expect_equal(output$fixed$height, 350) +# expect_match(output$fixed$src, "^data:image/png;base64,") + +# # Ensure that the plot defaults to a reasonable size. +# expect_equal(output$dynamic$width, 600) +# expect_equal(output$dynamic$height, 400) +# expect_match(output$dynamic$src, "^data:image/png;base64,") +# }) +#}) + +#test_that("testModule exposes the returned value from the module", { +# module <- function(input, output, session){ +# reactive({ +# return(input$a + input$b) +# }) +# } + +# testModule(module, { +# session$setInputs(a=1, b=2) +# expect_equal(session$returned(), 3) + +# # And retains reactivity +# session$setInputs(a=2) +# expect_equal(session$returned(), 4) +# }) +#}) + +#test_that("testModule handles synchronous errors", { +# module <- function(input, output, session, arg1, arg2){ +# output$err <- renderText({ +# stop("my error") +# }) + +# output$safe <- renderText({ +# stop(safeError("my safe error")) +# }) +# } + +# testModule(module, { +# expect_error(output$err, "my error") +# # TODO: helper for safe errors so users don't have to learn "shiny.custom.error"? +# expect_error(output$safe, "my safe error", class="shiny.custom.error") +# }) +#}) + +#test_that("accessing a non-existant output gives an informative message", { +# module <- function(input, output, session){} + +# testModule(module, { +# expect_error(output$dontexist, "hasn't been defined yet: output\\$dontexist") +# }) +#}) + +#test_that("testModule works with nested modules", { +# outerModule <- function(input, output, session) { +# r1 <- reactive({ input$x + 1}) +# r2 <- callModule(innerModule, "innerModule", r1) +# output$someVar <- renderText(r2()) +# } + +# innerModule <- function(input, output, session, r) { +# reactive(paste("a value:", r())) +# } + +# testModule(outerModule, { +# session$setInputs(x = 1) +# expect_equal(output$someVar, "a value: 2") +# }) +#}) + +#test_that("testModule calls can be nested", { +# outerModule <- function(input, output, session) { +# doubled <- reactive({ input$x * 2 }) +# innerModule <- function(input, output, session) { +# quadrupled <- reactive({ doubled() * 2 }) +# } +# } + +# testModule(outerModule, { +# session$setInputs(x = 1) +# expect_equal(doubled(), 2) +# testModule(innerModule, { +# expect_equal(quadrupled(), 4) +# }) +# }) +#}) + +#test_that("testModule returns a meaningful result", { +# result <- testModule(function(input, output, session) { +# reactive({ input$x * 2 }) +# }, { +# session$setInputs(x = 2) +# session$returned() +# }) +# expect_equal(result, 4) +#}) + +#test_that("assigning an output in a module function with a non-function errors", { +# module <- function(input, output, session) { +# output$someVar <- 123 +# } + +# expect_error(testModule(module, {}), "^Unexpected") +#}) + +#test_that("testServer works", { +# # app.R +# testServer({ +# session$setInputs(dist="norm", n=5) +# expect_length(d(), 5) + +# session$setInputs(dist="unif", n=6) +# expect_length(d(), 6) +# }, appDir=test_path("..", "test-modules", "06_tabsets")) + +# # server.R +# testServer({ +# session$setInputs(dist="norm", n=5) +# expect_length(d(), 5) + +# session$setInputs(dist="unif", n=6) +# expect_length(d(), 6) +# }, appDir=test_path("..", "test-modules", "server_r")) +#}) + +#test_that("testServer works when referencing external globals", { +# # If global is defined at the top of app.R outside of the server function. +# testServer({ +# expect_equal(get("global", session$env), 123) +# }, appDir=test_path("..", "test-modules", "06_tabsets")) +#}) + +#test_that("testModule allows lexical environment access through session$env", { +# m <- local({ +# a_var <- 123 +# function(input, output, session) { +# b_var <- 321 +# } +# }) +# expect_false(exists("a_var", inherits = FALSE)) +# testModule(m, { +# expect_equal(b_var, 321) +# expect_equal(get("a_var", session$env), 123) +# }) +#}) + +#test_that("Module shadowing can be mitigated with unquote", { +# i <- 0 +# inc <- function() i <<- i+1 + +# m <- local({ +# function(input, output, session) { +# inc <- function() stop("I should never be called") +# } +# }) + +# testModule(m, { +# expect_is(inc, "function") +# expect_false(identical(inc, !!inc)) +# !!inc() +# }) + +# expect_equal(i, 1) +#}) + +#test_that("testModule handles invalidateLater", { +# module <- function(input, output, session) { +# rv <- reactiveValues(x = 0) +# observe({ +# isolate(rv$x <- rv$x + 1) +# # We're only testing one invalidation +# if (isolate(rv$x) <= 1){ +# invalidateLater(50) +# } +# }) +# } + +# testModule(module, { +# # Should have run once +# expect_equal(rv$x, 1) + +# session$elapse(49) +# expect_equal(rv$x, 1) + +# session$elapse(1) +# # Should have been incremented now +# expect_equal(rv$x, 2) +# }) +#}) + +#test_that("session ended handlers work", { +# module <- function(input, output, session){} + +# testModule(module, { +# rv <- reactiveValues(closed = FALSE) +# session$onEnded(function(){ +# rv$closed <- TRUE +# }) + +# expect_equal(session$isEnded(), FALSE) +# expect_equal(session$isClosed(), FALSE) +# expect_false(rv$closed, FALSE) + +# session$close() + +# expect_equal(session$isEnded(), TRUE) +# expect_equal(session$isClosed(), TRUE) +# expect_false(rv$closed, TRUE) +# }) +#}) + +#test_that("session flush handlers work", { +# module <- function(input, output, session) { +# rv <- reactiveValues(x = 0, flushCounter = 0, flushedCounter = 0, +# flushOnceCounter = 0, flushedOnceCounter = 0) + +# onFlush(function(){rv$flushCounter <- rv$flushCounter + 1}, once=FALSE) +# onFlushed(function(){rv$flushedCounter <- rv$flushedCounter + 1}, once=FALSE) +# onFlushed(function(){rv$flushOnceCounter <- rv$flushOnceCounter + 1}, once=TRUE) +# onFlushed(function(){rv$flushedOnceCounter <- rv$flushedOnceCounter + 1}, once=TRUE) + +# observe({ +# rv$x <- input$x * 2 +# }) +# } + +# testModule(module, { +# session$setInputs(x=1) +# expect_equal(rv$x, 2) +# # We're not concerned with the exact values here -- only that they increase +# fc <- rv$flushCounter +# fdc <- rv$flushedCounter + +# session$setInputs(x=2) +# expect_gt(rv$flushCounter, fc) +# expect_gt(rv$flushedCounter, fdc) + +# # These should have only run once +# expect_equal(rv$flushOnceCounter, 1) +# expect_equal(rv$flushedOnceCounter, 1) + +# }) +#}) + +#test_that("findApp errors with no app", { +# calls <- 0 +# nothingExists <- function(path){ +# calls <<- calls + 1 +# FALSE +# } +# fa <- rewire(findApp, file.exists.ci=nothingExists) +# expect_error( +# expect_warning(fa("/some/path/here"), "No such file or directory"), # since we just made up a path +# "No shiny app was found in ") +# expect_equal(calls, 4 * 2) # Checks here, path, some, and / -- looking for app.R and server.R for each +#}) + +#test_that("findApp works with app in current or parent dir", { +# calls <- 0 +# cd <- normalizePath(".") +# mockExists <- function(path){ +# # Only TRUE if looking for server.R or app.R in current Dir +# calls <<- calls + 1 + +# path <- normalizePath(path, mustWork = FALSE) + +# appPath <- normalizePath(file.path(cd, "app.R"), mustWork = FALSE) +# serverPath <- normalizePath(file.path(cd, "server.R"), mustWork = FALSE) +# return(path %in% c(appPath, serverPath)) +# } +# fa <- rewire(findApp, file.exists.ci=mockExists) +# expect_equal(fa(), cd) +# expect_equal(calls, 1) # Should get a hit on the first call and stop + +# # Reset and point to the parent dir +# calls <- 0 +# cd <- normalizePath("..") # TODO: won't work if running tests in the root dir. +# f <- fa() +# expect_equal(normalizePath(f, mustWork = FALSE), cd) +# expect_equal(calls, 3) # Two for current dir and hit on the first in the parent +#}) From c4852cb45197ed4d3219518692090a493f13a7a5 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Wed, 25 Mar 2020 22:40:08 +0000 Subject: [PATCH 08/36] Desired environment semantics are working --- R/test-module.R | 16 ++++- tests/testthat/test-test-module.R | 100 ++++++++++++++++++------------ 2 files changed, 76 insertions(+), 40 deletions(-) diff --git a/R/test-module.R b/R/test-module.R index 21f099483f..6a68b73181 100644 --- a/R/test-module.R +++ b/R/test-module.R @@ -110,6 +110,7 @@ testModule <- function(module, expr, ...) { }) } +#' @noRd testCallModule <- function(module, id, session) { # TODO alan Figure out what to do with id here, necessary for nested usage body(module) <- rlang::expr({ @@ -124,6 +125,19 @@ testCallModule <- function(module, id, session) { ))) } +# Create a "data mask" suitable for passing to rlang::eval_tidy. Bindings in +# `env` and bindings in the parent of `env` are merged into a single named list. +# Bindings in `env` take precedence over bindings in the parent of `env`. +#' @noRd +makeMask <- function(env) { + stopifnot(length(rlang::env_parents(env)) > 1) + stopifnot(all(c("input", "output", "session") %in% ls(env))) + child <- as.list(env) + parent <- as.list(rlang::env_parent(env)) + parent_only <- setdiff(names(parent), names(child)) + append(child, parent[parent_only]) +} + #' Test an app's server-side logic #' @param appDir The directory root of the Shiny application. If `NULL`, this function #' will work up the directory hierarchy --- starting with the current directory --- @@ -146,7 +160,7 @@ testServer <- function(app, expr, ...) { withReactiveDomain( session, withr::with_options(list(`shiny.allowoutputreads`=TRUE), { - rlang::eval_tidy(quosure, as.list(session$env), rlang::caller_env()) + rlang::eval_tidy(quosure, makeMask(session$env), rlang::caller_env()) }) ) ) diff --git a/tests/testthat/test-test-module.R b/tests/testthat/test-test-module.R index d6362c3357..5ffb0e70d9 100644 --- a/tests/testthat/test-test-module.R +++ b/tests/testthat/test-test-module.R @@ -393,53 +393,75 @@ test_that("testModule handles async errors", { }) }) -#test_that("testModule handles modules with additional arguments", { -# module <- function(input, output, session, arg1, arg2){ -# output$txt1 <- renderText({ -# arg1 -# }) +test_that("testServer handles modules with additional arguments", { + server <- function(id, arg1, arg2) { + moduleServer(id, function(input, output, session){ + output$txt1 <- renderText({ + arg1 + }) -# output$txt2 <- renderText({ -# arg2 -# }) + output$txt2 <- renderText({ + arg2 + }) -# output$inp <- renderText({ -# input$x -# }) -# } + output$inp <- renderText({ + input$x + }) + }) + } -# testModule(module, { -# expect_equal(output$txt1, "val1") -# expect_equal(output$txt2, "val2") -# }, arg1="val1", arg2="val2") -#}) + testServer(server, { + expect_equal(output$txt1, "val1") + expect_equal(output$txt2, "val2") + }, arg1="val1", arg2="val2") +}) -#test_that("testModule captures htmlwidgets", { -# # TODO: use a simple built-in htmlwidget instead of something complex like dygraph -# if (!requireNamespace("dygraphs")){ -# testthat::skip("dygraphs not available to test htmlwidgets") -# } +test_that("testServer captures htmlwidgets", { + # TODO: use a simple built-in htmlwidget instead of something complex like dygraph + if (!requireNamespace("dygraphs")){ + testthat::skip("dygraphs not available to test htmlwidgets") + } -# if (!requireNamespace("jsonlite")){ -# testthat::skip("jsonlite not available to test htmlwidgets") -# } + if (!requireNamespace("jsonlite")){ + testthat::skip("jsonlite not available to test htmlwidgets") + } -# module <- function(input, output, session){ -# output$dy <- dygraphs::renderDygraph({ -# dygraphs::dygraph(data.frame(outcome=0:5, year=2000:2005)) -# }) -# } + server <- function(id) { + moduleServer(id, function(input, output, session){ + output$dy <- dygraphs::renderDygraph({ + dygraphs::dygraph(data.frame(outcome=0:5, year=2000:2005)) + }) + }) + } -# testModule(module, { -# # Really, this test should be specific to each htmlwidget. Here, we don't want to bind ourselves -# # to the current JSON structure of dygraphs, so we'll just check one element to see that the raw -# # JSON was exposed and is accessible in tests. -# d <- jsonlite::fromJSON(output$dy)$x$data -# expect_equal(d[1,], 0:5) -# expect_equal(d[2,], 2000:2005) -# }) -#}) + testServer(server, { + # Really, this test should be specific to each htmlwidget. Here, we don't want to bind ourselves + # to the current JSON structure of dygraphs, so we'll just check one element to see that the raw + # JSON was exposed and is accessible in tests. + d <- jsonlite::fromJSON(output$dy)$x$data + expect_equal(d[1,], 0:5) + expect_equal(d[2,], 2000:2005) + }) +}) +test_that("Variables outside of the module are inaccessible", { + server <- local({ + outside <- 123 + function(id, x) { + y <- x+1 + moduleServer(id, function(input, output, session) { + z <- y+1 + }) + } + }, envir = rlang::new_environment(parent = rlang::global_env())) + + testServer(server, { + expect_equal(x, 0) + expect_equal(y, 1) + expect_equal(z, 2) + expect_equal(exists("outside"), FALSE) + }, x = 0) +}) #test_that("testModule captures renderUI", { # module <- function(input, output, session){ # output$ui <- renderUI({ From ec2c9ecea0074606118501d127f174c0ed215886 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Fri, 27 Mar 2020 22:43:29 +0000 Subject: [PATCH 09/36] Split up and rename various tests --- tests/testthat/test-test-server-scope.R | 42 +++++++++++++++++++ ...{test-test-module.R => test-test-server.R} | 20 +-------- 2 files changed, 43 insertions(+), 19 deletions(-) create mode 100644 tests/testthat/test-test-server-scope.R rename tests/testthat/{test-test-module.R => test-test-server.R} (97%) diff --git a/tests/testthat/test-test-server-scope.R b/tests/testthat/test-test-server-scope.R new file mode 100644 index 0000000000..a6ee8632f5 --- /dev/null +++ b/tests/testthat/test-test-server-scope.R @@ -0,0 +1,42 @@ +context("testServer scope") + +library(shiny) +library(testthat) + +test_that("Variables outside of the module are inaccessible", { + server <- local({ + outside <- 123 + function(id, x) { + y <- x+1 + moduleServer(id, function(input, output, session) { + z <- y+1 + }) + } + }, envir = rlang::new_environment(parent = rlang::global_env())) + + testServer(server, { + expect_equal(x, 0) + expect_equal(y, 1) + expect_equal(z, 2) + expect_equal(exists("outside"), FALSE) + }, x = 0) +}) + +test_that("Variables outside the testServer() have correct visibility", { + server <- local({ + function(id, x) { + moduleServer(id, function(input, output, session) { + y <- 1 + }) + } + }, envir = rlang::new_environment(parent = rlang::global_env())) + + x <- 99 + z <- 123 + + testServer(server, { + expect_equal(x, 0) + expect_equal(y, 1) + expect_equal(z, 123) + }, x = 0) +}) diff --git a/tests/testthat/test-test-module.R b/tests/testthat/test-test-server.R similarity index 97% rename from tests/testthat/test-test-module.R rename to tests/testthat/test-test-server.R index 5ffb0e70d9..da7d95db0c 100644 --- a/tests/testthat/test-test-module.R +++ b/tests/testthat/test-test-server.R @@ -1,4 +1,4 @@ -context("testModule") +context("testServer") library(shiny) library(testthat) @@ -444,24 +444,6 @@ test_that("testServer captures htmlwidgets", { }) }) -test_that("Variables outside of the module are inaccessible", { - server <- local({ - outside <- 123 - function(id, x) { - y <- x+1 - moduleServer(id, function(input, output, session) { - z <- y+1 - }) - } - }, envir = rlang::new_environment(parent = rlang::global_env())) - - testServer(server, { - expect_equal(x, 0) - expect_equal(y, 1) - expect_equal(z, 2) - expect_equal(exists("outside"), FALSE) - }, x = 0) -}) #test_that("testModule captures renderUI", { # module <- function(input, output, session){ # output$ui <- renderUI({ From 0023418b94d08814943caece0c1295ac37e1fc94 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Mon, 30 Mar 2020 22:55:27 +0000 Subject: [PATCH 10/36] More test reorg --- tests/testthat/test-test-server-nesting.R | 33 ++ tests/testthat/test-test-server-scope.R | 33 ++ tests/testthat/test-test-server.R | 352 ++++++++++------------ 3 files changed, 218 insertions(+), 200 deletions(-) create mode 100644 tests/testthat/test-test-server-nesting.R diff --git a/tests/testthat/test-test-server-nesting.R b/tests/testthat/test-test-server-nesting.R new file mode 100644 index 0000000000..71745dc0b0 --- /dev/null +++ b/tests/testthat/test-test-server-nesting.R @@ -0,0 +1,33 @@ +#test_that("testModule works with nested modules", { +# outerModule <- function(input, output, session) { +# r1 <- reactive({ input$x + 1}) +# r2 <- callModule(innerModule, "innerModule", r1) +# output$someVar <- renderText(r2()) +# } + +# innerModule <- function(input, output, session, r) { +# reactive(paste("a value:", r())) +# } + +# testModule(outerModule, { +# session$setInputs(x = 1) +# expect_equal(output$someVar, "a value: 2") +# }) +#}) + +#test_that("testModule calls can be nested", { +# outerModule <- function(input, output, session) { +# doubled <- reactive({ input$x * 2 }) +# innerModule <- function(input, output, session) { +# quadrupled <- reactive({ doubled() * 2 }) +# } +# } + +# testModule(outerModule, { +# session$setInputs(x = 1) +# expect_equal(doubled(), 2) +# testModule(innerModule, { +# expect_equal(quadrupled(), 4) +# }) +# }) +#}) diff --git a/tests/testthat/test-test-server-scope.R b/tests/testthat/test-test-server-scope.R index a6ee8632f5..a36925e9f8 100644 --- a/tests/testthat/test-test-server-scope.R +++ b/tests/testthat/test-test-server-scope.R @@ -40,3 +40,36 @@ test_that("Variables outside the testServer() have correct visibility", { expect_equal(z, 123) }, x = 0) }) + +#test_that("testModule allows lexical environment access through session$env", { +# m <- local({ +# a_var <- 123 +# function(input, output, session) { +# b_var <- 321 +# } +# }) +# expect_false(exists("a_var", inherits = FALSE)) +# testModule(m, { +# expect_equal(b_var, 321) +# expect_equal(get("a_var", session$env), 123) +# }) +#}) + +#test_that("Module shadowing can be mitigated with unquote", { +# i <- 0 +# inc <- function() i <<- i+1 + +# m <- local({ +# function(input, output, session) { +# inc <- function() stop("I should never be called") +# } +# }) + +# testModule(m, { +# expect_is(inc, "function") +# expect_false(identical(inc, !!inc)) +# !!inc() +# }) + +# expect_equal(i, 1) +#}) diff --git a/tests/testthat/test-test-server.R b/tests/testthat/test-test-server.R index da7d95db0c..4b8450edde 100644 --- a/tests/testthat/test-test-server.R +++ b/tests/testthat/test-test-server.R @@ -444,228 +444,180 @@ test_that("testServer captures htmlwidgets", { }) }) -#test_that("testModule captures renderUI", { -# module <- function(input, output, session){ -# output$ui <- renderUI({ -# tags$a(href="https://rstudio.com", "hello!") -# }) -# } - -# testModule(module, { -# expect_equal(output$ui$deps, list()) -# expect_equal(as.character(output$ui$html), "hello!") -# }) -#}) - -#test_that("testModule captures base graphics outputs", { -# module <- function(input, output, session){ -# output$fixed <- renderPlot({ -# plot(1,1) -# }, width=300, height=350) - -# output$dynamic <- renderPlot({ -# plot(1,1) -# }) -# } - -# testModule(module, { -# # We aren't yet able to create reproducible graphics, so this test is intentionally pretty -# # limited. -# expect_equal(output$fixed$width, 300) -# expect_equal(output$fixed$height, 350) -# expect_match(output$fixed$src, "^data:image/png;base64,") - -# # Ensure that the plot defaults to a reasonable size. -# expect_equal(output$dynamic$width, 600) -# expect_equal(output$dynamic$height, 400) -# expect_match(output$dynamic$src, "^data:image/png;base64,") - -# # TODO: how do you customize automatically inferred plot sizes? -# # session$setPlotMeta("dynamic", width=600, height=300) ? -# }) -#}) - -#test_that("testModule captures ggplot2 outputs", { -# if (!requireNamespace("ggplot2")){ -# testthat::skip("ggplot2 not available") -# } - -# module <- function(input, output, session){ -# output$fixed <- renderPlot({ -# ggplot2::qplot(iris$Sepal.Length, iris$Sepal.Width) -# }, width=300, height=350) - -# output$dynamic <- renderPlot({ -# ggplot2::qplot(iris$Sepal.Length, iris$Sepal.Width) -# }) -# } - -# testModule(module, { -# expect_equal(output$fixed$width, 300) -# expect_equal(output$fixed$height, 350) -# expect_match(output$fixed$src, "^data:image/png;base64,") - -# # Ensure that the plot defaults to a reasonable size. -# expect_equal(output$dynamic$width, 600) -# expect_equal(output$dynamic$height, 400) -# expect_match(output$dynamic$src, "^data:image/png;base64,") -# }) -#}) - -#test_that("testModule exposes the returned value from the module", { -# module <- function(input, output, session){ -# reactive({ -# return(input$a + input$b) -# }) -# } - -# testModule(module, { -# session$setInputs(a=1, b=2) -# expect_equal(session$returned(), 3) - -# # And retains reactivity -# session$setInputs(a=2) -# expect_equal(session$returned(), 4) -# }) -#}) - -#test_that("testModule handles synchronous errors", { -# module <- function(input, output, session, arg1, arg2){ -# output$err <- renderText({ -# stop("my error") -# }) +test_that("testServer captures renderUI", { + server <- function(id) { + moduleServer(id, function(input, output, session){ + output$ui <- renderUI({ + tags$a(href="https://rstudio.com", "hello!") + }) + }) + } -# output$safe <- renderText({ -# stop(safeError("my safe error")) -# }) -# } + testServer(server, { + expect_equal(output$ui$deps, list()) + expect_equal(as.character(output$ui$html), "hello!") + }) +}) -# testModule(module, { -# expect_error(output$err, "my error") -# # TODO: helper for safe errors so users don't have to learn "shiny.custom.error"? -# expect_error(output$safe, "my safe error", class="shiny.custom.error") -# }) -#}) +test_that("testServer captures base graphics outputs", { + server <- function(id) { + moduleServer(id, function(input, output, session){ + output$fixed <- renderPlot({ + plot(1,1) + }, width=300, height=350) -#test_that("accessing a non-existant output gives an informative message", { -# module <- function(input, output, session){} + output$dynamic <- renderPlot({ + plot(1,1) + }) + }) + } -# testModule(module, { -# expect_error(output$dontexist, "hasn't been defined yet: output\\$dontexist") -# }) -#}) + testServer(server, { + # We aren't yet able to create reproducible graphics, so this test is intentionally pretty + # limited. + expect_equal(output$fixed$width, 300) + expect_equal(output$fixed$height, 350) + expect_match(output$fixed$src, "^data:image/png;base64,") + + # Ensure that the plot defaults to a reasonable size. + expect_equal(output$dynamic$width, 600) + expect_equal(output$dynamic$height, 400) + expect_match(output$dynamic$src, "^data:image/png;base64,") + + # TODO: how do you customize automatically inferred plot sizes? + # session$setPlotMeta("dynamic", width=600, height=300) ? + }) +}) -#test_that("testModule works with nested modules", { -# outerModule <- function(input, output, session) { -# r1 <- reactive({ input$x + 1}) -# r2 <- callModule(innerModule, "innerModule", r1) -# output$someVar <- renderText(r2()) -# } +test_that("testServer captures ggplot2 outputs", { + if (!requireNamespace("ggplot2")){ + testthat::skip("ggplot2 not available") + } -# innerModule <- function(input, output, session, r) { -# reactive(paste("a value:", r())) -# } + server <- function(id) { + moduleServer(id, function(input, output, session){ + output$fixed <- renderPlot({ + ggplot2::qplot(iris$Sepal.Length, iris$Sepal.Width) + }, width=300, height=350) -# testModule(outerModule, { -# session$setInputs(x = 1) -# expect_equal(output$someVar, "a value: 2") -# }) -#}) + output$dynamic <- renderPlot({ + ggplot2::qplot(iris$Sepal.Length, iris$Sepal.Width) + }) + }) + } -#test_that("testModule calls can be nested", { -# outerModule <- function(input, output, session) { -# doubled <- reactive({ input$x * 2 }) -# innerModule <- function(input, output, session) { -# quadrupled <- reactive({ doubled() * 2 }) -# } -# } + testServer(server, { + expect_equal(output$fixed$width, 300) + expect_equal(output$fixed$height, 350) + expect_match(output$fixed$src, "^data:image/png;base64,") + + # Ensure that the plot defaults to a reasonable size. + expect_equal(output$dynamic$width, 600) + expect_equal(output$dynamic$height, 400) + expect_match(output$dynamic$src, "^data:image/png;base64,") + }) +}) -# testModule(outerModule, { -# session$setInputs(x = 1) -# expect_equal(doubled(), 2) -# testModule(innerModule, { -# expect_equal(quadrupled(), 4) -# }) -# }) -#}) +test_that("testServer exposes the returned value from the module", { + server <- function(id) { + moduleServer(id, function(input, output, session){ + reactive({ + return(input$a + input$b) + }) + }) + } -#test_that("testModule returns a meaningful result", { -# result <- testModule(function(input, output, session) { -# reactive({ input$x * 2 }) -# }, { -# session$setInputs(x = 2) -# session$returned() -# }) -# expect_equal(result, 4) -#}) + testServer(server, { + session$setInputs(a=1, b=2) + expect_equal(session$returned(), 3) -#test_that("assigning an output in a module function with a non-function errors", { -# module <- function(input, output, session) { -# output$someVar <- 123 -# } + # And retains reactivity + session$setInputs(a=2) + expect_equal(session$returned(), 4) + }) +}) -# expect_error(testModule(module, {}), "^Unexpected") -#}) +test_that("testServer handles synchronous errors", { + server <- function(id) { + moduleServer(id, function(input, output, session, arg1, arg2){ + output$err <- renderText({ + stop("my error") + }) -#test_that("testServer works", { -# # app.R -# testServer({ -# session$setInputs(dist="norm", n=5) -# expect_length(d(), 5) + output$safe <- renderText({ + stop(safeError("my safe error")) + }) + }) + } -# session$setInputs(dist="unif", n=6) -# expect_length(d(), 6) -# }, appDir=test_path("..", "test-modules", "06_tabsets")) + testServer(server, { + expect_error(output$err, "my error") + # TODO: helper for safe errors so users don't have to learn "shiny.custom.error"? + expect_error(output$safe, "my safe error", class="shiny.custom.error") + }) +}) -# # server.R -# testServer({ -# session$setInputs(dist="norm", n=5) -# expect_length(d(), 5) +test_that("accessing a non-existent output gives an informative message", { + server <- function(id) { + moduleServer(id, function(input, output, session){}) + } -# session$setInputs(dist="unif", n=6) -# expect_length(d(), 6) -# }, appDir=test_path("..", "test-modules", "server_r")) -#}) + testServer(server, { + expect_error(output$dontexist, "hasn't been defined yet: output\\$dontexist") + }) +}) -#test_that("testServer works when referencing external globals", { -# # If global is defined at the top of app.R outside of the server function. -# testServer({ -# expect_equal(get("global", session$env), 123) -# }, appDir=test_path("..", "test-modules", "06_tabsets")) -#}) +test_that("testServer returns a meaningful result", { + result <- testServer(function(id) { + moduleServer(id, function(input, output, session) { + reactive({ input$x * 2 }) + }) + }, { + session$setInputs(x = 2) + session$returned() + }) + expect_equal(result, 4) +}) -#test_that("testModule allows lexical environment access through session$env", { -# m <- local({ -# a_var <- 123 -# function(input, output, session) { -# b_var <- 321 -# } -# }) -# expect_false(exists("a_var", inherits = FALSE)) -# testModule(m, { -# expect_equal(b_var, 321) -# expect_equal(get("a_var", session$env), 123) -# }) -#}) +test_that("assigning an output in a module function with a non-function errors", { + server <- function(id) { + moduleServer(id, function(input, output, session) { + output$someVar <- 123 -#test_that("Module shadowing can be mitigated with unquote", { -# i <- 0 -# inc <- function() i <<- i+1 + }) + } -# m <- local({ -# function(input, output, session) { -# inc <- function() stop("I should never be called") -# } -# }) + expect_error(testServer(server, {}), "^Unexpected") +}) -# testModule(m, { -# expect_is(inc, "function") -# expect_false(identical(inc, !!inc)) -# !!inc() -# }) +skip("TODO: testServer() with dir arg") +test_that("testServer works", { + # app.R + testServer({ + session$setInputs(dist="norm", n=5) + expect_length(d(), 5) + + session$setInputs(dist="unif", n=6) + expect_length(d(), 6) + }, appDir=test_path("..", "test-modules", "06_tabsets")) + + # server.R + testServer({ + session$setInputs(dist="norm", n=5) + expect_length(d(), 5) + + session$setInputs(dist="unif", n=6) + expect_length(d(), 6) + }, appDir=test_path("..", "test-modules", "server_r")) +}) -# expect_equal(i, 1) -#}) +skip("TODO: testServer() with dir arg") +test_that("testServer works when referencing external globals", { + # If global is defined at the top of app.R outside of the server function. + testServer({ + expect_equal(get("global", session$env), 123) + }, appDir=test_path("..", "test-modules", "06_tabsets")) +}) #test_that("testModule handles invalidateLater", { # module <- function(input, output, session) { From bb4aaa2a78270d57a38eea0951f48e39cbb30fff Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Tue, 31 Mar 2020 05:02:31 +0000 Subject: [PATCH 11/36] Bring back scope tests --- tests/testthat/test-test-server-scope.R | 62 ++++++++++++++----------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/tests/testthat/test-test-server-scope.R b/tests/testthat/test-test-server-scope.R index a36925e9f8..41fc2f5850 100644 --- a/tests/testthat/test-test-server-scope.R +++ b/tests/testthat/test-test-server-scope.R @@ -41,35 +41,41 @@ test_that("Variables outside the testServer() have correct visibility", { }, x = 0) }) -#test_that("testModule allows lexical environment access through session$env", { -# m <- local({ -# a_var <- 123 -# function(input, output, session) { -# b_var <- 321 -# } -# }) -# expect_false(exists("a_var", inherits = FALSE)) -# testModule(m, { -# expect_equal(b_var, 321) -# expect_equal(get("a_var", session$env), 123) -# }) -#}) +test_that("testServer allows lexical environment access through session$env", { + server <- local({ + a_var <- 123 + function(id) { + moduleServer(id, function(input, output, session) { + b_var <- 321 + }) + } + }) + + expect_false(exists("a_var", inherits = FALSE)) + + testServer(server, { + expect_equal(b_var, 321) + expect_equal(get("a_var", session$env), 123) + }) +}) -#test_that("Module shadowing can be mitigated with unquote", { -# i <- 0 -# inc <- function() i <<- i+1 +test_that("Shadowing can be mitigated with unquote", { + i <- 0 + inc <- function() i <<- i+1 -# m <- local({ -# function(input, output, session) { -# inc <- function() stop("I should never be called") -# } -# }) + server <- local({ + function(id) { + moduleServer(id, function(input, output, session) { + inc <- function() stop("I should never be called") + }) + } + }, envir = globalenv()) -# testModule(m, { -# expect_is(inc, "function") -# expect_false(identical(inc, !!inc)) -# !!inc() -# }) + testServer(server, { + expect_is(inc, "function") + expect_false(identical(inc, !!inc)) + !!inc() + }) -# expect_equal(i, 1) -#}) + expect_equal(i, 1) +}) From dd9e0343e8e1d25b08eac687900c00bdb1cdc8cb Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Tue, 31 Mar 2020 05:29:42 +0000 Subject: [PATCH 12/36] More test progress --- tests/testthat/test-test-server-app.R | 69 +++++++ tests/testthat/test-test-server-nesting.R | 4 + tests/testthat/test-test-server.R | 239 ++++++++-------------- 3 files changed, 153 insertions(+), 159 deletions(-) create mode 100644 tests/testthat/test-test-server-app.R diff --git a/tests/testthat/test-test-server-app.R b/tests/testthat/test-test-server-app.R new file mode 100644 index 0000000000..0c9a57d690 --- /dev/null +++ b/tests/testthat/test-test-server-app.R @@ -0,0 +1,69 @@ +context("testServer app") + +library(shiny) +library(testthat) + +#test_that("testServer works", { +# # app.R +# testServer({ +# session$setInputs(dist="norm", n=5) +# expect_length(d(), 5) +# +# session$setInputs(dist="unif", n=6) +# expect_length(d(), 6) +# }, appDir=test_path("..", "test-modules", "06_tabsets")) +# +# # server.R +# testServer({ +# session$setInputs(dist="norm", n=5) +# expect_length(d(), 5) +# +# session$setInputs(dist="unif", n=6) +# expect_length(d(), 6) +# }, appDir=test_path("..", "test-modules", "server_r")) +#}) +# +#test_that("testServer works when referencing external globals", { +# # If global is defined at the top of app.R outside of the server function. +# testServer({ +# expect_equal(get("global", session$env), 123) +# }, appDir=test_path("..", "test-modules", "06_tabsets")) +#}) +# +#test_that("findApp errors with no app", { +# calls <- 0 +# nothingExists <- function(path){ +# calls <<- calls + 1 +# FALSE +# } +# fa <- rewire(findApp, file.exists.ci=nothingExists) +# expect_error( +# expect_warning(fa("/some/path/here"), "No such file or directory"), # since we just made up a path +# "No shiny app was found in ") +# expect_equal(calls, 4 * 2) # Checks here, path, some, and / -- looking for app.R and server.R for each +#}) + +#test_that("findApp works with app in current or parent dir", { +# calls <- 0 +# cd <- normalizePath(".") +# mockExists <- function(path){ +# # Only TRUE if looking for server.R or app.R in current Dir +# calls <<- calls + 1 + +# path <- normalizePath(path, mustWork = FALSE) + +# appPath <- normalizePath(file.path(cd, "app.R"), mustWork = FALSE) +# serverPath <- normalizePath(file.path(cd, "server.R"), mustWork = FALSE) +# return(path %in% c(appPath, serverPath)) +# } +# fa <- rewire(findApp, file.exists.ci=mockExists) +# expect_equal(fa(), cd) +# expect_equal(calls, 1) # Should get a hit on the first call and stop + +# # Reset and point to the parent dir +# calls <- 0 +# cd <- normalizePath("..") # TODO: won't work if running tests in the root dir. +# f <- fa() +# expect_equal(normalizePath(f, mustWork = FALSE), cd) +# expect_equal(calls, 3) # Two for current dir and hit on the first in the parent +#}) diff --git a/tests/testthat/test-test-server-nesting.R b/tests/testthat/test-test-server-nesting.R index 71745dc0b0..d5a806f5e5 100644 --- a/tests/testthat/test-test-server-nesting.R +++ b/tests/testthat/test-test-server-nesting.R @@ -1,3 +1,7 @@ +context("testServer nesting") + +library(shiny) +library(testthat) #test_that("testModule works with nested modules", { # outerModule <- function(input, output, session) { # r1 <- reactive({ input$x + 1}) diff --git a/tests/testthat/test-test-server.R b/tests/testthat/test-test-server.R index 4b8450edde..f1903965fb 100644 --- a/tests/testthat/test-test-server.R +++ b/tests/testthat/test-test-server.R @@ -4,24 +4,6 @@ library(shiny) library(testthat) library(future) -#m <- function(id, multiplier = 2) { -# moduleServer(id, function(input, output, session) { -# multiplied <- reactive(input$x * multiplier) -# output$txt <- renderText(multiplied()) -# }) -#} -# -#test_that("it works", { -# expected <- 4 -# testServer(m, { -# session$setInputs(x = 2) -# expect_equal(multiplied(), expected) -# }, multiplier = 2) -#}) - -#library(promises) -#plan(multisession) - test_that("testServer passes dots", { server <- function(id, someArg) { expect_false(missing(someArg)) @@ -590,148 +572,87 @@ test_that("assigning an output in a module function with a non-function errors", expect_error(testServer(server, {}), "^Unexpected") }) -skip("TODO: testServer() with dir arg") -test_that("testServer works", { - # app.R - testServer({ - session$setInputs(dist="norm", n=5) - expect_length(d(), 5) - - session$setInputs(dist="unif", n=6) - expect_length(d(), 6) - }, appDir=test_path("..", "test-modules", "06_tabsets")) - - # server.R - testServer({ - session$setInputs(dist="norm", n=5) - expect_length(d(), 5) - - session$setInputs(dist="unif", n=6) - expect_length(d(), 6) - }, appDir=test_path("..", "test-modules", "server_r")) +test_that("testServer handles invalidateLater", { + server <- function(id) { + moduleServer(id, function(input, output, session) { + rv <- reactiveValues(x = 0) + observe({ + isolate(rv$x <- rv$x + 1) + # We're only testing one invalidation + if (isolate(rv$x) <= 1){ + invalidateLater(50) + } + }) + }) + } + + testServer(server, { + # Should have run once + expect_equal(rv$x, 1) + + session$elapse(49) + expect_equal(rv$x, 1) + + session$elapse(1) + # Should have been incremented now + expect_equal(rv$x, 2) + }) }) -skip("TODO: testServer() with dir arg") -test_that("testServer works when referencing external globals", { - # If global is defined at the top of app.R outside of the server function. - testServer({ - expect_equal(get("global", session$env), 123) - }, appDir=test_path("..", "test-modules", "06_tabsets")) +test_that("session ended handlers work", { + server <- function(id) { + moduleServer(id, function(input, output, session){}) + } + + testServer(server, { + rv <- reactiveValues(closed = FALSE) + session$onEnded(function(){ + rv$closed <- TRUE + }) + + expect_equal(session$isEnded(), FALSE) + expect_equal(session$isClosed(), FALSE) + expect_false(rv$closed, FALSE) + + session$close() + + expect_equal(session$isEnded(), TRUE) + expect_equal(session$isClosed(), TRUE) + expect_false(rv$closed, TRUE) + }) }) -#test_that("testModule handles invalidateLater", { -# module <- function(input, output, session) { -# rv <- reactiveValues(x = 0) -# observe({ -# isolate(rv$x <- rv$x + 1) -# # We're only testing one invalidation -# if (isolate(rv$x) <= 1){ -# invalidateLater(50) -# } -# }) -# } - -# testModule(module, { -# # Should have run once -# expect_equal(rv$x, 1) - -# session$elapse(49) -# expect_equal(rv$x, 1) - -# session$elapse(1) -# # Should have been incremented now -# expect_equal(rv$x, 2) -# }) -#}) - -#test_that("session ended handlers work", { -# module <- function(input, output, session){} - -# testModule(module, { -# rv <- reactiveValues(closed = FALSE) -# session$onEnded(function(){ -# rv$closed <- TRUE -# }) - -# expect_equal(session$isEnded(), FALSE) -# expect_equal(session$isClosed(), FALSE) -# expect_false(rv$closed, FALSE) - -# session$close() - -# expect_equal(session$isEnded(), TRUE) -# expect_equal(session$isClosed(), TRUE) -# expect_false(rv$closed, TRUE) -# }) -#}) - -#test_that("session flush handlers work", { -# module <- function(input, output, session) { -# rv <- reactiveValues(x = 0, flushCounter = 0, flushedCounter = 0, -# flushOnceCounter = 0, flushedOnceCounter = 0) - -# onFlush(function(){rv$flushCounter <- rv$flushCounter + 1}, once=FALSE) -# onFlushed(function(){rv$flushedCounter <- rv$flushedCounter + 1}, once=FALSE) -# onFlushed(function(){rv$flushOnceCounter <- rv$flushOnceCounter + 1}, once=TRUE) -# onFlushed(function(){rv$flushedOnceCounter <- rv$flushedOnceCounter + 1}, once=TRUE) - -# observe({ -# rv$x <- input$x * 2 -# }) -# } - -# testModule(module, { -# session$setInputs(x=1) -# expect_equal(rv$x, 2) -# # We're not concerned with the exact values here -- only that they increase -# fc <- rv$flushCounter -# fdc <- rv$flushedCounter - -# session$setInputs(x=2) -# expect_gt(rv$flushCounter, fc) -# expect_gt(rv$flushedCounter, fdc) - -# # These should have only run once -# expect_equal(rv$flushOnceCounter, 1) -# expect_equal(rv$flushedOnceCounter, 1) - -# }) -#}) - -#test_that("findApp errors with no app", { -# calls <- 0 -# nothingExists <- function(path){ -# calls <<- calls + 1 -# FALSE -# } -# fa <- rewire(findApp, file.exists.ci=nothingExists) -# expect_error( -# expect_warning(fa("/some/path/here"), "No such file or directory"), # since we just made up a path -# "No shiny app was found in ") -# expect_equal(calls, 4 * 2) # Checks here, path, some, and / -- looking for app.R and server.R for each -#}) - -#test_that("findApp works with app in current or parent dir", { -# calls <- 0 -# cd <- normalizePath(".") -# mockExists <- function(path){ -# # Only TRUE if looking for server.R or app.R in current Dir -# calls <<- calls + 1 - -# path <- normalizePath(path, mustWork = FALSE) - -# appPath <- normalizePath(file.path(cd, "app.R"), mustWork = FALSE) -# serverPath <- normalizePath(file.path(cd, "server.R"), mustWork = FALSE) -# return(path %in% c(appPath, serverPath)) -# } -# fa <- rewire(findApp, file.exists.ci=mockExists) -# expect_equal(fa(), cd) -# expect_equal(calls, 1) # Should get a hit on the first call and stop - -# # Reset and point to the parent dir -# calls <- 0 -# cd <- normalizePath("..") # TODO: won't work if running tests in the root dir. -# f <- fa() -# expect_equal(normalizePath(f, mustWork = FALSE), cd) -# expect_equal(calls, 3) # Two for current dir and hit on the first in the parent -#}) +test_that("session flush handlers work", { + server <- function(id) { + moduleServer(id, function(input, output, session) { + rv <- reactiveValues(x = 0, flushCounter = 0, flushedCounter = 0, + flushOnceCounter = 0, flushedOnceCounter = 0) + + onFlush(function(){rv$flushCounter <- rv$flushCounter + 1}, once=FALSE) + onFlushed(function(){rv$flushedCounter <- rv$flushedCounter + 1}, once=FALSE) + onFlushed(function(){rv$flushOnceCounter <- rv$flushOnceCounter + 1}, once=TRUE) + onFlushed(function(){rv$flushedOnceCounter <- rv$flushedOnceCounter + 1}, once=TRUE) + + observe({ + rv$x <- input$x * 2 + }) + }) + } + + testServer(server, { + session$setInputs(x=1) + expect_equal(rv$x, 2) + # We're not concerned with the exact values here -- only that they increase + fc <- rv$flushCounter + fdc <- rv$flushedCounter + + session$setInputs(x=2) + expect_gt(rv$flushCounter, fc) + expect_gt(rv$flushedCounter, fdc) + + # These should have only run once + expect_equal(rv$flushOnceCounter, 1) + expect_equal(rv$flushedOnceCounter, 1) + + }) +}) From 9d13cb644d9d81b0491b93a894f6acc743eb770c Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Tue, 31 Mar 2020 05:36:03 +0000 Subject: [PATCH 13/36] test-module.R => test-server.R --- DESCRIPTION | 2 +- R/{test-module.R => test-server.R} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename R/{test-module.R => test-server.R} (100%) diff --git a/DESCRIPTION b/DESCRIPTION index 162b750cda..494af31c71 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -170,7 +170,7 @@ Collate: 'snapshot.R' 'tar.R' 'test-export.R' - 'test-module.R' + 'test-server.R' 'test.R' 'update-input.R' RoxygenNote: 7.1.0 diff --git a/R/test-module.R b/R/test-server.R similarity index 100% rename from R/test-module.R rename to R/test-server.R From 65233cdd5c625d47825389372d04f14f4b89511c Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Tue, 31 Mar 2020 06:33:01 +0000 Subject: [PATCH 14/36] First passing app dir test for testServer overhaul --- R/test-server.R | 27 +++++++++++++++++++ tests/testthat/test-test-server-app.R | 38 +++++++++++++-------------- 2 files changed, 46 insertions(+), 19 deletions(-) diff --git a/R/test-server.R b/R/test-server.R index 6a68b73181..1123ff0e19 100644 --- a/R/test-server.R +++ b/R/test-server.R @@ -138,6 +138,11 @@ makeMask <- function(env) { append(child, parent[parent_only]) } +#' @noRd +isModuleServer <- function(x) { + is.function(x) && names(formals(x)) == "id" +} + #' Test an app's server-side logic #' @param appDir The directory root of the Shiny application. If `NULL`, this function #' will work up the directory hierarchy --- starting with the current directory --- @@ -145,9 +150,31 @@ makeMask <- function(env) { #' @rdname testModule #' @export testServer <- function(app, expr, ...) { + session <- MockShinySession$new() on.exit(if (!session$isClosed()) session$close()) + + if (is.character(app) && length(app) == 1) { + server <- shinyAppDir(app)$serverFuncSource() + message("Testing application found in: ", app) + fn_formals <- formals(server) + if (! "session" %in% names(fn_formals)) { + fn_formals$session <- bquote() + formals(server) <- fn_formals + } + body(server) <- rlang::expr({ + session$env <- base::environment() + session$setReturned({ !!!body(server) }) + }) + app <- function() { + server(input = session$input, output = session$output, session = session) + } + } else if (!isModuleServer(app)) { + stop("app must be either the location of a Shiny app or a module server function") + } + quosure <- rlang::enquo(expr) + isolate( withReactiveDomain( session, diff --git a/tests/testthat/test-test-server-app.R b/tests/testthat/test-test-server-app.R index 0c9a57d690..6b7923c11e 100644 --- a/tests/testthat/test-test-server-app.R +++ b/tests/testthat/test-test-server-app.R @@ -3,25 +3,25 @@ context("testServer app") library(shiny) library(testthat) -#test_that("testServer works", { -# # app.R -# testServer({ -# session$setInputs(dist="norm", n=5) -# expect_length(d(), 5) -# -# session$setInputs(dist="unif", n=6) -# expect_length(d(), 6) -# }, appDir=test_path("..", "test-modules", "06_tabsets")) -# -# # server.R -# testServer({ -# session$setInputs(dist="norm", n=5) -# expect_length(d(), 5) -# -# session$setInputs(dist="unif", n=6) -# expect_length(d(), 6) -# }, appDir=test_path("..", "test-modules", "server_r")) -#}) +test_that("testServer works with dir app", { + # app.R + testServer(test_path("..", "test-modules", "06_tabsets"), { + session$setInputs(dist="norm", n=5) + expect_length(d(), 5) + + session$setInputs(dist="unif", n=6) + expect_length(d(), 6) + }) + + # server.R + testServer(test_path("..", "test-modules", "server_r"), { + session$setInputs(dist="norm", n=5) + expect_length(d(), 5) + + session$setInputs(dist="unif", n=6) + expect_length(d(), 6) + }) +}) # #test_that("testServer works when referencing external globals", { # # If global is defined at the top of app.R outside of the server function. From cf9ab1c47b208130f6b480de013235b2ca6cde8c Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Tue, 31 Mar 2020 21:36:20 +0000 Subject: [PATCH 15/36] appobj coercion works --- R/test-server.R | 26 ++++++++++++++------------ tests/test-modules/06_tabsets/app.R | 2 +- tests/test-modules/server_r/server.R | 2 +- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/R/test-server.R b/R/test-server.R index 1123ff0e19..e65511cfed 100644 --- a/R/test-server.R +++ b/R/test-server.R @@ -143,6 +143,11 @@ isModuleServer <- function(x) { is.function(x) && names(formals(x)) == "id" } +#' @noRd +coercableToAppObj <- function(x) { + !is.null(getS3method("as.shiny.appobj", class(x), optional = TRUE)) +} + #' Test an app's server-side logic #' @param appDir The directory root of the Shiny application. If `NULL`, this function #' will work up the directory hierarchy --- starting with the current directory --- @@ -154,14 +159,11 @@ testServer <- function(app, expr, ...) { session <- MockShinySession$new() on.exit(if (!session$isClosed()) session$close()) - if (is.character(app) && length(app) == 1) { - server <- shinyAppDir(app)$serverFuncSource() - message("Testing application found in: ", app) - fn_formals <- formals(server) - if (! "session" %in% names(fn_formals)) { - fn_formals$session <- bquote() - formals(server) <- fn_formals - } + if (coercableToAppObj(app)) { + appobj <- as.shiny.appobj(app) + server <- appobj$serverFuncSource() + if (! "session" %in% names(formals(server))) + stop("Tested application server functions must declare input, output, and session arguments.") body(server) <- rlang::expr({ session$env <- base::environment() session$setReturned({ !!!body(server) }) @@ -169,12 +171,10 @@ testServer <- function(app, expr, ...) { app <- function() { server(input = session$input, output = session$output, session = session) } - } else if (!isModuleServer(app)) { - stop("app must be either the location of a Shiny app or a module server function") + } else if (!moduleServer(app)) { + stop("app argument must be a module function or coercable by as.shiny.appobj") } - quosure <- rlang::enquo(expr) - isolate( withReactiveDomain( session, @@ -183,6 +183,8 @@ testServer <- function(app, expr, ...) { }) ) ) + + quosure <- rlang::enquo(expr) isolate( withReactiveDomain( session, diff --git a/tests/test-modules/06_tabsets/app.R b/tests/test-modules/06_tabsets/app.R index 2b3ff527b0..1cc0bd3299 100644 --- a/tests/test-modules/06_tabsets/app.R +++ b/tests/test-modules/06_tabsets/app.R @@ -48,7 +48,7 @@ ui <- fluidPage( ) # Define server logic for random distribution app ---- -server <- function(input, output) { +server <- function(input, output, session) { # Reactive expression to generate the requested distribution ---- # This is called whenever the inputs change. The output functions diff --git a/tests/test-modules/server_r/server.R b/tests/test-modules/server_r/server.R index 9ec0e5dd22..335c9ed9a5 100644 --- a/tests/test-modules/server_r/server.R +++ b/tests/test-modules/server_r/server.R @@ -1,7 +1,7 @@ library(shiny) # Define server logic for random distribution app ---- -function(input, output) { +function(input, output, session) { # Reactive expression to generate the requested distribution ---- # This is called whenever the inputs change. The output functions From 58b4585b577ebfb2f8f99bb838a0cc29d9894c4c Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Wed, 1 Apr 2020 06:45:18 +0000 Subject: [PATCH 16/36] Doc and test updates --- R/test-server.R | 203 +++++++--------------- tests/testthat/test-test-server-app.R | 50 +----- tests/testthat/test-test-server-nesting.R | 68 ++++---- tests/testthat/test-test-server.R | 12 +- 4 files changed, 113 insertions(+), 220 deletions(-) diff --git a/R/test-server.R b/R/test-server.R index e65511cfed..86f462af16 100644 --- a/R/test-server.R +++ b/R/test-server.R @@ -1,35 +1,74 @@ +#' @noRd +testCallModule <- function(module, id, session) { + # TODO alan Figure out what to do with id here, necessary for nested usage? + body(module) <- rlang::expr({ + session$env <- base::environment() + !!!body(module) + }) + + session$setReturned(do.call(module, list( + input = session$input, + output = session$output, + session = session + ))) +} +# Create a "data mask" suitable for passing to rlang::eval_tidy. Bindings in +# `env` and bindings in the parent of `env` are merged into a single named list. +# Bindings in `env` take precedence over bindings in the parent of `env`. +#' @noRd +makeMask <- function(env) { + stopifnot(length(rlang::env_parents(env)) > 1) + stopifnot(all(c("input", "output", "session") %in% ls(env))) + child <- as.list(env) + parent <- as.list(rlang::env_parent(env)) + parent_only <- setdiff(names(parent), names(child)) + append(child, parent[parent_only]) +} -#' Integration testing for Shiny modules or server functions +#' @noRd +isModuleServer <- function(x) { + is.function(x) && names(formals(x))[1] == "id" +} + +#' @noRd +coercableToAppObj <- function(x) { + !is.null(getS3method("as.shiny.appobj", class(x), optional = TRUE)) +} + +#' Reactive testing for Shiny server functions and modules #' -#' Offer a way to test the reactive interactions in Shiny --- either in Shiny -#' modules or in the server portion of a Shiny application. For more -#' information, visit [the Shiny Dev Center article on integration -#' testing](https://shiny.rstudio.com/articles/integration-testing.html). -#' @param module The module to test +#' A way to test the reactive interactions in Shiny applications. Reactive +#' interactions are defined in the server function of applications and in +#' modules. +#' @param app The path to an application or module to test. In addition to +#' paths, applications may be represented by any object suitable for coercion +#' to an `appObj` by `as.shiny.appobj`. #' @param expr Test code containing expectations. The test expression will run -#' in the module's environment, meaning that the module's parameters (e.g. -#' `input`, `output`, and `session`) will be available along with any other -#' values created inside of the module. +#' in the server function environment, meaning that the parameters of the +#' server function (e.g. `input`, `output`, and `session`) will be available +#' along with any other values created inside of the server function. #' @param ... Additional arguments to pass to the module function. These #' arguments are processed with [rlang::list2()] and so are #' _[dynamic][rlang::dyn-dots]_. #' @return The result of evaluating `expr`. #' @include mock-session.R -#' @rdname testModule +#' @rdname testServer #' @examples -#' module <- function(input, output, session, multiplier = 2, prefix = "I am ") { -#' myreactive <- reactive({ -#' input$x * multiplier -#' }) -#' output$txt <- renderText({ -#' paste0(prefix, myreactive()) +#' server <- function(id, multiplier = 2, prefix = "I am ") { +#' moduleServer(id, function(input, output, session) { +#' myreactive <- reactive({ +#' input$x * multiplier +#' }) +#' output$txt <- renderText({ +#' paste0(prefix, myreactive()) +#' }) #' }) #' } #' #' # Basic Usage #' # ----------- -#' testModule(module, { +#' testServer(server, { #' session$setInputs(x = 1) #' # You're also free to use third-party #' # testing packages like testthat: @@ -47,7 +86,7 @@ #' # -------------- #' multiplier_arg_name = "multiplier" #' more_args <- list(prefix = "I am ") -#' testModule(module, { +#' testServer(server, { #' session$setInputs(x = 1) #' stopifnot(myreactive() == 2) #' stopifnot(output$txt == "I am 2") @@ -55,105 +94,6 @@ #' # into the testModule() argument list. #' }, !!multiplier_arg_name := 2, !!!more_args) #' @export -testModule <- function(module, expr, ...) { - .testModule( - module, - quosure = rlang::enquo(expr), - dots = rlang::list2(...), - env = rlang::caller_env() - ) -} - -#' @noRd -#' @importFrom withr with_options -.testModule <- function(module, quosure, dots, env) { - # Modify the module function locally by inserting `session$env <- - # environment()` at the beginning of its body. The dynamic environment of the - # module function is saved so that it may be referenced after the module - # function has returned. The saved dynamic environment is the basis for the - # `data` argument of tidy_eval() when used below to evaluate `quosure`, the - # test code expression. - body(module) <- rlang::expr({ - session$env <- base::environment() - !!!body(module) - }) - - session <- MockShinySession$new() - on.exit(if (!session$isClosed()) session$close()) - args <- append(dots, list(input = session$input, output = session$output, session = session)) - - isolate( - withReactiveDomain( - session, - withr::with_options(list(`shiny.allowoutputreads`=TRUE), { - # Assigning to `$returned` causes a flush to happen automatically. - session$returned <- do.call(module, args) - }) - ) - ) - - # Evaluate `quosure` in a reactive context, and in the provided `env`, but - # with `env` masked by a shallow view of `session$env`, the environment that - # was saved when the module function was invoked. flush is not needed before - # entering the loop because the first expr executed is `{`. - isolate({ - withReactiveDomain( - session, - withr::with_options(list(`shiny.allowoutputreads`=TRUE), { - rlang::eval_tidy( - quosure, - data = rlang::as_data_mask(as.list(session$env)), - env = env - ) - }) - ) - }) -} - -#' @noRd -testCallModule <- function(module, id, session) { - # TODO alan Figure out what to do with id here, necessary for nested usage - body(module) <- rlang::expr({ - session$env <- base::environment() - !!!body(module) - }) - - session$setReturned(do.call(module, list( - input = session$input, - output = session$output, - session = session - ))) -} - -# Create a "data mask" suitable for passing to rlang::eval_tidy. Bindings in -# `env` and bindings in the parent of `env` are merged into a single named list. -# Bindings in `env` take precedence over bindings in the parent of `env`. -#' @noRd -makeMask <- function(env) { - stopifnot(length(rlang::env_parents(env)) > 1) - stopifnot(all(c("input", "output", "session") %in% ls(env))) - child <- as.list(env) - parent <- as.list(rlang::env_parent(env)) - parent_only <- setdiff(names(parent), names(child)) - append(child, parent[parent_only]) -} - -#' @noRd -isModuleServer <- function(x) { - is.function(x) && names(formals(x)) == "id" -} - -#' @noRd -coercableToAppObj <- function(x) { - !is.null(getS3method("as.shiny.appobj", class(x), optional = TRUE)) -} - -#' Test an app's server-side logic -#' @param appDir The directory root of the Shiny application. If `NULL`, this function -#' will work up the directory hierarchy --- starting with the current directory --- -#' looking for a directory that contains an `app.R` or `server.R` file. -#' @rdname testModule -#' @export testServer <- function(app, expr, ...) { session <- MockShinySession$new() @@ -166,15 +106,16 @@ testServer <- function(app, expr, ...) { stop("Tested application server functions must declare input, output, and session arguments.") body(server) <- rlang::expr({ session$env <- base::environment() - session$setReturned({ !!!body(server) }) + !!!body(server) }) app <- function() { - server(input = session$input, output = session$output, session = session) + session$setReturned(server(input = session$input, output = session$output, session = session)) } - } else if (!moduleServer(app)) { + } else if (!isModuleServer(app)) { stop("app argument must be a module function or coercable by as.shiny.appobj") } + isolate( withReactiveDomain( session, @@ -194,25 +135,3 @@ testServer <- function(app, expr, ...) { ) ) } - -findApp <- function(startDir="."){ - dir <- normalizePath(startDir) - - # The loop will either return or stop() itself. - while (TRUE){ - if(file.exists.ci(file.path(dir, "app.R")) || file.exists.ci(file.path(dir, "server.R"))){ - return(dir) - } - - # Move up a directory - origDir <- dir - dir <- dirname(dir) - - # Testing for "root" path can be tricky. OSs differ and on Windows, network shares - # might have a \\ prefix. Easier to just see if we got stuck and abort. - if (dir == origDir){ - # We can go no further. - stop("No shiny app was found in ", startDir, " or any of its parent directories") - } - } -} diff --git a/tests/testthat/test-test-server-app.R b/tests/testthat/test-test-server-app.R index 6b7923c11e..bed5c6a36c 100644 --- a/tests/testthat/test-test-server-app.R +++ b/tests/testthat/test-test-server-app.R @@ -22,48 +22,10 @@ test_that("testServer works with dir app", { expect_length(d(), 6) }) }) -# -#test_that("testServer works when referencing external globals", { -# # If global is defined at the top of app.R outside of the server function. -# testServer({ -# expect_equal(get("global", session$env), 123) -# }, appDir=test_path("..", "test-modules", "06_tabsets")) -#}) -# -#test_that("findApp errors with no app", { -# calls <- 0 -# nothingExists <- function(path){ -# calls <<- calls + 1 -# FALSE -# } -# fa <- rewire(findApp, file.exists.ci=nothingExists) -# expect_error( -# expect_warning(fa("/some/path/here"), "No such file or directory"), # since we just made up a path -# "No shiny app was found in ") -# expect_equal(calls, 4 * 2) # Checks here, path, some, and / -- looking for app.R and server.R for each -#}) -#test_that("findApp works with app in current or parent dir", { -# calls <- 0 -# cd <- normalizePath(".") -# mockExists <- function(path){ -# # Only TRUE if looking for server.R or app.R in current Dir -# calls <<- calls + 1 - -# path <- normalizePath(path, mustWork = FALSE) - -# appPath <- normalizePath(file.path(cd, "app.R"), mustWork = FALSE) -# serverPath <- normalizePath(file.path(cd, "server.R"), mustWork = FALSE) -# return(path %in% c(appPath, serverPath)) -# } -# fa <- rewire(findApp, file.exists.ci=mockExists) -# expect_equal(fa(), cd) -# expect_equal(calls, 1) # Should get a hit on the first call and stop - -# # Reset and point to the parent dir -# calls <- 0 -# cd <- normalizePath("..") # TODO: won't work if running tests in the root dir. -# f <- fa() -# expect_equal(normalizePath(f, mustWork = FALSE), cd) -# expect_equal(calls, 3) # Two for current dir and hit on the first in the parent -#}) +test_that("testServer works when referencing external globals", { + # If global is defined at the top of app.R outside of the server function. + testServer(test_path("..", "test-modules", "06_tabsets"), { + expect_equal(get("global", session$env), 123) + }) +}) diff --git a/tests/testthat/test-test-server-nesting.R b/tests/testthat/test-test-server-nesting.R index d5a806f5e5..8d00d7bfc3 100644 --- a/tests/testthat/test-test-server-nesting.R +++ b/tests/testthat/test-test-server-nesting.R @@ -2,36 +2,46 @@ context("testServer nesting") library(shiny) library(testthat) -#test_that("testModule works with nested modules", { -# outerModule <- function(input, output, session) { -# r1 <- reactive({ input$x + 1}) -# r2 <- callModule(innerModule, "innerModule", r1) -# output$someVar <- renderText(r2()) -# } -# innerModule <- function(input, output, session, r) { -# reactive(paste("a value:", r())) -# } +test_that("testServer works with nested module servers", { + outerServer <- function(id) { + moduleServer(id, function(input, output, session) { + r1 <- reactive({ input$x + 1}) + r2 <- innerServer("inner", r1) + output$someVar <- renderText(r2()) + }) + } -# testModule(outerModule, { -# session$setInputs(x = 1) -# expect_equal(output$someVar, "a value: 2") -# }) -#}) + innerServer <- function(id, r) { + moduleServer(id, function(input, output, session) { + reactive(paste("a value:", r())) + }) + } -#test_that("testModule calls can be nested", { -# outerModule <- function(input, output, session) { -# doubled <- reactive({ input$x * 2 }) -# innerModule <- function(input, output, session) { -# quadrupled <- reactive({ doubled() * 2 }) -# } -# } + testServer(outerServer, { + session$setInputs(x = 1) + expect_equal(output$someVar, "a value: 2") + }) +}) -# testModule(outerModule, { -# session$setInputs(x = 1) -# expect_equal(doubled(), 2) -# testModule(innerModule, { -# expect_equal(quadrupled(), 4) -# }) -# }) -#}) +test_that("testServer calls can be nested", { + + outerServer <- function(id) { + moduleServer(id, function(input, output, session) { + doubled <- reactive({ input$x * 2 }) + innerServer <- function(id) { + moduleServer(id, function(input, output, session) { + quadrupled <- reactive({ doubled() * 2 }) + }) + } + }) + } + + testServer(outerServer, { + session$setInputs(x = 1) + expect_equal(doubled(), 2) + testServer(innerServer, { + expect_equal(quadrupled(), 4) + }) + }) +}) diff --git a/tests/testthat/test-test-server.R b/tests/testthat/test-test-server.R index f1903965fb..c6e621e566 100644 --- a/tests/testthat/test-test-server.R +++ b/tests/testthat/test-test-server.R @@ -97,14 +97,16 @@ test_that("testServer handles more complex expressions", { }) test_that("testServer handles reactiveVal", { - module <- function(input, output, session) { - x <- reactiveVal(0) - observe({ - x(input$y + input$z) + server <- function(id) { + moduleServer(id, function(input, output, session) { + x <- reactiveVal(0) + observe({ + x(input$y + input$z) + }) }) } - testModule(module, { + testServer(server, { session$setInputs(y=1, z=2) expect_equal(x(), 3) From 5475ec4f0c169e8363bc86c765e81dfe1d63e59d Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Wed, 1 Apr 2020 06:48:34 +0000 Subject: [PATCH 17/36] document --- NAMESPACE | 2 -- man/MockShinySession.Rd | 23 +++++++++++-- man/{testModule.Rd => testServer.Rd} | 49 +++++++++++++--------------- 3 files changed, 44 insertions(+), 30 deletions(-) rename man/{testModule.Rd => testServer.Rd} (50%) diff --git a/NAMESPACE b/NAMESPACE index 96c1f80493..4b138d1542 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -263,7 +263,6 @@ export(tagHasAttribute) export(tagList) export(tagSetChildren) export(tags) -export(testModule) export(testServer) export(textAreaInput) export(textInput) @@ -316,4 +315,3 @@ importFrom(grDevices,dev.cur) importFrom(grDevices,dev.set) importFrom(promises,"%...!%") importFrom(promises,"%...>%") -importFrom(withr,with_options) diff --git a/man/MockShinySession.Rd b/man/MockShinySession.Rd index b5df4e90e9..d480bc5c9a 100644 --- a/man/MockShinySession.Rd +++ b/man/MockShinySession.Rd @@ -21,6 +21,10 @@ s$setInputs(x=1, y=2) \describe{ \item{\code{env}}{The environment associated with the session.} +\item{\code{mask}}{The inner-module environment mask} + +\item{\code{returned}}{The value returned by the module.} + \item{\code{singletons}}{Hardcoded as empty. Needed for rendering HTML (i.e. renderUI)} \item{\code{clientData}}{Mock client data that always returns a size for plots} @@ -38,8 +42,6 @@ s$setInputs(x=1, y=2) \section{Active bindings}{ \if{html}{\out{
}} \describe{ -\item{\code{returned}}{The value returned from the module} - \item{\code{request}}{An empty environment where the request should be. The request isn't meaningfully mocked currently.} } \if{html}{\out{
}} @@ -83,6 +85,7 @@ s$setInputs(x=1, y=2) \item \href{#method-ns}{\code{MockShinySession$ns()}} \item \href{#method-flushReact}{\code{MockShinySession$flushReact()}} \item \href{#method-makeScope}{\code{MockShinySession$makeScope()}} +\item \href{#method-setReturned}{\code{MockShinySession$setReturned()}} \item \href{#method-clone}{\code{MockShinySession$clone()}} } } @@ -668,6 +671,22 @@ Create and return a namespace-specific session proxy. } } \if{html}{\out{
}} +\if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-setReturned}{}}} +\subsection{Method \code{setReturned()}}{ +\subsection{Usage}{ +\if{html}{\out{
}}\preformatted{MockShinySession$setReturned(value)}\if{html}{\out{
}} +} + +\subsection{Arguments}{ +\if{html}{\out{
}} +\describe{ +\item{\code{value}}{The value returned from the module} +} +\if{html}{\out{
}} +} +} +\if{html}{\out{
}} \if{html}{\out{}} \if{latex}{\out{\hypertarget{method-clone}{}}} \subsection{Method \code{clone()}}{ diff --git a/man/testModule.Rd b/man/testServer.Rd similarity index 50% rename from man/testModule.Rd rename to man/testServer.Rd index 8723bf148c..6e860b8a11 100644 --- a/man/testModule.Rd +++ b/man/testServer.Rd @@ -1,51 +1,48 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/test-module.R -\name{testModule} -\alias{testModule} +% Please edit documentation in R/test-server.R +\name{testServer} \alias{testServer} -\title{Integration testing for Shiny modules or server functions} +\title{Reactive testing for Shiny server functions and modules} \usage{ -testModule(module, expr, ...) - -testServer(expr, appDir = NULL) +testServer(app, expr, ...) } \arguments{ -\item{module}{The module to test} +\item{app}{The path to an application or module to test. In addition to +paths, applications may be represented by any object suitable for coercion +to an \code{appObj} by \code{as.shiny.appobj}.} \item{expr}{Test code containing expectations. The test expression will run -in the module's environment, meaning that the module's parameters (e.g. -\code{input}, \code{output}, and \code{session}) will be available along with any other -values created inside of the module.} +in the server function environment, meaning that the parameters of the +server function (e.g. \code{input}, \code{output}, and \code{session}) will be available +along with any other values created inside of the server function.} \item{...}{Additional arguments to pass to the module function. These arguments are processed with \code{\link[rlang:list2]{rlang::list2()}} and so are \emph{\link[rlang:dyn-dots]{dynamic}}.} - -\item{appDir}{The directory root of the Shiny application. If \code{NULL}, this function -will work up the directory hierarchy --- starting with the current directory --- -looking for a directory that contains an \code{app.R} or \code{server.R} file.} } \value{ The result of evaluating \code{expr}. } \description{ -Offer a way to test the reactive interactions in Shiny --- either in Shiny -modules or in the server portion of a Shiny application. For more -information, visit \href{https://shiny.rstudio.com/articles/integration-testing.html}{the Shiny Dev Center article on integration testing}. +A way to test the reactive interactions in Shiny applications. Reactive +interactions are defined in the server function of applications and in +modules. } \examples{ -module <- function(input, output, session, multiplier = 2, prefix = "I am ") { - myreactive <- reactive({ - input$x * multiplier - }) - output$txt <- renderText({ - paste0(prefix, myreactive()) +server <- function(id, multiplier = 2, prefix = "I am ") { + moduleServer(id, function(input, output, session) { + myreactive <- reactive({ + input$x * multiplier + }) + output$txt <- renderText({ + paste0(prefix, myreactive()) + }) }) } # Basic Usage # ----------- -testModule(module, { +testServer(server, { session$setInputs(x = 1) # You're also free to use third-party # testing packages like testthat: @@ -63,7 +60,7 @@ testModule(module, { # -------------- multiplier_arg_name = "multiplier" more_args <- list(prefix = "I am ") -testModule(module, { +testServer(server, { session$setInputs(x = 1) stopifnot(myreactive() == 2) stopifnot(output$txt == "I am 2") From 7e3deb5e3f45694eb9b5c24f45729aa895cfc228 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Wed, 1 Apr 2020 06:55:44 +0000 Subject: [PATCH 18/36] document --- R/test-server.R | 3 ++- man/testServer.Rd | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/R/test-server.R b/R/test-server.R index 86f462af16..301c115aa7 100644 --- a/R/test-server.R +++ b/R/test-server.R @@ -43,7 +43,8 @@ coercableToAppObj <- function(x) { #' modules. #' @param app The path to an application or module to test. In addition to #' paths, applications may be represented by any object suitable for coercion -#' to an `appObj` by `as.shiny.appobj`. +#' to an `appObj` by `as.shiny.appobj`. Application server functions must +#' include a `session` argument in order to be tested. #' @param expr Test code containing expectations. The test expression will run #' in the server function environment, meaning that the parameters of the #' server function (e.g. `input`, `output`, and `session`) will be available diff --git a/man/testServer.Rd b/man/testServer.Rd index 6e860b8a11..11f7534d1a 100644 --- a/man/testServer.Rd +++ b/man/testServer.Rd @@ -9,7 +9,8 @@ testServer(app, expr, ...) \arguments{ \item{app}{The path to an application or module to test. In addition to paths, applications may be represented by any object suitable for coercion -to an \code{appObj} by \code{as.shiny.appobj}.} +to an \code{appObj} by \code{as.shiny.appobj}. Application server functions must +include a \code{session} argument in order to be tested.} \item{expr}{Test code containing expectations. The test expression will run in the server function environment, meaning that the parameters of the From 7f80bfd2cb66faa46d74a2e542f9d08f3dbd65eb Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Wed, 1 Apr 2020 07:01:57 +0000 Subject: [PATCH 19/36] document --- R/mock-session.R | 2 -- inst/_pkgdown.yml | 2 +- man/MockShinySession.Rd | 2 -- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/R/mock-session.R b/R/mock-session.R index 57ae565d55..1777cd3d6e 100644 --- a/R/mock-session.R +++ b/R/mock-session.R @@ -83,8 +83,6 @@ MockShinySession <- R6Class( public = list( #' @field env The environment associated with the session. env = NULL, - #' @field mask The inner-module environment mask - mask = NULL, #' @field returned The value returned by the module. returned = NULL, #' @field singletons Hardcoded as empty. Needed for rendering HTML (i.e. renderUI) diff --git a/inst/_pkgdown.yml b/inst/_pkgdown.yml index 8112bf3ca3..7761a10802 100644 --- a/inst/_pkgdown.yml +++ b/inst/_pkgdown.yml @@ -217,5 +217,5 @@ reference: desc: Functions intended for testing of Shiny components contents: - runTests - - testModule + - testServer - MockShinySession diff --git a/man/MockShinySession.Rd b/man/MockShinySession.Rd index d480bc5c9a..2b08750df0 100644 --- a/man/MockShinySession.Rd +++ b/man/MockShinySession.Rd @@ -21,8 +21,6 @@ s$setInputs(x=1, y=2) \describe{ \item{\code{env}}{The environment associated with the session.} -\item{\code{mask}}{The inner-module environment mask} - \item{\code{returned}}{The value returned by the module.} \item{\code{singletons}}{Hardcoded as empty. Needed for rendering HTML (i.e. renderUI)} From 828567e0ce28d6303a607b4831880d5d359eb115 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Wed, 1 Apr 2020 21:31:39 +0000 Subject: [PATCH 20/36] Add failing proxy-related and ns() related tests --- tests/testthat/test-test-server-nesting.R | 32 +++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/testthat/test-test-server-nesting.R b/tests/testthat/test-test-server-nesting.R index 8d00d7bfc3..35f328d6ed 100644 --- a/tests/testthat/test-test-server-nesting.R +++ b/tests/testthat/test-test-server-nesting.R @@ -3,6 +3,38 @@ context("testServer nesting") library(shiny) library(testthat) +test_that("Nested modules", { + child <- function(id) { + moduleServer(id, function(input, output, session) { + output$txt <- renderText("bar") + }) + } + + parent <- function(id) { + moduleServer(id, function(input, output, session) { + output$txt <- renderText("foo") + child("child-id") + }) + } + + testServer(parent, { + expect_equal(output$txt, "foo") + }, id = "parent-id") + +}) + +test_that("Lack of ID", { + server <- function(id) { + moduleServer(id, function(input, output, session) { + output$txt <- renderText(session$ns("x")) + }) + } + + testServer(server, { + expect_equal(output$txt, "foo-x") + }, id = "foo") +}) + test_that("testServer works with nested module servers", { outerServer <- function(id) { moduleServer(id, function(input, output, session) { From e0ed443319213d680ab675276ee2caae1a3e8c4c Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Thu, 2 Apr 2020 21:33:57 +0000 Subject: [PATCH 21/36] WIP mock session scoped proxy --- R/mock-session.R | 26 +++++- R/test-server.R | 19 ++-- tests/testthat/test-test-server-nesting.R | 108 +++++++++++----------- 3 files changed, 86 insertions(+), 67 deletions(-) diff --git a/R/mock-session.R b/R/mock-session.R index 1777cd3d6e..be1ee1e08b 100644 --- a/R/mock-session.R +++ b/R/mock-session.R @@ -373,10 +373,10 @@ MockShinySession <- R6Class( #' @param export Not used #' @param format Not used getTestSnapshotUrl = function(input=TRUE, output=TRUE, export=TRUE, format="json") {}, - #' @description Returns the given id prefixed by `mock-session-`. + #' @description Returns the given id prefixed by this namespace's id. #' @param id The id to modify. ns = function(id) { - paste0("mock-session-", id) # TODO: does this need to be more complex/intelligent? + NS(private$nsPrefix, id) }, #' @description Trigger a reactive flush right now. flushReact = function(){ @@ -386,13 +386,30 @@ MockShinySession <- R6Class( #' @param namespace Character vector indicating a namespace. makeScope = function(namespace) { ns <- NS(namespace) - createSessionProxy( + env <- NULL + returned <- NULL + proxy <- createSessionProxy( self, input = .createReactiveValues(private$.input, readonly = TRUE, ns = ns), output = structure(.createOutputWriter(self, ns = ns), class = "shinyoutput"), - makeScope = function(namespace) self$makeScope(ns(namespace)) + makeScope = function(namespace) self$makeScope(ns(namespace)), + getEnv <- function() env, + setEnv <- function(env) { + env <<- env + }, + getReturned <- function() returned, + setReturned <- function(val) { + returned <<- val + private$flush() + val + } ) }, + getEnv = function() self$env, + setEnv = function(env) { + self$env <- env + }, + getReturned = function() self$returned, # If assigning to `returned`, proactively flush #' @param value The value returned from the module setReturned = function(value) { @@ -409,6 +426,7 @@ MockShinySession <- R6Class( timer = NULL, closed = FALSE, outs = list(), + nsPrefix = "", flush = function(){ isolate(private$flushCBs$invoke(..stacktraceon = TRUE)) diff --git a/R/test-server.R b/R/test-server.R index 301c115aa7..db19ef35f3 100644 --- a/R/test-server.R +++ b/R/test-server.R @@ -1,16 +1,17 @@ #' @noRd testCallModule <- function(module, id, session) { - # TODO alan Figure out what to do with id here, necessary for nested usage? body(module) <- rlang::expr({ - session$env <- base::environment() + session$setEnv(base::environment()) !!!body(module) }) - session$setReturned(do.call(module, list( - input = session$input, - output = session$output, - session = session - ))) + childSession <- session$makeScope(id) + + session$setReturned(module( + input = childSession$input, + output = childSession$output, + session = childSession + )) } # Create a "data mask" suitable for passing to rlang::eval_tidy. Bindings in @@ -106,7 +107,7 @@ testServer <- function(app, expr, ...) { if (! "session" %in% names(formals(server))) stop("Tested application server functions must declare input, output, and session arguments.") body(server) <- rlang::expr({ - session$env <- base::environment() + session$setEnv(base::environment()) !!!body(server) }) app <- function() { @@ -131,7 +132,7 @@ testServer <- function(app, expr, ...) { withReactiveDomain( session, withr::with_options(list(`shiny.allowoutputreads`=TRUE), { - rlang::eval_tidy(quosure, makeMask(session$env), rlang::caller_env()) + rlang::eval_tidy(quosure, makeMask(session$getEnv()), rlang::caller_env()) }) ) ) diff --git a/tests/testthat/test-test-server-nesting.R b/tests/testthat/test-test-server-nesting.R index 35f328d6ed..47bb07d34d 100644 --- a/tests/testthat/test-test-server-nesting.R +++ b/tests/testthat/test-test-server-nesting.R @@ -3,58 +3,58 @@ context("testServer nesting") library(shiny) library(testthat) -test_that("Nested modules", { - child <- function(id) { - moduleServer(id, function(input, output, session) { - output$txt <- renderText("bar") - }) - } - - parent <- function(id) { - moduleServer(id, function(input, output, session) { - output$txt <- renderText("foo") - child("child-id") - }) - } - - testServer(parent, { - expect_equal(output$txt, "foo") - }, id = "parent-id") - -}) - -test_that("Lack of ID", { - server <- function(id) { - moduleServer(id, function(input, output, session) { - output$txt <- renderText(session$ns("x")) - }) - } - - testServer(server, { - expect_equal(output$txt, "foo-x") - }, id = "foo") -}) - -test_that("testServer works with nested module servers", { - outerServer <- function(id) { - moduleServer(id, function(input, output, session) { - r1 <- reactive({ input$x + 1}) - r2 <- innerServer("inner", r1) - output$someVar <- renderText(r2()) - }) - } - - innerServer <- function(id, r) { - moduleServer(id, function(input, output, session) { - reactive(paste("a value:", r())) - }) - } - - testServer(outerServer, { - session$setInputs(x = 1) - expect_equal(output$someVar, "a value: 2") - }) -}) +#test_that("Nested modules", { +# child <- function(id) { +# moduleServer(id, function(input, output, session) { +# output$txt <- renderText("bar") +# }) +# } +# +# parent <- function(id) { +# moduleServer(id, function(input, output, session) { +# output$txt <- renderText("foo") +# child("child-id") +# }) +# } +# +# testServer(parent, { +# expect_equal(output$txt, "foo") +# }, id = "parent-id") +# +#}) +# +#test_that("Lack of ID", { +# server <- function(id) { +# moduleServer(id, function(input, output, session) { +# output$txt <- renderText(session$ns("x")) +# }) +# } +# +# testServer(server, { +# expect_equal(output$txt, "foo-x") +# }, id = "foo") +#}) +# +#test_that("testServer works with nested module servers", { +# outerServer <- function(id) { +# moduleServer(id, function(input, output, session) { +# r1 <- reactive({ input$x + 1}) +# r2 <- innerServer("inner", r1) +# output$someVar <- renderText(r2()) +# }) +# } +# +# innerServer <- function(id, r) { +# moduleServer(id, function(input, output, session) { +# reactive(paste("a value:", r())) +# }) +# } +# +# testServer(outerServer, { +# session$setInputs(x = 1) +# expect_equal(output$someVar, "a value: 2") +# }, id = "foo") +#}) test_that("testServer calls can be nested", { @@ -74,6 +74,6 @@ test_that("testServer calls can be nested", { expect_equal(doubled(), 2) testServer(innerServer, { expect_equal(quadrupled(), 4) - }) - }) + }, id = "bar") + }, id = "foo") }) From 953de733e76fc0eb294186d11355af0f1c2ba319 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Thu, 2 Apr 2020 23:26:02 +0000 Subject: [PATCH 22/36] nested module tests pass now, many others fail %-) --- R/mock-session.R | 21 ++++- R/modules.R | 19 +++- R/test-server.R | 28 ++---- tests/testthat/test-test-server-nesting.R | 108 +++++++++++----------- 4 files changed, 94 insertions(+), 82 deletions(-) diff --git a/R/mock-session.R b/R/mock-session.R index be1ee1e08b..7ced092192 100644 --- a/R/mock-session.R +++ b/R/mock-session.R @@ -69,6 +69,16 @@ extract <- function(promise) { stop("Single-bracket indexing of mockclientdata is not allowed.") } +#' @noRd +mapNames <- function(func, ...) { + vals <- list(...) + for (name in names(vals)) { + vals[[func(name)]] <- vals[[name]] + vals[[name]] <- NULL + } + vals +} + #' Mock Shiny Session #' #' @description @@ -393,17 +403,18 @@ MockShinySession <- R6Class( input = .createReactiveValues(private$.input, readonly = TRUE, ns = ns), output = structure(.createOutputWriter(self, ns = ns), class = "shinyoutput"), makeScope = function(namespace) self$makeScope(ns(namespace)), + ns = ns, getEnv <- function() env, - setEnv <- function(env) { - env <<- env - }, + setEnv <- function(env) env <<- env, getReturned <- function() returned, setReturned <- function(val) { returned <<- val private$flush() val - } + }, + setInputs = function(...) do.call(self$setInputs, mapNames(ns, ...)) ) + proxy }, getEnv = function() self$env, setEnv = function(env) { @@ -426,7 +437,7 @@ MockShinySession <- R6Class( timer = NULL, closed = FALSE, outs = list(), - nsPrefix = "", + nsPrefix = "mock-session", flush = function(){ isolate(private$flushCBs$invoke(..stacktraceon = TRUE)) diff --git a/R/modules.R b/R/modules.R index 455dc6d12d..4e70ce2876 100644 --- a/R/modules.R +++ b/R/modules.R @@ -37,6 +37,15 @@ createSessionProxy <- function(parentSession, ...) { `[[<-.session_proxy` <- `$<-.session_proxy` +#' @noRd +isMockSession <- function(session) { + if (inherits(session, "MockShinySession")) + return(TRUE) + if (inherits(session, "session_proxy")) + return(isMockSession(session$parent)) + FALSE +} + #' Shiny modules #' #' Shiny's module feature lets you break complicated UI and server logic into @@ -132,11 +141,13 @@ createSessionProxy <- function(parentSession, ...) { #' #' @export moduleServer <- function(id, module, session = getDefaultReactiveDomain()) { - if (inherits(session, "MockShinySession")) { - testCallModule(module, id, session) - } else { - callModule(module, id, session = session) + if (isMockSession(session)) { + body(module) <- rlang::expr({ + session$setEnv(base::environment()) + !!!body(module) + }) } + callModule(module, id, session = session) } diff --git a/R/test-server.R b/R/test-server.R index db19ef35f3..ae8b5d1973 100644 --- a/R/test-server.R +++ b/R/test-server.R @@ -1,19 +1,3 @@ -#' @noRd -testCallModule <- function(module, id, session) { - body(module) <- rlang::expr({ - session$setEnv(base::environment()) - !!!body(module) - }) - - childSession <- session$makeScope(id) - - session$setReturned(module( - input = childSession$input, - output = childSession$output, - session = childSession - )) -} - # Create a "data mask" suitable for passing to rlang::eval_tidy. Bindings in # `env` and bindings in the parent of `env` are merged into a single named list. # Bindings in `env` take precedence over bindings in the parent of `env`. @@ -98,6 +82,7 @@ coercableToAppObj <- function(x) { #' @export testServer <- function(app, expr, ...) { + args <- rlang::list2(...) session <- MockShinySession$new() on.exit(if (!session$isClosed()) session$close()) @@ -113,16 +98,21 @@ testServer <- function(app, expr, ...) { app <- function() { session$setReturned(server(input = session$input, output = session$output, session = session)) } - } else if (!isModuleServer(app)) { + if (length(args)) message("Discarding unused arguments to server function") + } else if (isModuleServer(app)) { + if (!("id" %in% names(args))) { + # If an id was not provided, one is generated. + args[["id"]] <- shiny::createUniqueId(bytes = 4) + } + } else { stop("app argument must be a module function or coercable by as.shiny.appobj") } - isolate( withReactiveDomain( session, withr::with_options(list(`shiny.allowoutputreads` = TRUE), { - rlang::exec(app, ...) + rlang::exec(app, !!!args) }) ) ) diff --git a/tests/testthat/test-test-server-nesting.R b/tests/testthat/test-test-server-nesting.R index 47bb07d34d..d7c135d9c2 100644 --- a/tests/testthat/test-test-server-nesting.R +++ b/tests/testthat/test-test-server-nesting.R @@ -3,58 +3,58 @@ context("testServer nesting") library(shiny) library(testthat) -#test_that("Nested modules", { -# child <- function(id) { -# moduleServer(id, function(input, output, session) { -# output$txt <- renderText("bar") -# }) -# } -# -# parent <- function(id) { -# moduleServer(id, function(input, output, session) { -# output$txt <- renderText("foo") -# child("child-id") -# }) -# } -# -# testServer(parent, { -# expect_equal(output$txt, "foo") -# }, id = "parent-id") -# -#}) -# -#test_that("Lack of ID", { -# server <- function(id) { -# moduleServer(id, function(input, output, session) { -# output$txt <- renderText(session$ns("x")) -# }) -# } -# -# testServer(server, { -# expect_equal(output$txt, "foo-x") -# }, id = "foo") -#}) -# -#test_that("testServer works with nested module servers", { -# outerServer <- function(id) { -# moduleServer(id, function(input, output, session) { -# r1 <- reactive({ input$x + 1}) -# r2 <- innerServer("inner", r1) -# output$someVar <- renderText(r2()) -# }) -# } -# -# innerServer <- function(id, r) { -# moduleServer(id, function(input, output, session) { -# reactive(paste("a value:", r())) -# }) -# } -# -# testServer(outerServer, { -# session$setInputs(x = 1) -# expect_equal(output$someVar, "a value: 2") -# }, id = "foo") -#}) +test_that("Nested modules", { + child <- function(id) { + moduleServer(id, function(input, output, session) { + output$txt <- renderText("bar") + }) + } + + parent <- function(id) { + moduleServer(id, function(input, output, session) { + output$txt <- renderText("foo") + child("child-id") + }) + } + + testServer(parent, { + expect_equal(output$txt, "foo") + }, id = "parent-id") + +}) + +test_that("Lack of ID", { + server <- function(id) { + moduleServer(id, function(input, output, session) { + output$txt <- renderText(session$ns("x")) + }) + } + + testServer(server, { + expect_equal(output$txt, "foo-x") + }, id = "foo") +}) + +test_that("testServer works with nested module servers", { + outerServer <- function(id) { + moduleServer(id, function(input, output, session) { + r1 <- reactive({ input$x + 1}) + r2 <- innerServer("inner", r1) + output$someVar <- renderText(r2()) + }) + } + + innerServer <- function(id, r) { + moduleServer(id, function(input, output, session) { + reactive(paste("a value:", r())) + }) + } + + testServer(outerServer, { + session$setInputs(x = 1) + expect_equal(output$someVar, "a value: 2") + }, id = "foo") +}) test_that("testServer calls can be nested", { @@ -74,6 +74,6 @@ test_that("testServer calls can be nested", { expect_equal(doubled(), 2) testServer(innerServer, { expect_equal(quadrupled(), 4) - }, id = "bar") - }, id = "foo") + }) + }) }) From 90f531888ce33d54cf5e272afd7f10589a86e034 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Fri, 3 Apr 2020 22:06:14 +0000 Subject: [PATCH 23/36] fix one failing test --- tests/testthat/test-test-server.R | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-test-server.R b/tests/testthat/test-test-server.R index c6e621e566..f46d6c78c8 100644 --- a/tests/testthat/test-test-server.R +++ b/tests/testthat/test-test-server.R @@ -547,7 +547,11 @@ test_that("accessing a non-existent output gives an informative message", { } testServer(server, { - expect_error(output$dontexist, "hasn't been defined yet: output\\$dontexist") + expect_error(output$dontexist, "hasn't been defined yet: output\\$server1-dontexist") + }, id = "server1") + + testServer(server, { + expect_error(output$dontexist, "hasn't been defined yet: output\\$.*-dontexist") }) }) From 70edcd62b97b643f636fded68004dbe9ff2c387e Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Fri, 3 Apr 2020 23:04:50 +0000 Subject: [PATCH 24/36] Getting there --- R/mock-session.R | 19 ++++++++++++++----- R/modules.R | 2 +- R/test-server.R | 22 +++++++++++++++++++--- tests/testthat/test-test-server-scope.R | 4 ++-- tests/testthat/test-test-server.R | 6 +++--- 5 files changed, 39 insertions(+), 14 deletions(-) diff --git a/R/mock-session.R b/R/mock-session.R index 7ced092192..f8333f6d15 100644 --- a/R/mock-session.R +++ b/R/mock-session.R @@ -404,29 +404,36 @@ MockShinySession <- R6Class( output = structure(.createOutputWriter(self, ns = ns), class = "shinyoutput"), makeScope = function(namespace) self$makeScope(ns(namespace)), ns = ns, - getEnv <- function() env, - setEnv <- function(env) env <<- env, - getReturned <- function() returned, - setReturned <- function(val) { + getEnv = function() env, + setEnv = function(env) env <<- env, + setReturned = function(val) { returned <<- val private$flush() val }, + getReturned = function() returned, setInputs = function(...) do.call(self$setInputs, mapNames(ns, ...)) ) + private$proxies$set(namespace, proxy) proxy }, getEnv = function() self$env, setEnv = function(env) { self$env <- env }, - getReturned = function() self$returned, # If assigning to `returned`, proactively flush #' @param value The value returned from the module setReturned = function(value) { self$returned <- value private$flush() value + }, + getReturned = function() self$returned, + genId = function() { + paste0("proxy", (private$proxyCounter <- private$proxyCounter+1)) + }, + getProxy = function(namespace) { + private$proxies$get(namespace) } ), private = list( @@ -438,6 +445,8 @@ MockShinySession <- R6Class( closed = FALSE, outs = list(), nsPrefix = "mock-session", + proxyCounter = 0, + proxies = fastmap::fastmap(), flush = function(){ isolate(private$flushCBs$invoke(..stacktraceon = TRUE)) diff --git a/R/modules.R b/R/modules.R index 4e70ce2876..f00f8f5d57 100644 --- a/R/modules.R +++ b/R/modules.R @@ -144,7 +144,7 @@ moduleServer <- function(id, module, session = getDefaultReactiveDomain()) { if (isMockSession(session)) { body(module) <- rlang::expr({ session$setEnv(base::environment()) - !!!body(module) + session$setReturned({ !!!body(module) }) }) } callModule(module, id, session = session) diff --git a/R/test-server.R b/R/test-server.R index ae8b5d1973..7d469c9af9 100644 --- a/R/test-server.R +++ b/R/test-server.R @@ -83,8 +83,12 @@ coercableToAppObj <- function(x) { testServer <- function(app, expr, ...) { args <- rlang::list2(...) - session <- MockShinySession$new() - on.exit(if (!session$isClosed()) session$close()) + + session <- getDefaultReactiveDomain() + if (is.null(session)) { + session <- MockShinySession$new() + on.exit(if (!session$isClosed()) session$close()) + } if (coercableToAppObj(app)) { appobj <- as.shiny.appobj(app) @@ -102,7 +106,7 @@ testServer <- function(app, expr, ...) { } else if (isModuleServer(app)) { if (!("id" %in% names(args))) { # If an id was not provided, one is generated. - args[["id"]] <- shiny::createUniqueId(bytes = 4) + args[["id"]] <- session$genId() } } else { stop("app argument must be a module function or coercable by as.shiny.appobj") @@ -118,6 +122,18 @@ testServer <- function(app, expr, ...) { ) quosure <- rlang::enquo(expr) + + # If app is a module server, we know that callModule() will be involved and + # will call session$makeScope(id). This is problematic for us because it means + # that `session` inside the module function will correspond to a proxy of the + # `session` in scope in this function, right now. To work around this, we rely + # on the fact that we know the `id` of the proxy. MockSession$makeScope() + # stores all the proxies it creates by name in a fastmap, so we can find the + # proxy we need by looking for the one with the id we have now. + if (isModuleServer(app)) { + session <- session$getProxy(args[["id"]]) + } + isolate( withReactiveDomain( session, diff --git a/tests/testthat/test-test-server-scope.R b/tests/testthat/test-test-server-scope.R index 41fc2f5850..070ea71fb1 100644 --- a/tests/testthat/test-test-server-scope.R +++ b/tests/testthat/test-test-server-scope.R @@ -41,7 +41,7 @@ test_that("Variables outside the testServer() have correct visibility", { }, x = 0) }) -test_that("testServer allows lexical environment access through session$env", { +test_that("testServer allows lexical environment access through session$getEnv()", { server <- local({ a_var <- 123 function(id) { @@ -55,7 +55,7 @@ test_that("testServer allows lexical environment access through session$env", { testServer(server, { expect_equal(b_var, 321) - expect_equal(get("a_var", session$env), 123) + expect_equal(get("a_var", session$getEnv()), 123) }) }) diff --git a/tests/testthat/test-test-server.R b/tests/testthat/test-test-server.R index f46d6c78c8..3e854d7181 100644 --- a/tests/testthat/test-test-server.R +++ b/tests/testthat/test-test-server.R @@ -513,11 +513,11 @@ test_that("testServer exposes the returned value from the module", { testServer(server, { session$setInputs(a=1, b=2) - expect_equal(session$returned(), 3) + expect_equal(session$getReturned()(), 3) # And retains reactivity session$setInputs(a=2) - expect_equal(session$returned(), 4) + expect_equal(session$getReturned()(), 4) }) }) @@ -562,7 +562,7 @@ test_that("testServer returns a meaningful result", { }) }, { session$setInputs(x = 2) - session$returned() + session$getReturned()() }) expect_equal(result, 4) }) From 3ca8b1017bf18e5fa3e646d246c64a7a0e0982b6 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Mon, 6 Apr 2020 22:36:09 +0000 Subject: [PATCH 25/36] Tests pass \o/ --- R/mock-session.R | 24 +++---------- R/modules.R | 12 +------ R/test-server.R | 44 ++++++++--------------- tests/testthat/test-test-server-nesting.R | 34 +++++++++++------- 4 files changed, 43 insertions(+), 71 deletions(-) diff --git a/R/mock-session.R b/R/mock-session.R index f8333f6d15..80d84c42cc 100644 --- a/R/mock-session.R +++ b/R/mock-session.R @@ -396,29 +396,19 @@ MockShinySession <- R6Class( #' @param namespace Character vector indicating a namespace. makeScope = function(namespace) { ns <- NS(namespace) - env <- NULL returned <- NULL - proxy <- createSessionProxy( + createSessionProxy( self, input = .createReactiveValues(private$.input, readonly = TRUE, ns = ns), output = structure(.createOutputWriter(self, ns = ns), class = "shinyoutput"), makeScope = function(namespace) self$makeScope(ns(namespace)), - ns = ns, - getEnv = function() env, - setEnv = function(env) env <<- env, - setReturned = function(val) { - returned <<- val - private$flush() - val - }, - getReturned = function() returned, + ns = function(namespace) ns(namespace), setInputs = function(...) do.call(self$setInputs, mapNames(ns, ...)) ) - private$proxies$set(namespace, proxy) - proxy }, getEnv = function() self$env, setEnv = function(env) { + stopifnot(is.null(self$env)) self$env <- env }, # If assigning to `returned`, proactively flush @@ -430,10 +420,7 @@ MockShinySession <- R6Class( }, getReturned = function() self$returned, genId = function() { - paste0("proxy", (private$proxyCounter <- private$proxyCounter+1)) - }, - getProxy = function(namespace) { - private$proxies$get(namespace) + paste0("proxy", (private$idCounter <- private$idCounter + 1)) } ), private = list( @@ -445,8 +432,7 @@ MockShinySession <- R6Class( closed = FALSE, outs = list(), nsPrefix = "mock-session", - proxyCounter = 0, - proxies = fastmap::fastmap(), + idCounter = 0, flush = function(){ isolate(private$flushCBs$invoke(..stacktraceon = TRUE)) diff --git a/R/modules.R b/R/modules.R index f00f8f5d57..aa12a9e3c9 100644 --- a/R/modules.R +++ b/R/modules.R @@ -36,16 +36,6 @@ createSessionProxy <- function(parentSession, ...) { `[[<-.session_proxy` <- `$<-.session_proxy` - -#' @noRd -isMockSession <- function(session) { - if (inherits(session, "MockShinySession")) - return(TRUE) - if (inherits(session, "session_proxy")) - return(isMockSession(session$parent)) - FALSE -} - #' Shiny modules #' #' Shiny's module feature lets you break complicated UI and server logic into @@ -141,7 +131,7 @@ isMockSession <- function(session) { #' #' @export moduleServer <- function(id, module, session = getDefaultReactiveDomain()) { - if (isMockSession(session)) { + if (inherits(session, "MockShinySession")) { body(module) <- rlang::expr({ session$setEnv(base::environment()) session$setReturned({ !!!body(module) }) diff --git a/R/test-server.R b/R/test-server.R index 7d469c9af9..f1094cee5d 100644 --- a/R/test-server.R +++ b/R/test-server.R @@ -16,11 +16,6 @@ isModuleServer <- function(x) { is.function(x) && names(formals(x))[1] == "id" } -#' @noRd -coercableToAppObj <- function(x) { - !is.null(getS3method("as.shiny.appobj", class(x), optional = TRUE)) -} - #' Reactive testing for Shiny server functions and modules #' #' A way to test the reactive interactions in Shiny applications. Reactive @@ -85,12 +80,20 @@ testServer <- function(app, expr, ...) { args <- rlang::list2(...) session <- getDefaultReactiveDomain() - if (is.null(session)) { - session <- MockShinySession$new() - on.exit(if (!session$isClosed()) session$close()) - } - if (coercableToAppObj(app)) { + if (inherits(session, "MockShinySession")) + stop("Test expressions may not call testServer()") + if (inherits(session, "session_proxy") + && inherits(get("parent", envir = session), "MockShinySession")) + stop("Modules may not call testServer()") + + session <- MockShinySession$new() + on.exit(if (!session$isClosed()) session$close()) + + if (isModuleServer(app)) { + if (!("id" %in% names(args))) + args[["id"]] <- session$genId() + } else { appobj <- as.shiny.appobj(app) server <- appobj$serverFuncSource() if (! "session" %in% names(formals(server))) @@ -102,14 +105,8 @@ testServer <- function(app, expr, ...) { app <- function() { session$setReturned(server(input = session$input, output = session$output, session = session)) } - if (length(args)) message("Discarding unused arguments to server function") - } else if (isModuleServer(app)) { - if (!("id" %in% names(args))) { - # If an id was not provided, one is generated. - args[["id"]] <- session$genId() - } - } else { - stop("app argument must be a module function or coercable by as.shiny.appobj") + if (length(args)) + message("Discarding unused arguments to server function") } isolate( @@ -123,17 +120,6 @@ testServer <- function(app, expr, ...) { quosure <- rlang::enquo(expr) - # If app is a module server, we know that callModule() will be involved and - # will call session$makeScope(id). This is problematic for us because it means - # that `session` inside the module function will correspond to a proxy of the - # `session` in scope in this function, right now. To work around this, we rely - # on the fact that we know the `id` of the proxy. MockSession$makeScope() - # stores all the proxies it creates by name in a fastmap, so we can find the - # proxy we need by looking for the one with the id we have now. - if (isModuleServer(app)) { - session <- session$getProxy(args[["id"]]) - } - isolate( withReactiveDomain( session, diff --git a/tests/testthat/test-test-server-nesting.R b/tests/testthat/test-test-server-nesting.R index d7c135d9c2..378a084739 100644 --- a/tests/testthat/test-test-server-nesting.R +++ b/tests/testthat/test-test-server-nesting.R @@ -56,24 +56,34 @@ test_that("testServer works with nested module servers", { }, id = "foo") }) -test_that("testServer calls can be nested", { +test_that("testServer calls do not nest in module functions", { + server <- function(id) { + moduleServer(id, function(input, output, session) { + x <- 1 + testServer(function(id) { + moduleServer(id, function(input, output, session) { + y <- x + 1 + }) + }) + }) + } - outerServer <- function(id) { + expect_error(testServer(server, {}), regexp = "Modules may not call testServer()") +}) + +test_that("testServer calls do not nest in test exprs", { + server <- function(id) { + x <- 1 moduleServer(id, function(input, output, session) { - doubled <- reactive({ input$x * 2 }) - innerServer <- function(id) { + inner <- function(id) { moduleServer(id, function(input, output, session) { - quadrupled <- reactive({ doubled() * 2 }) + y <- x + 1 }) } }) } - testServer(outerServer, { - session$setInputs(x = 1) - expect_equal(doubled(), 2) - testServer(innerServer, { - expect_equal(quadrupled(), 4) - }) - }) + expect_error(testServer(server, { + testServer(inner, {}) + }), regexp = "Test expressions may not call testServer()") }) From 1d9a6ea3c0b89c553d7f5b5a8a7d7fcd964e6e82 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Mon, 6 Apr 2020 23:11:51 +0000 Subject: [PATCH 26/36] getEnv() => env, docs --- R/mock-session.R | 2 -- R/test-server.R | 21 ++++----------------- tests/testthat/test-test-server-scope.R | 4 ++-- 3 files changed, 6 insertions(+), 21 deletions(-) diff --git a/R/mock-session.R b/R/mock-session.R index 80d84c42cc..764a850ed9 100644 --- a/R/mock-session.R +++ b/R/mock-session.R @@ -406,9 +406,7 @@ MockShinySession <- R6Class( setInputs = function(...) do.call(self$setInputs, mapNames(ns, ...)) ) }, - getEnv = function() self$env, setEnv = function(env) { - stopifnot(is.null(self$env)) self$env <- env }, # If assigning to `returned`, proactively flush diff --git a/R/test-server.R b/R/test-server.R index f1094cee5d..4ddc59a72f 100644 --- a/R/test-server.R +++ b/R/test-server.R @@ -31,7 +31,8 @@ isModuleServer <- function(x) { #' along with any other values created inside of the server function. #' @param ... Additional arguments to pass to the module function. These #' arguments are processed with [rlang::list2()] and so are -#' _[dynamic][rlang::dyn-dots]_. +#' _[dynamic][rlang::dyn-dots]_. If `app` is a module, and no `id` argument is +#' provided, one will be generated and supplied automatically. #' @return The result of evaluating `expr`. #' @include mock-session.R #' @rdname testServer @@ -47,8 +48,6 @@ isModuleServer <- function(x) { #' }) #' } #' -#' # Basic Usage -#' # ----------- #' testServer(server, { #' session$setInputs(x = 1) #' # You're also free to use third-party @@ -62,18 +61,6 @@ isModuleServer <- function(x) { #' stopifnot(output$txt == "I am 4") #' # Any additional arguments, below, are passed along to the module. #' }, multiplier = 2) -#' -#' # Advanced Usage -#' # -------------- -#' multiplier_arg_name = "multiplier" -#' more_args <- list(prefix = "I am ") -#' testServer(server, { -#' session$setInputs(x = 1) -#' stopifnot(myreactive() == 2) -#' stopifnot(output$txt == "I am 2") -#' # !!/:= and !!! from rlang are used below to splice computed arguments -#' # into the testModule() argument list. -#' }, !!multiplier_arg_name := 2, !!!more_args) #' @export testServer <- function(app, expr, ...) { @@ -123,8 +110,8 @@ testServer <- function(app, expr, ...) { isolate( withReactiveDomain( session, - withr::with_options(list(`shiny.allowoutputreads`=TRUE), { - rlang::eval_tidy(quosure, makeMask(session$getEnv()), rlang::caller_env()) + withr::with_options(list(`shiny.allowoutputreads` = TRUE), { + rlang::eval_tidy(quosure, makeMask(session$env), rlang::caller_env()) }) ) ) diff --git a/tests/testthat/test-test-server-scope.R b/tests/testthat/test-test-server-scope.R index 070ea71fb1..41fc2f5850 100644 --- a/tests/testthat/test-test-server-scope.R +++ b/tests/testthat/test-test-server-scope.R @@ -41,7 +41,7 @@ test_that("Variables outside the testServer() have correct visibility", { }, x = 0) }) -test_that("testServer allows lexical environment access through session$getEnv()", { +test_that("testServer allows lexical environment access through session$env", { server <- local({ a_var <- 123 function(id) { @@ -55,7 +55,7 @@ test_that("testServer allows lexical environment access through session$getEnv() testServer(server, { expect_equal(b_var, 321) - expect_equal(get("a_var", session$getEnv()), 123) + expect_equal(get("a_var", session$env), 123) }) }) From 9d8a6d01425482f7fa18dd274608fe9ea8d39710 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Tue, 7 Apr 2020 18:40:34 +0000 Subject: [PATCH 27/36] Document new R6 methods --- R/mock-session.R | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/R/mock-session.R b/R/mock-session.R index 764a850ed9..ca7316e82d 100644 --- a/R/mock-session.R +++ b/R/mock-session.R @@ -406,17 +406,22 @@ MockShinySession <- R6Class( setInputs = function(...) do.call(self$setInputs, mapNames(ns, ...)) ) }, + #' @description Set the environment associated with a testServer() call. + #' @param env The environment to retain. setEnv = function(env) { self$env <- env }, - # If assigning to `returned`, proactively flush + #' @description Set the value returned by the module call and proactively flush. #' @param value The value returned from the module setReturned = function(value) { self$returned <- value private$flush() value }, + #' @description Get the value returned by the module call. getReturned = function() self$returned, + #' @description Return a distinct character identifier for use as a proxy + #' namespace. genId = function() { paste0("proxy", (private$idCounter <- private$idCounter + 1)) } From 286f12522bd6fe1348eade48b38eede1b1f6ef5e Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Tue, 7 Apr 2020 18:40:40 +0000 Subject: [PATCH 28/36] document --- man/MockShinySession.Rd | 44 ++++++++++++++++++++++++++++++++++++++++- man/testServer.Rd | 17 ++-------------- 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/man/MockShinySession.Rd b/man/MockShinySession.Rd index 2b08750df0..93fc2ec3ba 100644 --- a/man/MockShinySession.Rd +++ b/man/MockShinySession.Rd @@ -83,7 +83,10 @@ s$setInputs(x=1, y=2) \item \href{#method-ns}{\code{MockShinySession$ns()}} \item \href{#method-flushReact}{\code{MockShinySession$flushReact()}} \item \href{#method-makeScope}{\code{MockShinySession$makeScope()}} +\item \href{#method-setEnv}{\code{MockShinySession$setEnv()}} \item \href{#method-setReturned}{\code{MockShinySession$setReturned()}} +\item \href{#method-getReturned}{\code{MockShinySession$getReturned()}} +\item \href{#method-genId}{\code{MockShinySession$genId()}} \item \href{#method-clone}{\code{MockShinySession$clone()}} } } @@ -628,7 +631,7 @@ No-op \if{html}{\out{}} \if{latex}{\out{\hypertarget{method-ns}{}}} \subsection{Method \code{ns()}}{ -Returns the given id prefixed by \verb{mock-session-}. +Returns the given id prefixed by this namespace's id. \subsection{Usage}{ \if{html}{\out{
}}\preformatted{MockShinySession$ns(id)}\if{html}{\out{
}} } @@ -669,9 +672,27 @@ Create and return a namespace-specific session proxy. } } \if{html}{\out{
}} +\if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-setEnv}{}}} +\subsection{Method \code{setEnv()}}{ +Set the environment associated with a testServer() call. +\subsection{Usage}{ +\if{html}{\out{
}}\preformatted{MockShinySession$setEnv(env)}\if{html}{\out{
}} +} + +\subsection{Arguments}{ +\if{html}{\out{
}} +\describe{ +\item{\code{env}}{The environment to retain.} +} +\if{html}{\out{
}} +} +} +\if{html}{\out{
}} \if{html}{\out{}} \if{latex}{\out{\hypertarget{method-setReturned}{}}} \subsection{Method \code{setReturned()}}{ +Set the value returned by the module call and proactively flush. \subsection{Usage}{ \if{html}{\out{
}}\preformatted{MockShinySession$setReturned(value)}\if{html}{\out{
}} } @@ -683,6 +704,27 @@ Create and return a namespace-specific session proxy. } \if{html}{\out{}} } +} +\if{html}{\out{
}} +\if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-getReturned}{}}} +\subsection{Method \code{getReturned()}}{ +Get the value returned by the module call. +\subsection{Usage}{ +\if{html}{\out{
}}\preformatted{MockShinySession$getReturned()}\if{html}{\out{
}} +} + +} +\if{html}{\out{
}} +\if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-genId}{}}} +\subsection{Method \code{genId()}}{ +Return a distinct character identifier for use as a proxy +namespace. +\subsection{Usage}{ +\if{html}{\out{
}}\preformatted{MockShinySession$genId()}\if{html}{\out{
}} +} + } \if{html}{\out{
}} \if{html}{\out{}} diff --git a/man/testServer.Rd b/man/testServer.Rd index 11f7534d1a..5973b07fee 100644 --- a/man/testServer.Rd +++ b/man/testServer.Rd @@ -19,7 +19,8 @@ along with any other values created inside of the server function.} \item{...}{Additional arguments to pass to the module function. These arguments are processed with \code{\link[rlang:list2]{rlang::list2()}} and so are -\emph{\link[rlang:dyn-dots]{dynamic}}.} +\emph{\link[rlang:dyn-dots]{dynamic}}. If \code{app} is a module, and no \code{id} argument is +provided, one will be generated and supplied automatically.} } \value{ The result of evaluating \code{expr}. @@ -41,8 +42,6 @@ server <- function(id, multiplier = 2, prefix = "I am ") { }) } -# Basic Usage -# ----------- testServer(server, { session$setInputs(x = 1) # You're also free to use third-party @@ -56,16 +55,4 @@ testServer(server, { stopifnot(output$txt == "I am 4") # Any additional arguments, below, are passed along to the module. }, multiplier = 2) - -# Advanced Usage -# -------------- -multiplier_arg_name = "multiplier" -more_args <- list(prefix = "I am ") -testServer(server, { - session$setInputs(x = 1) - stopifnot(myreactive() == 2) - stopifnot(output$txt == "I am 2") - # !!/:= and !!! from rlang are used below to splice computed arguments - # into the testModule() argument list. -}, !!multiplier_arg_name := 2, !!!more_args) } From bc2aa71888fe0ed6e6a7bdea14ddcb7a509a9662 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Wed, 8 Apr 2020 04:26:30 +0000 Subject: [PATCH 29/36] Use vapply in mapNames() --- R/mock-session.R | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/R/mock-session.R b/R/mock-session.R index ca7316e82d..2c464b3e31 100644 --- a/R/mock-session.R +++ b/R/mock-session.R @@ -72,10 +72,7 @@ extract <- function(promise) { #' @noRd mapNames <- function(func, ...) { vals <- list(...) - for (name in names(vals)) { - vals[[func(name)]] <- vals[[name]] - vals[[name]] <- NULL - } + names(vals) <- vapply(names(vals), func, character(1)) vals } From c0c02d290f6ad5ef468df8a437bea96a4810e7c2 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Wed, 8 Apr 2020 04:35:40 +0000 Subject: [PATCH 30/36] Remove unused variable --- R/mock-session.R | 1 - 1 file changed, 1 deletion(-) diff --git a/R/mock-session.R b/R/mock-session.R index 2c464b3e31..fdaa1140ba 100644 --- a/R/mock-session.R +++ b/R/mock-session.R @@ -393,7 +393,6 @@ MockShinySession <- R6Class( #' @param namespace Character vector indicating a namespace. makeScope = function(namespace) { ns <- NS(namespace) - returned <- NULL createSessionProxy( self, input = .createReactiveValues(private$.input, readonly = TRUE, ns = ns), From 2f8227e652aee341b85d006439a285da4c3a40f1 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Wed, 8 Apr 2020 04:36:00 +0000 Subject: [PATCH 31/36] Un-inline assignment --- R/mock-session.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/R/mock-session.R b/R/mock-session.R index fdaa1140ba..04c97f55ff 100644 --- a/R/mock-session.R +++ b/R/mock-session.R @@ -419,7 +419,8 @@ MockShinySession <- R6Class( #' @description Return a distinct character identifier for use as a proxy #' namespace. genId = function() { - paste0("proxy", (private$idCounter <- private$idCounter + 1)) + private$idCounter <- private$idCounter + 1 + paste0("proxy", private$idCounter) } ), private = list( From 214d721380d0aacb20ba578b380236faf74922a1 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Wed, 8 Apr 2020 04:41:51 +0000 Subject: [PATCH 32/36] Move session$env sanity check out of makeMask and into testServer --- R/test-server.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/R/test-server.R b/R/test-server.R index 4ddc59a72f..41f8dfcc78 100644 --- a/R/test-server.R +++ b/R/test-server.R @@ -4,7 +4,6 @@ #' @noRd makeMask <- function(env) { stopifnot(length(rlang::env_parents(env)) > 1) - stopifnot(all(c("input", "output", "session") %in% ls(env))) child <- as.list(env) parent <- as.list(rlang::env_parent(env)) parent_only <- setdiff(names(parent), names(child)) @@ -105,6 +104,8 @@ testServer <- function(app, expr, ...) { ) ) + stopifnot(all(c("input", "output", "session") %in% ls(session$env))) + quosure <- rlang::enquo(expr) isolate( From 7e2ffab62c59841b97d7d0d67d7633b26950add1 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Wed, 8 Apr 2020 04:44:25 +0000 Subject: [PATCH 33/36] Use base versions of a couple rlang::env_* functions --- tests/testthat/test-test-server-scope.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-test-server-scope.R b/tests/testthat/test-test-server-scope.R index 41fc2f5850..f5f504590a 100644 --- a/tests/testthat/test-test-server-scope.R +++ b/tests/testthat/test-test-server-scope.R @@ -12,7 +12,7 @@ test_that("Variables outside of the module are inaccessible", { z <- y+1 }) } - }, envir = rlang::new_environment(parent = rlang::global_env())) + }, envir = new.env(parent = globalenv())) testServer(server, { expect_equal(x, 0) @@ -29,7 +29,7 @@ test_that("Variables outside the testServer() have correct visibility", { y <- 1 }) } - }, envir = rlang::new_environment(parent = rlang::global_env())) + }, envir = new.env(parent = globalenv())) x <- 99 z <- 123 From fc6f535edd961a621930338d121bd4f6777e7eba Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Wed, 8 Apr 2020 04:54:32 +0000 Subject: [PATCH 34/36] Clarify testServer lexenv assertions --- tests/testthat/test-test-server-scope.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-test-server-scope.R b/tests/testthat/test-test-server-scope.R index f5f504590a..f841add67b 100644 --- a/tests/testthat/test-test-server-scope.R +++ b/tests/testthat/test-test-server-scope.R @@ -55,7 +55,8 @@ test_that("testServer allows lexical environment access through session$env", { testServer(server, { expect_equal(b_var, 321) - expect_equal(get("a_var", session$env), 123) + expect_equal(get("a_var", session$env, inherits = TRUE), 123) + expect_false(exists("a_var", inherits = FALSE)) }) }) From bac7299359b8daf41566813bbd113f1055c60914 Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Wed, 8 Apr 2020 04:57:14 +0000 Subject: [PATCH 35/36] Remove strings from expect_error --- tests/testthat/test-test-server.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-test-server.R b/tests/testthat/test-test-server.R index 3e854d7181..b6bf3746b3 100644 --- a/tests/testthat/test-test-server.R +++ b/tests/testthat/test-test-server.R @@ -68,8 +68,8 @@ test_that("inputs aren't directly assignable", { testServer(server, { session$setInputs(x = 0) - expect_error({ input$x <- 1 }, "Attempted to assign value to a read-only") - expect_error({ input$y <- 1 }, "Attempted to assign value to a read-only") + expect_error({ input$x <- 1 }) + expect_error({ input$y <- 1 }) }) }) From f4e3e5b61846f554a3a8c8c18599154eedc89c3c Mon Sep 17 00:00:00 2001 From: Alan Dipert Date: Wed, 8 Apr 2020 05:06:30 +0000 Subject: [PATCH 36/36] server => module --- tests/testthat/test-test-server-nesting.R | 20 ++-- tests/testthat/test-test-server-scope.R | 16 ++-- tests/testthat/test-test-server.R | 108 +++++++++++----------- 3 files changed, 72 insertions(+), 72 deletions(-) diff --git a/tests/testthat/test-test-server-nesting.R b/tests/testthat/test-test-server-nesting.R index 378a084739..c2383afc9a 100644 --- a/tests/testthat/test-test-server-nesting.R +++ b/tests/testthat/test-test-server-nesting.R @@ -24,40 +24,40 @@ test_that("Nested modules", { }) test_that("Lack of ID", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session) { output$txt <- renderText(session$ns("x")) }) } - testServer(server, { + testServer(module, { expect_equal(output$txt, "foo-x") }, id = "foo") }) test_that("testServer works with nested module servers", { - outerServer <- function(id) { + outerModule <- function(id) { moduleServer(id, function(input, output, session) { r1 <- reactive({ input$x + 1}) - r2 <- innerServer("inner", r1) + r2 <- innerModule("inner", r1) output$someVar <- renderText(r2()) }) } - innerServer <- function(id, r) { + innerModule <- function(id, r) { moduleServer(id, function(input, output, session) { reactive(paste("a value:", r())) }) } - testServer(outerServer, { + testServer(outerModule, { session$setInputs(x = 1) expect_equal(output$someVar, "a value: 2") }, id = "foo") }) test_that("testServer calls do not nest in module functions", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session) { x <- 1 testServer(function(id) { @@ -68,11 +68,11 @@ test_that("testServer calls do not nest in module functions", { }) } - expect_error(testServer(server, {}), regexp = "Modules may not call testServer()") + expect_error(testServer(module, {}), regexp = "Modules may not call testServer()") }) test_that("testServer calls do not nest in test exprs", { - server <- function(id) { + module <- function(id) { x <- 1 moduleServer(id, function(input, output, session) { inner <- function(id) { @@ -83,7 +83,7 @@ test_that("testServer calls do not nest in test exprs", { }) } - expect_error(testServer(server, { + expect_error(testServer(module, { testServer(inner, {}) }), regexp = "Test expressions may not call testServer()") }) diff --git a/tests/testthat/test-test-server-scope.R b/tests/testthat/test-test-server-scope.R index f841add67b..94c6c32f71 100644 --- a/tests/testthat/test-test-server-scope.R +++ b/tests/testthat/test-test-server-scope.R @@ -4,7 +4,7 @@ library(shiny) library(testthat) test_that("Variables outside of the module are inaccessible", { - server <- local({ + module <- local({ outside <- 123 function(id, x) { y <- x+1 @@ -14,7 +14,7 @@ test_that("Variables outside of the module are inaccessible", { } }, envir = new.env(parent = globalenv())) - testServer(server, { + testServer(module, { expect_equal(x, 0) expect_equal(y, 1) expect_equal(z, 2) @@ -23,7 +23,7 @@ test_that("Variables outside of the module are inaccessible", { }) test_that("Variables outside the testServer() have correct visibility", { - server <- local({ + module <- local({ function(id, x) { moduleServer(id, function(input, output, session) { y <- 1 @@ -34,7 +34,7 @@ test_that("Variables outside the testServer() have correct visibility", { x <- 99 z <- 123 - testServer(server, { + testServer(module, { expect_equal(x, 0) expect_equal(y, 1) expect_equal(z, 123) @@ -42,7 +42,7 @@ test_that("Variables outside the testServer() have correct visibility", { }) test_that("testServer allows lexical environment access through session$env", { - server <- local({ + module <- local({ a_var <- 123 function(id) { moduleServer(id, function(input, output, session) { @@ -53,7 +53,7 @@ test_that("testServer allows lexical environment access through session$env", { expect_false(exists("a_var", inherits = FALSE)) - testServer(server, { + testServer(module, { expect_equal(b_var, 321) expect_equal(get("a_var", session$env, inherits = TRUE), 123) expect_false(exists("a_var", inherits = FALSE)) @@ -64,7 +64,7 @@ test_that("Shadowing can be mitigated with unquote", { i <- 0 inc <- function() i <<- i+1 - server <- local({ + module <- local({ function(id) { moduleServer(id, function(input, output, session) { inc <- function() stop("I should never be called") @@ -72,7 +72,7 @@ test_that("Shadowing can be mitigated with unquote", { } }, envir = globalenv()) - testServer(server, { + testServer(module, { expect_is(inc, "function") expect_false(identical(inc, !!inc)) !!inc() diff --git a/tests/testthat/test-test-server.R b/tests/testthat/test-test-server.R index b6bf3746b3..23866c4898 100644 --- a/tests/testthat/test-test-server.R +++ b/tests/testthat/test-test-server.R @@ -5,17 +5,17 @@ library(testthat) library(future) test_that("testServer passes dots", { - server <- function(id, someArg) { + module <- function(id, someArg) { expect_false(missing(someArg)) moduleServer(id, function(input, output, session) { expect_equal(someArg, 123) }) } - testServer(server, {}, someArg = 123) + testServer(module, {}, someArg = 123) }) test_that("testServer passes dynamic dots", { - server <- function(id, someArg) { + module <- function(id, someArg) { expect_false(missing(someArg)) moduleServer(id, function(input, output, session) { expect_equal(someArg, 123) @@ -24,15 +24,15 @@ test_that("testServer passes dynamic dots", { # Test with !!! to splice in a whole named list constructed with base::list() moreArgs <- list(someArg = 123) - testServer(server, {}, !!!moreArgs) + testServer(module, {}, !!!moreArgs) # Test with !!/:= to splice in an argument name argName <- "someArg" - testServer(server, {}, !!argName := 123) + testServer(module, {}, !!argName := 123) }) test_that("testServer handles observers", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session) { rv <- reactiveValues(x = 0, y = 0) observe({ @@ -47,7 +47,7 @@ test_that("testServer handles observers", { }) } - testServer(server, { + testServer(module, { session$setInputs(x=1) expect_equal(rv$y, 2) expect_equal(rv$x, 2) @@ -61,12 +61,12 @@ test_that("testServer handles observers", { }) test_that("inputs aren't directly assignable", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session) { }) } - testServer(server, { + testServer(module, { session$setInputs(x = 0) expect_error({ input$x <- 1 }) expect_error({ input$y <- 1 }) @@ -74,7 +74,7 @@ test_that("inputs aren't directly assignable", { }) test_that("testServer handles more complex expressions", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session){ output$txt <- renderText({ input$x @@ -82,7 +82,7 @@ test_that("testServer handles more complex expressions", { }) } - testServer(server, { + testServer(module, { for (i in 1:5){ session$setInputs(x=i) expect_equal(output$txt, as.character(i)) @@ -97,7 +97,7 @@ test_that("testServer handles more complex expressions", { }) test_that("testServer handles reactiveVal", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session) { x <- reactiveVal(0) observe({ @@ -106,7 +106,7 @@ test_that("testServer handles reactiveVal", { }) } - testServer(server, { + testServer(module, { session$setInputs(y=1, z=2) expect_equal(x(), 3) @@ -120,7 +120,7 @@ test_that("testServer handles reactiveVal", { }) test_that("testServer handles reactives with complex dependency tree", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session) { x <- reactiveValues(x=1) r <- reactive({ @@ -132,7 +132,7 @@ test_that("testServer handles reactives with complex dependency tree", { }) } - testServer(server, { + testServer(module, { session$setInputs(a=1, b=2, c=3) expect_equal(r(), 4) expect_equal(r2(), 7) @@ -152,7 +152,7 @@ test_that("testServer handles reactives with complex dependency tree", { }) test_that("testServer handles reactivePoll", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session) { rv <- reactiveValues(x = 0) rp <- reactivePoll(50, session, function(){ rnorm(1) }, function(){ @@ -164,7 +164,7 @@ test_that("testServer handles reactivePoll", { }) } - testServer(server, { + testServer(module, { expect_equal(rv$x, 1) for (i in 1:4){ @@ -176,7 +176,7 @@ test_that("testServer handles reactivePoll", { }) test_that("testServer handles reactiveTimer", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session) { rv <- reactiveValues(x = 0) @@ -188,7 +188,7 @@ test_that("testServer handles reactiveTimer", { }) } - testServer(server, { + testServer(module, { expect_equal(rv$x, 1) session$elapse(200) @@ -198,7 +198,7 @@ test_that("testServer handles reactiveTimer", { }) test_that("testServer handles debounce/throttle", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session) { rv <- reactiveValues(t = 0, d = 0) react <- reactive({ @@ -219,7 +219,7 @@ test_that("testServer handles debounce/throttle", { }) } - testServer(server, { + testServer(module, { session$setInputs(y = TRUE) expect_equal(rv$d, 1) for (i in 2:5){ @@ -243,7 +243,7 @@ test_that("testServer wraps output in an observer", { testthat::skip("I'm not sure of a great way to test this without timers.") # And honestly it's so foundational in what we're doing now that it might not be necessary to test? - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session) { rv <- reactiveValues(x=0) rp <- reactiveTimer(50) @@ -254,7 +254,7 @@ test_that("testServer wraps output in an observer", { }) } - testServer(server, { + testServer(module, { session$setInputs(x=1) # Timers only tick if they're being observed. If the output weren't being # wrapped in an observer, we'd see the value of rv$x initialize to zero and @@ -281,7 +281,7 @@ test_that("testServer wraps output in an observer", { }) test_that("testServer works with async", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session) { output$txt <- renderText({ val <- input$x @@ -299,7 +299,7 @@ test_that("testServer works with async", { }) } - testServer(server, { + testServer(module, { session$setInputs(x=1) expect_equal(output$txt, "1") expect_equal(output$sync, "abc") @@ -317,7 +317,7 @@ test_that("testServer works with async", { }) test_that("testModule works with multiple promises in parallel", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session) { output$txt1 <- renderText({ future({ @@ -335,7 +335,7 @@ test_that("testModule works with multiple promises in parallel", { }) } - testServer(server, { + testServer(module, { # As we enter this test code, the promises will still be running in the background. # We'll need to give them ~2s (plus overhead) to complete startMS <- as.numeric(Sys.time()) * 1000 @@ -355,7 +355,7 @@ test_that("testModule works with multiple promises in parallel", { }) test_that("testModule handles async errors", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session, arg1, arg2){ output$err <- renderText({ future({ "my error"}) %...>% @@ -370,7 +370,7 @@ test_that("testModule handles async errors", { }) } - testServer(server, { + testServer(module, { expect_error(output$err, "my error") # TODO: helper for safe errors so users don't have to learn "shiny.custom.error"? expect_error(output$safe, "my safe error", class="shiny.custom.error") @@ -378,7 +378,7 @@ test_that("testModule handles async errors", { }) test_that("testServer handles modules with additional arguments", { - server <- function(id, arg1, arg2) { + module <- function(id, arg1, arg2) { moduleServer(id, function(input, output, session){ output$txt1 <- renderText({ arg1 @@ -394,7 +394,7 @@ test_that("testServer handles modules with additional arguments", { }) } - testServer(server, { + testServer(module, { expect_equal(output$txt1, "val1") expect_equal(output$txt2, "val2") }, arg1="val1", arg2="val2") @@ -410,7 +410,7 @@ test_that("testServer captures htmlwidgets", { testthat::skip("jsonlite not available to test htmlwidgets") } - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session){ output$dy <- dygraphs::renderDygraph({ dygraphs::dygraph(data.frame(outcome=0:5, year=2000:2005)) @@ -418,7 +418,7 @@ test_that("testServer captures htmlwidgets", { }) } - testServer(server, { + testServer(module, { # Really, this test should be specific to each htmlwidget. Here, we don't want to bind ourselves # to the current JSON structure of dygraphs, so we'll just check one element to see that the raw # JSON was exposed and is accessible in tests. @@ -429,7 +429,7 @@ test_that("testServer captures htmlwidgets", { }) test_that("testServer captures renderUI", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session){ output$ui <- renderUI({ tags$a(href="https://rstudio.com", "hello!") @@ -437,14 +437,14 @@ test_that("testServer captures renderUI", { }) } - testServer(server, { + testServer(module, { expect_equal(output$ui$deps, list()) expect_equal(as.character(output$ui$html), "hello!") }) }) test_that("testServer captures base graphics outputs", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session){ output$fixed <- renderPlot({ plot(1,1) @@ -456,7 +456,7 @@ test_that("testServer captures base graphics outputs", { }) } - testServer(server, { + testServer(module, { # We aren't yet able to create reproducible graphics, so this test is intentionally pretty # limited. expect_equal(output$fixed$width, 300) @@ -478,7 +478,7 @@ test_that("testServer captures ggplot2 outputs", { testthat::skip("ggplot2 not available") } - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session){ output$fixed <- renderPlot({ ggplot2::qplot(iris$Sepal.Length, iris$Sepal.Width) @@ -490,7 +490,7 @@ test_that("testServer captures ggplot2 outputs", { }) } - testServer(server, { + testServer(module, { expect_equal(output$fixed$width, 300) expect_equal(output$fixed$height, 350) expect_match(output$fixed$src, "^data:image/png;base64,") @@ -503,7 +503,7 @@ test_that("testServer captures ggplot2 outputs", { }) test_that("testServer exposes the returned value from the module", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session){ reactive({ return(input$a + input$b) @@ -511,7 +511,7 @@ test_that("testServer exposes the returned value from the module", { }) } - testServer(server, { + testServer(module, { session$setInputs(a=1, b=2) expect_equal(session$getReturned()(), 3) @@ -522,7 +522,7 @@ test_that("testServer exposes the returned value from the module", { }) test_that("testServer handles synchronous errors", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session, arg1, arg2){ output$err <- renderText({ stop("my error") @@ -534,7 +534,7 @@ test_that("testServer handles synchronous errors", { }) } - testServer(server, { + testServer(module, { expect_error(output$err, "my error") # TODO: helper for safe errors so users don't have to learn "shiny.custom.error"? expect_error(output$safe, "my safe error", class="shiny.custom.error") @@ -542,15 +542,15 @@ test_that("testServer handles synchronous errors", { }) test_that("accessing a non-existent output gives an informative message", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session){}) } - testServer(server, { + testServer(module, { expect_error(output$dontexist, "hasn't been defined yet: output\\$server1-dontexist") }, id = "server1") - testServer(server, { + testServer(module, { expect_error(output$dontexist, "hasn't been defined yet: output\\$.*-dontexist") }) }) @@ -568,18 +568,18 @@ test_that("testServer returns a meaningful result", { }) test_that("assigning an output in a module function with a non-function errors", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session) { output$someVar <- 123 }) } - expect_error(testServer(server, {}), "^Unexpected") + expect_error(testServer(module, {}), "^Unexpected") }) test_that("testServer handles invalidateLater", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session) { rv <- reactiveValues(x = 0) observe({ @@ -592,7 +592,7 @@ test_that("testServer handles invalidateLater", { }) } - testServer(server, { + testServer(module, { # Should have run once expect_equal(rv$x, 1) @@ -606,11 +606,11 @@ test_that("testServer handles invalidateLater", { }) test_that("session ended handlers work", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session){}) } - testServer(server, { + testServer(module, { rv <- reactiveValues(closed = FALSE) session$onEnded(function(){ rv$closed <- TRUE @@ -629,7 +629,7 @@ test_that("session ended handlers work", { }) test_that("session flush handlers work", { - server <- function(id) { + module <- function(id) { moduleServer(id, function(input, output, session) { rv <- reactiveValues(x = 0, flushCounter = 0, flushedCounter = 0, flushOnceCounter = 0, flushedOnceCounter = 0) @@ -645,7 +645,7 @@ test_that("session flush handlers work", { }) } - testServer(server, { + testServer(module, { session$setInputs(x=1) expect_equal(rv$x, 2) # We're not concerned with the exact values here -- only that they increase