search

uber / tchannel-java

A Java implementation of the TChannel protocol.
MIT License
134 stars 65 forks source link

Unfinalize classes to allow to mocking/subclassing #186

Closed achals closed 4 years ago

achals commented 7 years ago

Fixes #185.

codecov-io commented 7 years ago

Codecov Report

Merging #186 into master will increase coverage by 0.03%. The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #186      +/-   ##
============================================
+ Coverage     70.98%   71.02%   +0.03%     
  Complexity       10       10              
============================================
  Files            87       87              
  Lines          2630     2630              
  Branches        311      311              
============================================
+ Hits           1867     1868       +1     
  Misses          543      543              
+ Partials        220      219       -1
Impacted Files Coverage Δ Complexity Δ
.../src/main/java/com/uber/tchannel/api/TChannel.java 66.4% <ø> (ø) 0 <0> (ø) :arrow_down:
...ava/com/uber/tchannel/frames/InitRequestFrame.java 96% <ø> (ø) 0 <0> (ø) :arrow_down:
...uber/tchannel/frames/CallRequestContinueFrame.java 95.45% <ø> (ø) 0 <0> (ø) :arrow_down:
...main/java/com/uber/tchannel/frames/ErrorFrame.java 80.55% <ø> (ø) 0 <0> (ø) :arrow_down:
...java/com/uber/tchannel/messages/ErrorResponse.java 86.66% <ø> (ø) 0 <0> (ø) :arrow_down:
...ain/java/com/uber/tchannel/checksum/Checksums.java 38.46% <ø> (ø) 0 <0> (ø) :arrow_down:
...va/com/uber/tchannel/frames/CallResponseFrame.java 89.18% <ø> (ø) 0 <0> (ø) :arrow_down:
...in/java/com/uber/tchannel/codecs/MessageCodec.java 60% <ø> (ø) 0 <0> (ø) :arrow_down:
...va/com/uber/tchannel/frames/PingResponseFrame.java 87.5% <ø> (ø) 0 <0> (ø) :arrow_down:
...rc/main/java/com/uber/tchannel/api/SubChannel.java 77.88% <ø> (+0.96%) 0 <0> (ø) :arrow_down:
... and 19 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 77032af...60fedae. Read the comment docs.

alshopov commented 7 years ago

I get your point about mocking the classes. Since I am doing most of the things final - my reason for that is to explicitly show users that the classes are not meant for extension rather than hamper people. PowerMock really allows you to mock even final classes and static members. If @yborovikov is fine with your change I will not block it. I believe most proper way will be to extract interfaces, make these final classes implement them to allow for both final and mockito-ability. I am not sure whether this will be worth it.

achals commented 6 years ago

@yborovikov Bump, could you take a look at this when you had the chance?