-
Notifications
You must be signed in to change notification settings - Fork 11
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
#42 Removed Mockito from NetworkTest #64
base: master
Are you sure you want to change the base?
Conversation
Job #64 is now in scope, role is |
Codecov Report
@@ Coverage Diff @@
## master #64 +/- ##
============================================
- Coverage 93.03% 92.77% -0.27%
Complexity 49 49
============================================
Files 11 11
Lines 158 166 +8
Branches 10 11 +1
============================================
+ Hits 147 154 +7
Misses 11 11
- Partials 0 1 +1
Continue to review full report at Codecov.
|
final Remote lowremote = Mockito.mock(Remote.class); | ||
final Wallet wallet = Mockito.mock(Wallet.class); | ||
final Map<Long, Wallet> highwallets = new HashMap<>(); | ||
final Remote high = new Remote.Fake(20, highwallets); |
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.
@carlosmiranda don't you think Remote.Fake
should expose the underlying map instead of having to keep a reference around?
@victornoel please check new commit addressing your comments. Also added |
@carlosmiranda personally I like it better like that yes, also not that I am not the reviewer :) |
@victornoel doh. This is not even assigned yet, you are working for free. :P |
@carlosmiranda just defending my interests by ensuring my future usage of those classes is a joy :D |
@victornoel nevertheless it should be avoided, you are basically robbing both yourself and eventual assignee of the job (unless it turns out to be you). It's up to the architect to maintain joyful class usage. |
@carlosmiranda it's true, thanks for the reminder |
@llorllale are there no reviewers available? This has become a long running task |
@llorllale this is more than a week old now. Can you assign a |
@carlosmiranda it's a funding issue - let me try to resolve it as quickly as possible |
#42: Removed Mockito from
NetworkTest
. Added implementations ofpush()
andpull()
inRemote.Fake
that can be used for testing purposes.