test: expand HPC policy engine unit coverage - #3
Merged
Merged
Conversation
Adds 91 new deterministic unit tests (69 -> 160 passing) targeting policy evaluation, quota enforcement, scheduler stubs, request validation, and config edge cases. Fixes a coverage-visibility gap: eight modules (app/core/gpu.py, policies.py, quota.py, scheduler.py, app/services/ quota_service.py, scheduler_service.py, app/api/routes_policy.py, routes_quota.py) ship as pre-built Cython .so extensions alongside their .py source; on a host whose interpreter ABI matches the checked-in .so tag, a normal import silently resolves to the compiled binary, which coverage.py cannot trace -- so ~70 statements / 20 branches of policy-critical logic never appeared in a coverage report despite being exercised. New tests load the real .py source directly (tests/_srcload.py, same technique test_setup_module.py already used for setup.py) to measure it regardless of host architecture. No production source files were modified. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
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
Test-only PR. Audits
omnibioai-hpc-policy-engineand adds 91 new deterministic unit tests (69 → 160 passing) covering policy evaluation, quota enforcement, request validation/authorization, and configuration edge cases. Zero production source files were modified.Baseline (before this PR)
pytest --cov=app --cov-branchreported 100% on only 9 of 17 app modules (106 statements / 4 branches) —app/api/deps.py,core/config.py,db/models.py,db/session.py,main.py,models/decision.py,models/job.py,models/quota.py,services/usage_service.py.app/core/gpu.py,policies.py,quota.py,scheduler.py,app/services/quota_service.py,scheduler_service.py,app/api/routes_policy.py,routes_quota.py.Root cause of the coverage gap (infrastructure limitation, documented not fixed)
setup.pyCython-compiles those 8 modules into pre-built.soextensions that are checked into git alongside their.pysource (e.g.app/core/gpu.cpython-313-aarch64-linux-gnu.sonext toapp/core/gpu.py, from commitc213f05, "IP protection"). CPython's import machinery prefers a matching extension module over a same-named.pyfile. On a host whose interpreter ABI/arch matches the checked-in.sotag (cpython-313-aarch64-linux-gnu— this dev machine, Python 3.13.9 on aarch64), a plainimport app.core.gpusilently resolves to the compiled.so, not the.py.coverage.pycannot trace into a compiled extension, so those lines never show up in a coverage report even though the logic is exercised (indirectly, through the compiled binary) by the existing test suite.This is environment-specific: CI (
.github/workflows/ci.yml) runs Python 3.11, whose extension-suffix tag won't match the checked-in.sofiles, so.sois skipped automatically there and.pyis measured normally with no workaround needed. Verified the.pyand.so/.cwere last touched together in the same commit (7a95999), so they're in sync at HEAD — this is a coverage-attribution/tooling artifact of this specific dev machine, not evidence of stale or diverging binaries, and not a correctness issue.Fix (test-only)
Added
tests/_srcload.py, a helper that loads a module directly from its.pyfile path viaimportlib.util.spec_from_file_location(the same technique the existingtests/test_setup_module.pyalready uses forsetup.py).coverage.pyattributes execution by file path, not by module identity, so this makes the real.pysource get measured regardless of host architecture, without touching any production file or changing any behavior (Cython compiles these files essentially as-is).Tests added (11 new files, 91 tests)
tests/_srcload.py— shared test-only source-loader helper (not a test file itself)tests/test_core_gpu_source.py(9) —validate_gpu_access: zero/negative gpu counts, role membership (exact match, case-sensitive, no substring match), boundary casestests/test_core_policies_source.py(8) —validate_partition_access: dgx-a100 gating, unknown/case-mismatched/empty partition names pass through ungated (characterized, not fixed)tests/test_core_quota_source.py(12) —evaluate_quota: exact-boundary requests, cpu-checked-before-gpu precedence, negative requests and negative recorded usage accepted (characterized, not fixed), already-over-budget usagetests/test_core_scheduler_source.py(3) —SchedulerAdapter: stub return value, no failure path exists to test (see Infrastructure limitations)tests/test_quota_service_source.py(6) —QuotaService.evaluate: gpu/partition/quota check short-circuit ordering, denied-decision remaining-hours defaultstests/test_scheduler_service_source.py(3) —SchedulerService: delegation toSchedulerAdapter, not reachable from any routetests/test_routes_policy_source.py(17) —POST /jobs/evaluate: allow/deny precedence, malformed JSON body, wrong-typed fields (422), negative gpus/memory accepted (characterized), unknown extra fields ignored, org_id has no effect on the decision (characterized isolation gap)tests/test_routes_quota_source.py(11) —POST /quota/check: DB-mocked route wiring plus end-to-end (realQuotaService) gpu/dgx scenarios, malformed JSON, negative cpu_hours accepted (characterized)tests/test_config_module.py(7) —Config: safe defaults, env var overrides, non-numericMYSQL_PORT/DEFAULT_CPU_HOURS/MAX_CONCURRENT_JOBSraiseValueErrorat import time with no graceful handling (bug discovered, not fixed)tests/test_models_validation.py(15) —Decision/JobRequest/QuotaCheck: required fields, numeric coercion, mutable-default-list safety,QuotaCheckhas noorg_idfield at all (characterized isolation gap)No pre-existing test file was modified.
Final test results
Coverage (branch coverage enabled)
All 17 app modules now measured; 100% line and branch coverage. Command:
pytest --cov=app --cov-branch --cov-report=term-missing.Ruff
ruff check <the 11 new files>→ All checks passed. (Pre-existing lint findings in untouched files, e.g.tests/test_usage_service.py, are out of scope for this test-only change and were left as-is.)Policy/scheduler behaviors covered
gpus <= 0short-circuit, exact-role membership, case sensitivity)"dgx-a100"literal is gated; everything else passes)>semantics, negative/over-budget usage)QuotaServicecheck ordering/short-circuiting (gpu → partition → quota)/jobs/evaluateand/quota/checkrequest validation (missing/wrong-typed fields, malformed JSON, extra fields ignored)Remaining gaps / infrastructure limitations
SchedulerAdapter.get_cluster_load()is a hardcoded stub with no network/subprocess call, no error handling, no timeout, no retry logic — there is nothing to mock and no failure path to test.SchedulerService/SchedulerAdapterare also not wired into any API route (no reference underapp/api/), so scheduler-unavailable/scheduler-error/timeout/retry scenarios from the task's priority list don't apply to this codebase as it stands.validate_gpu_access,validate_partition_access) called in a fixed sequence, not a rule list. The sequence's own precedence is tested (seetest_quota_service_source.py).app/—/jobs/evaluateis a single stateless allow/deny check with no persistence. Nothing to test here without inventing functionality.JobRequest.org_idexists on the model but is never read byevaluate_job()(characterized intest_routes_policy_source.py);QuotaCheckhas noorg_idfield at all (characterized intest_models_validation.py). This is a real authorization gap, not a test gap — left as-is per the test-only scope.Bugs/gaps discovered but not fixed (production code untouched)
app/core/gpu.py/app/core/quota.py: no lower-bound validation ongpus,cpu_hours,gpu_hours,memory_gb— negative values are silently accepted end-to-end (model → route → policy function).app/core/config.py: a non-numericMYSQL_PORT,DEFAULT_CPU_HOURS,DEFAULT_GPU_HOURS, orMAX_CONCURRENT_JOBSenv var raises an uncaughtValueErrorat import time — the whole service fails to start with a raw traceback rather than a clear configuration error.app/core/policies.py: partition names are not validated against an allow-list — any string other than the literal"dgx-a100"is treated as an ungated partition, including typos.None of these were fixed, per the test-only scope of this change.
Confirmations
git diff main --stat -- . ':!tests'is empty; only files undertests/were added)test/hpc-policy-engine-unit-coverage🤖 Generated with Claude Code