Skip to content
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

wire: Add tests for BlockHeader (From)Bytes. #1600

Merged
merged 1 commit into from
Mar 18, 2019
Merged

Conversation

jozn
Copy link
Contributor

@jozn jozn commented Feb 9, 2019

This increase test coverage of BlockHeader.

In following days, I will increase the test coverage of wire. Some test suites need to be refactored to allow for error paths and wrong inputs, I will keep the test cases data but reorganize them and make them more structured this will allow for increase test coverage and easier understanding and covers more samples.
I will keep the PR short and separate for each Message.
After this, I will work with other packages and increase their test coverage and quality.

Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. In addition to the review comments, also please note that the commit title exceeds the maximum allowed length before the code contribution guidlines by 4 characters.

wire/blockheader_test.go Outdated Show resolved Hide resolved
wire/blockheader_test.go Outdated Show resolved Hide resolved
@davecgh davecgh added this to the 1.5.0 milestone Feb 13, 2019
@jozn
Copy link
Contributor Author

jozn commented Feb 13, 2019

Thanks Dave, I'll change the PR as requested and improve it.

@davecgh
Copy link
Member

davecgh commented Feb 15, 2019

Please let me know once this is updated.

@davecgh davecgh changed the title wire: Add test for BlockHeader Bytes() and FromBytes() wire: Add tests for BlockHeader (From)Bytes. Feb 15, 2019
@jozn jozn force-pushed the wire_header branch 3 times, most recently from f72a34d to acc4625 Compare February 18, 2019 14:12
@jozn
Copy link
Contributor Author

jozn commented Feb 18, 2019

Updated. Sorry I didn't see your comment in here.
At first, I wanted to improve this PR tests, for now I want to move to other packages and simultaneously add tests for other committed filters of wire.

@jozn jozn force-pushed the wire_header branch 3 times, most recently from c685cb5 to ee155b7 Compare February 20, 2019 10:23
Checking Bytes() and FromBytes() of BlockHeader works correctly.
@jozn
Copy link
Contributor Author

jozn commented Feb 20, 2019

This PR is finalized.

@davecgh davecgh merged commit f41b9fa into decred:master Mar 18, 2019
@jozn jozn deleted the wire_header branch March 18, 2019 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants