-
Notifications
You must be signed in to change notification settings - Fork 4
Use Isolated Test Database for Reliable and Consistent Test Runs #230
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
docker-compose.yml
Outdated
@@ -17,6 +17,20 @@ services: | |||
- POSTGRES_PASSWORD=${POSTGRES_PASSWORD?Variable not set} | |||
- POSTGRES_USER=${POSTGRES_USER?Variable not set} | |||
- POSTGRES_DB=${POSTGRES_DB?Variable not set} | |||
- POSTGRES_DB_TEST=${POSTGRES_DB_TEST?Variable not set} | |||
# Wait for main DB to be ready, then create test DB if it doesn't exist | |||
command: > |
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.
I am not sure if this is needed when we start the containers. It would be good to handle this during the test run.
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.
Can you also please resolve the conflicts.
backend/app/tests/conftest.py
Outdated
with Session(engine) as session: | ||
with Session(test_engine) as session: | ||
# Drop all tables and recreate them | ||
SQLModel.metadata.drop_all(test_engine) |
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.
Maybe drop the database instead of the tables.
@@ -18,7 +18,11 @@ jobs: | |||
POSTGRES_DB: ai_platform | |||
ports: | |||
- 5432:5432 | |||
options: --health-cmd "pg_isready -U postgres" --health-interval 10s --health-timeout 5s --health-retries 5 | |||
options: >- |
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.
Is this intentional?
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.
@kurund was part of my previous commit where I was modifying this file.
Either way it is fine
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.
Might be good to revert then
Target issue : #204
This PR addresses the issue where local test cases were running against the shared development database, causing inconsistent results due to residual or missing data. The updated implementation ensures that test cases now run against a dedicated test database (POSTGRES_DB_TEST), providing a clean and repeatable environment for every test session.