-
Notifications
You must be signed in to change notification settings - Fork 10
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
test: Add unit test to protocol module #165
Conversation
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.
Out of curiosity - do you see the test coverage bump up compared to here after this change?
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.
Yes. See the protocol.rs is now 100%.
Updated in the PR description.
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.
TODO : Add more coverage to the sections from report.
Also FYI, there's an open PR adding some testing to component.rs
- so no need to duplicate work there. Feel free to review that one: #127
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.
Thanks for pointing this out!
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.
Rust developer 🚀
@tanmayv25 @rmccorm4 Three of these tests don't do anything: They would only fail if Rust was broken. They effectively do Do you intend to enhance them in a follow-up commit? Otherwise we should remove them. |
@grahamking you mean the |
Yes the creation tests. They don't run any of our code. There is also only one way of creating an object in Rust, so there's no value in documenting how that's done. Can you make a PR to remove them? |
What does the PR do?
Adds the unit testing to protocol.rs module.
Selected it for simplicity from the report shared in #144 (comment)
TODO : Add more coverage to the sections from report.
Test coverage after adding this test.
Checklist
<commit_type>: <Title>
Commit Type:
Check the conventional commit type
box here and add the label to the github PR.