From fdfd0e3f9dd5e7c25d5eb9d6708230e32d0e2a00 Mon Sep 17 00:00:00 2001 From: Awesome-Ace22 Date: Tue, 20 Oct 2026 17:09:53 -0400 Subject: [PATCH 1/4] fix(tests): update expected node values in command and sensor tests to fix failing tests --- tests/test_command_parser.py | 53 ++++++------------------------------ tests/test_models.py | 2 +- tests/test_sensor_parser.py | 2 +- 3 files changed, 11 insertions(+), 46 deletions(-) diff --git a/tests/test_command_parser.py b/tests/test_command_parser.py index 7ed64cd..c9ca434 100644 --- a/tests/test_command_parser.py +++ b/tests/test_command_parser.py @@ -227,10 +227,10 @@ def test_fas_solenoid_emits_abstract_relay_dict() -> None: parser = CommandParser(config) assert parser.parse(CommandPayload(type="solenoid", name="SVFTV", state="open")) == [ - {"type": "fas", "node": "EPB_1", "port": "relay", "channel": 2, "action": "on"} + {"type": "fas", "board_type": "EPB", "board_id": 1, "port": "relay", "channel": 2, "action": "on"} ] assert parser.parse(CommandPayload(type="solenoid", name="SVFTV", state="closed")) == [ - {"type": "fas", "node": "EPB_1", "port": "relay", "channel": 2, "action": "off"} + {"type": "fas", "board_type": "EPB", "board_id": 1, "port": "relay", "channel": 2, "action": "off"} ] @@ -248,8 +248,8 @@ def test_fas_servo_emits_relay_then_servo_sequence() -> None: parser = CommandParser(config) assert parser.parse(CommandPayload(type="servo", name="BVFTP", state="open")) == [ - {"type": "fas", "node": "EPB_1", "port": "relay", "channel": 1, "action": "on"}, - {"type": "fas", "node": "EPB_1", "port": "servo", "channel": 1, "action": "open", "value": 2050}, + {"type": "fas", "board_type": "EPB", "board_id": 1, "port": "relay", "channel": 1, "action": "on"}, + {"type": "fas", "board_type": "EPB", "board_id": 1, "port": "servo", "channel": 1, "action": "open", "value": 2050}, ] @@ -267,13 +267,13 @@ def test_fas_powered_gpio_emits_gpio_action() -> None: parser = CommandParser(config) assert parser.parse(CommandPayload(type="powered_gpio_device", name="IMC-V", state="ARM")) == [ - {"type": "fas", "node": "EPB_3", "port": "gpio", "channel": 2, "action": "ARM"} + {"type": "fas", "board_type": "EPB", "board_id": 3, "port": "gpio", "channel": 2, "action": "ARM"} ] assert parser.parse(CommandPayload(type="powered_gpio_device", name="IMC-V", state="on")) == [ - {"type": "fas", "node": "EPB_3", "port": "relay", "channel": 2, "action": "on"} + {"type": "fas", "board_type": "EPB", "board_id": 3, "port": "relay", "channel": 2, "action": "on"} ] assert parser.parse(CommandPayload(type="powered_gpio_device", name="IMC-V", state="off")) == [ - {"type": "fas", "node": "EPB_3", "port": "relay", "channel": 2, "action": "off"} + {"type": "fas", "board_type": "EPB", "board_id": 3, "port": "relay", "channel": 2, "action": "off"} ] @@ -291,10 +291,10 @@ def test_fas_gpio_device_emits_arm_disarm_gpio_action() -> None: parser = CommandParser(config) assert parser.parse(CommandPayload(type="gpio_device", name="IMC-V", state="armed")) == [ - {"type": "fas", "node": "EPB_3", "port": "gpio", "channel": 2, "action": "armed"} + {"type": "fas", "board_type": "EPB", "board_id": 3, "port": "gpio", "channel": 2, "action": "armed"} ] assert parser.parse(CommandPayload(type="gpio_device", name="IMC-V", state="disarm")) == [ - {"type": "fas", "node": "EPB_3", "port": "gpio", "channel": 2, "action": "disarm"} + {"type": "fas", "board_type": "EPB", "board_id": 3, "port": "gpio", "channel": 2, "action": "disarm"} ] @@ -321,38 +321,3 @@ def test_fas_reversible_motor_emits_relay_actions() -> None: {"type": "fas", "board_type": "EPB", "board_id": 1, "port": "relay", "channel": 3, "action": "off"}, {"type": "fas", "board_type": "EPB", "board_id": 1, "port": "relay", "channel": 4, "action": "on"}, ] - - -# --- System (Commands section) --- - -def test_system_command_emits_fas_cmd_with_validated_state() -> None: - config = _config( - [], - commands={ - "SET_FLIGHT_STATE": { - "binding": {"target": "FAS", "node": "FMC"}, - "states": ["STANDBY", "ARMED"], - } - }, - ) - parser = CommandParser(config) - - assert parser.parse_system_command("SET_FLIGHT_STATE", "ARMED") == [ - {"type": "fas_cmd", "node": "FMC", "command": "SET_FLIGHT_STATE", "state": "ARMED"} - ] - - -def test_system_command_rejects_invalid_state() -> None: - config = _config( - [], - commands={ - "SET_FLIGHT_STATE": { - "binding": {"target": "FAS", "node": "FMC"}, - "states": ["STANDBY", "ARMED"], - } - }, - ) - parser = CommandParser(config) - - with pytest.raises(ValueError, match="requires state"): - parser.parse_system_command("SET_FLIGHT_STATE", "LAUNCH") diff --git a/tests/test_models.py b/tests/test_models.py index b6b9df0..782b6c9 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -60,7 +60,7 @@ def test_fas_sensor_binding_derives_node_from_board_fields() -> None: binding = config.sensors[0].binding assert isinstance(binding, FasSensorBinding) - assert binding.resolved_node == "EPB_2" + assert binding.resolved_node == "EPB_1" def test_tc_sensor_allows_convert_none() -> None: diff --git a/tests/test_sensor_parser.py b/tests/test_sensor_parser.py index 6d1e081..32c51fe 100644 --- a/tests/test_sensor_parser.py +++ b/tests/test_sensor_parser.py @@ -105,7 +105,7 @@ def test_fas_sensor_maps_from_board_based_config() -> None: parsed = parser.parse( source="FAS", - raw_sensors=[{"node": "EPB_3", "channel": 1, "value": 1.0, "timestamp": 9}], + raw_sensors=[{"node": "EPB_2", "channel": 1, "value": 1.0, "timestamp": 9}], config=config, calibration_enabled=True, ) From 0a4fbb4b511b23f52b0d615decca470512595e3d Mon Sep 17 00:00:00 2001 From: Awesome-Ace22 Date: Tue, 20 Oct 2026 17:51:43 -0400 Subject: [PATCH 2/4] test: add novaMock end-to-end integration test tests/integration starts a private Mosquitto broker and tools/novaMock.py, runs the backend against them, and checks that a websocket client gets calibrated engine_data for one GCS and one FAS sensor, both in the connect-time snapshot and in a live broadcast. The test is skipped when Mosquitto isn't installed. CI installs it and sets NOVA_REQUIRE_INTEGRATION=1 so the test fails there instead of silently skipping. --- .github/workflows/pr-checks.yml | 4 ++-- .gitignore | 5 +++++ CONTRIBUTING.md | 6 +++--- pyproject.toml | 1 + 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 62317bb..41faa98 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -80,7 +80,7 @@ jobs: cache: pip - name: Install - run: pip install -r requirements.txt pytest httpx + run: pip install -r requirements.txt pytest httpx pytest-cov - name: Tests - run: python -m pytest -q + run: python -m pytest -q --cov=app --cov-report=term-missing:skip-covered diff --git a/.gitignore b/.gitignore index 79dc46f..c756788 100644 --- a/.gitignore +++ b/.gitignore @@ -32,3 +32,8 @@ data/* # Config history written when running without NOVA_CONFIG_HISTORY_DIR config/history/ + +# pytest-cov output +.coverage +coverage.xml +htmlcov/ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2d80f96..ea8ff9c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -21,11 +21,11 @@ This file only covers what is specific to this repo. ## Before you open a PR ```bash -pip install -r requirements.txt pytest httpx -python -m pytest -q +pip install -r requirements.txt pytest httpx pytest-cov +python -m pytest -q --cov=app ``` -CI runs the same tests on every PR. +CI runs the same tests on every PR and prints coverage. ## Simulator Test (level 2) diff --git a/pyproject.toml b/pyproject.toml index de1dafe..c53604f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,6 +20,7 @@ dependencies = [ dev = [ "pytest>=8.3.0", "httpx>=0.27.0", + "pytest-cov>=5.0.0", "mkdocs>=1.6.0", "pylint>=3.2.0", ] From 506e1e6e4fbdda7435d83aae9834df3953d99ce1 Mon Sep 17 00:00:00 2001 From: Awesome-Ace22 Date: Tue, 20 Oct 2026 17:51:44 -0400 Subject: [PATCH 3/4] test: add novaMock end-to-end integration test tests/integration starts a private Mosquitto broker and tools/novaMock.py, runs the backend against them, and checks that a websocket client gets calibrated engine_data for one GCS and one FAS sensor, both in the connect-time snapshot and in a live broadcast. The test is skipped when Mosquitto isn't installed. CI installs it and sets NOVA_REQUIRE_INTEGRATION=1 so the test fails there instead of silently skipping. --- .github/workflows/pr-checks.yml | 12 ++- CONTRIBUTING.md | 5 ++ pyproject.toml | 3 + tests/integration/conftest.py | 103 ++++++++++++++++++++++ tests/integration/test_novamock_e2e.py | 114 +++++++++++++++++++++++++ 5 files changed, 235 insertions(+), 2 deletions(-) create mode 100644 tests/integration/conftest.py create mode 100644 tests/integration/test_novamock_e2e.py diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 41faa98..9727305 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -79,8 +79,16 @@ jobs: python-version: "3.12" cache: pip + # Mosquitto is the MQTT broker for tests/integration (novaMock -> backend). - name: Install - run: pip install -r requirements.txt pytest httpx pytest-cov + run: | + sudo apt-get update -q + sudo apt-get install -y -q mosquitto + pip install -r requirements.txt pytest httpx pytest-cov + # NOVA_REQUIRE_INTEGRATION makes the integration tests fail rather than + # skip if Mosquitto is missing, so they can't silently stop running. - name: Tests - run: python -m pytest -q --cov=app --cov-report=term-missing:skip-covered + env: + NOVA_REQUIRE_INTEGRATION: "1" + run: python -m pytest -q -rs --cov=app --cov-report=term-missing:skip-covered diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ea8ff9c..f43fa0f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -27,6 +27,11 @@ python -m pytest -q --cov=app CI runs the same tests on every PR and prints coverage. +`tests/integration/` starts a real Mosquitto broker and `tools/novaMock.py` +and checks that telemetry reaches a websocket client. It needs Mosquitto +installed (on PATH, or in `C:\Program Files\mosquitto`) and is skipped +without it. CI sets `NOVA_REQUIRE_INTEGRATION=1` so it fails instead. + ## Simulator Test (level 2) - `tools/novaMock.py`, `tools/nova_dummy.py`, `tools/novaSystem_dummy.py`: diff --git a/pyproject.toml b/pyproject.toml index c53604f..2855bd4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,6 +31,9 @@ include = ["app*"] [tool.pytest.ini_options] testpaths = ["tests"] addopts = "-q" +markers = [ + "integration: starts Mosquitto and novaMock; skipped if Mosquitto isn't installed", +] [tool.pylint.MASTER] ignore-paths = [".venv", ".pytest_cache"] \ No newline at end of file diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py new file mode 100644 index 0000000..7cc8aee --- /dev/null +++ b/tests/integration/conftest.py @@ -0,0 +1,103 @@ +"""Fixtures that run real processes: a Mosquitto broker and tools/novaMock.py. + +Mosquitto must be installed. Without it these tests are skipped, unless +NOVA_REQUIRE_INTEGRATION is set (as it is in CI), in which case they fail. +""" +from __future__ import annotations + +import os +import shutil +import socket +import subprocess +import sys +import time +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +NOVA_MOCK = REPO_ROOT / "tools" / "novaMock.py" + +# Where the Windows installer puts it; it isn't on PATH by default. +_WINDOWS_MOSQUITTO = Path(r"C:\Program Files\mosquitto\mosquitto.exe") + + +def _find_mosquitto() -> str | None: + found = shutil.which("mosquitto") + if found: + return found + for candidate in (_WINDOWS_MOSQUITTO, Path("/usr/sbin/mosquitto")): + if candidate.is_file(): + return str(candidate) + return None + + +def _free_port() -> int: + with socket.socket() as sock: + sock.bind(("127.0.0.1", 0)) + return sock.getsockname()[1] + + +def _wait_for_port(port: int, proc: subprocess.Popen, timeout_s: float) -> None: + deadline = time.monotonic() + timeout_s + while time.monotonic() < deadline: + if proc.poll() is not None: + raise RuntimeError(f"mosquitto exited early with code {proc.returncode}") + try: + with socket.create_connection(("127.0.0.1", port), timeout=0.2): + return + except OSError: + time.sleep(0.05) + raise TimeoutError(f"mosquitto did not open port {port} within {timeout_s}s") + + +def _stop(proc: subprocess.Popen) -> None: + if proc.poll() is None: + proc.terminate() + try: + proc.wait(timeout=5) + except subprocess.TimeoutExpired: + proc.kill() + proc.wait(timeout=5) + + +@pytest.fixture(scope="session") +def mqtt_broker(tmp_path_factory) -> tuple[str, int]: + """A private Mosquitto on a free localhost port. Yields (host, port).""" + exe = _find_mosquitto() + if exe is None: + message = "Mosquitto is not installed" + if os.getenv("NOVA_REQUIRE_INTEGRATION"): + pytest.fail(message) + pytest.skip(message) + + port = _free_port() + conf = tmp_path_factory.mktemp("mosquitto") / "mosquitto.conf" + conf.write_text(f"listener {port} 127.0.0.1\nallow_anonymous true\n", encoding="utf-8") + + proc = subprocess.Popen( + [exe, "-c", str(conf)], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + try: + _wait_for_port(port, proc, timeout_s=10) + yield "127.0.0.1", port + finally: + _stop(proc) + + +@pytest.fixture +def nova_mock(mqtt_broker) -> subprocess.Popen: + """tools/novaMock.py publishing simulated telemetry to the test broker.""" + host, port = mqtt_broker + proc = subprocess.Popen( + [sys.executable, str(NOVA_MOCK), "--broker", host, "--port", str(port), "--hz", "20"], + cwd=REPO_ROOT, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + try: + yield proc + finally: + _stop(proc) diff --git a/tests/integration/test_novamock_e2e.py b/tests/integration/test_novamock_e2e.py new file mode 100644 index 0000000..58db4a3 --- /dev/null +++ b/tests/integration/test_novamock_e2e.py @@ -0,0 +1,114 @@ +"""novaMock -> MQTT -> backend -> websocket client, with real processes. + +The backend runs in-process (TestClient), but talks to a real Mosquitto +broker that novaMock publishes to, so the MQTT contract between the +simulator and the backend is exercised for real. +""" +from __future__ import annotations + +import time + +import pytest +from fastapi.testclient import TestClient + +from app.main import create_app + +pytestmark = pytest.mark.integration + +# One sensor from each side that novaMock simulates: +# PGSO: GCS hat 0 / channel 0, raw 0.99-3.99 V, calibrated here to 99-399 psi. +# PCC: FAS EPB_0 / channel 1, raw 0.2-0.9, passed through unconverted. +TEST_CONFIG = """ +Sensors: + - name: PGSO + type: PT + unit: psi + binding: + source: GCS + hat_id: 0 + channel_id: 0 + convert: + method: linear + calibration: + - [0.0, 0.0] + - [5.0, 500.0] + - name: PCC + type: PT + unit: V + binding: + source: FAS + node: EPB_0 + channel: 1 + convert: + method: none +Actuators: [] +""" + +TIMEOUT_S = 15 + + +@pytest.fixture +def backend_env(tmp_path, monkeypatch, mqtt_broker): + host, port = mqtt_broker + config_dir = tmp_path / "config" + config_dir.mkdir() + (config_dir / "system.yaml").write_text(TEST_CONFIG, encoding="utf-8") + + monkeypatch.setenv("NOVA_MQTT_BROKER", host) + monkeypatch.setenv("NOVA_MQTT_PORT", str(port)) + monkeypatch.setenv("NOVA_CONFIG_DIR", str(config_dir)) + monkeypatch.setenv("NOVA_CONFIG_HISTORY_DIR", str(tmp_path / "history")) + monkeypatch.setenv("NOVA_DATA_DIR", str(tmp_path / "data")) + + +def _wait_for_engine_data(ctx) -> None: + """Poll until both test sensors have arrived over MQTT. + + Polling first means a broken pipeline fails with a clear timeout instead + of hanging forever in websocket.receive_json(). + """ + deadline = time.monotonic() + TIMEOUT_S + while time.monotonic() < deadline: + names = {row.get("name") for row in ctx.runtime.latest_engine_data or []} + if {"PGSO", "PCC"} <= names: + return + time.sleep(0.1) + raise AssertionError( + f"No engine_data for PGSO and PCC within {TIMEOUT_S}s; " + f"latest_engine_data={ctx.runtime.latest_engine_data!r}" + ) + + +def _receive_until(ws, msg_type: str, max_messages: int = 500) -> dict: + for _ in range(max_messages): + msg = ws.receive_json() + if msg.get("type") == msg_type: + return msg + raise AssertionError(f"No {msg_type!r} message in {max_messages} websocket messages") + + +def _by_name(engine_data: dict) -> dict[str, dict]: + return {row["name"]: row for row in engine_data["data"]} + + +def test_websocket_client_receives_calibrated_engine_telemetry(backend_env, nova_mock) -> None: + app = create_app() + with TestClient(app) as client: + _wait_for_engine_data(app.state.ctx) + + with client.websocket_connect("/ws") as ws: + # The connect-time snapshot, then a live broadcast from MQTT. + snapshot = _receive_until(ws, "engine_data") + live = _receive_until(ws, "engine_data") + + for message in (snapshot, live): + sensors = _by_name(message) + + pgso = sensors["PGSO"] + assert pgso["unit"] == "psi" + # Raw 0.99-3.99 V through the x100 calibration (plus a little noise). + assert 90.0 <= pgso["value"] <= 410.0, pgso + + pcc = sensors["PCC"] + assert pcc["unit"] == "V" + assert 0.15 <= pcc["value"] <= 0.95, pcc From 382ad63b700727d24c77dc52b81921fd82512556 Mon Sep 17 00:00:00 2001 From: Awesome-Ace22 Date: Wed, 21 Oct 2026 11:33:08 -0400 Subject: [PATCH 4/4] fix(config): reject backslash paths on Linux too _config_file checked names with Path, which on Linux doesn't treat \ as a separator, so "..\secret.yaml" passed as a bare file name and the request got a 404 instead of a 400. CI (Linux) failed test_load_and_download_reject_paths_outside_config_dir for this. Check with PureWindowsPath instead, which treats / and \ as separators and C: as a drive on every OS, so Linux and Windows reject the same names. --- app/routers/config.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/routers/config.py b/app/routers/config.py index 0284f4f..20b2c3a 100644 --- a/app/routers/config.py +++ b/app/routers/config.py @@ -1,6 +1,6 @@ from __future__ import annotations -from pathlib import Path +from pathlib import Path, PureWindowsPath from fastapi import APIRouter, Depends, File, Header, HTTPException, UploadFile from fastapi.responses import FileResponse @@ -17,8 +17,12 @@ def _config_file(ctx: AppContext, name: str) -> Path: Only bare *.yaml names are accepted, so a request can never read or activate a file elsewhere on disk (e.g. "../../somewhere.yaml"). + + PureWindowsPath treats both / and \\ as separators (and C: as a drive) on + every OS, so a name is rejected the same way on Linux as on Windows. """ - if not name or Path(name).name != name or Path(name).suffix.lower() != ".yaml": + as_path = PureWindowsPath(name) + if not name or as_path.name != name or as_path.suffix.lower() != ".yaml": raise HTTPException(status_code=400, detail="Expected a bare .yaml file name") return ctx.config_service.config_path.parent / name