4844: blob encoding version 2#8827
Conversation
bbacb98 to
a58011d
Compare
7871335 to
7ae1140
Compare
7ae1140 to
527335f
Compare
527335f to
cdd2ba8
Compare
trianglesphere
left a comment
There was a problem hiding this comment.
The parsing looks pretty good to me & I like the round trip tests. One class of tests I'd like to see is a one way test or test vectors. i.e. Given a known input data, we should expect it to yield a known output. We should check both ways (i.e. given some data, expect a blob output & given a blob, expect some data), but it's nice to have the specific output which can manually be verified.
OK to add test vectors as a followup PR? I created an issue for it to make sure it doesn't get dropped and added a TODO referencing it in blob_test.go. |
cdd2ba8 to
0399cc5
Compare
- use testify/require for testing - improve big blob test case - some performance improvements
0399cc5 to
b1ad019
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #8827 +/- ##
===========================================
- Coverage 34.85% 34.81% -0.05%
===========================================
Files 165 165
Lines 7106 7106
Branches 1198 1198
===========================================
- Hits 2477 2474 -3
- Misses 4477 4482 +5
+ Partials 152 150 -2
Flags with carried forward coverage won't be shown. Click here to find out more. |
trianglesphere
left a comment
There was a problem hiding this comment.
Doing 1 way tests is ok as a follow up
Description
This is the implementation based on the 2nd version of blob encoding spec.
Tests
Tested small and large data inputs, as well as edge cases.
Metadata