util-core: improve test coverage for Cancellable

[muachilin]

Problem

The test coverage of Cancellable still needs to be improved. Particularly the object of Cancellable has not been tested.

Solution

Add more tests for Cancellable.

Result

Improve the test coverage for Cancellable.

[caniszczyk]

Codecov Report

Merging #268 into develop will increase coverage by 0.02%. The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #268      +/-   ##
===========================================
+ Coverage    45.97%   45.99%   +0.02%     
===========================================
  Files          228      228              
  Lines        14447    14447              
  Branches       917      917              
===========================================
+ Hits          6642     6645       +3     
+ Misses        7805     7802       -3     

Impacted Files	Coverage Δ
util-core/src/main/scala/com/twitter/io/Buf.scala	92.16% <0.00%> (ø)	:arrow_up:
...core/src/main/scala/com/twitter/util/Promise.scala	78.31% <0.00%> (ø)	:arrow_up:
...ore/src/main/scala/com/twitter/util/Duration.scala	85.54% <0.00%> (+0.57%)	:arrow_up:
.../src/main/scala/com/twitter/util/Cancellable.scala	50.00% <0.00%> (+25.00%)	:arrow_up:

---

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 10c0bc9...4e7dd38. Read the comment docs.

[codecov-io]

Codecov Report

Merging #268 into develop will increase coverage by 0.02%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           develop    #268      +/-   ##
==========================================
+ Coverage    45.98%     46%   +0.02%     
==========================================
  Files          228     228              
  Lines        14447   14447              
  Branches       884     917      +33     
==========================================
+ Hits          6643    6646       +3     
+ Misses        7804    7801       -3

Impacted Files	Coverage Δ
util-core/src/main/scala/com/twitter/io/Buf.scala	92.16% <0%> (ø)	:arrow_up:
...core/src/main/scala/com/twitter/util/Promise.scala	78.71% <0%> (+0.4%)	:arrow_up:
.../src/main/scala/com/twitter/util/Cancellable.scala	50% <0%> (+25%)	:arrow_up:

---

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 4a61c2e...f810905. Read the comment docs.

[caniszczyk]

Codecov Report

Merging #268 into develop will increase coverage by 0.02%. The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #268      +/-   ##
===========================================
+ Coverage    45.97%   46.00%   +0.02%     
===========================================
  Files          228      228              
  Lines        14447    14447              
  Branches       917      917              
===========================================
+ Hits          6642     6646       +4     
+ Misses        7805     7801       -4     

Impacted Files	Coverage Δ
util-core/src/main/scala/com/twitter/io/Buf.scala	92.16% <0.00%> (ø)	:arrow_up:
...core/src/main/scala/com/twitter/util/Promise.scala	78.71% <0.00%> (+0.40%)	:arrow_up:
...ore/src/main/scala/com/twitter/util/Duration.scala	85.54% <0.00%> (+0.57%)	:arrow_up:
.../src/main/scala/com/twitter/util/Cancellable.scala	50.00% <0.00%> (+25.00%)	:arrow_up:

---

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 10c0bc9...4e7dd38. Read the comment docs.

[caniszczyk]

Codecov Report

Merging #268 into develop will increase coverage by 0.02%. The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #268      +/-   ##
===========================================
+ Coverage    45.97%   45.99%   +0.02%     
===========================================
  Files          228      228              
  Lines        14447    14447              
  Branches       917      917              
===========================================
+ Hits          6642     6645       +3     
+ Misses        7805     7802       -3     

Impacted Files	Coverage Δ
...in/scala/com/twitter/logging/QueueingHandler.scala	93.93% <0.00%> (-6.07%)	:arrow_down:
util-core/src/main/scala/com/twitter/io/Buf.scala	91.95% <0.00%> (-0.21%)	:arrow_down:
...core/src/main/scala/com/twitter/util/Promise.scala	78.71% <0.00%> (+0.40%)	:arrow_up:
...ore/src/main/scala/com/twitter/util/Duration.scala	85.54% <0.00%> (+0.57%)	:arrow_up:
.../src/main/scala/com/twitter/util/Cancellable.scala	75.00% <0.00%> (+50.00%)	:arrow_up:

