Fixes #1516 and #1631 : Allows @Spy with @InjectMocks to be injected into other @InjectMocks

[arnor2000]

[This PR is a cherry pick of #1710 to release/3.x]

check list

• 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

This PR aims to fix #1516 and #1631. I know and understand dependency injection is not priority of Mockito team but discussion on #1518 convinces me to propose a fix. Like @mockitoguy I think DI in Mockito is useful and most of reported issues denote users high expectations for a helpful feature rather than the fact it's broken.

Basically this PR allows fields annotated with @SPY and @Injectmocks to be injected into other fields annotated with @Injectmocks and so on.
It may also be a response to #174, the need to a @real annotation, actually I think most usage of @SPY and @Injectmocks combination is a workaround to such annotation.

Because PR ended to change a bunch of classes, I will highlight main changes.

"Entry point" of changes is InjectingAnnotationEngine, I add a loop of mock injection. The loop uses variants of existing field injection strategies documented in DefaultInjectionEngine.

To facilitate field injection strategies variants (StrictConstructorInjection and LenientPropertyAndSetterInjection), I merge ConstructorArgumentResolver (contract to find constructor arguments) and ConstructorInstantiator (contract to find and instantiate constructor) interfaces into an unique ConstructorResolver interface representing a strategy to find a constructor and its arguments. Constructor instantiation part comes back into FieldInitializer.

The fact to merge these two interfaces into ConstructorResolver leads to a better separation of concerns between finding constructor and its arguments in one side and instantiation, initialization and field injection on the other side. isResolvable() allows some tuning between the inclination of strategy implementations to fail with exception or handle failure softly. Moreover it's opening future smarter behaviors, for example BiggestConstructorResolver could try to resolve biggest constructor matching actual available mocks.

I hope you will appreciate this work.

[codecov-io]

Codecov Report

Merging #1711 into release/3.x will decrease coverage by 74.67%.
The diff coverage is 0%.

@@                Coverage Diff                 @@
##             release/3.x    #1711       +/-   ##
==================================================
- Coverage          86.84%   12.16%   -74.68%     
+ Complexity          2518      318     -2200     
==================================================
  Files                316      310        -6     
  Lines               6622     6525       -97     
  Branches             829      826        -3     
==================================================
- Hits                5751      794     -4957     
- Misses               672     5603     +4931     
+ Partials             199      128       -71

Impacted Files	Coverage Δ	Complexity Δ
...java/org/mockito/internal/exceptions/Reporter.java	0% <ø> (-93.52%)	0 <0> (-93)
...guration/injection/PropertyAndSetterInjection.java	0% <0%> (-100%)	0 <0> (-10)
...ernal/configuration/InjectingAnnotationEngine.java	11.11% <0%> (-88.89%)	1 <0> (-8)
...ito/internal/util/reflection/FieldInitializer.java	0% <0%> (-90.57%)	0 <0> (-18)
...internal/configuration/DefaultInjectionEngine.java	0% <0%> (-100%)	0 <0> (-2)
...l/configuration/injection/scanner/MockScanner.java	0% <0%> (-95.46%)	0 <0> (-13)
.../injection/filter/TerminalMockCandidateFilter.java	0% <0%> (-81.82%)	0 <0> (-3)
...to/internal/configuration/SpyAnnotationEngine.java	1.51% <0%> (-96.67%)	1 <0> (-23)
...guration/injection/StrictConstructorInjection.java	0% <0%> (ø)	0 <0> (?)
.../internal/util/reflection/ConstructorResolver.java	0% <0%> (ø)	0 <0> (?)
... and 304 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f61e187...85c7333. Read the comment docs.

[arnor2000]

Hi, I've rebased code onto current version 3.3.2.

Can I do something to help you more on this PR?

[adilkaraoz]

Hi guys,

Will you support this future soon or you already merged this via another PR? I have tested but @SPY and @Injectmocks objects didn't inject to the other @InjectsMocks object

[arnor2000]

Hi,
I rebased this PR on top of version 4.0.0 (or few commits behind but there is no conflict at this time).

@adilkaraoz, afaik there is no other PR superseding this one, I hope Mockito guys will have time to to review this PR and offer us this new powerful feature in next release ;)

[arnor2000]

Hi @mockitoguy @TimvdLippe @bric3

Please don't let this PR take the dust, I know it's a big PR to review but I made this one 2 years ago and each rebase is a pain...

And of course, I'm sure this feature will be useful for users!

[TimvdLippe]

Hey, sorry for the late reply. I think the primary reasons this PR has become stale are the fact that it is very large (e.g. very difficult to review) and it concerns @InjectMock, which I am generally not a fan about (#1518). That said, I know this feature is liked by the community, so I don't think removal is warranted, yet extending the feature is also not something I am thrilled about.

So truth be told, I am not sure if we will be able to make any progress here anytime soon and that it remains in the current state as it is 😢 You have definitely put a lot of effort in this PR already, which is why I would like the answer to be different, but at the same time I have to be realistic in both its size and intended purpose given the wider scope of Mockito.