BUG: .covignore not honored by review bot

[HaoZeke]

This is a strange one. My package has a .covignore file and local / on-repo pkgcheck passes. However, on the package's software review issue @ropensci-review-bot ignores the file, and flags the coverage of the submission.

[mpadge]

Thanks @HaoZeke, you're definitely right that this is a strange one, and indeed for the reasons you state. The strangeness is that covr::package_coverage() gets the correct coverage when called directly on our system. The actual results are, however, generated by goodpractice::gp(), which makes the same call, and yet in that case ignores your .covignore file.

This clearly reveals a bug in goodpractice. The problem there is that that package is no longer actively maintained, so a fix is unlikely at that end. We'll have to implement a work-around on our end. This issue will be closed when that is implemented. Sorry for any inconvenience.

[HaoZeke]

Thanks @HaoZeke, you're definitely right that this is a strange one, and indeed for the reasons you state. The strangeness is that covr::package_coverage() gets the correct coverage when called directly on our system. The actual results are, however, generated by goodpractice::gp(), which makes the same call, and yet in that case ignores your .covignore file.

This clearly reveals a bug in goodpractice. The problem there is that that package is no longer actively maintained, so a fix is unlikely at that end. We'll have to implement a work-around on our end. This issue will be closed when that is implemented. Sorry for any inconvenience.

Could it instead be something to do with the versions in the review bot? Especially since the Github action on the repo passes correctly:

That being said, its not an inconvinience at all, but would it be possible for you to confirm its an upstream bug on the ROpenSci issue? (just in case its not moving forward in the process while the bot isn't giving the all ok).

[mpadge]

Hmmm, that just thickens the plot... All versions of everything are identical. @assignUser can you help here? Running covr::package_coverage() on this package directly in our pkgcheck container ignores the ".covignore" file, but running it by calling pkgcheck-action/R/install.R-> pkgcheck-action/R/check.R generates the expected result by ignoring all files specified in ".covignore". Figuring out why is likely going to require dissecting all versions of everything installed via pak in the action ... unless you've got any other bright insights?

[mpadge]

@HaoZeke Should now be fixed, although maybe not on our actual build system until it re-deploys next week.

@assignUser This is an interesting one for you to keep in mind. The 🐛 was because the whole r-lib ecosystem depends on withr and callr to isolate processes and calls. Those have been constructed to always inherit all options set in the calling environment. That's an arbitrary design decision that makes sense, and avoids unduly cluttering the parameter spaces of the functions. The "covr" package depends on either an env var or an option to specify which file to use to define global ignore rules, and that file is hard-coded as ".covrignore". When called directly, however, neither the option nor the env var are picked up, yet they are somehow passed through in all calls otherwise made within the r-lib ecosystem of packages. I don't need to follow-through where that bug arises, because we can solve it with the above commit regardless.

All of which is a long-winded way of saying it might be useful for you to keep in mind that this is one case where the presumed inheritance of options breaks down. It does not fail in pkgcheck-action because you constently use r-lib throughout, but that consistency of functionality is actually only because of an assumption that is not necessarily always valid.

[mpadge]

r-lib/covr#538

[mpadge]

The above commit does not actually fix the issue here. Re-opening to implement an alternative fix

[mpadge]

@HaoZeke The bug has now been fixed upstream in the covr package. The above commit reverts pkgcheck to former state. You should notice no change, but please report just in case you notice anything stops working on your side. Thanks