---

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 10c0bc9...f810905. Read the comment docs.

[codecov-io]

Codecov Report

Merging #268 into develop will increase coverage by 0.01%. The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #268      +/-   ##
===========================================
+ Coverage    45.98%   45.99%   +0.01%     
===========================================
  Files          228      228              
  Lines        14447    14447              
  Branches       884      917      +33     
===========================================
+ Hits          6643     6645       +2     
+ Misses        7804     7802       -2

Impacted Files	Coverage Δ
...in/scala/com/twitter/logging/QueueingHandler.scala	93.93% <0%> (-6.07%)	:arrow_down:
util-core/src/main/scala/com/twitter/io/Buf.scala	91.95% <0%> (-0.21%)	:arrow_down:
...core/src/main/scala/com/twitter/util/Promise.scala	78.71% <0%> (+0.4%)	:arrow_up:
.../src/main/scala/com/twitter/util/Cancellable.scala	75% <0%> (+50%)	:arrow_up:

---

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 4a61c2e...f810905. Read the comment docs.

[caniszczyk]

Codecov Report

Merging #268 into develop will increase coverage by 0.04%. The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #268      +/-   ##
===========================================
+ Coverage    45.97%   46.01%   +0.04%     
===========================================
  Files          228      228              
  Lines        14447    14447              
  Branches       917      917              
===========================================
+ Hits          6642     6648       +6     
+ Misses        7805     7799       -6     

Impacted Files	Coverage Δ
...in/scala/com/twitter/logging/QueueingHandler.scala	93.93% <0.00%> (-6.07%)	:arrow_down:
...core/src/main/scala/com/twitter/util/Promise.scala	78.71% <0.00%> (+0.40%)	:arrow_up:
...ore/src/main/scala/com/twitter/util/Duration.scala	85.54% <0.00%> (+0.57%)	:arrow_up:
.../src/main/scala/com/twitter/util/Cancellable.scala	100.00% <0.00%> (+75.00%)	:arrow_up:

---

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 10c0bc9...471fbe4. Read the comment docs.

[codecov-io]

Codecov Report

Merging #268 into develop will decrease coverage by 0.07%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           develop    #268      +/-   ##
==========================================
- Coverage    45.98%   45.9%   -0.08%     
==========================================
  Files          228     227       -1     
  Lines        14447   14389      -58     
  Branches       884     915      +31     
==========================================
- Hits          6643    6605      -38     
+ Misses        7804    7784      -20

Impacted Files	Coverage Δ
...in/scala/com/twitter/logging/QueueingHandler.scala	93.93% <0%> (-6.07%)	:arrow_down:
.../src/main/scala/com/twitter/util/ThriftCodec.scala	80% <0%> (-5.72%)	:arrow_down:
.../main/scala/com/twitter/util/logging/Logging.scala	26.31% <0%> (-1.89%)	:arrow_down:
...l-zk/src/main/scala/com/twitter/zk/Connector.scala	61.29% <0%> (-1.21%)	:arrow_down:
...c/main/scala/com/twitter/logging/FileHandler.scala	83.15% <0%> (-0.85%)	:arrow_down:
...ore/src/main/scala/com/twitter/util/Duration.scala	84.97% <0%> (-0.58%)	:arrow_down:
util-core/src/main/scala/com/twitter/io/Buf.scala	91.75% <0%> (-0.42%)	:arrow_down:
util-zk/src/main/scala/com/twitter/zk/ZNode.scala	83.76% <0%> (-0.24%)	:arrow_down:
.../src/main/scala/com/twitter/concurrent/Spool.scala	80.43% <0%> (-0.22%)	:arrow_down:
util-app/src/main/scala/com/twitter/app/App.scala	91.75% <0%> (-0.09%)	:arrow_down:
... and 15 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 4a61c2e...1ad3a15. Read the comment docs.

