-
Notifications
You must be signed in to change notification settings - Fork 403
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
caliper-core
: add tests for Message
classes
#1561
Conversation
@aklenik Can you take a look |
On a quick glance, these seem ok to me, but really want @aklenik to take a look as he is more familiar with this part of the code I think |
}) | ||
}) | ||
|
||
describe("forRecipient", () => { |
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.
We need tests to show that it returns false as well
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.
My bad for missing this case! I have updated the test suite for the same. Thanks!
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.
One comment around a couple more tests being required, but these seem reasonable to me.
Signed-off-by: Eshaan Aggarwal <[email protected]>
Signed-off-by: Eshaan Aggarwal <[email protected]>
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.
Looks good to me, many thanks for the contribution
This PR introduces the tests for the base
Message
class incaliper-core
, and then introduces three similar test suites for the constructors of three types of messages. When approved, I can work on adding similar tests for all the message classes.Checklist
Issue/User story
Fixes part of #1557
Steps to Reproduce
Existing issues
Design of the fix
Validation of the fix
Automated Tests
What documentation has been provided for this pull request