tests(stat-model): Add unit tests to stat model server#118
tests(stat-model): Add unit tests to stat model server#118tonywu1999 merged 12 commits intodevelfrom
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (1)
Tip CodeRabbit can generate a title for your PR based on the changes with custom instructions.Add the WalkthroughThe changes migrate the statmodelServer Shiny module from a direct function pattern to the moduleServer architecture with an updated function signature (id-based scoping), standardize assignment operators in the server file, and introduce comprehensive test coverage for the statmodelServer module with mock data and multiple configuration scenarios. Changes
Sequence DiagramsequenceDiagram
participant Caller as Caller/Parent Module
participant Module as statmodelServer Module
participant Session as Shiny Session
rect rgb(240, 248, 255)
note over Caller,Module: Old Pattern: Direct Function Call
Caller->>Module: statmodelServer(input, output, session, ...)
Module->>Module: Execute module logic<br/>(direct closure)
Module-->>Caller: Return reactive outputs
end
rect rgb(255, 240, 245)
note over Caller,Module: New Pattern: moduleServer Architecture
Caller->>Module: statmodelServer(id, parent_session, ...)
Module->>Session: moduleServer(id, function(input,<br/>output, session) { ... })
Session->>Session: Create namespaced session<br/>(ns <- session$ns)
Module->>Module: Execute module logic<br/>(within module scope)
Module-->>Session: Reactive outputs<br/>(namespaced)
Session-->>Caller: Return reactive outputs<br/>(properly scoped)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring extra attention:
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Failed to generate code suggestions for PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
R/server.R (1)
13-15: Consider R community conventions for assignment operators.The changes replace
<-with=for assignments. While both are valid in R, the<-operator is strongly preferred by the R community (tidyverse style guide, Google R style guide) for variable assignments. Using=is typically reserved for function arguments.Consider maintaining consistency with R community conventions:
-server = function(input, output, session) { - isWebServer = Sys.getenv("SHINY_ENV", "development") == "production" - maxRequestSize = if (isWebServer) 250*1024^2 else 1000000*1024^2 +server <- function(input, output, session) { + isWebServer <- Sys.getenv("SHINY_ENV", "development") == "production" + maxRequestSize <- if (isWebServer) 250*1024^2 else 1000000*1024^2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
R/module-statmodel-server.R(1 hunks)R/server.R(2 hunks)tests/testthat/test-module-statmodel-server.R(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
tests/testthat/test-module-statmodel-server.R (2)
39-632: Excellent test coverage for the statmodelServer module.The test suite provides comprehensive coverage of the core module functionality:
- Initialization and reactive setup
- Group choices extraction across DDA/TMT/PTM configurations
- All matrix building scenarios (custom, all_pair, all_one, custom_np)
- Validation logic (identical group checks, zero-sum weight constraints)
- State management and reset behavior
- Edge cases (duplicates, empty comparisons)
The tests properly use
testServerwithMockShinySessionand are well-isolated.
30-36: Remove the unusedcreate_mock_raw_data()helper function.This function is defined at lines 30-36 but is never called anywhere in the test suite or codebase. It should be removed to keep the test file clean.
R/server.R (1)
45-53: Correct migration to direct moduleServer invocation.The change from
callModule(statmodelServer, ...)to direct invocationstatmodelServer(id = "statmodel", ...)is correct for modules that now usemoduleServer()internally. The id-based scoping aligns with the refactored module implementation.R/module-statmodel-server.R (3)
21-24: Correct migration to the moduleServer architecture.The function signature change and
moduleServer()wrapper follow the proper Shiny module pattern. Theidparameter enables namespace scoping, and the inner function receivesinput,output, andsessionfrom the module context.
57-113: Proper namespace usage for module UI elements.The consistent pattern of
ns = session$nsfollowed by wrapping IDs withns()ensures proper namespace scoping for the module. This prevents ID conflicts and enables multiple instantiations of the module.
912-920: Return value structure correctly preserved.The module returns a list with
inputanddataComparisonas expected by the calling code inserver.R. The structure is preserved from the original implementation.
1 participant
Note: When looking at diff on the Github PR, hide the whitespace.
Summary by CodeRabbit
Tests
Chores
Note: No user-facing behavior changes.