Skip to content

Commit 273eb4d

Browse files
author
Leon Sander
committed
Add contract-level test suite with CI matrix (Ubuntu/Windows)
20 pytest contract tests over the stable behavior layer: SQLite repositories, pure utils, and chat API request construction. Tests pin behavioral promises, never implementation structure, so forks can refactor internals freely (philosophy documented in TESTING.md). - tests/ run hermetically: import-time side effects are sandboxed into a temp dir, network access is blocked at the socket level - GitHub Actions matrix runs the suite on ubuntu + windows (Python 3.10) - UI, CSS, audio transcription and live model calls deliberately stay untested/manual (see TESTING.md)
1 parent 8461ef5 commit 273eb4d

9 files changed

Lines changed: 657 additions & 0 deletions

File tree

.github/workflows/tests.yml

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
name: tests
2+
3+
on:
4+
push:
5+
branches: [main]
6+
pull_request:
7+
branches: [main]
8+
9+
jobs:
10+
tests:
11+
strategy:
12+
fail-fast: false
13+
matrix:
14+
os: [ubuntu-latest, windows-latest]
15+
python-version: ["3.10"]
16+
runs-on: ${{ matrix.os }}
17+
timeout-minutes: 20
18+
steps:
19+
- uses: actions/checkout@v4
20+
21+
- uses: actions/setup-python@v5
22+
with:
23+
python-version: ${{ matrix.python-version }}
24+
cache: pip
25+
cache-dependency-path: requirements*.txt
26+
27+
- name: Install dependencies
28+
run: |
29+
python -m pip install --upgrade pip
30+
pip install -r requirements.txt -r requirements-dev.txt
31+
32+
# Catches import-time breakage (config paths, heavy deps) with a clear
33+
# error before the test run. torch is deliberately NOT installed: only
34+
# audio_handler needs it, and audio stays manual (see TESTING.md).
35+
- name: Smoke-import the modules under test
36+
run: python -c "import chat_api_handler, database_operations, utils"
37+
env:
38+
PYTHONPATH: src
39+
40+
- name: Run tests
41+
run: pytest

