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

Coverage with modules (e.g. klmr/box) #491

Closed
kamilzyla opened this issue Sep 27, 2021 · 6 comments · Fixed by #526
Closed

Coverage with modules (e.g. klmr/box) #491

kamilzyla opened this issue Sep 27, 2021 · 6 comments · Fixed by #526
Labels
help wanted ❤️ we'd love your help!

Comments

@kamilzyla
Copy link

Background

We use klmr/box (previously we used wahani/modules) to structure the code of our Shiny apps. These packages bring a module/import system similar to what you can find in languages like Python, JavaScript (ES6+) or Java.

Each module has its own namespace and multiple modules can export objects with identical names. For example, we can have modules named main.R and sidebar.R both of which export ui and server objects. When using them in some other module, they can be referred to e.g. as main$server and sidebar$server. This also works in our testthat unit tests - the test file itself imports the functions it needs to test.

Unfortunately the covr package is not prepared for this kind of usage. The file_coverage() function just sources all files passed in the source_files argument, so any duplicate names in the modules clash and overwrite each other. The generated coverage report has files missing.

Goal

We'd love to use covr to generate coverage report for our Shiny apps, but we find modules very important for structuring large apps, and these two tools are currently incompatible. I'd like to start a discussion - how can we change that?

I'm happy to provide more details, e.g. prepare a demo project.

@jimhester
Copy link
Member

I think you are generally better off using the namespace of a proper R package to handle imports and exports of R objects, even in shiny apps.

However if you or other community members would be willing to work on this feature and it didn't require too much additional code to support we would certainly consider it.

@jimhester jimhester added the help wanted ❤️ we'd love your help! label Sep 27, 2021
@kamilzyla
Copy link
Author

Thanks! We had many discussions at Appsilon about Shiny apps as R packages; we currently use box modules, as a single namespace provided by a package makes it quite hard to build a large app.

I do hope to find time to work on this feature - coverage reports for our apps is something we'd really like to have 🙂

@telegott
Copy link

telegott commented Mar 5, 2022

@kamilzyla Very interesting to hear that box is used in a professional context too! Did you find a workaround to lift the no-subdirectory restriction of testthat, e.g. having modules like R/context/module.R and an equivalent tests/testthat/context/test-module.R? Out of the box there's no recursive option in testthat to find test files as far as I know.

@kamilzyla
Copy link
Author

kamilzyla commented Mar 22, 2022

Hi @telegott! We did not find a way to use directories for structuring unit tests other than running test_dir() multiple times (but then you get multiple reports). It wouldn't be technically challenging, but we'd need to patch testthat.

@radbasa
Copy link
Contributor

radbasa commented Feb 10, 2024

@telegott There is a pending PR for recursive subdirectories in testthat. r-lib/testthat#1850

@radbasa
Copy link
Contributor

radbasa commented Feb 10, 2024

@kamilzyla The PR for this has been merged to main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted ❤️ we'd love your help!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants