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

Note: Unit test coverage amount #71

Open
lshep opened this issue May 8, 2020 · 8 comments
Open

Note: Unit test coverage amount #71

lshep opened this issue May 8, 2020 · 8 comments
Labels
Advanced Advanced level Hackathon project

Comments

@lshep
Copy link
Contributor

lshep commented May 8, 2020

Figure out unit test coverage amount and note if code is not sufficiently tested?

@lshep lshep added Advanced Advanced level Hackathon project Hackathon labels May 8, 2020
@lcolladotor
Copy link
Contributor

lcolladotor commented May 15, 2020

I can see this being useful and I'm happy that it's a NOTE, not more strict.

This gets really hard when you have packages with shiny apps (though there might be tools out there that I'm not aware of).

Also, everyone starts at 0% coverage. I guess that people developing a package could ignore this.

Btw, my package https://github.com/LieberInstitute/spatialLIBD has 0% coverage at the moment :P (in case those writing the code for this new note want to use it for testing their code)

@federicomarini
Copy link
Contributor

Could this be done with a direct call to the covr framework?
The problem is that it would (slightly) increase the runtime for the more mature packages - can this be optionally triggered with a boolean?

@mtmorgan
Copy link
Collaborator

I think there are two challenges with this issue

The first is that 'slightly increase' the run time is an understatement -- the entire package has to be checked with covr.

A second challenge is that the results of covr need to be effectively conveyed to the user, in part because of challenges the user has in emulating the build system (even in 'simple' ways, like using the correct version of R and Bioconductor).

@lazappi
Copy link

lazappi commented May 18, 2020

I was curious about the overhead for {covr} so here are the timings for running it on my package ({splatter}). Summary is that covr::package_coverage() takes about as long as running the tests.

> system.time(devtools::test())
   user  system elapsed 
 71.050   7.273  78.642 

> system.time(covr::package_coverage())
   user  system elapsed 
 97.117   9.077 106.933

> system.time(devtools::check())
   user  system elapsed 
336.414  24.696 365.059

I don't know if there is some other minimal way to estimate test coverage? Perhaps a compromise would be to add a message suggesting {covr} to the testing check section?

@federicomarini
Copy link
Contributor

Well covr kinda has to do at least test the whole package, so this is not entirely surprising.
I have no clue whether there are some "shallower" methods to estimate coverage - at least, not without at least testing...

@lazappi
Copy link

lazappi commented May 19, 2020

Yeah it's not surprising when you think about it but for some reason I had it in my head that it magically worked without running the tests 🎩.

I found this script which uses regex to find function calls in tests https://gist.github.com/cannin/819e73426b4ebd5752d5. Not as detailed as {covr} but might be enough to raise a note and suggest using {covr} to check in more detail?

@grimbough
Copy link
Contributor

grimbough commented May 19, 2020

I haven't given this much thought, but is there potential to turn off the 'standard' running of the tests, and rely on the run through with covr? Then it's not a doubling of the test runtime.

@lazappi
Copy link

lazappi commented May 19, 2020

I think the problem with that might be that if a test fails you just get an error from {covr} and you don't get any of the normal output about the success or not of individual tests.

@lshep lshep removed the Hackathon label Jun 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Advanced Advanced level Hackathon project
Projects
None yet
Development

No branches or pull requests

6 participants