-
Notifications
You must be signed in to change notification settings - Fork 52
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
Add tests - storage + worker #926
Conversation
✅ Deploy Preview for conda-store canceled.
|
✅ Deploy Preview for conda-store ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good!
from conda_store_server._internal.worker.app import CondaStoreWorker | ||
|
||
|
||
def test_worker_with_valid_config(conda_store_config, tmp_path): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A docstring for each test with a one-liner saying what it's supposed to be testing would be useful.
For example, is this just checking whether a worker can be initialized? If so, I guess if worker.initialize()
runs without generating an error then we pass the test? But should we check that the configuration settings in the valid config are actually being met, e.g. that the log level, concurrency, and watch_path
is being set correctly, or is that out of scope?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example, is this just checking whether a worker can be initialized? If so, I guess if worker.initialize() runs without generating an error then we pass the test?
Yep, this test is meant to ensure that the worker can init without an error. So, that is exercising all the validation functions, in this case, just _validate_config_file
.
But should we check that the configuration settings in the valid config are actually being met, e.g. that the log level, concurrency, and watch_path is being set correctly, or is that out of scope?
Good idea! Added a test for that as well.
partially fixes #914
Description
This PR adds a few unit tests to try to improve the code coverage. Tests covered:
Pull request checklist