Add unit tests for storage module #446
Conversation
|
Thoughts? :) |
| # from carbon.storage import loadStorageSchemas | ||
| # from carbon.exceptions import CarbonConfigException | ||
| # with self.assertRaises(CarbonConfigException): | ||
| # loadStorageSchemas() |
There was a problem hiding this comment.
Please don't add large swaths of code that is commented out. Uncomment it or remove it.
There was a problem hiding this comment.
As I explained in the PR, I was hoping someone might be able to enlighten me regarding this mock persistence across tests. If not, sure I’ll remove it.
There was a problem hiding this comment.
Gotcha. I can look at it more tomorrow or this weekend
|
More tests are always great, although I'm not the biggest fan of having to do the imports in every test. DRY Please remove or fix the commented code and we will happen my merge this however. Thanks for doing this! |
I think I had to do this because of the mock persistence, although I am not entirely sure anymore. |
Add unit tests for storage module
|
@JeanFred Thanks that makes sense. Merged! Feel free to write more if you feel so inclined. Sorry this took so long to get merged |
|
Thanks for merging @SEJeff! What about the commented out code − shall we just leave it there until someone figures it out ? Or let’s remove it as it will probably rot otherwise ? |
As I was trying to track down either a bug in Carbon parsing or a mistake in my
storage-aggregation.conffile (for the story, it turned out my carbon deamon just had to be restarted :-°), I ended up writing this test suite for the storage module.The all
NoConfigSchemaLoadingTestclass, whose purpose is to test what happens whenCONF_DIRleads nowhere, is commented out though. For some reason, the mock ofcarbon.conf.settingspersists between classes. (if I testNoConfigSchemaLoadingTestby itself it works fine). I have fiddled around with trying toreload(carbon.conf)in setUp or tearDown but to no avail. Maybe someone will have a better idea? Otherwise I suppose it’s fine removing this test class alltogether.Hope that helps!