Fix memory leak #979
Fix memory leak #979
Conversation
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
|
|
||
| def __contains__(self, item): | ||
| view_no, pp_seq_no = item | ||
| return pp_seq_no in self._batches[view_no] |
There was a problem hiding this comment.
What if viewno is not in self._batches?
There was a problem hiding this comment.
Then empty IntervalList will be returned, since self._batches is defaultdict(IntervalList). So, nothing bad here
There was a problem hiding this comment.
Actually this scenario is covered by test: https://github.com/hyperledger/indy-plenum/pull/979/files#diff-4eecd62b97800f596d964dc888f71cd4R132
| assert 3 not in intervals | ||
|
|
||
|
|
||
| def test_interval_list_can_have_multiple_blocks(intervals: IntervalList): |
There was a problem hiding this comment.
Please add more tests:
- add the same value multiple times
- add more tests for adding in different order, for example:
-- 2,1,4,3
-- 1,2,4,5,6,3
There was a problem hiding this comment.
Actually such cases are covered by a bit higher level randomized tests of OrderedTracker
There was a problem hiding this comment.
Also, it might be beneficial to employ Hypothesis framework (https://hypothesis.readthedocs.io/en/latest/) here.
…d tests Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
| return | ||
|
|
||
| for prev, next in zip(self._intervals, self._intervals[1:]): | ||
| if item == prev[1] + 1: |
There was a problem hiding this comment.
It would be great to have deterministic unit test for every if-else in add method
| @@ -92,6 +92,89 @@ def handleSync(self, msg: Any) -> Any: | |||
| self.replica.node.reportSuspiciousNodeEx(ex) | |||
|
|
|||
|
|
|||
| class IntervalList: | |||
There was a problem hiding this comment.
Cab we use IntervalTree (we already have this dependency) instead?
No description provided.