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

Missed coverage of function in a list. #556

Open
dmurdoch opened this issue Dec 24, 2023 · 1 comment
Open

Missed coverage of function in a list. #556

dmurdoch opened this issue Dec 24, 2023 · 1 comment

Comments

@dmurdoch
Copy link

dmurdoch commented Dec 24, 2023

In my recent PR igraph/rigraph#1021 to igraph, the coverage tests said that the testing didn't include the changed code, even though it did.

I put together a simple package with the same structure of running functions that have been assigned into a list, and covr::package_coverage(type="examples") also claimed there was no coverage.

The example package had this code:

test1 <- function() {
x <- 1 +
     2 +
     3 +
     4
}

test <- list(test1)

and it exported test. The example for the help page for test ran this:

test[[1]]()

A version of the package that simply assigned test <- test1 and ran test() showed 100% coverage.

The full package is available here: https://github.com/dmurdoch/testpkg/tree/test_coverage .

Here's my session info:

R version 4.3.1 (2023-06-16)
Platform: x86_64-apple-darwin20 (64-bit)
Running under: macOS Monterey 12.7.2

Matrix products: default
BLAS:   /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib 
LAPACK: /Library/Frameworks/R.framework/Versions/4.3-x86_64/Resources/lib/libRlapack.dylib;  LAPACK version 3.11.0

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

time zone: America/Toronto
tzcode source: internal

attached base packages:
[1] stats     graphics  grDevices utils     datasets 
[6] methods   base     

other attached packages:
[1] covr_3.6.4    testpkg_0.1.1

loaded via a namespace (and not attached):
[1] compiler_4.3.1 lazyeval_0.2.2 rex_1.2.1     
[4] tools_4.3.1 
@dmurdoch
Copy link
Author

I can see two kinds of solutions to this.

  1. Put the onus on the package writer: Allow a hook that (re)populates any lists or other containers after covr has installed its tracing code, and again when the code is removed.
  2. Modify covr to recursively handle lists.

2 is relatively straightforward, by adding a new type of record into the$replacements. The new type handles lists, not closures. A simple structure for it saves the environment where the list was found and the name of the list, and then has a list of replacements to make within that list, indexed by number rather than name since lists aren't always named.

A disadvantage of this is that lists aren't the only places where functions can be hidden -- packages could have environment objects, functions could be assigned as attributes of other objects, etc. Handling attributes wouldn't be hard if lists were handled, but handling environments recursively would have to watch out for infinite loops. Still, it's probably doable.

A disadvantage of 1 is that packages might need to be reorganized to do it, and it would be hard for the package writers to maintain.

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

No branches or pull requests

1 participant