Refactor: Improve CI workflow robustness and test environment variable management, and enable parallel quality test execution.
This commit is contained in:
@@ -58,6 +58,8 @@ jobs:
|
||||
|
||||
- name: Cache Cargo
|
||||
uses: actions/cache@v4
|
||||
timeout-minutes: 2
|
||||
continue-on-error: true
|
||||
with:
|
||||
path: |
|
||||
~/.cargo/bin/
|
||||
|
||||
@@ -25,6 +25,8 @@ jobs:
|
||||
|
||||
- name: Restore uv cache
|
||||
uses: actions/cache@v4
|
||||
timeout-minutes: 2
|
||||
continue-on-error: true
|
||||
with:
|
||||
path: ~/.cache/uv
|
||||
key: ${{ runner.os }}-py${{ env.PYTHON_VERSION }}-uv-${{ hashFiles('pyproject.toml', 'uv.lock') }}
|
||||
@@ -43,15 +45,15 @@ jobs:
|
||||
source .venv/bin/activate
|
||||
uv pip install -e ".[dev,all]"
|
||||
|
||||
- name: Run unit tests (non-integration)
|
||||
- name: Run unit tests (parallel)
|
||||
run: |
|
||||
source .venv/bin/activate
|
||||
pytest tests/ -m "not integration and not slow" --asyncio-mode=auto -v
|
||||
pytest tests/ -m "not integration and not slow and not stress" --asyncio-mode=auto -n auto -v
|
||||
|
||||
- name: Run integration tests (testcontainers)
|
||||
- name: Run integration tests (parallel - isolated databases per worker)
|
||||
run: |
|
||||
source .venv/bin/activate
|
||||
pytest tests/integration/ -m "integration" --asyncio-mode=auto -v
|
||||
pytest tests/integration/ -m "integration" --asyncio-mode=auto -n auto -v
|
||||
|
||||
test-typescript:
|
||||
runs-on: ubuntu-22.04
|
||||
@@ -112,6 +114,8 @@ jobs:
|
||||
|
||||
- name: Cache Cargo
|
||||
uses: actions/cache@v4
|
||||
timeout-minutes: 2
|
||||
continue-on-error: true
|
||||
with:
|
||||
path: |
|
||||
~/.cargo/bin/
|
||||
|
||||
@@ -68,6 +68,8 @@ jobs:
|
||||
|
||||
- name: Cache Cargo
|
||||
uses: actions/cache@v4
|
||||
timeout-minutes: 2
|
||||
continue-on-error: true
|
||||
with:
|
||||
path: |
|
||||
~/.cargo/bin/
|
||||
|
||||
@@ -22,6 +22,8 @@ jobs:
|
||||
|
||||
- name: Restore uv cache
|
||||
uses: actions/cache@v4
|
||||
timeout-minutes: 2
|
||||
continue-on-error: true
|
||||
with:
|
||||
path: ~/.cache/uv
|
||||
key: ${{ runner.os }}-py${{ env.PYTHON_VERSION }}-uv-${{ hashFiles('pyproject.toml', 'uv.lock') }}
|
||||
@@ -51,7 +53,7 @@ jobs:
|
||||
- name: Run Python quality tests
|
||||
run: |
|
||||
source .venv/bin/activate
|
||||
pytest tests/quality/ -q
|
||||
pytest tests/quality/ -n auto -q
|
||||
|
||||
quality-typescript:
|
||||
runs-on: ubuntu-22.04
|
||||
@@ -115,6 +117,8 @@ jobs:
|
||||
|
||||
- name: Cache Cargo
|
||||
uses: actions/cache@v4
|
||||
timeout-minutes: 2
|
||||
continue-on-error: true
|
||||
with:
|
||||
path: |
|
||||
~/.cargo/bin/
|
||||
|
||||
@@ -1,7 +1,15 @@
|
||||
"""PostgreSQL testcontainer fixtures and utilities."""
|
||||
"""PostgreSQL testcontainer fixtures and utilities.
|
||||
|
||||
Provides a singleton PostgreSQL container for integration tests. When running with
|
||||
pytest-xdist, each worker gets its own isolated database within the shared container
|
||||
for safe parallel execution.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import atexit
|
||||
import os
|
||||
import threading
|
||||
import time
|
||||
from importlib import import_module
|
||||
from typing import TYPE_CHECKING
|
||||
@@ -18,30 +26,36 @@ if TYPE_CHECKING:
|
||||
from sqlalchemy.ext.asyncio import AsyncConnection, AsyncEngine
|
||||
|
||||
|
||||
def get_xdist_worker_id() -> str:
|
||||
"""Get pytest-xdist worker ID (e.g., 'gw0', 'gw1') or 'main' for sequential runs."""
|
||||
return os.environ.get("PYTEST_XDIST_WORKER", "main")
|
||||
|
||||
|
||||
def get_worker_database_name() -> str:
|
||||
"""Get database name for current xdist worker (e.g., 'noteflow_test_gw0')."""
|
||||
worker_id = get_xdist_worker_id()
|
||||
if worker_id == "main":
|
||||
return "noteflow_test"
|
||||
return f"noteflow_test_{worker_id}"
|
||||
|
||||
|
||||
class PgTestContainer:
|
||||
"""Minimal Postgres testcontainer wrapper with custom readiness wait."""
|
||||
"""Minimal Postgres testcontainer wrapper with per-worker database support."""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
image: str = "pgvector/pgvector:pg16",
|
||||
username: str = "test",
|
||||
password: str = "test",
|
||||
dbname: str = "noteflow_test",
|
||||
dbname: str = "postgres",
|
||||
port: int = 5432,
|
||||
) -> None:
|
||||
"""Initialize the container configuration.
|
||||
|
||||
Args:
|
||||
image: Docker image to use.
|
||||
username: PostgreSQL username.
|
||||
password: PostgreSQL password.
|
||||
dbname: Database name.
|
||||
port: PostgreSQL port.
|
||||
"""
|
||||
self.username = username
|
||||
self.password = password
|
||||
self.dbname = dbname
|
||||
self.port = port
|
||||
self._created_databases: set[str] = set()
|
||||
self._db_lock = threading.Lock()
|
||||
|
||||
container_module = import_module("testcontainers.core.container")
|
||||
docker_container_cls = container_module.DockerContainer
|
||||
@@ -54,26 +68,68 @@ class PgTestContainer:
|
||||
)
|
||||
|
||||
def start(self) -> Self:
|
||||
"""Start the container."""
|
||||
self._container.start()
|
||||
self._wait_until_ready()
|
||||
return self
|
||||
|
||||
def stop(self) -> None:
|
||||
"""Stop the container."""
|
||||
self._container.stop()
|
||||
|
||||
def get_connection_url(self) -> str:
|
||||
"""Return a SQLAlchemy-style connection URL."""
|
||||
host = self._container.get_container_host_ip()
|
||||
port = self._container._get_exposed_port(self.port)
|
||||
def get_host(self) -> str:
|
||||
return str(self._container.get_container_host_ip())
|
||||
|
||||
def get_port(self) -> int:
|
||||
return int(self._container._get_exposed_port(self.port))
|
||||
|
||||
def get_connection_url(self, dbname: str | None = None) -> str:
|
||||
"""Return SQLAlchemy connection URL for specified database."""
|
||||
host = self.get_host()
|
||||
port = self.get_port()
|
||||
db = dbname or self.dbname
|
||||
quoted_password = quote(self.password, safe=" +")
|
||||
return (
|
||||
f"postgresql+psycopg2://{self.username}:{quoted_password}@{host}:{port}/{self.dbname}"
|
||||
)
|
||||
return f"postgresql+psycopg2://{self.username}:{quoted_password}@{host}:{port}/{db}"
|
||||
|
||||
def ensure_database_exists(self, dbname: str) -> None:
|
||||
"""Create database if it doesn't exist (thread-safe, idempotent)."""
|
||||
with self._db_lock:
|
||||
if dbname in self._created_databases:
|
||||
return
|
||||
|
||||
escaped_password = self.password.replace("'", "'\"'\"'")
|
||||
# Check if database exists, create if not
|
||||
check_cmd = [
|
||||
"sh",
|
||||
"-c",
|
||||
(
|
||||
f"PGPASSWORD='{escaped_password}' "
|
||||
f"psql --username {self.username} --dbname postgres --host 127.0.0.1 "
|
||||
f"-tAc \"SELECT 1 FROM pg_database WHERE datname='{dbname}'\""
|
||||
),
|
||||
]
|
||||
result = self._container.exec(check_cmd)
|
||||
if result.output and result.output.strip() == b"1":
|
||||
self._created_databases.add(dbname)
|
||||
return
|
||||
|
||||
# Create the database
|
||||
create_cmd = [
|
||||
"sh",
|
||||
"-c",
|
||||
(
|
||||
f"PGPASSWORD='{escaped_password}' "
|
||||
f"psql --username {self.username} --dbname postgres --host 127.0.0.1 "
|
||||
f"-c 'CREATE DATABASE {dbname}'"
|
||||
),
|
||||
]
|
||||
result = self._container.exec(create_cmd)
|
||||
if result.exit_code != 0:
|
||||
error_msg = result.output.decode(errors="ignore") if result.output else ""
|
||||
if "already exists" not in error_msg:
|
||||
raise RuntimeError(f"Failed to create database {dbname}: {error_msg}")
|
||||
|
||||
self._created_databases.add(dbname)
|
||||
|
||||
def _wait_until_ready(self, timeout: float = 30.0, interval: float = 0.5) -> None:
|
||||
"""Wait for Postgres to accept connections by running a simple query."""
|
||||
start_time = time.time()
|
||||
escaped_password = self.password.replace("'", "'\"'\"'")
|
||||
cmd = [
|
||||
@@ -103,45 +159,52 @@ class PgTestContainer:
|
||||
|
||||
# Module-level container singleton
|
||||
_container: PgTestContainer | None = None
|
||||
_database_url: str | None = None
|
||||
_container_lock = threading.Lock()
|
||||
_cleanup_registered = False
|
||||
|
||||
|
||||
def get_or_create_container() -> tuple[PgTestContainer, str]:
|
||||
"""Get or create the PostgreSQL container singleton.
|
||||
"""Get or create container and return (container, worker-specific database URL)."""
|
||||
global _container, _cleanup_registered
|
||||
|
||||
Returns:
|
||||
Tuple of (container, async_database_url).
|
||||
"""
|
||||
global _container, _database_url
|
||||
with _container_lock:
|
||||
if _container is None:
|
||||
_container = PgTestContainer().start()
|
||||
|
||||
if _container is None:
|
||||
container = PgTestContainer().start()
|
||||
_container = container
|
||||
url = container.get_connection_url()
|
||||
_database_url = url.replace("postgresql+psycopg2://", "postgresql+asyncpg://")
|
||||
if not _cleanup_registered:
|
||||
atexit.register(_atexit_cleanup)
|
||||
_cleanup_registered = True
|
||||
|
||||
assert _container is not None, "Container should be initialized"
|
||||
assert _database_url is not None, "Database URL should be initialized"
|
||||
return _container, _database_url
|
||||
# Get worker-specific database name and ensure it exists
|
||||
dbname = get_worker_database_name()
|
||||
_container.ensure_database_exists(dbname)
|
||||
|
||||
# Return async database URL for the worker's database
|
||||
url = _container.get_connection_url(dbname)
|
||||
async_url = url.replace("postgresql+psycopg2://", "postgresql+asyncpg://")
|
||||
return _container, async_url
|
||||
|
||||
|
||||
def stop_container() -> None:
|
||||
"""Stop and cleanup the container singleton."""
|
||||
def _atexit_cleanup() -> None:
|
||||
global _container
|
||||
if _container is not None:
|
||||
_container.stop()
|
||||
try:
|
||||
_container.stop()
|
||||
except Exception:
|
||||
pass
|
||||
_container = None
|
||||
|
||||
|
||||
def stop_container() -> None:
|
||||
global _container
|
||||
with _container_lock:
|
||||
if _container is not None:
|
||||
_container.stop()
|
||||
_container = None
|
||||
|
||||
|
||||
async def initialize_test_schema(conn: AsyncConnection) -> None:
|
||||
"""Initialize test database schema.
|
||||
|
||||
Creates the pgvector extension and noteflow schema with all tables.
|
||||
Also seeds the default workspace and user required by FK constraints.
|
||||
|
||||
Args:
|
||||
conn: Async database connection.
|
||||
"""
|
||||
"""Initialize test database schema with pgvector extension and all tables."""
|
||||
await conn.execute(text("CREATE EXTENSION IF NOT EXISTS vector"))
|
||||
await conn.execute(text("DROP SCHEMA IF EXISTS noteflow CASCADE"))
|
||||
await conn.execute(text("CREATE SCHEMA noteflow"))
|
||||
@@ -169,23 +232,10 @@ async def initialize_test_schema(conn: AsyncConnection) -> None:
|
||||
|
||||
|
||||
async def cleanup_test_schema(conn: AsyncConnection) -> None:
|
||||
"""Drop the test schema.
|
||||
|
||||
Args:
|
||||
conn: Async database connection.
|
||||
"""
|
||||
await conn.execute(text("DROP SCHEMA IF EXISTS noteflow CASCADE"))
|
||||
|
||||
|
||||
def create_test_session_factory(engine: AsyncEngine) -> async_sessionmaker[AsyncSession]:
|
||||
"""Create standard test session factory.
|
||||
|
||||
Args:
|
||||
engine: SQLAlchemy async engine.
|
||||
|
||||
Returns:
|
||||
Configured session factory.
|
||||
"""
|
||||
return async_sessionmaker(
|
||||
engine,
|
||||
class_=AsyncSession,
|
||||
@@ -196,12 +246,4 @@ def create_test_session_factory(engine: AsyncEngine) -> async_sessionmaker[Async
|
||||
|
||||
|
||||
def create_test_engine(database_url: str) -> AsyncEngine:
|
||||
"""Create test database engine.
|
||||
|
||||
Args:
|
||||
database_url: Async database URL.
|
||||
|
||||
Returns:
|
||||
SQLAlchemy async engine.
|
||||
"""
|
||||
return create_async_engine(database_url, echo=False)
|
||||
|
||||
@@ -564,14 +564,21 @@ class TestPartialBufferComparisonBenchmark:
|
||||
def test_preallocated_append_only(
|
||||
self, benchmark: BenchmarkFixture, audio_chunk: NDArray[np.float32]
|
||||
) -> None:
|
||||
"""Benchmark pre-allocated buffer append only (no get_audio)."""
|
||||
buffer = PartialAudioBuffer(sample_rate=SAMPLE_RATE)
|
||||
"""Benchmark pre-allocated buffer append only (no get_audio).
|
||||
|
||||
def append_only() -> None:
|
||||
buffer.append(audio_chunk)
|
||||
Uses pedantic mode to ensure setup runs before each iteration,
|
||||
preventing buffer overflow across benchmark iterations.
|
||||
"""
|
||||
|
||||
benchmark(append_only)
|
||||
assert buffer.samples_buffered > 0, "Should have appended"
|
||||
def setup() -> tuple[tuple[PartialAudioBuffer, NDArray[np.float32]], dict[str, object]]:
|
||||
buffer = PartialAudioBuffer(sample_rate=SAMPLE_RATE)
|
||||
return (buffer, audio_chunk), {}
|
||||
|
||||
def append_only(buffer: PartialAudioBuffer, chunk: NDArray[np.float32]) -> None:
|
||||
buffer.append(chunk)
|
||||
|
||||
pedantic = getattr(benchmark, "pedantic")
|
||||
pedantic(append_only, setup=setup, rounds=5000)
|
||||
|
||||
def test_preallocated_get_audio_only(
|
||||
self, benchmark: BenchmarkFixture, audio_chunk: NDArray[np.float32]
|
||||
|
||||
@@ -88,21 +88,10 @@ def mock_oauth_manager() -> MagicMock:
|
||||
|
||||
_TEST_DATABASE_URL = "postgresql+asyncpg://test:test@localhost:5432/noteflow_test"
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True, scope="session")
|
||||
def set_test_database_url() -> Generator[None, None, None]:
|
||||
"""Set dummy database URL for tests that instantiate Settings.
|
||||
|
||||
Many tests create NoteFlowServicer which calls get_settings() which
|
||||
requires NOTEFLOW_DATABASE_URL. This fixture sets a dummy value.
|
||||
"""
|
||||
original = os.environ.get("NOTEFLOW_DATABASE_URL")
|
||||
# Set database URL at module load time (before any fixtures run)
|
||||
# This is safe for xdist because each worker process has isolated env
|
||||
if "NOTEFLOW_DATABASE_URL" not in os.environ:
|
||||
os.environ["NOTEFLOW_DATABASE_URL"] = _TEST_DATABASE_URL
|
||||
yield
|
||||
if original is None:
|
||||
del os.environ["NOTEFLOW_DATABASE_URL"]
|
||||
else:
|
||||
os.environ["NOTEFLOW_DATABASE_URL"] = original
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
|
||||
@@ -43,6 +43,7 @@ def _install_fake_keyring(monkeypatch: pytest.MonkeyPatch) -> dict[tuple[str, st
|
||||
|
||||
def test_get_or_create_master_key_creates_and_reuses(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Master key should be created once and then reused."""
|
||||
monkeypatch.delenv(keystore.ENV_VAR_NAME, raising=False)
|
||||
storage = _install_fake_keyring(monkeypatch)
|
||||
ks = keystore.KeyringKeyStore(service_name="svc", key_name="key")
|
||||
|
||||
@@ -59,6 +60,7 @@ def test_get_or_create_master_key_falls_back_to_file(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""Keyring errors should fall back to file-based key storage."""
|
||||
monkeypatch.delenv(keystore.ENV_VAR_NAME, raising=False)
|
||||
|
||||
class DummyErrors:
|
||||
class KeyringError(Exception): ...
|
||||
|
||||
@@ -25,7 +25,7 @@ from numpy.typing import NDArray
|
||||
|
||||
from noteflow.config.constants import DEFAULT_SAMPLE_RATE
|
||||
from noteflow.domain.entities import Meeting, Segment
|
||||
from noteflow.domain.value_objects import MeetingId, MeetingState
|
||||
from noteflow.domain.value_objects import AudioSource, MeetingId, MeetingState, SpeakerRole
|
||||
from noteflow.grpc.mixins.streaming import StreamingMixin
|
||||
from noteflow.grpc.proto import noteflow_pb2
|
||||
from noteflow.grpc.service import NoteFlowServicer
|
||||
@@ -286,7 +286,12 @@ class TestStreamSegmentPersistence:
|
||||
|
||||
def _create_stream_mocks(self, audio: NDArray[np.float32]) -> MeetingStreamState:
|
||||
"""Create mocked stream state with VAD and segmenter."""
|
||||
mock_segment = MagicMock(audio=audio, start_time=0.0)
|
||||
mock_segment = MagicMock(
|
||||
audio=audio,
|
||||
start_time=0.0,
|
||||
audio_source=AudioSource.UNKNOWN,
|
||||
speaker_role=SpeakerRole.UNKNOWN,
|
||||
)
|
||||
segmenter = MagicMock(
|
||||
process_audio=MagicMock(return_value=[mock_segment]), flush=MagicMock(return_value=None)
|
||||
)
|
||||
|
||||
@@ -3,7 +3,6 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
@@ -53,19 +52,12 @@ async def _crud_roundtrip(database_url: str, meetings_dir: Path) -> None:
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
def test_migrations_upgrade_and_downgrade(tmp_path: Path) -> None:
|
||||
def test_migrations_upgrade_and_downgrade(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Upgrade to head, run CRUD, then downgrade to base without errors."""
|
||||
_, database_url = get_or_create_container()
|
||||
original_url = os.environ.get("NOTEFLOW_DATABASE_URL")
|
||||
os.environ["NOTEFLOW_DATABASE_URL"] = database_url
|
||||
monkeypatch.setenv("NOTEFLOW_DATABASE_URL", database_url)
|
||||
|
||||
try:
|
||||
asyncio.run(_reset_schema(database_url))
|
||||
command.upgrade(_alembic_config(), "head")
|
||||
asyncio.run(_crud_roundtrip(database_url, tmp_path))
|
||||
command.downgrade(_alembic_config(), "base")
|
||||
finally:
|
||||
if original_url is None:
|
||||
os.environ.pop("NOTEFLOW_DATABASE_URL", None)
|
||||
else:
|
||||
os.environ["NOTEFLOW_DATABASE_URL"] = original_url
|
||||
asyncio.run(_reset_schema(database_url))
|
||||
command.upgrade(_alembic_config(), "head")
|
||||
asyncio.run(_crud_roundtrip(database_url, tmp_path))
|
||||
command.downgrade(_alembic_config(), "base")
|
||||
|
||||
@@ -25,6 +25,9 @@ from noteflow.infrastructure.logging import get_logger
|
||||
|
||||
logger = get_logger(__name__)
|
||||
|
||||
# Mark all tests in this module as slow (excluded from -n auto parallel runs)
|
||||
pytestmark = pytest.mark.slow
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Generator
|
||||
|
||||
|
||||
Reference in New Issue
Block a user