New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
4844: blob encoding version 2 #8827
4844: blob encoding version 2 #8827
Conversation
bbacb98
to
a58011d
Compare
7871335
to
7ae1140
Compare
7ae1140
to
527335f
Compare
527335f
to
cdd2ba8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing 1 way tests is ok as a follow up
721a24d
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