Skip to content

Conversation

@hadley
Copy link
Member

@hadley hadley commented Apr 26, 2023

  • In package environment for interactive use
  • In actual test environment for testing + S3 support

* In package environment for interactive use
* In actual test environment for testing + S3 support
@hadley hadley requested review from jennybc and lionel- April 26, 2023 17:01
R/mock2.R Outdated
#' incorporating what we've learned from the mockr, mockery, and mockthat
#' packages.
#'
#' `with_mocked_bindings()` and `local_mocked_bindings()` work by temporarily
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: the slack message about "any good description of mocking that people might grok"

Maybe: with_mocked_bindings() and local_mocked_bindings()` work by temporarily redefining an object (usually a function) provided by a package.

I feel like "(re)defining an object" might speak to more people than talking about bindings.

R/mock2.R Outdated
#' implementation changes.
#' Generally, it's only safe to mock packages that you own.
#'
#' These functions do not currently affect registered S3 methods.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still accurate?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried to clarify this a bit more — I think the danger is that you're not just affecting the functions in your package, but all the other functions in that package. (i.e. in the rlang case you're affecting every rlang function that calls check_installed(), not just your functions).

hadley and others added 3 commits April 27, 2023 13:18
Co-authored-by: Jennifer (Jenny) Bryan <jenny.f.bryan@gmail.com>
@hadley hadley merged commit 757a9a9 into main Apr 27, 2023
@hadley hadley deleted the more-mocking-envs branch April 27, 2023 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants