Best-effort error reporting for interactions on final methods

[AndreasTu]

Add error reporting code to the byte-buddy` mock maker to report interaction on final methods, which can't be handled.

The IMockInteractionValidator interface allows IMockMakers to implement different kinds of validations on mock interactions.

Fixes #2039

[codecov]

Codecov Report

Attention: Patch coverage is 92.79279% with 8 lines in your changes missing coverage. Please review.

Project coverage is 81.95%. Comparing base (2c7db77) to head (5d5a3e2).
Report is 159 commits behind head on master.

Files with missing lines	Patch %	Lines
...va/org/spockframework/mock/runtime/MockObject.java	71.42%	1 Missing and 3 partials ⚠️
...ock/runtime/ByteBuddyMockInteractionValidator.java	95.74%	1 Missing and 1 partial ⚠️
...ckframework/spring/mock/DelegatingInterceptor.java	33.33%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #2040      +/-   ##
============================================
+ Coverage     80.44%   81.95%   +1.50%     
- Complexity     4337     4647     +310     
============================================
  Files           441      450       +9     
  Lines         13534    14513     +979     
  Branches       1707     1834     +127     
============================================
+ Hits          10888    11894    +1006     
+ Misses         2008     1942      -66     
- Partials        638      677      +39     

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

[leonard84]

🤔 I wonder if we should have three dedicated methods instead that delegate to this. propteryToGetterMethodName/propteryToSetterMethodName/ propteryToIsMethodName (not sure about the last one.)

[AndreasTu]

I introduced propertyToGetterMethodName, propertyToSetterMethodName and propertyToBooleanGetterMethodName.

[leonard84]

The method name suggests that we should get an error here, but the test does not reflect that.

[AndreasTu]

It fails to mock the method,. because one of the overloads is final, so there is not error message, because the parameters could have matched both the final or the non-final method, so we can't detect that right now => No error message.

This is the best-effort part of this PR.

[leonard84]

Is this really specific to ByteBuddy, or does it apply to CgLib as well?

[AndreasTu]

It would also work for the cglib mock maker, but I did not want to implement the static field cache in cglib, because it is already deprecated.

If you want that supported, let me know, then I will have a look.

[Vampire]

I didn't have time for a proper review, but is the validation done at compile-time or runtime?
If it was done at runtime, how much is the performance impact?

[AndreasTu]

@Vampire It is done at runtime.
During compile-time, it would not be possible to know the used mock maker, therefore it would lead to false positives, if the user would use e.g. mockito or an external mock maker.

During runtime, the expensive operation of calculating the final methods is done, once per class and only if an interaction is specified. Note: Currently I am only checking for the public finals, => class.getMethods() not for anything else, as I said best-effort. But this could be extended in the future.

The final methods Set cache is attached to the generated mock class as static field, so the cache is GCed without any weakrefs etc., when the mock class is GCed.

For each interaction specification the cost is to loop over the contained IInvocationConstraints in the IMockInteraction and make a Set lookup for the method name, and two Set lookups for a property name, if the byteBuddy mock maker is used.

For mockito and java-proxy there is not overhead, except of the getter of the spock mock obj and a null check.