Skip to content

Add unit tests for CRC and BitReader#63

Merged
erikd merged 5 commits into
xiph:masterfrom
enzo1982:tests
May 20, 2018
Merged

Add unit tests for CRC and BitReader#63
erikd merged 5 commits into
xiph:masterfrom
enzo1982:tests

Conversation

@enzo1982

@enzo1982 enzo1982 commented May 19, 2018

Copy link
Copy Markdown
Contributor

This PR adds unit tests for CRC calculation and for the BitReader. See PR #57 for reference.

The CRC tests verify CRC8 and CRC16 calculation against simple reference CRC implementations.
The BitReader test includes code to test unaligned small reads (less than a word) that would trigger the bug found by @lvqcl, fixed in commit d57fb5c in PR #57.

In addition, obsolete CRC8 functions are removed (commit 8cebcf7) and some checks and messages in the BitWriter test are fixed (commit d93eec3).

@enzo1982

enzo1982 commented May 19, 2018

Copy link
Copy Markdown
Contributor Author

The Travis check was terminated because it ran for too long. Everything looked good up to that point though. Is this a configurable timeout or a hard limit?

Do you know if there a way to restart it without pushing new code? For now, I think I'll just try my luck by pushing the last commit again.

Update:

It timed out again. Unfortunately, the timeout for Travis jobs on public repositories seems to be fixed at 50 minutes. See https://docs.travis-ci.com/user/customizing-the-build#Build-Timeouts for reference.

Any idea how to work around this? Running the 32 and 64 bit words configurations in separate jobs should help, but I don't know how to do it or if it's even possible.

Update 2:

I found a way to run the configurations in separate jobs and modified .travis.yml accordingly. Now building...

Comment thread .travis.yml

- os: linux
compiler: gcc
env: CONFIGURE_OPTS=--enable-64-bit-words

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's a nice solution to this issue. Thanks!

@erikd

erikd commented May 20, 2018

Copy link
Copy Markdown
Member

Very useful contribution. Thanks!

@erikd erikd merged commit fed5ad4 into xiph:master May 20, 2018
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