Moar tests #100
Moar tests #100
Conversation
Also move the caught exceptions around os.unlink() to be more explicit
If you set whisper.CACHE_HEADERS to True and run whisper.info to cache the headers, the cached version will always be used even if whisper.CACHE_HEADERS is still used.
Once the coverage is a bit more up to spec, I can actually refactor some of these bits and still sleep at night.
This was removed when I rebase removed my py3k support changes as @brutasse opened graphite-project#70 long before I had this all ready to merge.
This ups the test coverage to 86% for whisper.py
This lets us track the improvements in test coverage this branch
brings see this website for more information:
https://coveralls.io/r/graphite-project/whisper
|
I also setup the bits for coveralls.io to measure our coverage for whisper. We can use whisper as a test bed and if people like it, we can enable it for the other repos: cc: @graphite-project/committers |
| info = __headerCache.get(fh.name) | ||
| if info: | ||
| return info | ||
| if CACHE_HEADERS: |
There was a problem hiding this comment.
FYI the code was buggy before. If you set CACHE_HEADERS=True, ran whisper.info() on a whisper file, set CACHE_HEADERS=False, and then used whisper.resize() or something to change the archive in the same python process (like a test that re-uses the same archive name over multiple tests), it will always and only use the cached version.
That behavior is blatantly broken as you shouldn't use the cache if CACHE_HEADERS is False. This was to prevent it from happening again.
This is for setAggregationMethod
Unlike unittest.TestCase.assertRaises, this context manager will also validate the contents of the raised exception and not just the type. There are places in the whisper codebase where an InvalidConfiguration or CorruptWhisperFile exception is raised with different values. This ensures we are getting the expected exceptions when we think we are.
This ensures that when we are testing for an exception, it is in the code path we think it is hitting.
For more context see this minimal reproducer: https://gist.github.com/SEJeff/891fb08dc908ec58a6cb This bug is total bs
|
@SEJeff is this still ok to merge or should we wait on more? |
|
Good to go for now sir! I was waiting for 1 other person to ack it and preferably merge it |
So I started working on py3k support, but since I was re-reminded of @brutasse's #70, I rebased and deleted those commits. Additionally, due to #98, which I also did, I rebase deleted those commits as @amosshapira took care of that first :)
Anyways, this brings up the total test coverage of whisper.py up from 67 --> 88%. I'll be slowly continuing to update this, but it can be merged now.