with_mock() function

[krlmlr]

As discussed in #159. Interface as suggested by @hadley, supports nesting, mocking more than one function, not mocking at all, and returning a value.

Early draft, with tests and a documentation stub. Comments are welcome.

Fixes #159.

[hadley]

What if we got rid of the env argument and made it base::all.equal = ...

[krlmlr]

That's a neat idea, but there's a catch:

> list(a::b = 5)
Error: unexpected '=' in "list(a::b ="
> list(`a::b` = 5)
$`a::b`
[1] 5

So it'd have to be ``base::all.equal = ... . But how about:

with_mock({ ... }, list(compare = ...), base = list(all.equal = ...))

?

[hadley]

Oh yeah, I meant to say that but forget that I needed multiple backticks. Still, I think that's better than a named list for each environment. Most of time you'll only be mocking one function in one.env so that use case needs to be concise.

[krlmlr]

Should we do a second review round?

[hadley]

Maybe a style like this would look better?

with_mock(
  `base::all.equal` = function(x, y, ...) TRUE,
  {
    expect_equal(3, 5),
  }
)

[krlmlr]

I just tried this: with_mock <- function(..., .code, .env = topenv()) { ... }

Then it only works when I explicitly use the .code parameter:

with_mock(
  `base::all.equal` = function(x, y, ...) TRUE,
  .code = {
    expect_equal(3, 5),
  }
)

Otherwise, the code is part of ..., and nothing is assigned to .code.

I probably could make it "just work" by using the unnamed parameters of ... as code to be mocked. This would also permit

with_mock(
  `base::all.equal` = function(x, y, ...) TRUE,
  expect_equal(2 * 3, 4),
  expect_equal(3 * 3, 6),
  expect_equal(3 * 3 + 4, 9)
)

However, I haven't captured unevaluated ... before.

What do you think? Is it ok to import pryr?

[hadley]

I actually rather like that idea. But you don't need to import pryr, just inline the dots function.

[krlmlr]

@hadley: I have incorporated your comments except for the set pattern. Thanks for your feedback.

[hadley]

I'm not sure this will work - if nothing is named then mock_qual_names() will be NULL

[krlmlr]

On my system:

> NULL == ""
logical(0)
> all(NULL == "")
[1] TRUE

True, this is not very explicit -- but this code path is tested in the tests. Do you prefer an explicit check for NULL?

[hadley]

Ok, that's fine.

[hadley]

Thanks!