Skip to content

Commit

Permalink
fix: #1628 Improve local testcontainer startup to avoid overriding ru…
Browse files Browse the repository at this point in the history
…nning main docker db instance (#1632)
  • Loading branch information
ianliuwk1019 authored Oct 16, 2024
1 parent 465f066 commit 2371396
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 51 deletions.
19 changes: 6 additions & 13 deletions .github/workflows/sonar.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ on:
jobs:
ci-auth:
runs-on: ubuntu-latest
env:
POSTGRES_USER: postgres
POSTGRES_PASSWORD: postgres
steps:
- uses: actions/checkout@v4
with:
Expand All @@ -24,6 +21,10 @@ jobs:

- name: Tests and coverage
env:
POSTGRES_USER: postgres
POSTGRES_PASSWORD: postgres
POSTGRES_PORT: ${{ vars.LOCAL_POSTGRES_PORT }}
POSTGRES_PORT_TESTCONTAINER: ${{ vars.LOCAL_POSTGRES_PORT_TESTCONTAINER }}
FC_API_TOKEN_TEST: ${{ secrets.FOREST_CLIENT_API_API_KEY_TEST }}
run: |
cd server/auth_function
Expand Down Expand Up @@ -53,11 +54,6 @@ jobs:
env:
environment: dev
organization: bcgov
POSTGRES_USER: fam_proxy_api
POSTGRES_PASSWORD: test
POSTGRES_HOST: localhost
POSTGRES_DB: fam
POSTGRES_PORT: 5432
USE_POSTGRES: false
steps:
- uses: actions/checkout@v4
Expand All @@ -80,6 +76,7 @@ jobs:
POSTGRES_HOST: ${{ vars.LOCAL_POSTGRES_HOST }}
POSTGRES_DB: ${{ vars.LOCAL_POSTGRES_DB }}
POSTGRES_PORT: ${{ vars.LOCAL_POSTGRES_PORT }}
POSTGRES_PORT_TESTCONTAINER: ${{ vars.LOCAL_POSTGRES_PORT_TESTCONTAINER }}
COGNITO_REGION: ${{ vars.LOCAL_COGNITO_REGION }}
COGNITO_USER_POOL_ID: ${{ vars.LOCAL_COGNITO_USER_POOL_ID }}
COGNITO_CLIENT_ID: ${{ vars.LOCAL_COGNITO_CLIENT_ID }}
Expand Down Expand Up @@ -113,11 +110,6 @@ jobs:
env:
environment: dev
organization: bcgov
POSTGRES_USER: fam_admin_management_api
POSTGRES_PASSWORD: test
POSTGRES_HOST: localhost
POSTGRES_DB: fam
POSTGRES_PORT: 5432
steps:
- uses: actions/checkout@v4
with:
Expand All @@ -138,6 +130,7 @@ jobs:
POSTGRES_HOST: ${{ vars.LOCAL_POSTGRES_HOST }}
POSTGRES_DB: ${{ vars.LOCAL_POSTGRES_DB }}
POSTGRES_PORT: ${{ vars.LOCAL_POSTGRES_PORT }}
POSTGRES_PORT_TESTCONTAINER: ${{ vars.LOCAL_POSTGRES_PORT_TESTCONTAINER }}
COGNITO_REGION: ${{ vars.LOCAL_COGNITO_REGION }}
COGNITO_USER_POOL_ID: ${{ vars.LOCAL_COGNITO_USER_POOL_ID }}
COGNITO_CLIENT_ID: ${{ vars.LOCAL_COGNITO_CLIENT_ID }}
Expand Down
5 changes: 0 additions & 5 deletions docker-base-services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,17 @@
services:
fam-database:
image: postgres:14.1-alpine
container_name: fam-postgres
mem_limit: 128m
environment:
- POSTGRES_USER=postgres
- POSTGRES_PASSWORD=postgres
- POSTGRES_DB=fam
- POSTGRES_HOST_AUTH_METHOD=password
- POSTGRES_PORT=5432
healthcheck:
test: ["CMD-SHELL", "pg_isready -U postgres"]
interval: 5s
timeout: 5s
retries: 6
ports:
- "5432:5432"
networks:
- fam

Expand All @@ -31,7 +27,6 @@ services:
context: server/flyway
dockerfile: Dockerfile
environment:
- FLYWAY_URL=jdbc:postgresql://fam-postgres:5432/fam
- FLYWAY_USER=postgres
- FLYWAY_PASSWORD=postgres
- FLYWAY_BASELINE_ON_MIGRATE=true
Expand Down
20 changes: 16 additions & 4 deletions docker-compose-testcontainer.yml
Original file line number Diff line number Diff line change
@@ -1,16 +1,28 @@
# Note, the compose service name and container_name for "fam-database" and "fam-flyway" are named differently
# than the main application docker-compose so that when testcontainer is up it does not delete previous running main container.
---

services:
fam-database:
fam-database-testcontainer:
extends:
file: docker-base-services.yml
service: fam-database
container_name: fam-postgres-testcontainer
environment:
- PGPORT=${POSTGRES_PORT_TESTCONTAINER} # the easiest way to change postgres db default port from '5432' to PGPORT.
ports:
- "${POSTGRES_PORT_TESTCONTAINER}:${POSTGRES_PORT_TESTCONTAINER}" # use host-port: POSTGRES_PORT_TESTCONTAINER ='5433' for testcontainer db instance than '5432' for main db.

fam-flyway:
fam-flyway-testcontainer:
extends:
file: docker-base-services.yml
service: fam-flyway
environment:
# note: this environment is separated out from docker-base-services.yml for testcontainer to avoid
# colliding with main app db instance.
- FLYWAY_URL=jdbc:postgresql://fam-postgres-testcontainer:${POSTGRES_PORT_TESTCONTAINER}/fam
depends_on:
fam-database:
fam-database-testcontainer:
condition: service_healthy

fam-test-nginx:
Expand All @@ -19,7 +31,7 @@ services:
ports:
- "8181:80"
depends_on:
fam-flyway:
fam-flyway-testcontainer:
condition: service_completed_successfully

networks:
Expand Down
5 changes: 5 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@ services:
extends:
file: docker-base-services.yml
service: fam-database
container_name: fam-postgres
ports:
- "${POSTGRES_PORT}:${POSTGRES_PORT}"

fam-flyway:
extends:
file: docker-base-services.yml
service: fam-flyway
environment:
- FLYWAY_URL=jdbc:postgresql://fam-postgres:${POSTGRES_PORT}/fam
depends_on:
fam-database:
condition: service_healthy
Expand Down
2 changes: 1 addition & 1 deletion server/admin_management/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def db_pg_connection(db_pg_container):
+ f"{os.environ.get('POSTGRES_USER')}:"
+ f"{os.environ.get('POSTGRES_PASSWORD')}@"
+ f"{os.environ.get('POSTGRES_HOST')}:"
f"{os.environ.get('POSTGRES_PORT')}/"
f"{os.environ.get('POSTGRES_PORT_TESTCONTAINER')}/"
f"{os.environ.get('POSTGRES_DB')}"
)

