Skip to content
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

Improve tests #41

Closed
mgeier opened this issue May 27, 2014 · 4 comments
Closed

Improve tests #41

mgeier opened this issue May 27, 2014 · 4 comments

Comments

@mgeier
Copy link
Contributor

mgeier commented May 27, 2014

  • we shouldn't write a file with PySoundFile and then test reading it with PySoundFile. If there is a systematic error in reading and writing, we won't find it
  • we shouldn't write a file in each test case. It would be better to create one (or few) very small test file(s) that we ship with the repo (just a few hundred bytes). We should only write files when we're testing writing files.
  • we shouldn't fill the test file with copies of the same number. The samples should be different.
@mgeier
Copy link
Contributor Author

mgeier commented May 27, 2014

Apart from this, there also some formal problems:

$ pep8 test_pysoundfile.py 
test_pysoundfile.py:7:1: E302 expected 2 blank lines, found 1
test_pysoundfile.py:14:80: E501 line too long (84 > 79 characters)
test_pysoundfile.py:39:80: E501 line too long (83 > 79 characters)
test_pysoundfile.py:45:80: E501 line too long (85 > 79 characters)
test_pysoundfile.py:46:80: E501 line too long (84 > 79 characters)
test_pysoundfile.py:129:80: E501 line too long (81 > 79 characters)
test_pysoundfile.py:145:33: E231 missing whitespace after ','
test_pysoundfile.py:160:80: E501 line too long (84 > 79 characters)
test_pysoundfile.py:229:80: E501 line too long (80 > 79 characters)
test_pysoundfile.py:235:80: E501 line too long (84 > 79 characters)
test_pysoundfile.py:265:80: E501 line too long (85 > 79 characters)
test_pysoundfile.py:271:49: E231 missing whitespace after ','
test_pysoundfile.py:272:46: E231 missing whitespace after ','
test_pysoundfile.py:275:80: E501 line too long (94 > 79 characters)
test_pysoundfile.py:290:80: E501 line too long (84 > 79 characters)
test_pysoundfile.py:297:1: E302 expected 2 blank lines, found 1
test_pysoundfile.py:301:33: E231 missing whitespace after ','
test_pysoundfile.py:311:37: E231 missing whitespace after ','
test_pysoundfile.py:316:32: E231 missing whitespace after ','
test_pysoundfile.py:324:33: E231 missing whitespace after ','
test_pysoundfile.py:324:47: E261 at least two spaces before inline comment
test_pysoundfile.py:335:33: E231 missing whitespace after ','

@mgeier mgeier mentioned this issue May 27, 2014
@bastibe
Copy link
Owner

bastibe commented May 28, 2014

These are very valid points. Thank you!

@mgeier
Copy link
Contributor Author

mgeier commented May 29, 2014

I think the tests about precision are not really necessary because they only test libsndfile.
I think it would be better to check for exact equality without the PCM <--> float conversion.

So either make a float test file and check with float32/float64 or make a PCM test file and test with int16/int32.
Or both.

It would be probably best to make a floating point test file with values which are accurately representable in both decimal and binary.
Something like [1.0, 0.5, 0.0, -0.5, -1.0], probably?
In stereo this could be [[1.0, -1.0], [0.5, -0.5], [0.0, 0.0], [-0.5, 0.5], [-1.0, 1.0]].
I guess 5 frames would be enough for most test cases.

@bastibe
Copy link
Owner

bastibe commented Jun 17, 2014

I am closing this issue. Discussion continues in #48 .

@bastibe bastibe closed this as completed Jun 17, 2014
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

No branches or pull requests

2 participants