Add tests for config #212
Add tests for config #212
Conversation
|
It seems the automated GitHub tests fail because of the ctime comparisons. Putting an artificial micro sleep in the tests seemed like the easiest approach to ensure the ctime would be different if recreating the directories immediately but it seems tox doesn't agree and the sleep would need to be larger. |
| def test_clear_cache(self) -> None: | ||
| """Test clearing cache does delete and recreate cache directory.""" | ||
| self.assert_not_equal_ctime("cache_dir", config.clear_cache) | ||
|
|
||
| def test_clear_db(self) -> None: | ||
| """Test clearing database does delete and recreate database directory.""" | ||
| self.assert_not_equal_ctime("db_dir", config.clear_db) | ||
|
|
||
| def test_clear_data(self) -> None: | ||
| """Test clearing data does delete and recreate data directory.""" | ||
| self.assert_not_equal_ctime("data_dir", config.clear_data) |
There was a problem hiding this comment.
I get what you're doing here, but I think instead of testing for deletion/recreation a better test might be to check whether these directories are empty. i.e. stick a file in, then assert that the directory is empty after the clear_data.
The reasoning is that technically speaking, a clear_anything function ought to result in an empty directory. Whether that's done by deleting and recreating the directory, or by deleting every single file in a directory, is kinda irrelevant. The latter would result in the same desirable outcome but fail the test.
There was a problem hiding this comment.
That's what I meant to do at the start but the check for an empty directory doesn't actually work because data_dir is the parent to cache_dir and db_dir in the default config. That means they always get recreated before clear_data returns. I could do a recursive check for only empty directories in the tree starting at the directory of interest but that's a bit smelly to me.
So checking for empty makes sense in the case of db and cache but not data, and I figured since I needed a different check for data that I might as well use the same logic for the other two.
There was a problem hiding this comment.
I guess I could check for empty except for cache_dir and db_dir but this means that any time we add a default directory in the config, we need to check if it's inside another directory described in the config and if it is, we have to change the tests.
I'm not too happy with the ctime check either but I just didn't find any better option.
| import tempfile | ||
| import time | ||
| import typing | ||
| import unittest |
There was a problem hiding this comment.
Not really an issue but any particular reason to prefer unittest vs pytest? I've always used the latter but don't have a preference, just curious
There was a problem hiding this comment.
Same reason (and one fewer pip install or dependency).
Closes #211.
This PR provides a first batch of tests for the
Configclass and related functions.I've tried to handle most use cases but I'm more than happy to add other tests if there are things I forgot.