Fixes #2905 : ThreadLocal classes can be mocked.

[robertotru]

Fixes #2905

Changes

• MockMethodAdvice way of checking if an object was a mock didn't work for ThreadLocal classes. The only change here is in method MockMethodAdvice#isMocked(Object) where the two conditions are flipped in order to avoid hitting the get() method from ThreadLocal used to check if the object was calling itself.

Note: this was working in Mockito 4.9.0, didn't check what happens in 5.0.0 and 5.1.0.

Checklist

• Read the contributing guide
• PR should be motivated, i.e. what does it fix, why, and if relevant how
• If possible / relevant include an example in the description, that could help all readers
  including project members to get a better picture of the change
• Avoid other runtime dependencies
• Meaningful commit history ; intention is important please rebase your commit history so that each
  commit is meaningful and help the people that will explore a change in 2 years
• The pull request follows coding style
• Mention Fixes #<issue number> in the description if relevant
• At least one commit should mention Fixes #<issue number> if relevant

[robertotru]

Hello,

This PR is a possible fix for the issue reported in #2905
Hope it helps.
Best
R

[raphw]

While this will solve the issue for thread local classes, it will introduce it for map objects. Let me have a look if this can be solved differently.

[TimvdLippe]

it will introduce it for map objects

Can we make sure we have a regression test for that as well? I would have merged this PR since all tests are green. Would be good to codify this requirement to ensure we don't regress.

[robertotru]

@TimvdLippe and @raphw

Got it. I can add additional tests, just give me a list of use cases you want me to cover.

Best
R

[raphw]

Turns out the recursion check is one level deeper. For the case I meant, the self-call-check is pointless anyways. Turning this the way as suggested should not be an issue.

[robertotru]

Thanks a lot for the quick processing.