[caniszczyk]

Codecov Report

Merging #268 into develop will increase coverage by 0.03%. The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #268      +/-   ##
===========================================
+ Coverage    45.97%   46.00%   +0.03%     
===========================================
  Files          228      228              
  Lines        14447    14447              
  Branches       917      917              
===========================================
+ Hits          6642     6647       +5     
+ Misses        7805     7800       -5     

Impacted Files	Coverage Δ
...in/scala/com/twitter/logging/QueueingHandler.scala	93.93% <0.00%> (-6.07%)	:arrow_down:
util-core/src/main/scala/com/twitter/io/Buf.scala	91.95% <0.00%> (-0.21%)	:arrow_down:
...core/src/main/scala/com/twitter/util/Promise.scala	79.11% <0.00%> (+0.80%)	:arrow_up:
.../src/main/scala/com/twitter/util/Cancellable.scala	100.00% <0.00%> (+75.00%)	:arrow_up:

---

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 10c0bc9...1ad3a15. Read the comment docs.

[codecov-io]

Codecov Report

Merging #268 into develop will increase coverage by 0.04%. The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #268      +/-   ##
===========================================
+ Coverage    45.98%   46.03%   +0.04%     
===========================================
  Files          228      228              
  Lines        14447    14447              
  Branches       884      917      +33     
===========================================
+ Hits          6643     6650       +7     
+ Misses        7804     7797       -7

Impacted Files	Coverage Δ
util-core/src/main/scala/com/twitter/io/Buf.scala	91.95% <0%> (-0.21%)	:arrow_down:
...core/src/main/scala/com/twitter/util/Promise.scala	79.11% <0%> (+0.8%)	:arrow_up:
.../src/main/scala/com/twitter/util/Cancellable.scala	100% <0%> (+75%)	:arrow_up:

---

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 4a61c2e...55e6a21. Read the comment docs.

[caniszczyk]

Codecov Report

Merging #268 into develop will increase coverage by 0.05%. The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #268      +/-   ##
===========================================
+ Coverage    45.97%   46.03%   +0.05%     
===========================================
  Files          228      228              
  Lines        14447    14447              
  Branches       917      917              
===========================================
+ Hits          6642     6650       +8     
+ Misses        7805     7797       -8     

Impacted Files	Coverage Δ
util-core/src/main/scala/com/twitter/io/Buf.scala	91.95% <0.00%> (-0.21%)	:arrow_down:
...ore/src/main/scala/com/twitter/util/Duration.scala	85.54% <0.00%> (+0.57%)	:arrow_up:
...core/src/main/scala/com/twitter/util/Promise.scala	79.11% <0.00%> (+0.80%)	:arrow_up:
.../src/main/scala/com/twitter/util/Cancellable.scala	100.00% <0.00%> (+75.00%)	:arrow_up:

---

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 10c0bc9...55e6a21. Read the comment docs.

[codecov-io]

Codecov Report

Merging #268 into develop will increase coverage by 0.04%. The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #268      +/-   ##
===========================================
+ Coverage    45.98%   46.03%   +0.04%     
===========================================
  Files          228      228              
  Lines        14447    14447              
  Branches       884      917      +33     
===========================================
+ Hits          6643     6650       +7     
+ Misses        7804     7797       -7

Impacted Files	Coverage Δ
util-core/src/main/scala/com/twitter/io/Buf.scala	91.95% <0%> (-0.21%)	:arrow_down:
...core/src/main/scala/com/twitter/util/Promise.scala	79.11% <0%> (+0.8%)	:arrow_up:
.../src/main/scala/com/twitter/util/Cancellable.scala	100% <0%> (+75%)	:arrow_up:

---

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 4a61c2e...55e6a21. Read the comment docs.

[muachilin]

