From fcb2388197ff2cc1e5d99ff55f85bd04efa53f91 Mon Sep 17 00:00:00 2001 From: Manish Kumar Date: Thu, 3 Sep 2026 21:12:48 -0500 Subject: [PATCH] test: fix Docker fixture volume mount, raise coverage, gate at 95% Baseline was 95% but with a genuinely FAILING test, not just a coverage gap: test_manifest_headers_and_cors (tests/test_generated_infra.py) was hitting a real 404 because its docker_service fixture builds and runs the image without ever mounting content/ at /videos -- nginx.conf's /videos.json and /videos/ locations always read from /videos, never from the image's own /usr/share/nginx/html/ (that only backs the "/" SPA route). In production, omnibioai-studio's docker-compose supplies this via `${VIDEO_DIR}:/videos:ro`; the test fixture never did. Verified against the live production container (read-only, over HTTP) that nginx.conf/Dockerfile are correct as shipped -- production's own /videos.json request behaves exactly as expected once the mount is present. Confirmed the failure and the fix by building the image locally and running it with/without the mount on a scratch port before touching anything. Fix: docker_service now mounts content/ at /videos:ro, matching production's real contract. The previously-failing test now genuinely passes (not just silenced). Also closed the two remaining honestly-closable coverage gaps: - conftest.py's pytest_runtest_setup: both Docker-unavailable skip branches (missing executable, unreachable daemon), via a fake pytest item rather than actually uninstalling Docker. - test_manifest.py: the malformed-JSON -> pytest.fail path, via a scoped MANIFEST_PATH swap to a deliberately broken temp file. Left alone on purpose: - test_static_contracts.py's two already-`xfail`(strict)-marked, already-documented production defects (index.html's video URL scheme, zero-byte placeholder videos) -- not this change's call to fix. - tests/test_video_service.py: a superseded duplicate of test_generated_infra.py's own Docker tests, hardcoded to port 8086 -- the same port the real production videos container binds to, so it can never start alongside a running deployment (works fine on a clean CI runner with nothing on 8086). Already excluded from the coverage measurement by this repo's own prior .coveragerc; now documented why in pyproject.toml instead of silently omitted. Deleting it is a repo-hygiene call for whoever owns this repo. Result: 97.73% (was 95%), 23 passed + 2 expected xfails. Added --cov-fail-under=95 to pyproject.toml's addopts so this is enforced on every `pytest` invocation, not just measured ad hoc. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01VevfQdYwX27LkcGqfRAxQL --- pyproject.toml | 8 ++++++- tests/test_conftest_hooks.py | 42 +++++++++++++++++++++++++++++++++++ tests/test_generated_infra.py | 14 ++++++++++-- tests/test_manifest.py | 11 +++++++++ 4 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 tests/test_conftest_hooks.py diff --git a/pyproject.toml b/pyproject.toml index 30a9083..1d72581 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,10 +1,16 @@ [tool.pytest.ini_options] -addopts = "--cov=tests --cov-report=term-missing" +addopts = "--cov=tests --cov-report=term-missing --cov-fail-under=95" markers = [ "docker: tests that build and exercise the nginx image through Docker", ] [tool.coverage.run] +# test_video_service.py is a superseded duplicate of test_generated_infra.py's +# Part 2 (same build-and-run-the-image approach, but hardcoded to port 8086 -- +# the same port omnibioai-studio's docker-compose binds the real videos +# service to, so it can never start alongside a running deployment). Kept +# out of the coverage measurement rather than deleted outright -- that's a +# repo-hygiene call for whoever owns this repo, not this change. omit = ["tests/test_video_service.py"] [tool.coverage.report] diff --git a/tests/test_conftest_hooks.py b/tests/test_conftest_hooks.py new file mode 100644 index 0000000..4f23428 --- /dev/null +++ b/tests/test_conftest_hooks.py @@ -0,0 +1,42 @@ +"""Direct unit coverage for conftest.py's Docker-availability skip hook. + +pytest_runtest_setup only ever runs (and its skip branches only ever +trigger) inside a real pytest session collecting a real test item, so the +two skip paths -- docker not installed, docker daemon unreachable -- are +exercised here by calling the hook directly against a minimal fake item, +rather than by actually uninstalling Docker. +""" + +import shutil +import subprocess + +import pytest + +import conftest as _conftest + + +class _FakeItem: + def __init__(self, keywords): + self.keywords = keywords + + +def test_pytest_runtest_setup_ignores_non_docker_items(): + item = _FakeItem(keywords={}) + _conftest.pytest_runtest_setup(item) # must not raise or skip + + +def test_pytest_runtest_setup_skips_when_docker_executable_missing(monkeypatch): + monkeypatch.setattr(shutil, "which", lambda name: None) + item = _FakeItem(keywords={"docker": True}) + with pytest.raises(pytest.skip.Exception, match="docker executable"): + _conftest.pytest_runtest_setup(item) + + +def test_pytest_runtest_setup_skips_when_docker_daemon_unreachable(monkeypatch): + monkeypatch.setattr(shutil, "which", lambda name: "/usr/bin/docker") + monkeypatch.setattr( + subprocess, "run", lambda *a, **k: subprocess.CompletedProcess(a, returncode=1) + ) + item = _FakeItem(keywords={"docker": True}) + with pytest.raises(pytest.skip.Exception, match="Docker daemon"): + _conftest.pytest_runtest_setup(item) diff --git a/tests/test_generated_infra.py b/tests/test_generated_infra.py index 6a2e797..9a213d9 100644 --- a/tests/test_generated_infra.py +++ b/tests/test_generated_infra.py @@ -2,6 +2,8 @@ import json import subprocess import time +from pathlib import Path + import pytest import requests @@ -63,10 +65,18 @@ def docker_service(): # Build subprocess.run(["docker", "build", "-t", IMAGE_NAME, "."], capture_output=True, check=True) - # Run + # Run -- mount content/ at /videos:ro, exactly as omnibioai-studio's + # docker-compose does with ${VIDEO_DIR}:/videos:ro in production. + # nginx.conf's /videos.json and /videos/ locations always read from + # /videos (never from the image's own /usr/share/nginx/html/, which + # only backs the "/" SPA route) -- without this mount they 404 against + # any freshly-built container, mount or not. + content_dir = Path(__file__).resolve().parents[1] / "content" subprocess.run([ "docker", "run", "-d", "--name", CONTAINER_NAME, - "-p", f"{PORT}:8086", IMAGE_NAME + "-p", f"{PORT}:8086", + "-v", f"{content_dir}:/videos:ro", + IMAGE_NAME ], capture_output=True, check=True) # Wait diff --git a/tests/test_manifest.py b/tests/test_manifest.py index d6e7fd9..5663f98 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -17,6 +17,17 @@ def test_manifest_is_valid_json(): except json.JSONDecodeError as e: pytest.fail(f"Manifest is not valid JSON: {e}") +def test_manifest_is_valid_json_reports_decode_errors(tmp_path): + global MANIFEST_PATH + bad_manifest = tmp_path / "bad.json" + bad_manifest.write_text("{not valid json") + original, MANIFEST_PATH = MANIFEST_PATH, str(bad_manifest) + try: + with pytest.raises(pytest.fail.Exception, match="not valid JSON"): + test_manifest_is_valid_json() + finally: + MANIFEST_PATH = original + def test_manifest_schema(): with open(MANIFEST_PATH, 'r') as f: data = json.load(f)