Improve code coverage by adding more tests #9
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9 +/- ##
==========================================
+ Coverage 89.96% 96.71% +6.75%
==========================================
Files 24 26 +2
Lines 947 1067 +120
==========================================
+ Hits 852 1032 +180
+ Misses 95 35 -60 ☔ View full report in Codecov by Sentry. |
pkhalaj
force-pushed
the
tests/improve-coverage
branch
from
b3e1ec6 to
8510179
Compare
This concerns two methods in the `ResponseError` class. This bug came to my attention while adding more tests!
This is needed as we are using `loguru` instead of the Python built-in logging package.
Our test is not spawning child processes.
This is to ensure that we are calling `MongoDB.close()` only if the client has been properly initialized.
This is to clarify that the return value cannot be readily used in the initialization of MongoDB. As a result, `test_recorder.py` has been updated accordingly with the new `make_test_app_config_as_dict()`.
pkhalaj
force-pushed
the
tests/improve-coverage
branch
from
ee52af0 to
7161505
Compare
A test will be added in the next commit.
We also improve the structure of previous tests in the module. In particular, we now assert the log messages and use parametrization.
Moreover, perform some minor refactoring of the tests in the module.
…)` and `MongoDB().get_database()`
TODO: These tests need to be improved so that they cover all the code paths.
…ing an exception
pkhalaj
force-pushed
the
tests/improve-coverage
branch
from
01d89c4 to
a6f136f
Compare
We have added `fastapi_app.py`. This simplifies the testing and also is aligned with separation of concerns.
This results in a correct code coverage for tests. As a result, `api_server_process_context()` is no longer needed and has been removed.
The former scheme is not recommended when designing REST APIs.
This led to the reintroduction of `api_server_process_context()` but it now lies in the `test_utils` module.
This concerns the case when the input to the function is a response.
pkhalaj
force-pushed
the
tests/improve-coverage
branch
from
fa710c7 to
27733f7
Compare
This mainly concerns invalid ObjectID values and non-existing documents.
mraspaud
reviewed
Jun 12, 2024
| @@ -46,18 +46,13 @@ def id_must_be_valid(id_like_string: str) -> ObjectId: | |||
| try: | |||
| return ObjectId(id_like_string) | |||
| except InvalidId as e: | |||
| raise ValueError from e | |||
| raise ValueError(f"{id_like_string} is not a valid value for an ID.") from e | |||
There was a problem hiding this comment.
The reason is that as per the requirements of Pydantic, one can either raise a ValueError or AssertionError in a custom validator. Here I have defined a custom validator for a MongoDB object ID. When it fails it raises InvalidId which is not a valid exception to signify validation failure in Pydantic, hence the need to catch the error and raise a different one.
Please let me know if my understanding of this is correct.
Reference: https://docs.pydantic.dev/latest/concepts/validators/#handling-errors-in-validators
There was a problem hiding this comment.
I have added a note on this to the validator's docstring.
mraspaud
reviewed
Jun 12, 2024
This concerns the internal function (auxiliary) in the `check_log()` fixture.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is mainly about the code coverage as mentioned in the title.
However, as a result of adding more tests, we might need to add some new testing utilities to the test_utils package and perform some minor refactoring.