bcoin
bcoin copied to clipboard
test: golomb filters
Adding a specific file for golomb filter implementation test.
I was made aware that there already is some test coverage for the compact filters implementation at the /test/block-test.js
file. But that test only takes care of the generation of the proper filters starting from the raw items.
What this does is the opposite: given a filter, we create an instance of the Golomb
class using the fromRaw()
function and then use its match()
function to make sure all of the provided scripts do match as expected.
I did not know there was some test already in place at the block-test.js
that's why I introduced my own test file. We could move this over there, bring that test case to this file, or discard this PR altogether if you don't think it is necessary.
Codecov Report
Merging #898 into master will increase coverage by
0.29%
. The diff coverage isn/a
.
@@ Coverage Diff @@
## master #898 +/- ##
==========================================
+ Coverage 61.72% 62.02% +0.29%
==========================================
Files 155 155
Lines 26058 26058
==========================================
+ Hits 16085 16163 +78
+ Misses 9973 9895 -78
Impacted Files | Coverage Δ | |
---|---|---|
lib/script/script.js | 80.04% <0%> (+0.07%) |
:arrow_up: |
lib/script/opcode.js | 76.98% <0%> (+0.83%) |
:arrow_up: |
lib/golomb/golomb.js | 62.06% <0%> (+22.06%) |
:arrow_up: |
lib/golomb/reader.js | 86.79% <0%> (+81.13%) |
: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 f991b0a...10aeb88. Read the comment docs.
This should be moved to be next to the existing block filter tests in test/block-test.js
?
Yeah, agreed with this being a part of block-test.js
.
What's up with this test PR? I moved all of the code to the block-tests.js
and applied the modifications as requested. Is there anything else you think should be done?
I think rebase and squash into a single commit.
@pinheadmz is this issue stale or does it need to be worked upon?
@masterchief164 I think we can close it, thanks