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

Avoid overwrite warning using more specific method #119

Merged
merged 3 commits into from
Jul 13, 2024
Merged

Conversation

omus
Copy link
Member

@omus omus commented Jul 12, 2024

As of Julia 1.10 we receive warnings like the following when running Mocking.activate():

WARNING: Method definition activated() in module Mocking at /Users/cvogt/.julia/dev/Mocking/src/Mocking.jl:15 overwritten at /Users/cvogt/.julia/dev/Mocking/src/Mocking.jl:24.

As the method efinition overwrite is intentional here as we purposefully make use of invalidation to support @mock it would be best to silence this warning for end users.

We can use the Julia flag --warn-overwrite=no to do this (which is the default) but running Pkg.test() seems to enable this flag.

In order to silence this error message we're now avoiding overwriting an existing method and now we are instead just defining a more specific method which ends up having the same result but without the warning. We can however no longer cleanly support multiple calls to activate/deactivate as unless we want to utilize an arbitrarily deep type hierarchy. Due to this new restriction I've opted to deprecate both deactivate and activate(f) which I'm doubtful were used externally.

@omus
Copy link
Member Author

omus commented Jul 12, 2024

Note we still see some of these warnings due to the multiple activate calls in test/activate.jl. Once the deprecated functions are no longer tested these warnings will disappear.

@omus omus merged commit b028151 into master Jul 13, 2024
4 checks passed
@omus omus deleted the cv/silent-activate branch July 13, 2024 06:29
@nickrobinson251
Copy link
Contributor

nickrobinson251 commented Aug 12, 2024

We use activate(f) extensively throughout our testing codebase at RAI, and removing it (as was done in the v0.8 release) prevents us from updating Mocking.jl

I think we're fine with the warnings WARNING: Method definition ... and would be happy to live with them in our tests for the convenience/safety of having the activate(f) method. What do you think about reinstating this method and have it redefine _activate(::Int), so that only activate(f) would give the warning, and activate() would be warning-free?

cc @omus

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.

2 participants