New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #1597 : Adds lenient for BDD Mockito #2048
base: main
Are you sure you want to change the base?
Conversation
Based on closed PR mockito#1598 by konradkluz
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
public void bdd_unused_and_lenient_stubbings() throws Exception { | ||
given(mock1.simpleMethod(1)).willReturn("1"); | ||
given(mock1.simpleMethod(2)).willReturn("2"); | ||
lenientBDD().given(mock2.simpleMethod(3)).willReturn("3"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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. |
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: 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! |
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. |
@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 |
9c90ce5
to
5fa02c6
Compare
I'll try to review in more detail in the next few days and merge. Thanks! |
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
It's taking me a while but I'm working on it ;-) |
@holubec-petr, do you mind rebasing with latest release/3.x? Thanks! |
7b3ce60
to
a285d5f
Compare
- started using 'ProductionCode' class so that we are emulating the behavior correctly - refactored to use lambdas
a285d5f
to
8bf1bb6
Compare
@mockitoguy Done ;-) |
Any progress here @mockitoguy ? |
Came across this one and eager to get it merged. @mockitoguy Any chance you can look at it again? Thanks. |
Hi, solution for lenient and BddMockito would be really handy. |
+1, this would be great to have! Any reason it couldn't be merged today @mockitoguy? |
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
including project members to get a better picture of the change
commit is meaningful and help the people that will explore a change in 2 years
Fixes #<issue number>
in the description if relevantFixes #<issue number>
if relevant