README.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,17 @@ Running Ollama inside Docker is slow on Windows (system calls are translated bet
116116

117117
Key settings live in `config.yaml` (Ollama base URL and embedding model, Whisper model, Chroma path/collection, chat-sessions DB path). Runtime options (endpoint, model, chunk size/overlap, retrieved chunks, chat memory) are adjustable in the sidebar and persisted per user.
118118

119+
## Testing
120+
121+
A small contract-level test suite covers the SQLite layer, utility functions, and API request construction (UI, audio, and real model calls stay manual by design):
122+
123+
```bash
124+
pip install -r requirements-dev.txt
125+
pytest
126+
```
127+
128+
CI runs the suite on Ubuntu and Windows. See [TESTING.md](./TESTING.md) for the philosophy and what is deliberately not tested.
129+
119130
## Roadmap
120131

121132
- [ ] Additional model providers (Gemini, others)

TESTING.md

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
# Testing
2+
3+
This repo has a small, deliberate test suite: **20 contract tests** over the parts of the code whose behavior is stable by design. It is meant to stay cheap to maintain — including for forks.
4+
5+
## Philosophy: contracts, not structure
6+
7+
A test here pins a **behavioral promise** (what a function guarantees to its callers), never an implementation detail. The intended property: **a test only fails when behavior actually changed.** If you fork this repo, rewrite internals freely — a red test means you changed something user-visible, which is exactly when you want to know.
8+
9+
A few places accept **medium churn** deliberately, each documented inline where it occurs:
10+
11+
- `test_pdf_chat_stuffs_retrieved_context_into_prompt` patches the internal `load_vectordb` seam (the alternative is a real Chroma + Ollama instance).
12+
- `test_command_dispatch` patches the internal `pull_model_in_background` seam (the alternative is asyncio + a live Ollama pull).
13+
- The handler tests patch two values that src binds **at import time** (`chat_api_handler.config`, `chat_api_handler.openai_api_key`). A behavior-preserving refactor to call-time lookups would require updating the two fixtures at the top of `tests/test_chat_api_handler.py` — a two-line fix, accepted as the cost of avoiding real env vars and network.
14+
15+
## What is covered
16+
17+
| File | Contracts |
18+
|------|-----------|
19+
| `tests/test_database_operations.py` (9) | message roundtrips (text + binary), LLM-context feed ordering/filtering, per-session delete isolation, session id listing, settings read-through defaults + persistence, restart durability |
20+
| `tests/test_utils.py` (3) | base64/data-URL wire format, `/command` dispatch, config save/load roundtrip |
21+
| `tests/test_chat_api_handler.py` (8) | endpoint routing (ollama/openai/unknown), request bodies + auth headers, image wire formats (Ollama bare-b64 vs OpenAI data URL), RAG context stuffing, error-shape handling (string vs nested object) |
22+
23+
## What deliberately stays untested (and why)
24+
25+
- **UI structure and CSS** (`app.py`, `html_templates.py`) — Streamlit class names churn every release; tests would break without any behavior change.
26+
- **Audio transcription** (`audio_handler.py`) — needs Whisper model downloads; the browser microphone path can't be unit-tested anyway.
27+
- **Real model calls / PDF ingestion** — network- and model-dependent; the request *construction* is tested, the live call is not.
28+
- **Trivial helpers** (`convert_ns_to_seconds`, `get_avatar`, timestamp formatting) — near-zero regression value for the count they add.
29+
- **The manual smoke test remains**: launch the app, send a text message, record audio once. That is the irreducible browser part.
30+
31+
## Running
32+
33+
From the **repo root**:
34+
35+
```bash
36+
pip install -r requirements-dev.txt # includes requirements.txt
37+
pytest
38+
```
39+
40+
CI runs the same suite on Ubuntu and Windows (Python 3.10) on every push/PR to `main` — which also doubles as a "does it still install on Windows" check.
41+
42+
## Gotchas when writing new tests
43+
44+
The src modules bind several things **at import time** — patch the right namespace:
45+
46+
| To fake | Patch | Why |
47+
|---------|-------|-----|
48+
| OpenAI key | `chat_api_handler.openai_api_key` | `os.getenv` ran at import; `monkeypatch.setenv` does nothing |
49+
| Ollama base URL | `chat_api_handler.config` | module-level `load_config()` froze it |
50+
| Vector DB | `chat_api_handler.load_vectordb` | from-import binding; patching `vectordb_handler` does nothing |
51+
| HTTP | `requests.post` (module attribute) | call-time lookup, reaches all src modules |
52+
| Session state | `streamlit.session_state``FakeSessionState` (conftest) | never rely on bare-mode session_state |
53+
54+
Other invariants:
55+
56+
- An autouse `no_network` fixture replaces `requests.get/post` and blocks raw socket connects (which also covers `aiohttp` and `requests.Session`) — a test that reaches for the network fails loudly instead of silently hitting a real endpoint.
57+
- DB tests build their own `DatabaseManager` on `tmp_path` and must `close()` it (Windows cannot delete open sqlite files). Never assert through the module-level singleton in `database_operations`.
58+
- `tests/conftest.py` chdirs into a throwaway **sandbox** (with its own minimal `config.yaml` and `chat_sessions/`) before anything imports the src modules — their import-time side effects (config read, sqlite creation) land in the sandbox, the repo's real `config.yaml` and `chat_sessions/` are never read or written, and pytest works from any invocation directory.

pytest.ini

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[pytest]
2+
pythonpath = src
3+
testpaths = tests

requirements-dev.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
-r requirements.txt
2+
pytest==9.1.1

tests/conftest.py

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
"""Shared fixtures for the contract-level test suite.
2+
3+
IMPORTANT - import-time side effects in src/ are load-bearing: utils.py,
4+
chat_api_handler.py and database_operations.py read the CWD-relative
5+
config.yaml when first imported, and database_operations.py opens a sqlite
6+
database at the configured path at import time. The module-level code below
7+
therefore chdirs into a throwaway sandbox seeded with a minimal config.yaml
8+
BEFORE any test module (and hence any src module) is imported: every
9+
import-time side effect lands in the sandbox, the repo's real config.yaml and
10+
chat_sessions/ are never read or written, and the suite works from any
11+
invocation directory.
12+
"""
13+
import atexit
14+
import os
15+
import shutil
16+
import socket
17+
import tempfile
18+
from pathlib import Path
19+
20+
import pytest
21+
import requests
22+
23+
_SANDBOX = Path(tempfile.mkdtemp(prefix="local-multimodal-chat-tests-"))
24+
(_SANDBOX / "chat_sessions").mkdir()
25+
(_SANDBOX / "config.yaml").write_text(
26+
"\n".join(
27+
[
28+
"ollama:",
29+
' embedding_model: "sandbox-embed"',
30+
" base_url: http://sandbox-ollama:11434",
31+
"",
32+
'whisper_model: "sandbox-whisper"',
33+
"",
34+
"chromadb:",
35+
' chromadb_path: "chroma_db"',
36+
' collection_name: "pdfs"',
37+
"",
38+
'chat_sessions_database_path: "./chat_sessions/chat_sessions.db"',
39+
"",
40+
]
41+
)
42+
)
43+
# Best-effort cleanup; ignore_errors because the module-level singleton in
44+
# database_operations holds its sandbox db open for the whole session.
45+
atexit.register(shutil.rmtree, _SANDBOX, ignore_errors=True)
46+
47+
48+
def pytest_configure(config):
49+
# chdir in the configure hook, not at conftest import: pytest resolves
50+
# testpaths against the CWD first, and collection (which imports the test
51+
# modules and therefore the src modules) only starts afterwards - so the
52+
# import-time side effects still land in the sandbox.
53+
os.chdir(_SANDBOX)
54+
55+
56+
class FakeSessionState(dict):
57+
"""Stand-in for st.session_state covering the four access patterns used in
58+
src/: st.session_state["key"], .get("key", default), attribute access, and
59+
membership tests. Never rely on Streamlit's bare-mode session_state - it is
60+
a process-global singleton with undocumented semantics."""
61+
62+
def __getattr__(self, name):
63+
try:
64+
return self[name]
65+
except KeyError:
66+
raise AttributeError(name) from None
67+
68+
def __setattr__(self, name, value):
69+
self[name] = value
70+
71+
72+
class DummyResponse:
73+
"""Minimal requests.Response stand-in; .json() is repeatably callable."""
74+
75+
def __init__(self, payload):
76+
self._payload = payload
77+
78+
def json(self):
79+
return self._payload
80+
81+
82+
class RecordingPost:
83+
"""Callable that records every requests.post(...) call and returns a
84+
canned response. The signature mirrors the real
85+
requests.post(url, data=None, json=None, ...) so that a refactor in src to
86+
positional arguments cannot silently record the body under the wrong key."""
87+
88+
def __init__(self, response_payload):
89+
self.response_payload = response_payload
90+
self.calls = []
91+
92+
def __call__(self, url=None, data=None, json=None, headers=None, **kwargs):
93+
self.calls.append({"url": url, "data": data, "json": json, "headers": headers})
94+
return DummyResponse(self.response_payload)
95+
96+
97+
@pytest.fixture(autouse=True)
98+
def no_network(monkeypatch):
99+
"""Fail loudly if any test hits the network without explicitly patching
100+
it: requests.get/post are replaced for a clear error message, and raw
101+
socket connects are blocked as a catch-all (this also covers aiohttp and
102+
requests.Session). captured_post overrides requests.post for tests that
103+
expect an HTTP call."""
104+
105+
def _blocked(*args, **kwargs):
106+
raise RuntimeError(
107+
"Network access is disabled in tests; use the captured_post "
108+
"fixture or patch the network call explicitly."
109+
)
110+
111+
monkeypatch.setattr(requests, "get", _blocked)
112+
monkeypatch.setattr(requests, "post", _blocked)
113+
monkeypatch.setattr(socket.socket, "connect", _blocked)
114+
115+
116+
@pytest.fixture
117+
def captured_post(monkeypatch):
118+
"""Factory: captured_post(payload) patches requests.post to record calls
119+
and answer with `payload`. Works because src modules do `import requests`
120+
and look up requests.post at call time."""
121+
122+
def _install(response_payload):
123+
recorder = RecordingPost(response_payload)
124+
monkeypatch.setattr(requests, "post", recorder)
125+
return recorder
126+
127+
return _install
128+
129+
130+
@pytest.fixture
131+
def fake_session_state(monkeypatch):
132+
"""Replace st.session_state for every src module at once (they all do
133+
`import streamlit as st`, so patching the module attribute reaches all)."""
134+
import streamlit as st
135+
136+
state = FakeSessionState()
137+
monkeypatch.setattr(st, "session_state", state)
138+
return state
139+
140+
141+
@pytest.fixture
142+
def db_manager(tmp_path):
143+
"""A DatabaseManager on a throwaway sqlite file. close() in teardown is
144+
mandatory: an open handle makes tmp_path cleanup fail on Windows."""
145+
from database_operations import DatabaseManager
146+
147+
manager = DatabaseManager(str(tmp_path / "test.db"))
148+
yield manager
149+
manager.close()

0 commit comments

Comments
 (0)