[Sealing] AssignmentCollectorTree unit tests
#1134
Conversation
…/5719-assignment-collector-unit-tests
…d mocks for approvals package
…/5719-assignment-collector-unit-tests
…/5719-assignment-collector-unit-tests
Codecov Report
@@ Coverage Diff @@
## master #1134 +/- ##
==========================================
+ Coverage 55.80% 56.17% +0.37%
==========================================
Files 484 484
Lines 29788 29807 +19
==========================================
+ Hits 16622 16743 +121
+ Misses 10897 10779 -118
- Partials 2269 2285 +16
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| // leveled forest doesn't treat this case as error, we shouldn't create collectors | ||
| // for vertexes lower that forest.LowestLevel | ||
| if vertex.Level() < t.forest.LowestLevel { | ||
| return nil, fmt.Errorf("could not add collector with height lower than the lowest level") | ||
| } |
There was a problem hiding this comment.
If so, then we could move it up right after the lock
There was a problem hiding this comment.
Agree with Leo:
to me it would also be more intuitive if this check happened right after
flow-go/engine/consensus/approvals/assignment_collector_tree.go
Lines 254 to 255 in 3210889
…/5719-assignment-collector-unit-tests
| // leveled forest doesn't treat this case as error, we shouldn't create collectors | ||
| // for vertexes lower that forest.LowestLevel | ||
| if vertex.Level() < t.forest.LowestLevel { | ||
| return nil, fmt.Errorf("could not add collector with height lower than the lowest level") | ||
| } |
There was a problem hiding this comment.
Agree with Leo:
to me it would also be more intuitive if this check happened right after
flow-go/engine/consensus/approvals/assignment_collector_tree.go
Lines 254 to 255 in 3210889
Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>
flow-go/issues/5719
Context:
This PR adds separate test suite for
AssignmentCollectorTree. Contains some refactoring of testing logic of approvals suite.