-
Notifications
You must be signed in to change notification settings - Fork 879
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
feat(ipld): integration test for namespace hasher #1256
feat(ipld): integration test for namespace hasher #1256
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1256 +/- ##
==========================================
- Coverage 56.16% 55.74% -0.42%
==========================================
Files 161 163 +2
Lines 9880 10027 +147
==========================================
+ Hits 5549 5590 +41
- Misses 3781 3880 +99
- Partials 550 557 +7
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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.
-
can we call
nmt_hasher_test.go
and we can just, in the docs forTestNamespaceHasher_CorruptedData
, explain that it's an integration test, etc. This will get run as part of unit testing though (unless we put it in swamp tests) -
another nit, I prefer to have test functions first and all mocks used in those test functions defined below
-
additionally noticed
SubNetNode
is dangling
…nd renaming test file
Co-authored-by: rene <41963722+renaynay@users.noreply.github.com>
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 test logic is legit. Only leaving some nits here
} | ||
|
||
func (b *CorruptBlock) String() string { | ||
return fmt.Sprintf("[Block %s]", b.Cid()) |
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.
return fmt.Sprintf("[Block %s]", b.Cid()) | |
return fmt.Sprintf("[Block %s]", b.Cid().String()) |
lights1, lights2 := make( | ||
[]*availability_test.TestNode, lightNodes/2), | ||
make([]*availability_test.TestNode, lightNodes/2) |
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.
Nit: Personally for me it is much harder to read code as 3 line like that than if it was 2 lines:
lights1, lights2 := make( | |
[]*availability_test.TestNode, lightNodes/2), | |
make([]*availability_test.TestNode, lightNodes/2) | |
lights1 := make([]*availability_test.TestNode, lightNodes/2) | |
lights2 := make([]*availability_test.TestNode, lightNodes/2) |
Closes #1249