Skip to content

test: expand HPC policy engine unit coverage - #3

Merged
man4ish merged 1 commit into
mainfrom
test/hpc-policy-engine-unit-coverage
Aug 26, 2026
Merged

man4ish merged 1 commit into
mainfrom
test/hpc-policy-engine-unit-coverage

Conversation

@man4ish

@man4ish man4ish commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

Test-only PR. Audits omnibioai-hpc-policy-engine and 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)

  • 69 tests passing, 0.30s
  • pytest --cov=app --cov-branch reported 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.
  • The other 8 modules never appeared in the coverage report at all: 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.py Cython-compiles those 8 modules into pre-built .so extensions that are checked into git alongside their .py source (e.g. app/core/gpu.cpython-313-aarch64-linux-gnu.so next to app/core/gpu.py, from commit c213f05, "IP protection"). CPython's import machinery prefers a matching extension module over a same-named .py file. On a host whose interpreter ABI/arch matches the checked-in .so tag (cpython-313-aarch64-linux-gnu — this dev machine, Python 3.13.9 on aarch64), a plain import app.core.gpu silently resolves to the compiled .so, not the .py. coverage.py cannot 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 .so files, so .so is skipped automatically there and .py is measured normally with no workaround needed. Verified the .py and .so/.c were 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 .py file path via importlib.util.spec_from_file_location (the same technique the existing tests/test_setup_module.py already uses for setup.py). coverage.py attributes execution by file path, not by module identity, so this makes the real .py source 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 cases
  • tests/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 usage
  • tests/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 defaults
  • tests/test_scheduler_service_source.py (3) — SchedulerService: delegation to SchedulerAdapter, not reachable from any route
  • tests/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 (real QuotaService) gpu/dgx scenarios, malformed JSON, negative cpu_hours accepted (characterized)
  • tests/test_config_module.py (7) — Config: safe defaults, env var overrides, non-numeric MYSQL_PORT/DEFAULT_CPU_HOURS/MAX_CONCURRENT_JOBS raise ValueError at 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, QuotaCheck has no org_id field at all (characterized isolation gap)

No pre-existing test file was modified.

Final test results

160 passed in 0.69s

Coverage (branch coverage enabled)

Name                                Stmts   Miss Branch BrPart  Cover
-------------------------------------------------------------------------
app/api/deps.py                         6      0      0      0   100%
app/api/routes_policy.py               14      0      4      0   100%
app/api/routes_quota.py                12      0      0      0   100%
app/core/config.py                     12      0      0      0   100%
app/core/gpu.py                         6      0      4      0   100%
app/core/policies.py                    5      0      4      0   100%
app/core/quota.py                       9      0      4      0   100%
app/core/scheduler.py                   3      0      0      0   100%
app/db/models.py                        9      0      0      0   100%
app/db/session.py                       7      0      0      0   100%
app/main.py                            34      0      2      0   100%
app/models/decision.py                  6      0      0      0   100%
app/models/job.py                      11      0      0      0   100%
app/models/quota.py                     8      0      0      0   100%
app/services/quota_service.py          15      0      4      0   100%
app/services/scheduler_service.py       6      0      0      0   100%
app/services/usage_service.py          13      0      2      0   100%
-------------------------------------------------------------------------
TOTAL                                 176      0     24      0   100%

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

  • GPU access gate (gpus <= 0 short-circuit, exact-role membership, case sensitivity)
  • DGX partition gate (only "dgx-a100" literal is gated; everything else passes)
  • Quota evaluation (cpu-before-gpu precedence, exact-boundary > semantics, negative/over-budget usage)
  • QuotaService check ordering/short-circuiting (gpu → partition → quota)
  • /jobs/evaluate and /quota/check request validation (missing/wrong-typed fields, malformed JSON, extra fields ignored)
  • Config defaults, env var overrides, and unhandled invalid-config failure mode
  • Pydantic model defaults, required fields, and mutable-default-list safety

Remaining gaps / infrastructure limitations

  • No real Slurm/PBS/LSF/Kubernetes/cloud scheduler integration exists. 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/SchedulerAdapter are also not wired into any API route (no reference under app/api/), so scheduler-unavailable/scheduler-error/timeout/retry scenarios from the task's priority list don't apply to this codebase as it stands.
  • No declarative policy/rule engine. "Multiple matching rules," "rule precedence," "conflicting rules," and "malformed policy" (as a data structure) don't map onto this codebase — policy is two independent Python functions (validate_gpu_access, validate_partition_access) called in a fixed sequence, not a rule list. The sequence's own precedence is tested (see test_quota_service_source.py).
  • No job lifecycle exists. There's no submit/running/completed/cancelled state machine anywhere in app/ — /jobs/evaluate is a single stateless allow/deny check with no persistence. Nothing to test here without inventing functionality.
  • No org/project isolation is enforced. JobRequest.org_id exists on the model but is never read by evaluate_job() (characterized in test_routes_policy_source.py); QuotaCheck has no org_id field at all (characterized in test_models_validation.py). This is a real authorization gap, not a test gap — left as-is per the test-only scope.
  • Tests run entirely without a real HPC cluster; DB is mocked via FastAPI dependency override (no MySQL connection needed), matching the existing test suite's approach.

Bugs/gaps discovered but not fixed (production code untouched)

  1. app/core/gpu.py / app/core/quota.py: no lower-bound validation on gpus, cpu_hours, gpu_hours, memory_gb — negative values are silently accepted end-to-end (model → route → policy function).
  2. app/core/config.py: a non-numeric MYSQL_PORT, DEFAULT_CPU_HOURS, DEFAULT_GPU_HOURS, or MAX_CONCURRENT_JOBS env var raises an uncaught ValueError at import time — the whole service fails to start with a raw traceback rather than a clear configuration error.
  3. 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.
  4. Organization/project isolation gap described above.
  5. Coverage-visibility gap on the 8 Cython-shadowed modules described above (environment-specific, not a CI/production issue — root-caused and worked around in this PR's test infrastructure).

None of these were fixed, per the test-only scope of this change.

Confirmations

  • Production source files modified: 0 (git diff main --stat -- . ':!tests' is empty; only files under tests/ were added)
  • No coverage exclusions or pragmas added
  • No coverage thresholds lowered (none existed before; none added)
  • No pre-existing tests modified or weakened
  • Branch: test/hpc-policy-engine-unit-coverage
  • PR left OPEN, not merged

🤖 Generated with Claude Code

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>
@man4ish
man4ish merged commit ed9994c into main Aug 26, 2026
1 of 2 checks passed
@man4ish
man4ish deleted the test/hpc-policy-engine-unit-coverage branch August 26, 2026 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant