-
-
Notifications
You must be signed in to change notification settings - Fork 1
503 shinytest2 : migrate test to shinytest2 using TealAppDriver #375
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Code Coverage Summary
Diff against main
Results for commit: fb6f8e3 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Unit Tests Summary 1 files 16 suites 11s ⏱️ Results for commit fb6f8e3. |
Unit Test Performance Difference
Additional test case details
Results for commit 0a542d1 ♻️ This comment has been updated with latest results. |
Hi @kartikeyakirar , what is the state of this PR? |
Hey @danielinteractive, Just wanted to update you on a couple of PRs: https://github.com/insightsengineering/teal.modules.helios/pull/130 we're prioritized the addition of shinytest2 in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR migrates module tests from shinytest
to shinytest2
by removing old test scripts and adding new end-to-end tests powered by TealAppDriver
.
- Removed obsolete app scripts and legacy
test-*.R
files that used the old framework - Deleted corresponding snapshots and helper functions for
shinytest
- Introduced
test-shinytest2-*.R
files covering all modules (volcanoplot, scatterplot, barplot, boxplot, pca, quality, km, forest, geneSpec, sampleVarSpec, assaySpec, adtteSpec, experimentSpec)
Reviewed Changes
Copilot reviewed 48 out of 68 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
tests/testthat/volcanoplot/app.R | Removed old Shiny app script for the volcano plot example |
tests/testthat/test-volcanoplot.R | Deleted legacy unit and integration tests for volcanoplot |
tests/testthat/test-shinytest2-volcanoplot.R | Added shinytest2-based e2e tests for the volcanoplot module |
tests/testthat/test-shinytest2-scatterplot.R | Added shinytest2-based e2e tests for the scatterplot module |
tests/testthat/test-shinytest2-barplot.R | Added shinytest2-based e2e tests for the barplot module |
Comments suppressed due to low confidence (3)
tests/testthat/test-shinytest2-barplot.R:56
- The input ID "genes" doesn't match the later use of "x-genes". Update this to
get_active_module_input("x-genes")
for consistency.
res <- app$get_active_module_input("genes")
tests/testthat/test-shinytest2-boxplot.R:12
- The
.test
argument is referring to an undefined variable. It should be set explicitly (e.g.,.test = TRUE
).
.test = .test
tests/testthat/test-shinytest2-barplot.R:13
- The
.test
parameter is undefined here. Use.test = TRUE
(or another literal) to enable test mode.
.test = .test
test_that("e2e: tm_g_scatterplot creates expected HTML", { | ||
data <- teal.data::teal_data(MAE = hermes::multi_assay_experiment) | ||
app <- teal:::TealAppDriver$new( | ||
data = teal_data(MAE = helios::large_helios_data), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The data
argument in TealAppDriver$new()
is using a new teal_data()
call instead of the data
variable defined on the previous line. Replace with data = data
to use the intended dataset.
data = teal_data(MAE = helios::large_helios_data), | |
data = data, |
Copilot uses AI. Check for mistakes.
part of https://github.com/insightsengineering/coredev-tasks/issues/503