Files
chatbot_v3/docs/adr/0016-testing-strategy-and-quality-gates.md
Ali Zarinkolah fd70ad01af docs(architecture): adopt inline synchronous ingestion (ADR-0017)
Why:
- ingestion is inline in the request instead of dispatched through RabbitMQ/outbox/worker; ADR-0014 is superseded (not deleted) and named as the design to adopt once ingestion needs to move off the request path.

Changes:
- new ADR-0017 plus amendments to every ADR/plan that referenced the job-shaped/broker contract, so none silently contradict it.

Impact:
- no broker, outbox, or worker code; rabbitmq test marker removed.
2026-08-16 11:53:59 +03:30

229 lines
10 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 0016. Testing strategy and quality gates
## Status
Proposed
> Amended by [ADR-0017](0017-synchronous-ingestion-in-the-request-path.md):
> there is no broker, outbox, or queue, so the `rabbitmq` marker and
> `tests/integration/rabbitmq/` are not carried until ADR-0014 is adopted.
> Ingestion is exercised through the upload request itself, which now returns a
> terminal result. Every reliability invariant below still applies — retrying an
> upload stands in for redelivery.
## Context
The project has ADRs for tenant-scoped ingestion, explicit resource ownership,
MinIO object storage, inline request-path ingestion,
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; the source file and its job row commit
atomically; job execution is safe under at-least-once semantics; 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-0017 controls ingestion: `POST /v1/files` parses, chunks, embeds, and
indexes inline, then returns a terminal `201`. `ingestion_jobs` records the
attempt. There is no outbox, queue, or publisher to test — but the request's
bounds (size, timeout, capacity) and its two-transaction shape are testable
contracts.
## 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`, or `qdrant` for the real service used by an integration
test (a `rabbitmq` marker returns with ADR-0014);
- `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/
│ └── 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, 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, 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 of the running web process, which performs
ingestion inline under ADR-0017. 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 job payloads 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 first upload transaction records `source_files` and a `running`
`ingestion_jobs` row atomically, and commits before any parse/embed work; no
session or transaction is held open across that work.
- Every terminating path — success, parse failure, embedder failure, timeout —
writes a terminal job status and its `ingestion_job_events` row. A job is
never left in `running` by a handled failure.
- Each bound maps to its status code: oversized upload `413`, capacity `503`,
timeout `504`, embedder failure `502`.
- Retrying an upload does not regress terminal jobs, inflate counters, or create
duplicate logical chunks; identical content is recognized rather than
re-ingested.
- Embedding is batched and concurrency-bounded rather than serial per chunk, and
blocking work is executed off the event loop under an explicit limiter.
- 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, transaction boundaries under real sessions, 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
transaction/connection 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.