Run shinytest2 app tests on CI; fix duplicate input IDs and test warn…#30
Conversation
amcim
left a comment
There was a problem hiding this comment.
Excellent work. The CI is working and we have plenty of evidence now that the dashboard is functional across platforms.
Here is the quote from the Bioconductor handbook "We highly recommend that reviewers use the shinytest2 package to test the visual and computational aspects of the Shiny app in addition to standard unit tests for plain functions."
Currently the shinytest2 tests implemented check that the dashboard launches and that the tabs are rendered. This in combination with the unit tests that tests internal logic, I think we fulfill this requirement and things are good for submission on this front.
Now that this is functional we can add more tests to check that plots render correctly and more through tests on this front. This is not high priority but should be straightforward if we desire them or if a reviewer requests them.
One note I am making is that I wasn't sure if skip_on_cran was correct in our case because we aren't submitting to cran, but the name is misleading and it is the correct guard that we want.
Approved for merge
Summary
Follow-up to PR #16. Gets the shinytest2 app tests running on CI across all platforms, and clears the warnings those tests were showing.
Changes
Run app tests on CI
app-tests.yamlworkflow runs the app tests on Ubuntu/macOS/Windows withdevtools::test()— separately fromR CMD check, so the browser's temp files don't cause the "detritus in the temp directory" warning. The normalR-CMD-checkstill skips them.withr) that gets deleted when the tests finish, so they don't pile up in a reviewer's session.Fix the app-test warnings
data_typeID (already used by the Model performance tab) and defined the bug selectors twice. Renamed the feature tab's input tofeature_data_typeand define each bug selector once.Result
Notes
R-CMD-checkstays clean.