fix: augment PieceAggregateCommP to return the resulting aggregate size #18
Conversation
doing so here *massively* simplifies downstream code in lotus
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #18 +/- ##
=========================================
Coverage ? 78.12%
=========================================
Files ? 4
Lines ? 192
Branches ? 0
=========================================
Hits ? 150
Misses ? 27
Partials ? 15
|
commd_test.go
Outdated
| @@ -26,7 +26,7 @@ func TestGenerateUnsealedCID(t *testing.T) { | |||
|
|
|||
| expCommD := cidMustParse("baga6ea4seaqiw3gbmstmexb7sqwkc5r23o3i7zcyx5kr76pfobpykes3af62kca") | |||
|
|
|||
| commD, _ := commp.PieceAggregateCommP( | |||
| commD, _, _ := commp.PieceAggregateCommP( | |||
There was a problem hiding this comment.
I think you could work out the expected value here and add an assertion for it so we at least have some coverage
There was a problem hiding this comment.
@rvagg the codecov is incorrect: the thing is quite covered via https://github.com/filecoin-project/go-commp-utils/pull/18/files#diff-26d4fcf1ea0693620c557d21296e08c0517e901e2b01c8d4d93ac5ac5ffd4582R153
pushed https://github.com/filecoin-project/go-commp-utils/actions/runs/10259961154/job/28385226864 as proof
pushed an addition as requested as well: the test is based on an actual mainnert sector, so the size is expected at 32GiB
|
yeah, I'm fine with this, probably should bump 2.1.0 for this at least though; I would like a test to at least touch this though |
Arguably this is an API-breaking change. But: