bcoin icon indicating copy to clipboard operation
bcoin copied to clipboard

test: golomb filters

Open bilthon opened this issue 5 years ago • 5 comments

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.

bilthon avatar Oct 29 '19 18:10 bilthon

Codecov Report

Merging #898 into master will increase coverage by 0.29%. The diff coverage is n/a.

Impacted file tree graph

@@            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.

codecov-io avatar Oct 29 '19 23:10 codecov-io

This should be moved to be next to the existing block filter tests in test/block-test.js?

braydonf avatar Oct 31 '19 19:10 braydonf

Yeah, agreed with this being a part of block-test.js.

tuxcanfly avatar Nov 01 '19 16:11 tuxcanfly

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?

bilthon avatar Dec 03 '19 14:12 bilthon

I think rebase and squash into a single commit.

braydonf avatar Dec 03 '19 21:12 braydonf

@pinheadmz is this issue stale or does it need to be worked upon?

masterchief164 avatar Nov 30 '22 07:11 masterchief164

@masterchief164 I think we can close it, thanks

pinheadmz avatar Nov 30 '22 19:11 pinheadmz