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

tests: add varints_bitpatterns test #7849

Merged
merged 1 commit into from Apr 14, 2016

Conversation

Projects
None yet
5 participants
@laanwj
Member

laanwj commented Apr 9, 2016

The current tests for varint only check that serialization-deserialization is a roundtrip. That is a useful test, but it is also good to check for some exact bit patterns, to prevent a code change that changes the serialization format from going undetected (or a similar incompatibility between platforms).

As the varint functions are templated, also check with different types.

I wrote this test in the course of diagnosing #7848 and everything turned out to be good, but it makes sense anyhow.

@laanwj laanwj added the Tests label Apr 9, 2016

tests: add varints_bitpatterns test
The current tests for varint only check that
serialization-deserialization is a roundtrip. That is a useful test, but
it is also good to check for some exact bit patterns, to prevent a code
change that changes the serialization format from going undetected.

As the varint functions are templated, also check with different types.
@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Apr 9, 2016

Member

Indeed, makes sense. Concept ACK.

Member

MarcoFalke commented Apr 9, 2016

Indeed, makes sense. Concept ACK.

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Apr 11, 2016

Member

ACK 4521f00.
More tests are always good.

Member

jonasschnelli commented Apr 11, 2016

ACK 4521f00.
More tests are always good.

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Apr 12, 2016

Member

utACK 4521f00

Member

sipa commented Apr 12, 2016

utACK 4521f00

@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Apr 12, 2016

Contributor

ACK 4521f00

Contributor

paveljanik commented Apr 12, 2016

ACK 4521f00

@laanwj laanwj merged commit 4521f00 into bitcoin:master Apr 14, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Apr 14, 2016

Merge #7849: tests: add varints_bitpatterns test
4521f00 tests: add varints_bitpatterns test (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge #7849: tests: add varints_bitpatterns test
4521f00 tests: add varints_bitpatterns test (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #7849: tests: add varints_bitpatterns test
4521f00 tests: add varints_bitpatterns test (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Dec 20, 2017

Merge #7849: tests: add varints_bitpatterns test
4521f00 tests: add varints_bitpatterns test (Wladimir J. van der Laan)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment