Closed sbellem closed 7 years ago
Agreed that this is a buggy line of code, and also that this issue should require us to show test coverage for that codepath too.
It looks like this line of code should be run in any schedule where some node receives a message for the next block r+1
before determining that block r
has been committed. But that should happen easily under ordinary circumstances, so I'm surprised this isn't triggered by the multi-block test. https://github.com/amiller/HoneyBadgerBFT/blob/dev/test/test_honeybadger.py
Merging #27 into dev will increase coverage by
6.96%
. The diff coverage isn/a
.
@@ Coverage Diff @@
## dev #27 +/- ##
==========================================
+ Coverage 58.96% 65.92% +6.96%
==========================================
Files 18 18
Lines 943 942 -1
==========================================
+ Hits 556 621 +65
+ Misses 387 321 -66
Impacted Files | Coverage Δ | |
---|---|---|
honeybadgerbft/core/honeybadger.py | 97.08% <ø> (+10.54%) |
:arrow_up: |
honeybadgerbft/core/commoncoin.py | 87.17% <0%> (-10.26%) |
:arrow_down: |
honeybadgerbft/core/binaryagreement.py | 92.2% <0%> (+3.89%) |
:arrow_up: |
honeybadgerbft/core/commonsubset.py | 100% <0%> (+9.67%) |
:arrow_up: |
honeybadgerbft/core/reliablebroadcast.py | 88.48% <0%> (+17.98%) |
:arrow_up: |
honeybadgerbft/core/honeybadger_block.py | 93.33% <0%> (+46.66%) |
: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 cb69dbd...e6263da. Read the comment docs.
Please forgive if I am the one omitting a detail as it appeared to me that there is no variable
round
, butself.round
there is.round
being a built-in in Python the expression:should throw an
AssertionError
, e.g.:Since that part of the code is untested, a test should be added as part of this PR.