twitter / util

Wonderful reusable code from Twitter
https://twitter.github.io/util
Apache License 2.0
2.68k stars 580 forks source link

util-core: improve test coverage for method parse in Duration #265

Closed krisgun closed 4 years ago

krisgun commented 4 years ago

Problem

The way it is currently implemented, the test will pass after the first NumberFormatException for the string "". The rest of the strings are therefore not tested.

Solution

Moving the assertion to within the foreach will ensure that all strings in the Seq are tested.

Result

Improves code coverage in parse. Two additional branches in the code are now checked.

claassistantio commented 4 years ago

CLA assistant check
All committers have signed the CLA.

caniszczyk commented 4 years ago

Codecov Report

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

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #265      +/-   ##
===========================================
+ Coverage    45.87%   45.90%   +0.02%     
===========================================
  Files          228      228              
  Lines        14436    14438       +2     
  Branches       893      881      -12     
===========================================
+ Hits          6623     6628       +5     
+ Misses        7813     7810       -3     
Impacted Files Coverage Δ
...core/src/main/scala/com/twitter/util/Promise.scala 78.31% <0.00%> (ø) :arrow_up:
util-app/src/main/scala/com/twitter/app/App.scala 91.83% <0.00%> (+0.17%) :arrow_up:
util-core/src/main/scala/com/twitter/io/Buf.scala 92.21% <0.00%> (+0.21%) :arrow_up:
...ore/src/main/scala/com/twitter/util/Duration.scala 85.54% <0.00%> (+1.15%) :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 ccfb944...2235173. Read the comment docs.

codecov-io commented 4 years ago

Codecov Report

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

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #265      +/-   ##
===========================================
+ Coverage    45.87%   45.89%   +0.02%     
===========================================
  Files          228      228              
  Lines        14436    14438       +2     
  Branches       893      881      -12     
===========================================
+ Hits          6623     6627       +4     
+ Misses        7813     7811       -2
Impacted Files Coverage Δ
...core/src/main/scala/com/twitter/util/Promise.scala 77.91% <0%> (-0.41%) :arrow_down:
util-app/src/main/scala/com/twitter/app/App.scala 91.83% <0%> (+0.17%) :arrow_up:
util-core/src/main/scala/com/twitter/io/Buf.scala 92.21% <0%> (+0.21%) :arrow_up:
...ore/src/main/scala/com/twitter/util/Duration.scala 85.54% <0%> (+1.15%) :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 ccfb944...2235173. Read the comment docs.

hamdiallam commented 4 years ago

nice catch

yufangong commented 4 years ago

thanks @krisgun , this is merged at https://github.com/twitter/util/commit/b99fe186391d47876738173bfdb8124866b1b418.