Is there anything that I need to modify in the tests for Cancellable? Thank you!

[ryanoneill]

Hi @muachilin, would you mind rebasing off of the latest master? If so, then I can get this merged in internally. Thanks.

[muachilin]

Okay!

[caniszczyk]

Codecov Report

Merging #268 into develop will not change coverage by %. The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #268   +/-   ##
========================================
  Coverage    46.04%   46.04%           
========================================
  Files          228      228           
  Lines        14443    14443           
  Branches       922      922           
========================================
  Hits          6651     6651           
  Misses        7792     7792           

Impacted Files	Coverage Δ
...in/scala/com/twitter/logging/QueueingHandler.scala	93.93% <0.00%> (-6.07%)	:arrow_down:
...ore/src/main/scala/com/twitter/util/Duration.scala	84.97% <0.00%> (-0.58%)	:arrow_down:
util-core/src/main/scala/com/twitter/io/Buf.scala	91.95% <0.00%> (-0.42%)	:arrow_down:
...core/src/main/scala/com/twitter/util/Promise.scala	78.36% <0.00%> (-0.41%)	:arrow_down:
.../src/main/scala/com/twitter/util/Cancellable.scala	100.00% <0.00%> (+75.00%)	:arrow_up:

---

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 2b0b3dc...8a6a176. Read the comment docs.

[codecov-io]

Codecov Report

Merging #268 into develop will not change coverage. The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #268   +/-   ##
========================================
  Coverage    46.04%   46.04%           
========================================
  Files          228      228           
  Lines        14443    14443           
  Branches       922      922           
========================================
  Hits          6651     6651           
  Misses        7792     7792

Impacted Files	Coverage Δ
...in/scala/com/twitter/logging/QueueingHandler.scala	93.93% <0%> (-6.07%)	:arrow_down:
...ore/src/main/scala/com/twitter/util/Duration.scala	84.97% <0%> (-0.58%)	:arrow_down:
util-core/src/main/scala/com/twitter/io/Buf.scala	91.95% <0%> (-0.42%)	:arrow_down:
...core/src/main/scala/com/twitter/util/Promise.scala	78.36% <0%> (-0.41%)	:arrow_down:
.../src/main/scala/com/twitter/util/Cancellable.scala	100% <0%> (+75%)	:arrow_up:

---

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 2b0b3dc...8a6a176. Read the comment docs.

[caniszczyk]

Codecov Report

Merging #268 into develop will increase coverage by 0.00%. The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #268   +/-   ##
========================================
  Coverage    46.04%   46.05%           
========================================
  Files          228      228           
  Lines        14443    14443           
  Branches       922      922           
========================================
+ Hits          6651     6652    +1     
+ Misses        7792     7791    -1     

Impacted Files	Coverage Δ
...in/scala/com/twitter/logging/QueueingHandler.scala	93.93% <0.00%> (-6.07%)	:arrow_down:
util-core/src/main/scala/com/twitter/io/Buf.scala	91.95% <0.00%> (-0.42%)	:arrow_down:
...core/src/main/scala/com/twitter/util/Promise.scala	78.36% <0.00%> (-0.41%)	:arrow_down:
.../src/main/scala/com/twitter/util/Cancellable.scala	100.00% <0.00%> (+75.00%)	:arrow_up:

---

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 2b0b3dc...8a6a176. Read the comment docs.

[ryanoneill]

Sorry, I meant to say 'develop' branch, not 'master'. Also, it seems like there are still some old commits in the pull request beyond the 'CancellableTest' one which is the problem. The goal is to have the patch url be just your test change here, otherwise it won't apply cleanly internally. When that's the case we can merge it in and make sure you get proper attribution. Sorry for the hoops to jump through.

[muachilin]

I see. Can I create another branch for simply the "Cancellable Test"? Since my current branch has other commits (Improve test coverage in StorageUnit)... Sorry!! I'll close this PR and open another one instead.

I opened the new PR #270