MockingProgress: replaced getVerificationListeners() with fireVerificationEvent(..)

[ChristianSchwarz]

With this PR MockingProgress don't need to expose its listeners. This keeps the notification of listeners and the logic in one place to avoid "solution sprawl". Later on we can unify or improve the notification behaviour in MockingProgressImpl.

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (release/2.x@a59cc57). Click here to learn what that means.
The diff coverage is 100%.

@@              Coverage Diff               @@
##             release/2.x     #965   +/-   ##
==============================================
  Coverage               ?   86.69%           
  Complexity             ?     2255           
==============================================
  Files                  ?      287           
  Lines                  ?     5733           
  Branches               ?      663           
==============================================
  Hits                   ?     4970           
  Misses                 ?      574           
  Partials               ?      189

Impacted Files	Coverage Δ	Complexity Δ
...ternal/verification/MockAwareVerificationMode.java	94.73% <100%> (ø)	4 <2> (?)
...mockito/internal/progress/MockingProgressImpl.java	98.61% <100%> (ø)	29 <ø> (?)
...rc/main/java/org/mockito/internal/MockitoCore.java	97.59% <100%> (ø)	33 <ø> (?)

---

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 a59cc57...abce062. Read the comment docs.

[ChristianSchwarz]

blub

[TimvdLippe]

I think I am okay with this, but the original solution was working out just fine thus far 😄

[ChristianSchwarz]

@TimvdLippe

[...] but the original solution was working out just fine thus far

Thats true for almost every refactoring on this planet ;-).

Exposing implementation details to other classes are a no go even if it looks handy. I've seen more than one product that became unmaintainable after the sum of all technical depts get out of control. Every tiny improvement is a good invest for a healty product.

[TimvdLippe]

The reason I made that comment is that I am not aware of any planned changes to that class. So I would rather refactor if we see the need to, than to make these changes.

Yes it is a different design, but also this one has some disadvantaged. Therefore I am inclined to wait, until we find a reason to change the implementation and also apply these changes 😄

[ChristianSchwarz]

okay let's see what the other core-members say

[mockitoguy]

I'm happy to review a PR if it brings some feature in :) Otherwise, it's hard prioritize PRs that are only refactorings without clear value. Normally, for every refactoring-only PR, I glance at the code change and if it does not stick out immediately as useful, I move on without leaving a comment. I'd rather allocate my limited resources to reviewing issues / PRs / coding stuff that adds clear value (or makes me happy).

Here's an example of a refactoring that removes unnecessary code and is needed to implement a feature: #1011 (it removes ~20 lines of build logic code that is magic to most folks besides build authors). The refactoring in this PR adds ~30 lines of code (net).

I'd rather avoid getting into a debate how / if the refactorings in this PR are useful. I'm sure you will convince us ;) I'm happy to review this PR once I clean up all PRs that clearly add value (new features, improved behavior), and once my personal backlog of favorite features is implemented.

Thanks for reading and for supporting the team!!!

[mockitoguy]

@ChristianSchwarz, do you want to take a stab at this one #1013? :)

[ChristianSchwarz]

@szczepiq

do you want to take a stab at this one #1013? :)

I really like to! Sadly I have no time until September, also I want to finish 936 Unification of timeout()/after().

[TimvdLippe]

Closing this one for now. Feel free to apply these refactorings when the behavior of the classes has to be changed 😄