-
Notifications
You must be signed in to change notification settings - Fork 338
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
chore: add tests for NewOverlayFromEthereumAddress method #4485
chore: add tests for NewOverlayFromEthereumAddress method #4485
Conversation
@notanatol I have reopened the PR with the suggested updates. |
NewOverlayFromEthereumAddress
NewOverlayFromEthereumAddress
NewOverlayFromEthereumAddress
thanks, but you don't have to re-open the PR, just push the fix commits to this branch, they are squashed anyway as we merge to master. |
…ss & made the suggested changes
t.Errorf("got address length %v, want %v", l, swarm.HashSize) | ||
} | ||
|
||
wantAddress, err := swarm.ParseHexAddress(tc.expectedAddress) |
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.
This step can be avoided by using swarm.MustParseHexAddress
function in the test table.
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.
only line 303 or from 299?
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.
because MustParseHexAddress
is not taking account of the length of the hash
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.
This is just for the tc.expectedAddress
.
added 4 test cases under
TestNewOverlayFromEthereumAddress
to testNewOverlayFromEthereumAddress
of crypto libraryChecklist
Description
Open API Spec Version Changes (if applicable)
Motivation and Context (Optional)
Related Issue (Optional)
Screenshots (if appropriate):