Skip to content

Run shinytest2 app tests on CI; fix duplicate input IDs and test warn…#30

Merged
eboyer221 merged 3 commits into
mainfrom
dev-app-tests-ci
Jun 4, 2026
Merged

Run shinytest2 app tests on CI; fix duplicate input IDs and test warn…#30
eboyer221 merged 3 commits into
mainfrom
dev-app-tests-ci

Conversation

@eboyer221

Copy link
Copy Markdown
Contributor

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

  • New app-tests.yaml workflow runs the app tests on Ubuntu/macOS/Windows with devtools::test() — separately from R CMD check, so the browser's temp files don't cause the "detritus in the temp directory" warning. The normal R-CMD-check still skips them.
  • The app tests now put Chrome's temp files in their own folder (using withr) that gets deleted when the tests finish, so they don't pile up in a reviewer's session.

Fix the app-test warnings

  • Duplicate input IDs: the Bug/Drug feature comparison tab was reusing the data_type ID (already used by the Model performance tab) and defined the bug selectors twice. Renamed the feature tab's input to feature_data_type and define each bug selector once.

Result

  • App tests: 20 pass, 0 warnings.
  • Full test suite: 168 pass, 0 fail.

Notes

  • The app tests only run in the new workflow (and locally), R-CMD-check stays clean.
  • Keep an eye on the Windows result: deleting Chrome's temp folder can fail there if the browser hasn't fully closed.

@amcim amcim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@eboyer221 eboyer221 merged commit 62405e1 into main Jun 4, 2026
7 checks passed
@eboyer221 eboyer221 deleted the dev-app-tests-ci branch June 4, 2026 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants