Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix up the scoping for the @mock macro. #113

Merged
merged 1 commit into from
Apr 28, 2024

Conversation

kuszmaul
Copy link
Contributor

Previously, the macro required Mocking to be visible in the caller's context: It was invoking Mocking.activated(), for exmaple. Now it invokes $activated(), which makes sure that the code uses the right version of activated. Similarly for get_alternate and Patch.

Also, the macro was doing some unceessary gensym calls, which macro hygene can take care of automatically.

Previously, the macro required `Mocking` to be visible in the caller's context:
It was invoking `Mocking.activated()`, for exmaple.  Now it invokes
`$activated()`, which makes sure that the code uses the right version of
`activated`.  Similarly for `get_alternate` and `Patch`.

Also, the macro was doing some unceessary `gensym` calls, which macro hygene can
take care of automatically.
Copy link
Contributor

@nickrobinson251 nickrobinson251 left a comment

Choose a reason for hiding this comment

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

Thanks, @kuszmaul! I think this LGTM, but i'll hold off merging for now to give other contributors here a chance to review (cc @omus @iamed2 @oxinabox @rofinn)

Also, this is a nice clean-up, but curious if there was an issue you ran into which prompted it? if so, is it something worth adding as a test case?

@kuszmaul
Copy link
Contributor Author

kuszmaul commented Apr 26, 2024

Yes, I was doing some cleanup of the imports in some RAI code, which were something like:

Old:

import Mocking
...
   @mock something()

New:

using Mocking: @mock
...
  @mock something()

Partly because the RAI style guide says that's how we want to do things.

Since the module didn't otherwise using Mocking, I thought it would be fine. But it doesn't work with the current code., where I need to write

using Mocking: Mocking, @mock
...
  @mock something()

@oxinabox oxinabox merged commit 00ad8f1 into JuliaTesting:master Apr 28, 2024
11 checks passed
@oxinabox
Copy link
Member

googe improvement, It would be possible to create a test for this but i do not think worth the effort

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.

3 participants