Skip to content

Commit 3d7e0e9

Browse files
Krishcalinclaude
andcommitted
CI runs the store tests, and the server's dependencies are declared
Two gaps I flagged when the server half landed, both of the same kind: something that looked covered and was not. CI NOW EXERCISES THE SQL ON EVERY PUSH A postgres:16-alpine service on every matrix leg, with OT_TEST_DSN in the job env. fail-fast: false, so one Python version failing does not hide the others. The service is the easy part. The real problem was that a SKIPPED test in CI reads exactly like a passing one on the summary page -- the same looks-clean-but-was-never-checked failure this whole product is built around, one level up in the toolchain. So there are two guards: * test_server_store.py REFUSES to skip when CI=true and raises with an explanation. Verified both ways: it errors under CI=true with no DSN, and stays an ordinary skip on a developer machine. * a final CI step re-runs that file and greps for "passed" with no "skipped", so a service that fails to come up turns the build red rather than quietly green having exercised no SQL at all. Simulated locally against a real container the way CI will run it: 243 passed, 0 skipped. DEPENDENCIES DECLARED, AND KEPT APART ot_server/requirements.txt -- fastapi, uvicorn, psycopg[binary] -- separate from the collector's, which stays dpkt alone. A collector is an appliance in a substation and should carry as little as it can; a server is operator-hosted and may carry a web framework and a database driver. Merging them would put a web framework on every Pi. requirements-dev.txt gains the server deps because the suite covers all three components. Without them the server tests do not fail, they ERROR at import, which reads as a broken checkout rather than a missing install. CHECKED RATHER THAN ASSUMED The matrix still includes Python 3.8, so before adding these I verified in a python:3.8-slim container that fastapi 0.124.4 and psycopg 3.2.13 actually install there. Had they not, this commit would have broken the 3.8 leg on the next push -- and the breakage would have looked like a dependency problem rather than a decision. The POSTGRES_PASSWORD in the workflow is a throwaway for an ephemeral per-job container reachable only from the runner, and is commented as such. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 5d9b505 commit 3d7e0e9

4 files changed

Lines changed: 75 additions & 0 deletions

File tree

.github/workflows/ci.yml

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,38 @@ jobs:
1010
test:
1111
runs-on: ubuntu-latest
1212
strategy:
13+
# Report every leg. A failure on one Python version is information, not a
14+
# reason to stop testing the others.
15+
fail-fast: false
1316
matrix:
1417
python-version: ["3.8", "3.10", "3.12"]
1518

19+
# PostgreSQL for the server store tests (Q1: PostgreSQL only).
20+
#
21+
# Without this the store tests SKIP, and a skipped test in CI reads exactly
22+
# like a passing one on the summary page — which is the same
23+
# looks-clean-but-was-never-checked failure the whole product is built to
24+
# avoid. tests/test_server_store.py refuses to skip when CI=true, so a
25+
# service that fails to start is a red build rather than a quiet one.
26+
services:
27+
postgres:
28+
image: postgres:16-alpine
29+
env:
30+
POSTGRES_USER: otfleet
31+
# Ephemeral, per-job container reachable only from this runner.
32+
POSTGRES_PASSWORD: ci-throwaway
33+
POSTGRES_DB: otfleet
34+
ports:
35+
- 5432:5432
36+
options: >-
37+
--health-cmd "pg_isready -U otfleet -d otfleet"
38+
--health-interval 10s
39+
--health-timeout 5s
40+
--health-retries 5
41+
42+
env:
43+
OT_TEST_DSN: postgresql://otfleet:ci-throwaway@localhost:5432/otfleet
44+
1645
steps:
1746
- uses: actions/checkout@v4
1847

@@ -31,3 +60,14 @@ jobs:
3160
- name: Run tests
3261
working-directory: ot_scanner
3362
run: python -m pytest tests/ -v --tb=short
63+
64+
- name: Confirm the store tests actually ran
65+
working-directory: ot_scanner
66+
# Belt and braces on top of the in-test guard: if the Postgres service
67+
# is unreachable these deselect to zero and the run would otherwise be
68+
# green having tested no SQL at all.
69+
run: |
70+
python -m pytest tests/test_server_store.py -q \
71+
| tee /tmp/store.txt
72+
grep -qE "[0-9]+ passed" /tmp/store.txt
73+
! grep -q "skipped" /tmp/store.txt

ot_scanner/requirements-dev.txt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,12 @@
1+
# Test dependencies for the whole repository — scanner, collector AND server.
2+
#
3+
# The suite in ot_scanner/tests/ covers all three, so running it needs the
4+
# server's dependencies too. Without them the server tests do not fail, they
5+
# ERROR at import, which reads as a broken checkout rather than a missing
6+
# install.
17
pytest>=7.0
8+
9+
# Server (ot_server/). Kept here rather than referenced with -r so a single
10+
# `pip install -r requirements-dev.txt` from ot_scanner/ sets up everything.
11+
fastapi>=0.110
12+
psycopg[binary]>=3.1

ot_scanner/tests/test_server_store.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,19 @@
2626
sys.path.insert(0, _ROOT)
2727

2828
DSN = os.environ.get("OT_TEST_DSN")
29+
30+
# A SKIPPED test in CI reads exactly like a passing one on the summary page.
31+
# That is the same looks-clean-but-was-never-checked failure this whole product
32+
# is built to avoid, so the skip is allowed on a developer machine and REFUSED
33+
# in CI: if the Postgres service did not come up, the build goes red rather than
34+
# quietly green having exercised no SQL at all.
35+
_IN_CI = os.environ.get("CI", "").lower() in ("1", "true", "yes")
36+
if _IN_CI and not DSN:
37+
raise RuntimeError(
38+
"OT_TEST_DSN is unset in CI. The PostgreSQL store tests would skip, and "
39+
"a skipped test on the summary page is indistinguishable from a passing "
40+
"one. Start the postgres service or remove this suite deliberately.")
41+
2942
pytestmark = pytest.mark.skipif(
3043
not DSN, reason="set OT_TEST_DSN to run the PostgreSQL store tests")
3144

ot_server/requirements.txt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# OT Sensor Fleet — analysis server runtime (OTS-SRS-001 Phase 3+).
2+
#
3+
# Deliberately NOT the collector's dependencies. A collector is an appliance in
4+
# a substation and carries as little as it can (dpkt alone, see
5+
# ot_scanner/collector/manifest.py RUNTIME_REQUIRES); the server is an operator-
6+
# hosted service and may carry a web framework and a database driver.
7+
#
8+
# Verified to install on Python 3.8, which the CI matrix still covers.
9+
fastapi>=0.110
10+
uvicorn>=0.27
11+
psycopg[binary]>=3.1 # Q1: PostgreSQL only

0 commit comments

Comments
 (0)