covr icon indicating copy to clipboard operation
covr copied to clipboard

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

Open kamilzyla opened this issue 3 years ago • 4 comments

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.

kamilzyla avatar Sep 27 '21 09:09 kamilzyla

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 avatar Sep 27 '21 15:09 jimhester

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 :slightly_smiling_face:

kamilzyla avatar Sep 28 '21 06:09 kamilzyla

@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.

telegott avatar Mar 05 '22 22:03 telegott

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.

kamilzyla avatar Mar 22 '22 08:03 kamilzyla

@telegott There is a pending PR for recursive subdirectories in testthat. https://github.com/r-lib/testthat/pull/1850

radbasa avatar Feb 10 '24 11:02 radbasa

@kamilzyla The PR for this has been merged to main

radbasa avatar Feb 10 '24 11:02 radbasa