Fixes #1597 : Adds lenient for BDD Mockito

[holubec-petr]

This PR add lenient support for BDD style of mocking.
Based on closed PR #1598 by konradkluz.

Example:
lenientBDD().given(mock.differentMethod("2")).willReturn("2");

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

[codecov-commenter]

Codecov Report

Merging #2048 into release/3.x will increase coverage by 0.00%.
The diff coverage is 88.23%.

@@              Coverage Diff               @@
##             release/3.x    #2048   +/-   ##
==============================================
  Coverage          84.89%   84.90%           
- Complexity          2704     2705    +1     
==============================================
  Files                325      325           
  Lines               8204     8221   +17     
  Branches             979      979           
==============================================
+ Hits                6965     6980   +15     
- Misses               968      970    +2     
  Partials             271      271           

Impacted Files	Coverage Δ	Complexity Δ
src/main/java/org/mockito/BDDMockito.java	88.57% <88.23%> (-0.11%)	11.00 <1.00> (+1.00)	⬇️

---

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 b6ae6cf...9c90ce5. Read the comment docs.

[mockitoguy]

Can you come up with a strategy so that we can write:

lenient().given(...)...

We'd like to understand what are options and trade-offs.

[mockitoguy]

I'm very happy you picked this work! Can you address my feedback.

I'll work with you on this one, and I should be more accessible than in the recent months.

[holubec-petr]

1. BDDMockito class will not extend Mockito class

   • it would allow us to have both versions of lenient() method with the same method name
   • but it will break backward compatibility due to missing general methods like any() or inOrder() in BDDMockito class
   • IMO we cannot break backward compatibility

2. Use still different but better naming e.g. lenientGiven() or givenLenient()

   • it will be more verbose and fit to BDD style than lenientBDD()
   • still we won't be able to write lenient().given(...)...
   • IMO current state of this PR just with better naming, doable

3. BDDLenientStubber will extend LenientStubber

   • it will allow us to have both versions of lenient() method
   • but it will introduce a bunch of meaningless methods from LenientStubber to BDDLenientStubber where they don't have any sense and will allow mixing concepts e.g. BDDMockito.lenient(...).doReturn()
   • IMO allows mixing concepts of standard Mockito and BDD style but if you insist on the same naming, this option would be best from the options listed here

4. Have one more separate BDDMockito class with just lenient method

   • we would have 2 lenient() methods each returning stubber with the desired scope
   • it will mess up imports and confuse users which class they should import
   • IMO it will be very confusing for users, I'd vote to not go this way

These are my ideas with their PROS and CONS, some of them are more or less stupid but I wanted to list as many as possible options.
Maybe I'm missing some internal logic to provide more reasonable options. Do you have any other ideas?

[mockitoguy]

Thank you, @holubec-petr, this is super useful to list all the options (even the ones that are very unlikely to be picked).

Options 2 and 3 make sense, and here's how I see the tradeoffs:
Option 2) Usability inconvenience: overloaded, non-BDD methods show up with autocompletion. This option also leads to more code that needs to be maintained (and potentially implementing runtime validation that throws exceptions if the dev uses non-BDD method in BDD tests).
Option 3) Usability inconvenience: inconsistent method naming "lenient()" vs. "lenient2()" (or however we call it)

Both options generate usability inconvenience and it's hard to objectively decide which inconvenience generates more cognitive load on engineers. Option 2) requires more code to implement and maintain (objectively inferior to option 3).

Let's go for 2). @TimvdLippe, thoughts?

I'd prefer "leniently()" instead of "lenientBDD()" method. It reads better in the code.

Can you document in the javadoc why we needed to pick a method that is not consistent, and link to this discussion? Thanks!

[TimvdLippe]

I am fine with option 2. I am personally not a fan of BDDMockito duplicating our whole API, but I guess that ship has sailed.

[holubec-petr]

@mockitoguy Did you switch tradeoffs of option 2) and option 3)? :-)

I picked up that we want to go with different method names and the BDD method should be called leniently().
I'll also document the process of decision linking to this thread.

[mockitoguy]

I'll try to review in more detail in the next few days and merge. Thanks!

[codecov-io]

Codecov Report

Merging #2048 (7b3ce60) into release/3.x (18da8f2) will increase coverage by 0.08%.
The diff coverage is 74.35%.

@@                Coverage Diff                @@
##             release/3.x    #2048      +/-   ##
=================================================
+ Coverage          84.79%   84.88%   +0.08%     
- Complexity          2715     2721       +6     
=================================================
  Files                325      325              
  Lines               8266     8294      +28     
  Branches             988      989       +1     
=================================================
+ Hits                7009     7040      +31     
- Misses               982      983       +1     
+ Partials             275      271       -4     

Impacted Files	Coverage Δ	Complexity Δ
...al/creation/bytebuddy/InlineBytecodeGenerator.java	89.44% <63.63%> (-3.31%)	39.00 <0.00> (+1.00)	⬇️
src/main/java/org/mockito/BDDMockito.java	88.57% <88.23%> (-0.11%)	11.00 <1.00> (+1.00)	⬇️
.../internal/creation/bytebuddy/MockMethodAdvice.java	77.61% <0.00%> (-0.30%)	23.00% <0.00%> (ø%)
...ito/internal/invocation/InterceptedInvocation.java	78.72% <0.00%> (+2.12%)	26.00% <0.00%> (+1.00%)
...l/creation/bytebuddy/InlineByteBuddyMockMaker.java	70.31% <0.00%> (+2.88%)	51.00% <0.00%> (+3.00%)
...rnal/util/reflection/ReflectionMemberAccessor.java	75.00% <0.00%> (+8.33%)	8.00% <0.00%> (ø%)

---

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 18da8f2...7b3ce60. Read the comment docs.

[mockitoguy]

It's taking me a while but I'm working on it ;-)

[mockitoguy]

@holubec-petr, do you mind rebasing with latest release/3.x? Thanks!

[holubec-petr]

@mockitoguy Done ;-)

[holubec-petr]

Any progress here @mockitoguy ?

[MichaelKunze]

Came across this one and eager to get it merged. @mockitoguy Any chance you can look at it again? Thanks.

[bugs84]

Hi, solution for lenient and BddMockito would be really handy.
If possible - Please solve this issue. @mockitoguy
Workaround with mixing BddMockito and Mockito style together creates a real mess in my tests :-/

[atwoosnam]

+1, this would be great to have! Any reason it couldn't be merged today @mockitoguy?