Skip to content
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

testthat/integration tests for 5 apps #2

Merged
merged 6 commits into from
Jan 22, 2020
Merged

testthat/integration tests for 5 apps #2

merged 6 commits into from
Jan 22, 2020

Conversation

alandipert
Copy link
Contributor

@alandipert alandipert commented Jan 18, 2020

This adds testthat-based unit/integration tests to 5 applications that would be exercised by the testthat action in rstudio/shinycoreci#8

The most important lesson I learned was probably that in order for modules to be available in the test environment, it's important to provide the env argument to testthat::test_file(). Otherwise, testthat loads test files in a clean environment without the definitions added to the environment by shiny::runTests().

It also occurred to me to add a first-class way to "click" an actionButton(); I created rstudio/shiny#2745 to track this.

I ran across what I think is a bug in shiny::runTests(): rstudio/shiny#2746

Overall, I had some difficulty adding what I would consider to be "helpful" tests.

# TODO Determine the ideal way for module-reliant test code to load an app
with_dir(shiny:::findApp(), {
local({
source("app.R")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this still needed given you are passing in env?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You know, I think it actually is, because of this bug that I just filed: rstudio/shiny#2746

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing that out though, I had forgotten why it was necessary.

@alandipert alandipert merged commit 3d8a298 into master Jan 22, 2020
@alandipert alandipert deleted the test-5-apps branch January 22, 2020 21:24
@alandipert alandipert removed their assignment May 29, 2020
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