Expand Down
2 changes: 1 addition & 1 deletion server/auth_function/test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def get_local_db_string():
password = os.getenv("POSTGRES_PASSWORD", "postgres") # test
host = os.getenv("POSTGRES_HOST", "localhost")
dbname = os.getenv("POSTGRES_DB", "fam")
port = os.getenv("POSTGRES_PORT", "5432")
port = os.getenv("POSTGRES_PORT_TESTCONTAINER", "5433")

db_conn_string = (
f"user={username} password={password} host={host} "
Expand Down
2 changes: 1 addition & 1 deletion server/backend/testspg/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def db_pg_connection(db_pg_container):
+ f"{os.environ.get('POSTGRES_USER')}:"
+ f"{os.environ.get('POSTGRES_PASSWORD')}@"
+ f"{os.environ.get('POSTGRES_HOST')}:"
f"{os.environ.get('POSTGRES_PORT')}/"
f"{os.environ.get('POSTGRES_PORT_TESTCONTAINER')}/"
f"{os.environ.get('POSTGRES_DB')}"
)

Expand Down
36 changes: 10 additions & 26 deletions server/backend/testspg/router/test_router_permission_audit.py
Original file line number Diff line number Diff line change
@@ -1,33 +1,17 @@
import pytest
from fastapi.testclient import TestClient
from api.app.main import app, apiPrefix
from api.app.main import apiPrefix
from testspg.fixture.permission_audit_fixture import (
APPLICATION_ID_1,
USER_ID_1,
MOCKED_PERMISSION_HISTORY_RESPONSE,
)
APPLICATION_ID_1, MOCKED_PERMISSION_HISTORY_RESPONSE, USER_ID_1)

