search

ejikeugba / serp

Smooth Effects on Response Penalty for CLM
https://ejikeugba.github.io/serp
1 stars 0 forks source link

Use regression tests #9

Closed Bisaloo closed 2 years ago

Bisaloo commented 3 years ago

I see your package has a good code coverage but upon closer inspection of what is tested, most of the tests related to errors. It would be good to have some regression tests, where you compare the numerical output of some functions to what is known to be correct (e.g., from earlier versions). This prevents the introduction of a bug that would return incorrect values in the future.

Snapshot testing in testthat provides a convenient way to address this issue: https://testthat.r-lib.org/reference/expect_snapshot.html

This issue is part of https://github.com/openjournals/joss-reviews/issues/3705.

ejikeugba commented 3 years ago

Thanks for mentioning this point. I actually didn't use snapshot tests at this early stage for being a bit fragile and sensitive to little changes. However, I hope to give it a try in the nearest future, as the package gain more ground stability-wise.

Meanwhile, the unpenalized-model test in the testthat files, compare regression outputs from serp with those from VGAM-vglm function under similar conditions.

Bisaloo commented 3 years ago

Okay, thanks for your answer. I think still it's valuable to have snapshot tests since the package is already on CRAN. But at this stage, this is a simple recommendation, and not a blocker for the article acceptance.

ejikeugba commented 3 years ago

Sure, I'll do that as soon as possible. Thanks.