Reintroduce activate(f) and deactivate()

[nickrobinson251]

These were dropped in the initial v0.8 release
(see PR#119) but were being used in the wild so
this PR brings them back, but with documentation
warning about the downsides of their usage.

As mentioned at #119 (comment)

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

(Originally added in #102)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.03%. Comparing base (856b399) to head (47116d0).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #135      +/-   ##
==========================================
+ Coverage   97.91%   98.03%   +0.12%     
==========================================
  Files           6        6              
  Lines         144      153       +9     
==========================================
+ Hits          141      150       +9     
  Misses          3        3              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[omus]

Do you need deactivate? Unless you also require this for your code base it's probably best to avoid re-introducing this

[nickrobinson251]

No, just for implementing (and testing) activate(f), so perhaps we should delete the doc-string and name this _deactivate()?

[omus]

That would be good with me.

[omus]

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

Just to double check: what is the problem with doing a removal on all of the activate(f) calls and adding one top-level activate()? How many activate(f) calls are we talking about?

[nickrobinson251]

what is the problem with doing a removal on all of the activate(f) calls and adding one top-level activate()?

I think there are two reasons we prefer activate(f):

• we want to minimise global state, and have every @testitem control enabling Mocking iff it's required for that @testitem (an @testitem from ReTestItems.jl is basically just a testset that runs in its own isolated module and which may be run on a different processes, i.e. each @testitem must contain everything it needs to run)
• we found that running with mocking always enabled slowed down the tests, so prefer to not have it enabled when not required (although i don't have numbers on this to hand).

[nickrobinson251]

mocking always enabled slowed down the tests

A quick look at internal issues, and i can't find a reference for this so i might be misremembering 🤔 i'll need to check more carefully next week

(And if most tests need Mocking, not paying the recompilation cost of enabling/disabling will be more performant)

i think the "reduce global state" does matter 🤔 I suppose an alternative is to just have every testitem that needs it call Mocking.activate(), since it's a no-op to call this multiple times, and then every testitem is still doing everything it needs

[omus]

we found that running with mocking always enabled slowed down the tests, so prefer to not have it enabled when not required (although i don't have numbers on this to hand).

If this is a concern it should be possible to improve performance on mocked calls where there are no applied patches. I've been prioritizing performance when Mocking is not activated to date.

[nickrobinson251]

Okay, i compared running our whole test suite under Mocking.activate(), and any perf impact isn't visible above noise, so i think actually v0.8 will work for us as-is. Thanks for talking it through!