Re-factor tests

[mgeier]

The new py.test-based tests are great, but the fixtures seem overly complicated given the claim of py.test to be low-overhead.

There should be a fixture for a read-only file, parametrized with three options, returning either a filename, a file descriptor or a file-like object.
Same for write-only.
For 'rw' we might need two separate groups: "existing file" and "new file".
These can then be used in further fixtures which open a pysoundfile.SoundFile.

So far, this is no problem with py.test.
But it would be really useful to have a way to combine two or more of them into a new fixture.
For example, it would be nice to have a new fixture (called "readable" or similar) that combines the read-only fixture (three parameters) with the "existing 'rw' file" fixture (three parameters), thus leading to six test cases in total.

Sadly, I couldn't come up with a way to implement this.

I asked on stackoverflow, but no response until now: http://stackoverflow.com/questions/24340681/how-to-concatenate-several-parametrized-fixtures-into-a-new-fixture-in-py-test

Any ideas how this could be done?

Other possible improvements:

• use tmpdir when writing files (http://pytest.org/latest/tmpdir.html)
• don't open the test WAV file in 'rw' mode, make a copy first
• use yield_fixture for closing files and other cleanup (http://pytest.org/latest/yieldfixture.html)

[bastibe]

This is related to #48, where the py.test-based test bench was initially written.

I totally agree that the fixtures are very complicated. Over the course of developing the tests, I tried several different approaches, none of which looked particularly appealing.

• use tmpdir when writing files (http://pytest.org/latest/tmpdir.html)

I think that it is actually nicer to have the files lying around in the test directory. That way, it is easier to see when something goes wrong and wrongly created files are easier to examine. What advantage would there be in using a temp directory instead?

• don't open the test WAV file in 'rw' mode, make a copy first

Very true. It can really mess with your test cases if one test overwrites data for the next test case. The downside is that copying slows down testing some. Maybe we should think about separating the test bench into a fast set and a complete set?

• use yield_fixture for closing files and other cleanup (http://pytest.org/latest/yieldfixture.html)

I think that yield_fixture might actually be a solution for our main problem. In particular, we would need a yield-fixture that can yield more than once, and then just yield from several generators. However, I did not use the yield_fixture initially, since it is still marked as experimental. I am very much hoping that they will implement a proper generator/multi-yield fixture soon.

[mgeier]

What advantage would there be in using a temp directory instead?

The tests would also work if the user doesn't have write permissions to the test directory.
If something goes wrong, the test directory wouldn't be polluted with garbage files (what you see as an advantage).

I tried to implement this, but it is actually quite a hassle to get all those paths right ... so I stopped trying.

I still think it would be a good feature, but probably not worth the effort.
We can re-visit this later if we feel like it.

The downside is that copying slows down testing some.

When I tried it, it changed the running time from 1.4x to 1.5x seconds, making it 0.1 seconds (0.2 seconds max) slower.
I think we can live with that.

Initially I wanted to make the copy also in a tmpdir, but as mentioned above, this is quite hairy.

Maybe we should think about separating the test bench into a fast set and a complete set?

Sure, at some point we could do that, but for now there is no need.

In particular, we would need a yield-fixture that can yield more than once

Currently, only one value can be yielded.
It is possible that multiple values will be supported some time in the future, but I wouldn't hold my breath.

... it is still marked as experimental

It didn't change in version 2.5, so it will probably stay.
If not, it's easy to revert to a normal fixture.

I pushed 2 commits, what do you think about them?

[bastibe]

Looks good.

I still not sure about using floating point wave files. Most audio programs won't even open a floating point wave file. But I guess we'll have to trust libsndfile to do the right thing here.

On a different note, I think we should come up with a canonical shorthand for pysoundfile. I guess sf works well. Do yo know of any popular package this might collide with?

[mgeier]

I still not sure about using floating point wave files. Most audio programs won't even open a floating point wave file. But I guess we'll have to trust libsndfile to do the right thing here.

Yes. We should try to test our code, not libsndfile's.

I think it's just much easier to handle exact floating point values instead of values that are somehow "close". It's also much nicer to have 1.0 in an assertion error than 0.99996948.

Don't you think the comparisons in a99ac16 are much simpler now?

And although I know that 32-bit float WAVs are kinda seldomly used, I think they should be used more often.

And I changed the mono file to PCM_16, so we also use the ubiquitous subtype.

On a different note, I think we should come up with a canonical shorthand for pysoundfile. I guess sf works well.

I like that.
I think it's quite obvious.
snd would be another possibility, but I like sf more (I like the sound of it and it's shorter).

Are there even any other real possibilities?

The situation is less obvious with PySoundCard, probably we should open an issue there?

Anyway, I also think it's important that we agree on a canonical shorthand and use that in all example code and documentation (and of course tests).

Do yo know of any popular package this might collide with?

I didn't see any conflicts yet.

A quick search brings up pySFML: http://www.python-sfml.org/

They have their own audio file handling, so I guess this isn't really a problem.

[Whistler7]

I use the REAPER (reaper.fm) digital audio workstation (DAW) application regularly. I am sure it writes/renders 16-bit PCM, 24-bit PCM, 32-bit float and 64-bit float formats. I think I have also read 32-bit PCM files with REAPER. I use 44.1k, 48k, 88.2k, 96k, 176.4k and 192k sample rates. I would like to read and write all these formats with PySoundFile.

REAPER may be useful for testers of PySoundFile since REAPER has a free evaluation, and it uses the honor system (nag screen) to enforce the 60-day evaluation period. The 60 USD price for personal use is quite reasonable also.

Another interesting free app for testing is Wavosaur (wavosaur.com). However, I find that it doesn't play 32-bit PCM files or 192k sample rate.

Yet another interesting free app for testing is SoX (sox.sourceforge.net). It will read all the formats I mentioned above, but it will not write to 64-bit formats.

[mgeier]

@Whistler7: Thanks for the info about those applications!

libsndfile, and therefore PySoundFile, should be able to read and write all the mentioned formats.
If you should find out that any of them doesn't work, please tell us.

[bastibe]

snd would be another possibility, but I like sf more (I like the sound of it and it's shorter).

Also, pysoundfile will probably be frequently used together with pysoundcard. snd would be ambiguous in that context. Let's go with sf!

[mgeier]

I added a few commits.
I solved the problem with writing PCM in 'rw' mode by removing the whole test function.
I also added the check for consistent arguments that I announced earlier.

This is about everything I want to do in this PR.
Any more comments?

[bastibe]

Looks good. Just out of curiosity, did the number of tests or the test coverage change much during this PR?

[mgeier]

I tried to keep the assertions more or less exactly the same. I combined a few test functions to decrease test runtime, but I didn't (with very few exceptions) remove assertions. I turned the 157 test functions into 117.
I didn't find a counter for the number of assertions, though. Is there one?
I guess I could just grep through the file ...

I didn't reduce coverage, I added a few lines of coverage by testing the functions instead of the methods.
The coverage before:

Name          Stmts   Miss  Cover
---------------------------------
pysoundfile     313     65    79%

... and after:

Name          Stmts   Miss  Cover
---------------------------------
pysoundfile     313     50    84%

The test runtime varies quite a lot, but the average runtime should be shorter now.

[bastibe]

OK, looks great! Merge when your're ready!