-
Notifications
You must be signed in to change notification settings - Fork 108
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
Add a simple testsuite #79
Conversation
55756eb
to
f7025ac
Compare
Now I spot that #73 also contains a test suite, which looks more comprehensive than this one. |
Awesome! These tests look great already—but you're right; it looks like we let that other PR languish. I don't see any outstanding issues with that PR, so maybe we should merge that one and try to unify the two test suites. |
I've looked at PR #73 and found several issues: the seeking support is incomplete, the tests don't pass for me, and the commit history is a bit messed up. I could make a new PR with the test suite from #73 minus the 'seeking' feature that's not finished. The main difference from this branch is that it uses the Python 'unittest' module rather than Pytest. Also, it has a tool to generate a wav file and assert in the tests that the data is read correctly from the file. It will take some time to get this working though as the expected results don't seem to match the generated file. I'm not actually sure if this kind of test will work on different platforms as I don't know if different versions of an MP3 decoder are guaranteed to return the same bit stream as each other ... I'm not interested in the 'seeking' feature, but I am willing to do a bit more work to ensure the project has a test suite. I could do a new branch using the 'unittest' tests from #73 as a base, but without the parts that don't work ... or we could merge this branch now and ask @pconerly to rebase his branch on top of master if/when he gets a chance to revisit. What's your preference @sampsyo ? |
Got it. Thanks for investigating. Let's forge ahead with merging a test suite and worry about seeking separately. I think starting with files on disk (instead of generating them) seems simplest, especially if we can keep the files small! Let's do that. As for unittest vs. py.test: I typically opt for the built-in stdlib module, which makes it easy to run the tests without any dependencies using any old test runner (e.g., nose), but I don't have any objection to py.test if that's what you're more familiar with. One thing that would be great to steal form #79 is the tox configuration, which makes it easy to run the tests without worrying about dependencies (and across different Python versions). |
The testsuite can be run simply by executing 'pytest' or 'pytest-3' from the top directory of the project.
This makes it possible to integrate Py.test with setup.py.
This allows running the test suite with Python 2.6 and Python 3.6, with dependencies automatically installed, just by running `tox`.
I happen to prefer py.test, I find the test code a lot easier to read. If you're happy with that then let's go with this branch for a test suite. I've added a Tox configuration; I have left the Travis CI config running pytest directly because it seems to already manage multiple versions of Python. |
Looks great; thanks!! I'll merge this now. |
I wrote a simple testsuite using py.test while investigating #78.
Currently it only tests the audioread.audio_open() method. I'd like to add a separate set of tests for each backend, as I think there are some bugs in both the GStreamer and libmad backends that are causing everything to end up being run through ffmpeg. I don't know if I'll get time for that though.