search

beetbox / audioread

cross-library (GStreamer + Core Audio + MAD + FFmpeg) audio decoding for Python
MIT License
483 stars 108 forks source link

Add a simple testsuite #79

Closed ssssam closed 5 years ago

ssssam commented 5 years ago

I wrote a simple testsuite using py.test while investigating https://github.com/beetbox/audioread/pull/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.

ssssam commented 5 years ago

Now I spot that https://github.com/beetbox/audioread/pull/73/ also contains a test suite, which looks more comprehensive than this one.

sampsyo commented 5 years ago

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.

ssssam commented 5 years ago

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 ?

sampsyo commented 5 years ago

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).

ssssam commented 5 years ago

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.

sampsyo commented 5 years ago

Looks great; thanks!! I'll merge this now.