moduleServer testing #2783
moduleServer testing #2783
Conversation
R/modules.R
Outdated
| @@ -121,23 +121,40 @@ createSessionProxy <- function(parentSession, ...) { | |||
| #' | |||
| #' @export | |||
| moduleServer <- function(id, module, session = getDefaultReactiveDomain()) { | |||
| if (inherits(session, c("MockShinySession"))) session$isModuleServer <- TRUE | |||
There was a problem hiding this comment.
Instead of adding session$isModuleServer here and checking it in callModule, I think it would be clearer if the modifications to module were done in this function. You could skip the call to callModule in the case where session is a MockShinySession.
I also still feel a weird about embedding this test-related code into the moduleServer or callModule functions, but I don't see a better way to do it at the moment.
R/modules.R
Outdated
| # performed by .testModule() if the module is *not* called through | ||
| # moduleServer() but is under test. See .testModule() for details. | ||
| body(module) <- rlang::expr({ | ||
| base::assign("env", base::environment(), envir = !!session) |
There was a problem hiding this comment.
Can this line just be the same as the one down below? The other one is a more idiomatic way of doing it.
| @@ -64,33 +64,55 @@ testModule <- function(module, expr, ...) { | |||
| ) | |||
| } | |||
|
|
|||
| isOldModule <- function(func) { | |||
There was a problem hiding this comment.
It would be helpful to have a comment explaining the purpose of this function (and how it works).
R/modules.R
Outdated
| base::assign("env", base::environment(), envir = !!session) | ||
| !!!body(module) | ||
| }) | ||
| isolate( |
There was a problem hiding this comment.
Is this isolate(withReactiveDomain(..., with_options stuff necessary? That stuff is also present in test-module.R, line 106.
| stopifnot(is.function(func)) | ||
| required <- c("input", "output", "session") | ||
| declared <- names(formals(func)) | ||
| setequal(required, intersect(required, declared)) |
There was a problem hiding this comment.
A bit more idiomatic to do something like:
all(required %in% declared)
|
Continued by #2807 |
No description provided.