Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.45%. Comparing base (cf8b820) to head (be64f3f).
Report is 237 commits behind head on main.

@@             Coverage Diff              @@
##               main    #3063      +/-   ##
============================================
- Coverage     85.45%   85.45%   -0.01%     
- Complexity     2888     2889       +1     
============================================
  Files           329      329              
  Lines          8801     8811      +10     
  Branches       1093     1093              
============================================
+ Hits           7521     7529       +8     
- Misses          992      993       +1     
- Partials        288      289       +1     

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

Yes this is an area where we have not yet figured out how we should solve this. One question I have before we analyze the various options: are you sure that on the slow path you are using the Stackwalker? We have seen significant improvements when that's used, to the extent that the requests for improved performance mostly were gone.

Yes, StackWalker is used on the slow path, I also just tested it with the Java 8 Location implementation and there it is even worse with 4 minutes and 44 seconds.

StackWalker: 1 min 58 sec
Java 8 / Throwable: 4 min 44 sec
Empty Location: 40 sec

Bumping this. Without some way to disable this, mockito is an order of magnitude more expensive than a simple Java proxy

I do think this is a legitimate issue to be fixed, but I am unsure whether this is the best way to solve it. Ideally, I would like to explore how to best integrate this with CI/IDE tooling, so that they can take advantage of this. E.g. in case a test passes, we don't need all of this information. But when a test fails, we do need this information. I can imagine a workflow where a test fails, on rerun we want more explicit information. And on CI I can imagine that we want the full stacktrace, to ease debugging. By making it overridable, we do have the building blocks, but the way to use it is not straightforward. Similar to what we have done with mockito-inline, we need a low entry barrier here so that we can make Mockito perform the best in each scenario, while still providing sufficient information for debuggability.

Given this feature, in which scenarios do you think it is worthwhile to drop the location information, keeping in mind we have to balance performance and debuggability in for example CI environments?

Given this feature, in which scenarios do you think it is worthwhile to drop the location information, keeping in mind we have to balance performance and debuggability in for example CI environments?

Pretty much all local tests on my IDE, if they fail and need the location information to debug that, then I can re-enable as necessary.

The location information takes the time for certain tests from seconds to minutes

Given that this is predominantly useful when builds fail and there is benefit of a run-twice scenario, I wonder if we should write a Gradle/Maven plugin for this. What I am thinking is that, similar to the Gradle retry plugin, we can instruct Mockito to run without location on a first run. If a test fails, we can enable location data when we run that test and try again.

This sounds definitely interesting, how would you like to taggle this? Should I make a PoC? or what is the process in Mockito for such things?

Something which also maybe worth looking into is a JUnit 5 extensions, since it is possible to intercept the method invocation it may be possible to call the test twice if it fails, although here it is harder to handle cleanup stuff.

Before we invest a lot of time into writing such a plugin, let's first design it to see what is possible. Some initial thoughts/questions:

1. Users shouldn't have to change test source code to get the behavior. E.g. a separate extension would not be ideal to me
2. This should be an optional optimization, since rerunning tests can have unintended side-effects or a performance hit (if the amount of time spent in Mockito is vastly smaller than the rest of the test)
3. Can we only rerun tests when Mockito fails and any other exception we ignore? Based on what can we determine whether the location info is missing and can we only rerun tests in those cases?
4. If we go down the route of integrating with build systems, which ones do we choose and how much effort is it to keep up? Do we keep that as part of this repository or do we make it a separate project similar to mockito-testng?

I have a very similar situation and would like to see some type of change to enable the squashing of the reporting the location. I'm wondering if this approach / some of the options might be over-engineering this problem. In my particular use-case, I only really care about squashing the location for a specific mock that my test interacts a lot with. I (largely) do not care on any of the other mocks and fine with the performance hit. Because of this, I think it might make sense to implement this feature via the MockSettings rather than via a Plugin. This setting could also be applied to stubOnly() mocks as I think it would fit with the theme of a stubOnly mock and give users the performance increase without and code changes. This approach could yield a pretty simple solution that would see the performance uplift when it is important and would retain the StackTraces when the performance is not important. It could be leveraged simply via:

Object ex = mock(Object.class, withSettings().noLocation());

and then in MockMethodAdvice#advice, the mock settings could be checked to determine whether to report the actual location.
ex:

if mockHandler(instance).getSettings().disabledLocationReporting()
    return new ReturnValueWrapper(
        interceptor.doIntercept(
                instance, origin, arguments, realMethod, new Location () {
                    String getLocation() {
                        return "This mock has disabled location reporting" + mockHandler(instance).getSettings().getName();
                    }
                });
else
    return new ReturnValueWrapper(
        interceptor.doIntercept(
            instance, origin, arguments, realMethod, LocationFactory.create(true)));

Looking at this further, I think it is actually incorrect to be collecting the real Location on stubOnly mocks. As far as I can tell, Location is only reported when a verification fails. With StubOnly mocks, mockito already throws an exception when attempting to verify on a stub only mock, so I don't think it is possible to get mockito to report these locations OOTB and collecting these locations is wasting resources.

This approach has the added benefit of still supporting calls into LocationFactory when a user could fail a verification check and would want to report the location. This also does not affect the reporting of UnfinishedStubbingException for stubOnly mocks so we could still vend location information on stubbed mock that is being miss used.

I suspect that in many of the situations that a user is interacting with a mock millions of times the mock is going to already be a StubOnly mock because of the memory issues of tracking millions invocations.