Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Optionally skip tests requiring 'mockr' #434

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MichaelChirico
Copy link
Contributor

It would be simpler to add the skip() inside the helper:

with_mock <- function(..., .parent = parent.frame()) {
mockr::with_mock(..., .parent = .parent, .env = "googledrive")
}

But the tests would have to be rewritten a bit, e.g.

with_mock(
root_id = function() "",
{
out <- resolve_paths(as_dribble(x), ancestors)
}
)
expect_equal(out$path, "e")

Would probably become:

with_mock(
  root_id = function() "",
  expect_equal(resolve_paths(as_dribble(x), ancestors)$path, "e")
)

That format of test looks more natural to me anyway, but filing as is now pending feedback.

@jennybc
Copy link
Member

jennybc commented Jun 27, 2023

I've been removing mockr dependencies in favor of testthat::local_mocked_bindings(). Apparently I just haven't done so here yet. So I'm not going to take a change that touches this stuff in any other way; it's not worth fiddling with it.

@MichaelChirico
Copy link
Contributor Author

Makes sense! I'll leave this open for now in case it serves as a reminder, but feel free to close at any time.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants