Undercover checks are all about making sure that no changed line of code is merged without test coverage. Ideally, this happens in code review with warnings auto-delivered as review comments on a method-by-method basis
I was recently contacted by Aleksandr in this GitHub issue for
pronto-undercover who proposed making undercover warnings less strict with configurable required percentage. After all, pull requests in projects with low or nonexistent initial test coverage can likely generate large numbers of hard-to-deal-with warnings which you might not want to address all at once. This was the case for the issue author.
You can set the undercover PR check to optional, which I normally recommend, but it won’t fix the noise generated by a poorly tested codebase.
Should you then just rely on a global coverage threshold (e.g.
SimpleCov.minimum_coverage) that allows more wiggle room, e.g. 50%? I don’t think so, because random lines might slip through review and make bugs more likely to happen. But, I recently found out about a better and more explicit test coverage strategy thanks to SimpleCov.
✨ SimpleCov lets you wrap any Ruby code in
:nocov: comments to disable coverage reporting, which also works great for suppressing any undercover warnings. I didn’t know about this. ✨
As I wrote earlier, my general undercover CI workflow was to set undercover as an optional check, so that anyone can merge changes even when some acknowledged lines and methods are missing tests. It’s good, because these are surfaced and scrutinized in code review.
But what about this strategy instead:
- mark the
coveragepull request check as required instead of optional
- in case you want to acknowledge and skip a particular warning, update the code and tag relevant lines with
:nocov:(could be fully automated by UndercoverCI with repo write permission in the future!)
- only ship on green, with potential untested lines flagged and silenced
Why? This way information loss is avoided and your future self (and team) will clearly know which code parts went to production untested thanks to all the
Let me know what you think about this. Will you ever get back to those comments to fix them? 😅 Did you know about this handy SimpleCov feature? Maybe it’s a good idea to update your test coverage strategy to allow untested changes only when wrapped in
:nocov: syntax has now a dedicated section in UndercoverCI docs: skipping/ignoring coverage.