client = TestClient(app)
ENDPOINT_ROOT = "permission-audit-history"


@pytest.fixture(scope="function", autouse=True)
def mock_get_db(mocker, db_pg_session):
# This will mock the get_db dependency for all tests in this module
mocker.patch(
"api.app.routers.router_permission_audit.database.get_db",
return_value=db_pg_session,
)


# Test successful retrieval
def test_get_permission_audit_history_success(mocker, auth_headers):
def test_get_permission_audit_history_success(mocker, test_client_fixture, auth_headers):
mocker.patch(
"api.app.routers.router_permission_audit.read_permission_audit_history_by_user_and_application",
return_value=MOCKED_PERMISSION_HISTORY_RESPONSE,
)

response = client.get(
response = test_client_fixture.get(
f"{apiPrefix}/{ENDPOINT_ROOT}?user_id={USER_ID_1}&application_id={APPLICATION_ID_1}",
headers=auth_headers,
)
Expand All @@ -40,13 +24,13 @@ def test_get_permission_audit_history_success(mocker, auth_headers):


# Test retrieval with no records
def test_get_permission_audit_history_bad_request(mocker, auth_headers):
def test_get_permission_audit_history_bad_request(mocker, test_client_fixture, auth_headers):
mocker.patch(
"api.app.routers.router_permission_audit.read_permission_audit_history_by_user_and_application",
return_value=[],
)

response = client.get(
response = test_client_fixture.get(
f"{apiPrefix}/{ENDPOINT_ROOT}?user_id=999&application_id=999",
headers=auth_headers,
)
Expand All @@ -55,8 +39,8 @@ def test_get_permission_audit_history_bad_request(mocker, auth_headers):


# Test handling of invalid user_id
def test_get_permission_audit_history_invalid_user_id_type(auth_headers):
response = client.get(
def test_get_permission_audit_history_invalid_user_id_type(auth_headers, test_client_fixture):
response = test_client_fixture.get(
f"{apiPrefix}/{ENDPOINT_ROOT}?user_id=invalid_user_id&application_id={APPLICATION_ID_1}",
headers=auth_headers,
)
Expand All @@ -65,13 +49,13 @@ def test_get_permission_audit_history_invalid_user_id_type(auth_headers):


# Test unauthorized access
def test_get_permission_audit_history_unauthorized(mocker):
def test_get_permission_audit_history_unauthorized(mocker, test_client_fixture):
mocker.patch(
"api.app.routers.router_permission_audit.read_permission_audit_history_by_user_and_application",
side_effect=Exception("Unauthorized"),
)

response = client.get(
response = test_client_fixture.get(
f"{apiPrefix}/{ENDPOINT_ROOT}?user_id={USER_ID_1}&application_id={APPLICATION_ID_1}"
)

Expand Down

0 comments on commit 2371396

Please sign in to comment.