Switch the durable ingestion/maintenance job-dispatch broker decision from NATS JetStream to RabbitMQ (aio-pika), rewriting ADR-0014 and propagating the terminology change through ADR-0015, ADR-0016, and the ingestion vertical-slice plan. Adds aio-pika as a runtime dependency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
216 lines
9.6 KiB
Markdown
216 lines
9.6 KiB
Markdown
# 0016. Testing strategy and quality gates
|
||
|
||
## Status
|
||
|
||
Proposed
|
||
|
||
## Context
|
||
|
||
The project has ADRs for tenant-scoped ingestion, explicit resource ownership,
|
||
MinIO object storage, transactional outbox dispatch, RabbitMQ workers,
|
||
Qdrant indexing, and a modular monolith. It has no test runner, test fixtures,
|
||
or executable test suite yet.
|
||
|
||
The first CSV ingestion slice has correctness properties that cannot be left to
|
||
manual testing: Alembic is the only schema-management path; tenant identity is
|
||
trusted server-side context; a file, job, and outbox event commit atomically;
|
||
workers are safe under at-least-once delivery; and generated Qdrant points are
|
||
idempotent and tenant-filtered. ADR-0015 already reserves a test layout by
|
||
boundary, while ADR-0012 requires explicit dependencies and resource lifetimes
|
||
that should make tests practical without import-time client patching.
|
||
|
||
Tests need to give fast feedback during implementation without replacing
|
||
integration coverage with mocks or making routine development depend on Docker,
|
||
provider credentials, live models, or Langfuse availability.
|
||
|
||
ADR-0014's transactional-outbox decision controls ingestion dispatch. The upload
|
||
path records durable dispatch intent; a separate outbox publisher publishes it.
|
||
|
||
## Decision
|
||
|
||
### Use pytest with explicit async support
|
||
|
||
Use pytest as the test runner. Add `pytest`, `pytest-asyncio`, `httpx`,
|
||
`asgi-lifespan`, `testcontainers`, `pytest-timeout`, and `pytest-cov` as
|
||
development dependencies.
|
||
|
||
Configure `pytest-asyncio` in strict mode. Async tests and fixtures must be
|
||
explicit. FastAPI tests use `httpx.AsyncClient`, `ASGITransport`, and
|
||
`LifespanManager` so they exercise application startup and shutdown according to
|
||
ADR-0012. `FastAPI.TestClient` is not the default project test client.
|
||
|
||
Register these markers:
|
||
|
||
- one primary boundary marker per test: `unit`, `integration`, or `e2e`;
|
||
- `postgres`, `minio`, `rabbitmq`, or `qdrant` for the real service used by an
|
||
integration test;
|
||
- `slow` only where a test materially exceeds the normal integration feedback
|
||
target;
|
||
- `live_provider` for an opt-in, credential-gated external-provider smoke test.
|
||
|
||
Name tests `test_<unit>_<scenario>_<outcome>` and use Arrange–Act–Assert.
|
||
|
||
### Test by architectural boundary
|
||
|
||
Use the test layout reserved by ADR-0015, with shared support for fixtures and
|
||
assertions:
|
||
|
||
```text
|
||
tests/
|
||
├── conftest.py
|
||
├── fakes.py
|
||
├── support/
|
||
│ ├── factories.py
|
||
│ └── assertions.py
|
||
├── unit/
|
||
│ ├── application/
|
||
│ └── agent/
|
||
├── integration/
|
||
│ ├── postgres/
|
||
│ ├── minio/
|
||
│ ├── rabbitmq/
|
||
│ └── qdrant/
|
||
└── e2e/
|
||
```
|
||
|
||
- **Unit tests** are the default development feedback loop. They cover pure
|
||
application policy, validation, lifecycle transitions, tenant propagation,
|
||
deterministic IDs, CSV chunking, event construction, and LangGraph
|
||
control-flow policy.
|
||
- **Integration tests** validate a production infrastructure adapter against its
|
||
real backing service and its deployment-relevant behavior.
|
||
- **End-to-end tests** prove a small vertical-slice acceptance contract through
|
||
the real application composition. They do not replace faster unit or adapter
|
||
tests.
|
||
|
||
Hand-written fakes and spies implement narrow application-owned ports, not
|
||
MinIO, RabbitMQ, Qdrant, or model SDK-shaped interfaces. Scripted model, embedder,
|
||
retrieval, clock, and UUID fakes make normal test runs deterministic.
|
||
|
||
### Apply pragmatic TDD
|
||
|
||
For application behavior, HTTP contracts, database migrations, reliability
|
||
rules, and defects, first write a focused failing test that describes the
|
||
observable requirement. Make the smallest change that passes it, then refactor
|
||
while the relevant suite is green.
|
||
|
||
TDD applies to behavior and regressions, not as an artificial ritual for pure
|
||
refactors or configuration-only changes with no observable behavior change.
|
||
Those changes must preserve and extend existing relevant coverage as needed.
|
||
|
||
Before introducing a concrete adapter, write tests for the consuming
|
||
application port. Before each Alembic migration, write the empty-database
|
||
migration test or extend the existing migration test. Add the corresponding
|
||
real-adapter integration test before declaring that boundary complete.
|
||
|
||
### Use disposable real infrastructure in integration tests
|
||
|
||
Use Testcontainers as the standard automated integration-test resource mechanism
|
||
for Postgres, MinIO, RabbitMQ, and Qdrant.
|
||
|
||
- Tests never connect to a developer's local services or Langfuse-owned storage
|
||
and credentials.
|
||
- Start containers at suite or session scope, then isolate data per test with
|
||
unique data, object-key prefixes, exchange/queue/binding names, and collection
|
||
names.
|
||
- Disable parallel integration execution until fixture isolation and cleanup are
|
||
proven worker-safe.
|
||
- Fixtures expose typed settings or connection values. Tests create production
|
||
adapters through their normal constructors.
|
||
|
||
Docker Compose remains the mechanism for manual local validation and a later,
|
||
serialized operational smoke test where web, outbox-publisher, and worker run as
|
||
independent processes. It is not the default pytest fixture mechanism.
|
||
|
||
### Treat invariants as reusable contracts
|
||
|
||
Test the following requirements at the applicable application, adapter, and E2E
|
||
boundaries:
|
||
|
||
- Tenant identity comes from server-side authenticated context. Request payloads,
|
||
query parameters, object metadata, and broker messages cannot override it.
|
||
- Cross-tenant access does not disclose tenant-owned data. Public routes normally
|
||
return `404` for inaccessible resources.
|
||
- Alembic creates the schema from an empty database. Tests never use
|
||
`Base.metadata.create_all()`, and FastAPI startup performs readiness checks only,
|
||
never DDL.
|
||
- The upload transaction records `source_files`, a queued `ingestion_jobs` row,
|
||
and an unpublished `outbox_events` row atomically. The HTTP route does not
|
||
directly publish the ingestion event.
|
||
- Broker messages contain durable identifiers and correlation metadata only. The
|
||
worker reloads job and source-file records from Postgres before tenant-scoped
|
||
side effects.
|
||
- Worker acknowledgement follows durable progress or terminal-state persistence.
|
||
Duplicate publication and redelivery do not regress terminal jobs, inflate
|
||
counters, or create duplicate logical chunks.
|
||
- MinIO keys are server-derived internal paths. Qdrant reads and mutations use a
|
||
server-derived tenant filter, deterministic point IDs, and upsert semantics.
|
||
|
||
### Keep correctness tests separate from model-quality evaluation
|
||
|
||
Pytest verifies deterministic application behavior: graph policy, schemas,
|
||
redaction, correlation metadata, retry budgets, and graceful observability
|
||
failure handling. Normal pytest runs never call a paid or live model provider.
|
||
|
||
Langfuse datasets and experiments evaluate prompts, model behavior, retrieval,
|
||
citations, and response quality. They are promotion evidence, not a replacement
|
||
for application correctness tests. Live-provider smoke tests, if introduced,
|
||
are opt-in, credential-gated, rate-limited, and excluded from ordinary local and
|
||
pull-request runs.
|
||
|
||
### Establish phased quality gates
|
||
|
||
Initially require Ruff format checking, Ruff linting, Ty type checking, and unit
|
||
tests. Require Docker-capable integration tests as their adapters are
|
||
implemented. Run the separate-process Compose E2E smoke as a serialized
|
||
pre-release or scheduled gate until it is reliable enough for every pull request.
|
||
|
||
Collect coverage reports but do not set a percentage threshold before the first
|
||
vertical slice has meaningful implementation. Later introduce a scoped,
|
||
ratcheting threshold rather than encouraging low-value coverage.
|
||
|
||
## Consequences
|
||
|
||
### Positive
|
||
|
||
- Unit tests provide fast, deterministic TDD feedback for core application
|
||
behavior.
|
||
- Real-service tests cover the behaviors least safe to simulate: Alembic
|
||
migrations, object storage, RabbitMQ acknowledgements/redelivery, and Qdrant
|
||
filtering/upserts.
|
||
- Explicit fakes reinforce the dependency direction and resource ownership rules
|
||
from ADR-0012 and ADR-0015.
|
||
- The ingestion path has concrete tenant-isolation and reliability contracts,
|
||
rather than only a happy-path demonstration.
|
||
- Live model quality can improve through Langfuse experiments without making
|
||
application tests nondeterministic or expensive.
|
||
|
||
### Negative
|
||
|
||
- Docker is required for integration and E2E suites.
|
||
- Testcontainers add setup time and require careful fixture cleanup.
|
||
- Maintaining real-service coverage and test data isolation adds engineering
|
||
effort.
|
||
- E2E tests do not prove semantic quality of LLM responses; that remains an
|
||
evaluation responsibility.
|
||
|
||
## Alternatives Considered
|
||
|
||
- **Mock all external SDKs**: rejected. Mocks cannot prove migrations, real
|
||
RabbitMQ acknowledgement/redelivery behavior, MinIO semantics, or Qdrant
|
||
tenant filtering.
|
||
- **Use full-stack Compose tests only**: rejected. They are slow and opaque for
|
||
the default development loop and make failures difficult to localize.
|
||
- **Run all integration containers on every pytest invocation**: rejected. Test
|
||
boundaries should be selected deliberately for fast local feedback.
|
||
- **Use live model providers in routine tests**: rejected because they are
|
||
nondeterministic, costly, slow, credential-dependent, and hard to assert.
|
||
- **Use Langfuse as the primary regression-test runner**: rejected. Langfuse is
|
||
the quality/evaluation plane; pytest remains the deterministic application test
|
||
framework.
|
||
- **Create schemas with `Base.metadata.create_all()` in fixtures**: rejected. It
|
||
bypasses the production Alembic migration path.
|
||
- **Require strict test-first work for every non-behavioral refactor**: rejected.
|
||
TDD should protect observable behavior and regressions, not add ceremony where
|
||
no behavior changes.
|