bluefoxr / COINr

COINr
https://bluefoxr.github.io/COINr/
Other
22 stars 7 forks source link

Improve coverage of unit tests #7

Closed bauer-alex closed 1 year ago

bauer-alex commented 1 year ago

Issue based on the JOSS revision process (see the corresponding issue).

Based on devtools::test_coverage(), the unit tests in the package currently only cover 25.80% of the relevant code. This number needs to be higher to accept the paper. JOSS doesn't state a specific required test coverage and of course reaching 100% is usually not realistic (as of a lot of specific subfunctionalities when making use of individual function arguments), but currently a lot of functions have no corresponding unit tests at all. Please include unit tests for all functions of the package.

bluefoxr commented 1 year ago

Thanks, it's a fair point and I will improve this but this will take a while due to the number of functions. I'll put some updates here as I go along.

bluefoxr commented 1 year ago

Hi @bauer-alex I have added a lot more unit tests and the coverage is now up over 82%. Could you have a look and see if this is sufficient for now, or if not please point to other tests to perform. One untested function you may notice is import_coin_tool() which I have left out because it would involve downloading a fairly heavy spreadsheet from the web and I worry about this causing problems with CRAN. Still, if you think this should go ahead please let me know as well. Thanks.

bauer-alex commented 1 year ago

Seems good to me now. :+1: Did you deliberately not test COIN_to_coin? Seems like the only function left for which unit tests would be reasonable.

bluefoxr commented 1 year ago

It is actually tested but requires another package to be downloaded, perhaps it is not picked up if you don't have that installed. See https://github.com/bluefoxr/COINr/blob/master/tests/testthat/test-convert.R

bauer-alex commented 1 year ago

Indeed. I see. Alright, then I'm happy to close the issue.

bluefoxr commented 1 year ago

Thanks and by the way improving the test coverage helped me uncover a few small bugs, so it has helped to improve the package considerably.