Feature/add smoke tests api db#30
Conversation
Signed-off-by: Prateek Kumar <85689959+PrateekKumar1709@users.noreply.github.com>
Signed-off-by: Prateek Kumar <85689959+PrateekKumar1709@users.noreply.github.com>
Signed-off-by: Prateek Kumar <85689959+PrateekKumar1709@users.noreply.github.com>
fearnworks
left a comment
There was a problem hiding this comment.
Some issues that need to be addressed for the api test before merging. But looks mostly good.
modules/odr_api/Taskfile.yml
Outdated
| - cmd: python -m debugpy --listen 0.0.0.0:5678 --wait-for-client ./modules/odr_api/odr_api/main.py | ||
| platforms: [windows, linux, darwin] | ||
|
|
||
| test_api: |
There was a problem hiding this comment.
This should be included in the api-test taskfile https://github.com/Open-Model-Initiative/OMI-Data-Pipeline/blob/main/modules/odr_api/tests/Taskfile.yml .
To keep naming in alignment across tasks use - and not _ in task names, (change to test-api). I would also just rename this one to health-check and add it as the first command ran in the test-all command in the odr_api/test/Taskfile.yml
There was a problem hiding this comment.
Seems like something is a bit off when running this command when in the root directory:
PS C:\OMI-Data-Pipeline> task api:test_api
task: [api:test_api] pytest -s modules/odr_api/tests/integration/models/test_health.py
========================================= test session starts ==========================================
platform win32 -- Python 3.12.0, pytest-8.3.2, pluggy-1.5.0
rootdir: C:\OMI-Data-Pipeline\modules\odr_api
configfile: pyproject.toml
plugins: anyio-4.4.0
collected 0 items / 1 error
================================================ ERRORS ================================================
_______________________ ERROR collecting tests/integration/models/test_health.py _______________________
ImportError while importing test module 'C:\OMI-Data-Pipeline\modules\odr_api\tests\integration\models\test_health.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
C:\AppData\Local\Programs\Python\Python312\Lib\importlib\__init__.py:90: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
modules\odr_api\tests\integration\models\__init__.py:1: in <module>
from models.test_users import TestUserLifecycle
E ModuleNotFoundError: No module named 'models'
======================================= short test summary info ========================================
ERROR modules\odr_api\tests\integration\models\test_health.py
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
=========================================== 1 error in 0.08s ===========================================
task: Failed to run task "api:test_api": exit status 2
PS C:\OMI-Data-Pipeline>
modules/odr_api/odr_api/main.py
Outdated
| app.include_router(annotation_router, prefix=settings.API_V1_STR) | ||
| app.include_router(auth_router, prefix=settings.API_V1_STR) | ||
| app.include_router(embedding_router, prefix=settings.API_V1_STR) | ||
| app.include_router(health.router) |
There was a problem hiding this comment.
Try and match to the existing routes. Should have a prefix and same naming style
Signed-off-by: Prateek Kumar <85689959+PrateekKumar1709@users.noreply.github.com>
…l task Signed-off-by: Prateek Kumar <85689959+PrateekKumar1709@users.noreply.github.com>
Signed-off-by: Prateek Kumar <85689959+PrateekKumar1709@users.noreply.github.com>
Signed-off-by: Prateek Kumar <85689959+PrateekKumar1709@users.noreply.github.com>
Signed-off-by: Prateek Kumar <85689959+PrateekKumar1709@users.noreply.github.com>
Signed-off-by: Prateek Kumar <85689959+PrateekKumar1709@users.noreply.github.com>
|
Thank you for the review @fearnworks. I've addressed all the points raised and made the necessary changes. Please let me know if anything else needs attention |
|
Hitting what looks like some pytest config issues. Are you able to run |
Signed-off-by: jphillips <josh.phillips@fearnworks.com>
|
Had to make the changes from the last two commits to get running : https://github.com/Open-Model-Initiative/OMI-Data-Pipeline/tree/feature/add-smoke-tests-api-db |
|
If you can get those integrated into your PR I think we are good to merge this in. |
Signed-off-by: jphillips <120260158+fearnworks@users.noreply.github.com>
fearnworks
left a comment
There was a problem hiding this comment.
Reviewed, tests performed and merged into PR by author. Merging
Problem Statement
We need to implement smoke tests to verify that our core API and database services are up and running after deployment. This will help ensure the reliability of our deployments and quickly identify any issues.
Related Issue
Fixes #3 - Add smoke tests for services
Proposed Changes
API Smoke Test
modules/odr_api/odr_api/api/endpoints/health.pymodules/odr_api/odr_api/api/main.pyto include the health check routermodules/odr_api/tests/integration/models/test_health.pymodules/odr_api/Taskfile.ymlto include a smoke test taskDatabase Smoke Test
modules/odr_database/tests/test_db_connection.pymodules/odr_database/Taskfile.ymlto include a smoke test taskCombined Smoke Tests
Taskfile.ymlto include a task that runs both API and database smoke tests