test: fix Docker fixture volume mount, raise coverage, gate at 95% - #5
Merged
Merged
Conversation
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VevfQdYwX27LkcGqfRAxQL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Baseline was 95% but with a genuinely failing test, not just a coverage gap:
test_manifest_headers_and_corswas hitting a real 404 because its Docker fixture builds and runs the image without ever mountingcontent/at/videos—nginx.conf's/videos.jsonand/videos/locations always read from/videos, populated only by a runtime bind mount in production (omnibioai-studio's docker-compose:${VIDEO_DIR}:/videos:ro). Verified against the live production container (read-only, over HTTP) thatnginx.conf/Dockerfileare correct as shipped; confirmed the failure and the fix by building the image locally with/without the mount on a scratch port before touching anything.Fix: the fixture now mounts
content/at/videos:ro, matching production's real contract. The previously-failing test now genuinely passes.Also closed the two remaining honestly-closable coverage gaps (
conftest.py's Docker-availability skip branches,test_manifest.py's malformed-JSON path).Left alone on purpose:
test_static_contracts.py's two already-xfail(strict)-marked, already-documented production defects — not this change's call to fix.tests/test_video_service.py: a superseded duplicate oftest_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). Already excluded from coverage by this repo's own prior.coveragerc; now documented why 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=95topyproject.toml's addopts.🤖 Generated with Claude Code