Shim code and revise tests for upcoming release of testthat #135
Conversation
… allow mocking functions from base packages
Codecov Report
@@ Coverage Diff @@
## master #135 +/- ##
==========================================
- Coverage 87.44% 87.41% -0.04%
==========================================
Files 87 87
Lines 4980 4984 +4
==========================================
+ Hits 4355 4357 +2
- Misses 625 627 +2
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #135 +/- ##
==========================================
- Coverage 87.44% 87.41% -0.04%
==========================================
Files 87 87
Lines 4980 4984 +4
==========================================
+ Hits 4355 4357 +2
- Misses 625 627 +2
Continue to review full report at Codecov.
|
| @@ -1,7 +1,7 @@ | |||
| with_fake_input <- function (input, expr) { | |||
| with_mock( | |||
| `crunch:::is.interactive`=function () return(TRUE), | |||
| `base::readline`=function (...) input, | |||
| `crunch:::read_input`=function (...) input, | |||
There was a problem hiding this comment.
The crunch::: doesn't appear to be strictly necessary here. I tried removing them and running (against both release and master branch testthat) and the examples don't have them: https://github.com/hadley/testthat/blob/master/R/mock.R#L34
However, I don't mind leaving them in since they make it more explicit what's going on and make sure we aren't accidentally mocking from another namespace.
There was a problem hiding this comment.
As it turns out R CMD check fails if you remove the crunch::: from these, like Function is.interactive not found in environment testthat.
r-lib/testthat#553 makes it so that
with_mockcan't mock functions in base packages anymore. See r-lib/testthat#546 and various other issues linked to it for a discussion. They keep saying that mocking doesn't work with the current development version of R, but I've never seen any test failures on it locally or on Travis, so /me shrugs. Apparently they landed on a compromise of disallowing mocking of base packages and not preventing it for anything outside of your own package, which would have been a disaster for us."Fixing" it here generally means adding a function to
crunchthat passes through to the base/utils/whatever function, and then mocking the crunch version in the tests.Since we're due for a CRAN release anyway, and I can see that they're gearing up for a
testthatrelease too, let's get in front of the reverse-dependency check notification and update now. I ran everything locally with the current master branch of testthat and it passes.