From 73e23807c2f572c7969322db8ed105ff755d7536 Mon Sep 17 00:00:00 2001 From: Manish Kumar Date: Tue, 25 Aug 2026 22:37:42 -0500 Subject: [PATCH] test: expand HPC policy engine unit coverage 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 --- tests/_srcload.py | 44 +++++++ tests/test_config_module.py | 113 +++++++++++++++++ tests/test_core_gpu_source.py | 64 ++++++++++ tests/test_core_policies_source.py | 58 +++++++++ tests/test_core_quota_source.py | 138 +++++++++++++++++++++ tests/test_core_scheduler_source.py | 38 ++++++ tests/test_models_validation.py | 121 ++++++++++++++++++ tests/test_quota_service_source.py | 111 +++++++++++++++++ tests/test_routes_policy_source.py | 162 +++++++++++++++++++++++++ tests/test_routes_quota_source.py | 156 ++++++++++++++++++++++++ tests/test_scheduler_service_source.py | 34 ++++++ 11 files changed, 1039 insertions(+) create mode 100644 tests/_srcload.py create mode 100644 tests/test_config_module.py create mode 100644 tests/test_core_gpu_source.py create mode 100644 tests/test_core_policies_source.py create mode 100644 tests/test_core_quota_source.py create mode 100644 tests/test_core_scheduler_source.py create mode 100644 tests/test_models_validation.py create mode 100644 tests/test_quota_service_source.py create mode 100644 tests/test_routes_policy_source.py create mode 100644 tests/test_routes_quota_source.py create mode 100644 tests/test_scheduler_service_source.py diff --git a/tests/_srcload.py b/tests/_srcload.py new file mode 100644 index 0000000..6e7276f --- /dev/null +++ b/tests/_srcload.py @@ -0,0 +1,44 @@ +""" +Test-only helper: import a module directly from its .py source file on disk, +bypassing normal package import resolution. + +Why this exists: setup.py (see EXTENSIONS there) Cython-compiles eight +modules (app/core/gpu.py, policies.py, quota.py, scheduler.py, +app/services/quota_service.py, scheduler_service.py, and +app/api/routes_policy.py, routes_quota.py) 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. + +CPython's default import machinery prefers a matching extension module over +a same-named .py file. On a host whose interpreter ABI/arch happens to match +the checked-in .so tag (cpython-313-aarch64-linux-gnu), a plain +`import app.core.gpu` silently resolves to the compiled .so, not the .py. +coverage.py cannot trace execution inside a compiled extension, so on such a +host those modules' lines never appear in a coverage report at all -- even +though their logic *is* being exercised (indirectly, through the compiled +binary) by tests that import them normally. On hosts where the .so tag does +not match (e.g. this repo's CI, which runs Python 3.11), the .so is skipped +automatically and the .py import already gets measured -- no workaround +needed there. + +Loading the .py file directly by path (the same technique +tests/test_setup_module.py already uses for setup.py) sidesteps the .so/.py +shadowing so the real .py source is what gets executed and measured, +regardless of host architecture. It does not change any production +behavior -- Cython compiles these files essentially as-is, so the .py +source and the compiled extension implement the same logic. +""" +import importlib.util +import os + +_REPO_ROOT = os.path.normpath(os.path.join(os.path.dirname(__file__), "..")) + + +def load(relative_path: str): + """Load and return the .py module at `relative_path` (repo-root-relative).""" + full_path = os.path.join(_REPO_ROOT, relative_path) + module_name = "srcload_" + relative_path.replace("/", "_").replace(".", "_") + spec = importlib.util.spec_from_file_location(module_name, full_path) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod diff --git a/tests/test_config_module.py b/tests/test_config_module.py new file mode 100644 index 0000000..d6c8414 --- /dev/null +++ b/tests/test_config_module.py @@ -0,0 +1,113 @@ +""" +Unit tests for app/core/config.py::Config. + +Config.* are plain class attributes evaluated once, at module-exec time, +from os.getenv(...). To exercise different env-var combinations +deterministically (without mutating the single Config object every other +test file in this session imports via `app.core.config`), each test here +loads a *fresh* copy of config.py by file path (tests/_srcload.py) inside an +os.environ patch, so the env vars are only visible to that one exec and +nothing else in the suite is affected. +""" +import os +from unittest.mock import patch + +from tests._srcload import load + + +def _load_config_with_env(env: dict): + # os.environ is read at class-body execution time inside config.py, so + # the patch must be active for the load() call itself. + clean_env = {k: v for k, v in os.environ.items() if not k.startswith(( + "MYSQL_", "REDIS_URL", "DEFAULT_CPU_HOURS", "DEFAULT_GPU_HOURS", "MAX_CONCURRENT_JOBS", + ))} + with patch.dict(os.environ, {**clean_env, **env}, clear=True): + return load("app/core/config.py") + + +# --------------------------------------------------------------------------- +# Safe defaults when nothing is configured +# --------------------------------------------------------------------------- + +def test_defaults_used_when_no_env_vars_set(): + cfg = _load_config_with_env({}) + assert cfg.Config.MYSQL_HOST == "mysql" + assert cfg.Config.MYSQL_PORT == 3306 + assert cfg.Config.MYSQL_DB == "omnibioai_hpc" + assert cfg.Config.MYSQL_USER == "root" + assert cfg.Config.MYSQL_PASSWORD == "root" + assert cfg.Config.REDIS_URL == "redis://redis:6379" + assert cfg.Config.DEFAULT_CPU_HOURS == 120 + assert cfg.Config.DEFAULT_GPU_HOURS == 24 + assert cfg.Config.MAX_CONCURRENT_JOBS == 5 + assert cfg.Config.APP_NAME == "OmniBioAI HPC Policy Engine" + + +# --------------------------------------------------------------------------- +# Valid overrides +# --------------------------------------------------------------------------- + +def test_env_vars_override_defaults(): + cfg = _load_config_with_env({ + "MYSQL_HOST": "db.internal", + "MYSQL_PORT": "5432", + "MYSQL_DB": "custom_db", + "MYSQL_USER": "svc", + "MYSQL_PASSWORD": "hunter2", + "REDIS_URL": "redis://cache:6380", + "DEFAULT_CPU_HOURS": "500", + "DEFAULT_GPU_HOURS": "50", + "MAX_CONCURRENT_JOBS": "20", + }) + assert cfg.Config.MYSQL_HOST == "db.internal" + assert cfg.Config.MYSQL_PORT == 5432 + assert cfg.Config.MYSQL_DB == "custom_db" + assert cfg.Config.MYSQL_USER == "svc" + assert cfg.Config.MYSQL_PASSWORD == "hunter2" + assert cfg.Config.REDIS_URL == "redis://cache:6380" + assert cfg.Config.DEFAULT_CPU_HOURS == 500 + assert cfg.Config.DEFAULT_GPU_HOURS == 50 + assert cfg.Config.MAX_CONCURRENT_JOBS == 20 + + +def test_zero_quota_defaults_are_respected_verbatim(): + """A deliberately-zeroed quota env var is honored, not silently + replaced by a nonzero default.""" + cfg = _load_config_with_env({"DEFAULT_CPU_HOURS": "0", "DEFAULT_GPU_HOURS": "0"}) + assert cfg.Config.DEFAULT_CPU_HOURS == 0 + assert cfg.Config.DEFAULT_GPU_HOURS == 0 + + +# --------------------------------------------------------------------------- +# Invalid configuration -- no graceful handling exists (audit gap) +# --------------------------------------------------------------------------- + +def test_non_numeric_mysql_port_raises_at_import_time(): + """Characterizes current behavior: MYSQL_PORT is parsed with a bare + int(...) call and nothing catches a malformed value -- the module fails + to import at all (ValueError) rather than falling back to the default + or raising a clear configuration error. Documented as a "bugs + discovered but not fixed" item in the PR description; not fixed here + per the test-only scope of this change.""" + import pytest + with pytest.raises(ValueError): + _load_config_with_env({"MYSQL_PORT": "not-a-port"}) + + +def test_non_numeric_default_cpu_hours_raises_at_import_time(): + import pytest + with pytest.raises(ValueError): + _load_config_with_env({"DEFAULT_CPU_HOURS": "unlimited"}) + + +def test_non_numeric_max_concurrent_jobs_raises_at_import_time(): + import pytest + with pytest.raises(ValueError): + _load_config_with_env({"MAX_CONCURRENT_JOBS": "many"}) + + +def test_empty_string_mysql_host_is_accepted_verbatim(): + """Characterizes current behavior: string-typed settings have no + non-empty validation, so an explicitly-empty value is accepted as-is.""" + cfg = _load_config_with_env({"MYSQL_HOST": ""}) + assert cfg.Config.MYSQL_HOST == "" diff --git a/tests/test_core_gpu_source.py b/tests/test_core_gpu_source.py new file mode 100644 index 0000000..e2f8d60 --- /dev/null +++ b/tests/test_core_gpu_source.py @@ -0,0 +1,64 @@ +""" +Direct-source unit tests for app/core/gpu.py::validate_gpu_access. + +Loaded via tests/_srcload.py so its lines/branches are measured even on a +host where the checked-in .so shadows the .py import (see _srcload.py for +why). Behavior is already exercised indirectly via test_quota_service.py +and test_routes_policy.py; this file targets the function directly with the +full input space, including edge cases those higher-level tests don't hit. +""" +from tests._srcload import load + +gpu = load("app/core/gpu.py") + + +def test_zero_gpus_needs_no_role(): + ok, reason = gpu.validate_gpu_access([], 0) + assert ok is True + assert reason == "no gpu needed" + + +def test_negative_gpus_short_circuits_to_no_gpu_needed(): + """Characterizes current behavior: `gpus <= 0` is a single check, so a + negative gpu count is (silently) treated the same as zero/none.""" + ok, reason = gpu.validate_gpu_access([], -3) + assert ok is True + assert reason == "no gpu needed" + + +def test_positive_gpus_without_any_roles_denied(): + ok, reason = gpu.validate_gpu_access([], 1) + assert ok is False + assert reason == "gpu access denied" + + +def test_positive_gpus_with_unrelated_roles_denied(): + ok, _reason = gpu.validate_gpu_access(["researcher", "viewer"], 1) + assert ok is False + + +def test_positive_gpus_with_gpu_user_role_allowed(): + ok, reason = gpu.validate_gpu_access(["researcher", "gpu_user"], 1) + assert ok is True + assert reason == "gpu allowed" + + +def test_role_match_is_exact_not_substring(): + """'gpu_user_temp' must not satisfy the 'gpu_user' membership check.""" + ok, _reason = gpu.validate_gpu_access(["gpu_user_temp"], 1) + assert ok is False + + +def test_role_match_is_case_sensitive(): + ok, _reason = gpu.validate_gpu_access(["GPU_USER"], 1) + assert ok is False + + +def test_large_gpu_request_with_role_allowed(): + ok, _reason = gpu.validate_gpu_access(["gpu_user"], 64) + assert ok is True + + +def test_empty_roles_list_with_zero_gpus_allowed(): + ok, _reason = gpu.validate_gpu_access([], 0) + assert ok is True diff --git a/tests/test_core_policies_source.py b/tests/test_core_policies_source.py new file mode 100644 index 0000000..66ca4fb --- /dev/null +++ b/tests/test_core_policies_source.py @@ -0,0 +1,58 @@ +""" +Direct-source unit tests for app/core/policies.py::validate_partition_access. + +Loaded via tests/_srcload.py (see that module's docstring) so its lines are +measured even where the checked-in .so shadows the .py import. +""" +from tests._srcload import load + +policies = load("app/core/policies.py") + + +def test_dgx_partition_denied_without_dgx_access_role(): + ok, reason = policies.validate_partition_access([], "dgx-a100") + assert ok is False + assert reason == "dgx partition denied" + + +def test_dgx_partition_denied_with_unrelated_roles(): + ok, _reason = policies.validate_partition_access(["gpu_user", "researcher"], "dgx-a100") + assert ok is False + + +def test_dgx_partition_allowed_with_dgx_access_role(): + ok, reason = policies.validate_partition_access(["dgx_access"], "dgx-a100") + assert ok is True + assert reason == "partition allowed" + + +def test_cpu_partition_allowed_with_no_roles(): + ok, _reason = policies.validate_partition_access([], "cpu") + assert ok is True + + +def test_gpu_partition_allowed_with_no_roles(): + """Only the literal 'dgx-a100' partition is gated -- any other partition + name (including 'gpu') is allowed regardless of roles.""" + ok, _reason = policies.validate_partition_access([], "gpu") + assert ok is True + + +def test_unknown_partition_name_allowed_by_default(): + """Characterizes current behavior: partition names aren't validated + against an allow-list, so an unrecognized/typo'd partition passes + through as allowed rather than being rejected.""" + ok, _reason = policies.validate_partition_access([], "totally-made-up-partition") + assert ok is True + + +def test_empty_partition_string_allowed(): + ok, _reason = policies.validate_partition_access([], "") + assert ok is True + + +def test_partition_check_is_case_sensitive(): + """'DGX-A100' does not match the literal 'dgx-a100' gate, so it's + treated as an ungated partition and allowed.""" + ok, _reason = policies.validate_partition_access([], "DGX-A100") + assert ok is True diff --git a/tests/test_core_quota_source.py b/tests/test_core_quota_source.py new file mode 100644 index 0000000..c90be4d --- /dev/null +++ b/tests/test_core_quota_source.py @@ -0,0 +1,138 @@ +""" +Direct-source unit tests for app/core/quota.py::evaluate_quota. + +Loaded via tests/_srcload.py (see that module's docstring) so its lines are +measured even where the checked-in .so shadows the .py import. + +evaluate_quota reads its limits from app.core.config.Config at call time +(Config.DEFAULT_CPU_HOURS / DEFAULT_GPU_HOURS are plain class attributes), +so tests patch those attributes directly to pin deterministic limits rather +than depending on whatever env vars happen to be set for the process. +""" +from unittest.mock import patch + +from tests._srcload import load + +quota = load("app/core/quota.py") + + +def _patched_limits(cpu_limit, gpu_limit): + return patch.multiple( + quota.Config, + DEFAULT_CPU_HOURS=cpu_limit, + DEFAULT_GPU_HOURS=gpu_limit, + ) + + +# --------------------------------------------------------------------------- +# Within-limits / boundary behavior +# --------------------------------------------------------------------------- + +def test_within_both_limits_allowed(): + with _patched_limits(100, 20): + ok, reason, rem_cpu, rem_gpu = quota.evaluate_quota(10, 5, 5, 5) + assert ok is True + assert reason == "quota ok" + assert rem_cpu == 90 + assert rem_gpu == 15 + + +def test_request_exactly_equal_to_remaining_cpu_is_allowed(): + """Boundary: the check is strictly `>`, so a request equal to what's + left is not an overage.""" + with _patched_limits(100, 20): + ok, _reason, rem_cpu, _rem_gpu = quota.evaluate_quota(90, 0, 10, 0) + assert ok is True + assert rem_cpu == 10 + + +def test_request_one_over_remaining_cpu_denied(): + with _patched_limits(100, 20): + ok, reason, _rem_cpu, _rem_gpu = quota.evaluate_quota(90, 0, 10.0001, 0) + assert ok is False + assert reason == "cpu quota exceeded" + + +def test_request_exactly_equal_to_remaining_gpu_is_allowed(): + with _patched_limits(100, 20): + ok, _reason, _rem_cpu, rem_gpu = quota.evaluate_quota(0, 15, 0, 5) + assert ok is True + assert rem_gpu == 5 + + +def test_request_one_over_remaining_gpu_denied(): + with _patched_limits(100, 20): + ok, reason, _rem_cpu, _rem_gpu = quota.evaluate_quota(0, 15, 0, 5.0001) + assert ok is False + assert reason == "gpu quota exceeded" + + +# --------------------------------------------------------------------------- +# Exceeded quota +# --------------------------------------------------------------------------- + +def test_cpu_exceeded_denies_before_checking_gpu(): + """cpu is checked first: a request that blows both budgets is reported + as a cpu overage, not a gpu one.""" + with _patched_limits(10, 10): + ok, reason, _rem_cpu, _rem_gpu = quota.evaluate_quota(9, 9, 5, 5) + assert ok is False + assert reason == "cpu quota exceeded" + + +def test_gpu_exceeded_when_cpu_is_within_limits(): + with _patched_limits(100, 10): + ok, reason, _rem_cpu, _rem_gpu = quota.evaluate_quota(0, 9, 1, 5) + assert ok is False + assert reason == "gpu quota exceeded" + + +def test_remaining_hours_reported_even_when_denied(): + """The (remaining_cpu, remaining_gpu) tuple reflects current usage vs. + the limit -- not usage vs. the (denied) request -- regardless of the + allow/deny outcome.""" + with _patched_limits(100, 20): + ok, _reason, rem_cpu, rem_gpu = quota.evaluate_quota(95, 0, 10, 0) + assert ok is False + assert rem_cpu == 5 + assert rem_gpu == 20 + + +# --------------------------------------------------------------------------- +# Zero / negative / already-over-budget edge cases +# --------------------------------------------------------------------------- + +def test_zero_request_always_allowed_even_at_zero_remaining(): + with _patched_limits(10, 10): + ok, _reason, rem_cpu, rem_gpu = quota.evaluate_quota(10, 10, 0, 0) + assert ok is True + assert rem_cpu == 0 + assert rem_gpu == 0 + + +def test_usage_already_over_limit_denies_any_positive_request(): + """current usage can already exceed the configured limit (e.g. the + limit was lowered after usage accrued); remaining goes negative, and + any positive request is denied.""" + with _patched_limits(10, 10): + ok, _reason, rem_cpu, _rem_gpu = quota.evaluate_quota(15, 0, 0.01, 0) + assert ok is False + assert rem_cpu == -5 + + +def test_negative_request_is_allowed_through(): + """Characterizes current behavior: there is no floor/validation on the + requested amount, so a negative request_cpu is always <= remaining and + is silently allowed (no guard against malformed/negative input).""" + with _patched_limits(10, 10): + ok, _reason, _rem_cpu, _rem_gpu = quota.evaluate_quota(0, 0, -5, 0) + assert ok is True + + +def test_negative_current_usage_inflates_remaining(): + """Characterizes current behavior: negative recorded usage (e.g. a data + bug) is not clamped and simply inflates the remaining budget.""" + with _patched_limits(10, 10): + ok, _reason, rem_cpu, _rem_gpu = quota.evaluate_quota(-5, 0, 14, 0) + assert ok is True + assert rem_cpu == 15 diff --git a/tests/test_core_scheduler_source.py b/tests/test_core_scheduler_source.py new file mode 100644 index 0000000..c13fa21 --- /dev/null +++ b/tests/test_core_scheduler_source.py @@ -0,0 +1,38 @@ +""" +Direct-source unit tests for app/core/scheduler.py::SchedulerAdapter. + +Loaded via tests/_srcload.py (see that module's docstring) so its lines are +measured even where the checked-in .so shadows the .py import. + +Audit note: SchedulerAdapter is not a real Slurm/PBS/LSF/Kubernetes/cloud +integration -- get_cluster_load() is a stub that always returns the same +hardcoded dict, with no network/subprocess call, no error handling, no +timeout, and no retry logic. There is nothing to mock and no failure path +to exercise here (see PR description "remaining gaps" / "infrastructure +limitations" for the corresponding audit note); these tests characterize +the stub's actual current behavior rather than inventing scheduler-failure +scenarios the code doesn't implement. SchedulerAdapter/SchedulerService are +also not wired into any API route (grep of app/api/ finds no reference to +either), so there is no HTTP-level test to add for them. +""" +import asyncio + +from tests._srcload import load + +scheduler = load("app/core/scheduler.py") + + +def test_get_cluster_load_returns_expected_keys(): + result = asyncio.run(scheduler.SchedulerAdapter().get_cluster_load()) + assert set(result.keys()) == {"cpu_load", "gpu_load", "running_jobs"} + + +def test_get_cluster_load_values_are_stable_stub_values(): + result = asyncio.run(scheduler.SchedulerAdapter().get_cluster_load()) + assert result == {"cpu_load": 0.45, "gpu_load": 0.60, "running_jobs": 21} + + +def test_get_cluster_load_is_independent_across_instances(): + a = asyncio.run(scheduler.SchedulerAdapter().get_cluster_load()) + b = asyncio.run(scheduler.SchedulerAdapter().get_cluster_load()) + assert a == b diff --git a/tests/test_models_validation.py b/tests/test_models_validation.py new file mode 100644 index 0000000..5b213c5 --- /dev/null +++ b/tests/test_models_validation.py @@ -0,0 +1,121 @@ +""" +Pydantic model validation tests for app/models/{decision,job,quota}.py. + +These models aren't Cython-compiled (not in setup.py's EXTENSIONS list), so +they're already traced normally by coverage; this file targets validation +behavior -- defaults, required fields, type coercion, and the absence of +value constraints -- that the route-level tests don't exhaustively cover. +""" +import pytest +from pydantic import ValidationError + +from app.models.decision import Decision +from app.models.job import JobRequest +from app.models.quota import QuotaCheck + +# --------------------------------------------------------------------------- +# Decision +# --------------------------------------------------------------------------- + +def test_decision_requires_allow_and_reason(): + with pytest.raises(ValidationError): + Decision() + + +def test_decision_remaining_hours_default_to_zero(): + d = Decision(allow=True, reason="ok") + assert d.remaining_cpu_hours == 0 + assert d.remaining_gpu_hours == 0 + + +def test_decision_reason_must_be_string(): + with pytest.raises(ValidationError): + Decision(allow=True, reason=123) + + +# --------------------------------------------------------------------------- +# JobRequest +# --------------------------------------------------------------------------- + +def test_job_request_requires_user_id(): + with pytest.raises(ValidationError): + JobRequest() + + +def test_job_request_defaults(): + r = JobRequest(user_id="u1") + assert r.cpu_hours == 0 + assert r.gpu_hours == 0 + assert r.gpus == 0 + assert r.memory_gb == 0 + assert r.partition == "cpu" + assert r.roles == [] + assert r.org_id is None + + +def test_job_request_numeric_string_coerced_to_float(): + r = JobRequest(user_id="u1", cpu_hours="4.5") + assert r.cpu_hours == 4.5 + + +def test_job_request_negative_values_accepted_no_lower_bound(): + """No ge=0 constraint exists on any resource field.""" + r = JobRequest(user_id="u1", cpu_hours=-1, gpu_hours=-1, gpus=-1, memory_gb=-1) + assert r.cpu_hours == -1 + assert r.gpus == -1 + + +def test_job_request_roles_default_is_not_shared_between_instances(): + """Guards against a mutable-default-argument style bug: two instances + must not share the same underlying roles list object.""" + a = JobRequest(user_id="a") + b = JobRequest(user_id="b") + a.roles.append("gpu_user") + assert b.roles == [] + + +def test_job_request_roles_reject_non_string_items(): + with pytest.raises(ValidationError): + JobRequest(user_id="u1", roles=[1, 2, 3]) + + +def test_job_request_org_id_accepts_none_explicitly(): + r = JobRequest(user_id="u1", org_id=None) + assert r.org_id is None + + +# --------------------------------------------------------------------------- +# QuotaCheck +# --------------------------------------------------------------------------- + +def test_quota_check_requires_user_id(): + with pytest.raises(ValidationError): + QuotaCheck() + + +def test_quota_check_defaults(): + q = QuotaCheck(user_id="u1") + assert q.cpu_hours == 0 + assert q.gpu_hours == 0 + assert q.gpus == 0 + assert q.partition == "cpu" + assert q.roles == [] + + +def test_quota_check_has_no_org_id_field(): + """Audit finding: unlike JobRequest, QuotaCheck has no org_id field at + all -- /quota/check has no organization/project-scoping concept + whatsoever, not even an unused one. See PR description's "remaining + gaps" note.""" + q = QuotaCheck(user_id="u1") + assert not hasattr(q, "org_id") + + +def test_quota_check_negative_values_accepted_no_lower_bound(): + q = QuotaCheck(user_id="u1", cpu_hours=-5, gpu_hours=-5, gpus=-5) + assert q.cpu_hours == -5 + + +def test_quota_check_roles_reject_non_string_items(): + with pytest.raises(ValidationError): + QuotaCheck(user_id="u1", roles=[{"not": "a string"}]) diff --git a/tests/test_quota_service_source.py b/tests/test_quota_service_source.py new file mode 100644 index 0000000..bb83ff7 --- /dev/null +++ b/tests/test_quota_service_source.py @@ -0,0 +1,111 @@ +""" +Direct-source unit tests for app/services/quota_service.py::QuotaService. + +Loaded via tests/_srcload.py (see that module's docstring) so its own lines +are measured even where the checked-in .so shadows the .py import. The +module-level `from app.core.gpu import validate_gpu_access` (etc.) inside +quota_service.py still resolves through normal package import machinery -- +i.e. it calls whatever app.core.gpu/policies/quota normally resolve to on +this host (.so or .py) -- which is fine: those modules' own lines are +measured independently by test_core_gpu_source.py / test_core_policies_source.py +/ test_core_quota_source.py, and their logic is Cython-compiled as-is from +the same .py source, so behavior is identical either way. + +Scenarios mirror tests/test_quota_service.py (kept intact, not modified); +this file adds check-ordering/precedence coverage that file doesn't target. +""" +from unittest.mock import MagicMock, patch + +from tests._srcload import load + +quota_service_mod = load("app/services/quota_service.py") +QuotaService = quota_service_mod.QuotaService + + +def _usage(cpu_hours=0.0, gpu_hours=0.0): + u = MagicMock() + u.cpu_hours = cpu_hours + u.gpu_hours = gpu_hours + return u + + +def _request(gpus=0, partition="cpu", cpu_hours=1.0, gpu_hours=0.0): + r = MagicMock() + r.gpus = gpus + r.partition = partition + r.cpu_hours = cpu_hours + r.gpu_hours = gpu_hours + return r + + +def test_gpu_denied_short_circuits_before_partition_and_quota_checks(): + """When the gpu check fails, validate_partition_access and + evaluate_quota must not run at all.""" + with patch.object(quota_service_mod, "validate_partition_access") as mock_partition, \ + patch.object(quota_service_mod, "evaluate_quota") as mock_quota: + decision = QuotaService.evaluate( + usage=_usage(), + request=_request(gpus=2, partition="dgx-a100"), + roles=[], + ) + assert decision.allow is False + assert decision.reason == "gpu access denied" + mock_partition.assert_not_called() + mock_quota.assert_not_called() + + +def test_partition_denied_short_circuits_before_quota_check(): + """When gpu passes but partition fails, evaluate_quota must not run.""" + with patch.object(quota_service_mod, "evaluate_quota") as mock_quota: + decision = QuotaService.evaluate( + usage=_usage(), + request=_request(gpus=0, partition="dgx-a100"), + roles=[], + ) + assert decision.allow is False + assert decision.reason == "dgx partition denied" + mock_quota.assert_not_called() + + +def test_all_checks_pass_falls_through_to_quota_result(): + decision = QuotaService.evaluate( + usage=_usage(cpu_hours=10.0, gpu_hours=5.0), + request=_request(gpus=1, partition="cpu", cpu_hours=5.0, gpu_hours=1.0), + roles=["gpu_user"], + ) + assert decision.allow is True + assert decision.reason == "quota ok" + + +def test_denied_decision_has_zero_remaining_hours_defaults(): + """When denied at the gpu/partition stage (before evaluate_quota runs), + the Decision falls back to its model defaults (0) for remaining hours + rather than reporting the caller's real remaining budget.""" + decision = QuotaService.evaluate( + usage=_usage(cpu_hours=1.0, gpu_hours=1.0), + request=_request(gpus=2, partition="cpu"), + roles=[], + ) + assert decision.allow is False + assert decision.remaining_cpu_hours == 0 + assert decision.remaining_gpu_hours == 0 + + +def test_quota_exceeded_after_passing_gpu_and_partition_checks(): + decision = QuotaService.evaluate( + usage=_usage(cpu_hours=119.5), + request=_request(gpus=0, partition="cpu", cpu_hours=1.0), + roles=[], + ) + assert decision.allow is False + assert decision.reason == "cpu quota exceeded" + + +def test_returns_decision_instance(): + from app.models.decision import Decision + decision = QuotaService.evaluate( + usage=_usage(), + request=_request(), + roles=[], + ) + assert isinstance(decision, Decision) diff --git a/tests/test_routes_policy_source.py b/tests/test_routes_policy_source.py new file mode 100644 index 0000000..87785d9 --- /dev/null +++ b/tests/test_routes_policy_source.py @@ -0,0 +1,162 @@ +""" +Direct-source unit tests for app/api/routes_policy.py (POST /jobs/evaluate). + +Loaded via tests/_srcload.py (see that module's docstring) so its own lines +are measured even where the checked-in .so shadows the .py import. Exercised +through a real FastAPI TestClient (not by calling the handler function +directly) so request validation, JSON parsing, and response shaping all run +for real, same as tests/test_routes_policy.py (kept intact, not modified). +This file adds validation/edge-case coverage that file doesn't target: +negative resource values, malformed roles, unknown/extra fields, malformed +JSON bodies, and the org_id no-op characterization. +""" +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from tests._srcload import load + +routes_policy_mod = load("app/api/routes_policy.py") + + +@pytest.fixture +def client(): + app = FastAPI() + app.include_router(routes_policy_mod.router) + return TestClient(app) + + +# --------------------------------------------------------------------------- +# Core allow/deny precedence (mirrors test_routes_policy.py, source-loaded) +# --------------------------------------------------------------------------- + +def test_default_request_approved(client): + response = client.post("/jobs/evaluate", json={"user_id": "u1"}) + assert response.status_code == 200 + assert response.json() == {"allow": True, "reason": "job approved", "partition": "cpu"} + + +def test_gpu_request_without_role_denied(client): + response = client.post("/jobs/evaluate", json={"user_id": "u1", "gpus": 2}) + data = response.json() + assert data["allow"] is False + assert data["reason"] == "gpu access denied" + + +def test_dgx_request_with_gpu_role_but_no_dgx_role_denied(client): + response = client.post("/jobs/evaluate", json={ + "user_id": "u1", "gpus": 1, "partition": "dgx-a100", "roles": ["gpu_user"], + }) + data = response.json() + assert data["allow"] is False + assert data["reason"] == "dgx partition denied" + + +def test_dgx_request_with_both_roles_allowed(client): + response = client.post("/jobs/evaluate", json={ + "user_id": "u1", "gpus": 1, "partition": "dgx-a100", + "roles": ["gpu_user", "dgx_access"], + }) + assert response.json()["allow"] is True + + +# --------------------------------------------------------------------------- +# Validation edge cases +# --------------------------------------------------------------------------- + +def test_missing_user_id_rejected(client): + response = client.post("/jobs/evaluate", json={"gpus": 1}) + assert response.status_code == 422 + + +def test_empty_body_rejected(client): + response = client.post("/jobs/evaluate", json={}) + assert response.status_code == 422 + + +def test_malformed_json_body_rejected(client): + response = client.post( + "/jobs/evaluate", + content="{not valid json", + headers={"content-type": "application/json"}, + ) + assert response.status_code == 422 + + +def test_roles_as_wrong_type_rejected(client): + """roles must be a list[str]; a bare string is not coerced into one.""" + response = client.post("/jobs/evaluate", json={"user_id": "u1", "roles": "gpu_user"}) + assert response.status_code == 422 + + +def test_gpus_as_wrong_type_rejected(client): + response = client.post("/jobs/evaluate", json={"user_id": "u1", "gpus": "not-a-number"}) + assert response.status_code == 422 + + +def test_negative_gpus_accepted_and_treated_as_no_gpu_needed(client): + """Characterizes current behavior (see test_core_gpu_source.py): there + is no ge=0 constraint on gpus, and validate_gpu_access's `gpus <= 0` + check means a negative value slips through as if no GPU was requested.""" + response = client.post("/jobs/evaluate", json={"user_id": "u1", "gpus": -1}) + assert response.status_code == 200 + assert response.json()["allow"] is True + + +def test_negative_memory_gb_accepted_without_validation(client): + """Characterizes current behavior: memory_gb has no lower-bound + constraint and is not used by evaluate_job's decision at all, so a + negative value is accepted silently.""" + response = client.post("/jobs/evaluate", json={"user_id": "u1", "memory_gb": -16}) + assert response.status_code == 200 + + +def test_unknown_extra_fields_are_ignored(client): + """Characterizes current behavior: pydantic's default extra-field + policy is "ignore", so an unrecognized field neither errors nor + changes the decision.""" + response = client.post("/jobs/evaluate", json={ + "user_id": "u1", "not_a_real_field": "surprise", + }) + assert response.status_code == 200 + assert response.json()["allow"] is True + + +def test_duplicate_role_entries_do_not_change_outcome(client): + response = client.post("/jobs/evaluate", json={ + "user_id": "u1", "gpus": 1, "roles": ["gpu_user", "gpu_user", "gpu_user"], + }) + assert response.json()["allow"] is True + + +# --------------------------------------------------------------------------- +# Authorization / org isolation +# --------------------------------------------------------------------------- + +def test_org_id_accepted_but_has_no_effect_on_decision(client): + """Audit finding: JobRequest.org_id exists on the model but + evaluate_job() never reads it -- two requests that differ only in + org_id get an identical decision. There is no organization/project + isolation enforced by this endpoint; see PR description's + "remaining gaps" note.""" + shared = {"user_id": "u1", "gpus": 1, "roles": ["gpu_user"], "partition": "dgx-a100"} + resp_a = client.post("/jobs/evaluate", json={**shared, "org_id": "org-a"}) + resp_b = client.post("/jobs/evaluate", json={**shared, "org_id": "org-b"}) + assert resp_a.json() == resp_b.json() + + +def test_missing_org_id_defaults_to_none_and_still_evaluates(client): + response = client.post("/jobs/evaluate", json={"user_id": "u1"}) + assert response.status_code == 200 + + +def test_empty_user_id_string_is_accepted_by_validation(client): + """Characterizes current behavior: user_id has no min_length constraint, + so an empty string is a valid (if meaningless) identity.""" + response = client.post("/jobs/evaluate", json={"user_id": ""}) + assert response.status_code == 200 + + +def test_user_id_wrong_type_rejected(client): + response = client.post("/jobs/evaluate", json={"user_id": 12345}) + assert response.status_code == 422 diff --git a/tests/test_routes_quota_source.py b/tests/test_routes_quota_source.py new file mode 100644 index 0000000..0a7d271 --- /dev/null +++ b/tests/test_routes_quota_source.py @@ -0,0 +1,156 @@ +""" +Direct-source unit tests for app/api/routes_quota.py (POST /quota/check). + +Loaded via tests/_srcload.py (see that module's docstring) so its own lines +are measured even where the checked-in .so shadows the .py import. Exercised +through a real FastAPI TestClient with the DB dependency overridden by a +MagicMock (same approach as tests/test_routes_quota.py, kept intact, not +modified) -- no real MySQL connection is used. This file adds +validation/edge-case and identity coverage that file doesn't target. + +UsageService.get_or_create_user_usage and QuotaService.evaluate are mocked +at the routes_quota module boundary in most tests here so the route's own +DB-wiring/response-shaping/role-forwarding logic is what's under test (the +underlying QuotaService/quota logic itself is covered by +test_quota_service_source.py and test_core_quota_source.py). A couple of +tests run the real QuotaService end to end to prove the route's roles/ +partition forwarding actually reaches policy-critical decisions. +""" +from unittest.mock import MagicMock, patch + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from app.models.decision import Decision +from tests._srcload import load + +routes_quota_mod = load("app/api/routes_quota.py") + + +def _allow(**kwargs): + defaults = {"allow": True, "reason": "quota ok", "remaining_cpu_hours": 100.0, "remaining_gpu_hours": 20.0} + defaults.update(kwargs) + return Decision(**defaults) + + +@pytest.fixture +def client(): + app = FastAPI() + app.include_router(routes_quota_mod.router) + mock_db = MagicMock() + app.dependency_overrides[routes_quota_mod.get_db] = lambda: mock_db + return TestClient(app), mock_db + + +# --------------------------------------------------------------------------- +# Core happy/deny path (mirrors test_routes_quota.py, source-loaded) +# --------------------------------------------------------------------------- + +def test_quota_check_allow(client): + tc, _mock_db = client + usage = MagicMock(cpu_hours=10.0, gpu_hours=2.0) + with patch.object(routes_quota_mod.UsageService, "get_or_create_user_usage", return_value=usage), \ + patch.object(routes_quota_mod.QuotaService, "evaluate", return_value=_allow()): + response = tc.post("/quota/check", json={"user_id": "u1", "cpu_hours": 1.0}) + assert response.status_code == 200 + assert response.json()["allow"] is True + + +def test_quota_check_deny(client): + tc, _mock_db = client + usage = MagicMock(cpu_hours=119.0, gpu_hours=0.0) + denied = Decision(allow=False, reason="cpu quota exceeded", remaining_cpu_hours=1.0, remaining_gpu_hours=24.0) + with patch.object(routes_quota_mod.UsageService, "get_or_create_user_usage", return_value=usage), \ + patch.object(routes_quota_mod.QuotaService, "evaluate", return_value=denied): + response = tc.post("/quota/check", json={"user_id": "u2", "cpu_hours": 5.0}) + assert response.json()["allow"] is False + + +def test_roles_forwarded_to_quota_service_not_hardcoded(client): + tc, _mock_db = client + usage = MagicMock(cpu_hours=0.0, gpu_hours=0.0) + with patch.object(routes_quota_mod.UsageService, "get_or_create_user_usage", return_value=usage), \ + patch.object(routes_quota_mod.QuotaService, "evaluate", return_value=_allow()) as mock_eval: + tc.post("/quota/check", json={"user_id": "u3", "roles": ["viewer"]}) + assert mock_eval.call_args.kwargs["roles"] == ["viewer"] + + +def test_roles_default_to_empty_list_when_unsupplied(client): + tc, _mock_db = client + usage = MagicMock(cpu_hours=0.0, gpu_hours=0.0) + with patch.object(routes_quota_mod.UsageService, "get_or_create_user_usage", return_value=usage), \ + patch.object(routes_quota_mod.QuotaService, "evaluate", return_value=_allow()) as mock_eval: + tc.post("/quota/check", json={"user_id": "u4"}) + assert mock_eval.call_args.kwargs["roles"] == [] + + +# --------------------------------------------------------------------------- +# End-to-end (real QuotaService, no mocking of policy logic) +# --------------------------------------------------------------------------- + +def test_end_to_end_gpu_request_denied_without_role(client): + tc, _mock_db = client + usage = MagicMock(cpu_hours=0.0, gpu_hours=0.0) + with patch.object(routes_quota_mod.UsageService, "get_or_create_user_usage", return_value=usage): + response = tc.post("/quota/check", json={"user_id": "u5", "gpus": 2}) + data = response.json() + assert data["allow"] is False + assert data["reason"] == "gpu access denied" + + +def test_end_to_end_dgx_partition_allowed_with_correct_roles(client): + tc, _mock_db = client + usage = MagicMock(cpu_hours=0.0, gpu_hours=0.0) + with patch.object(routes_quota_mod.UsageService, "get_or_create_user_usage", return_value=usage): + response = tc.post("/quota/check", json={ + "user_id": "u6", "gpus": 1, "gpu_hours": 1.0, + "partition": "dgx-a100", "roles": ["gpu_user", "dgx_access"], + }) + assert response.json()["allow"] is True + + +# --------------------------------------------------------------------------- +# Validation / identity edge cases +# --------------------------------------------------------------------------- + +def test_missing_user_id_rejected(client): + tc, _mock_db = client + response = tc.post("/quota/check", json={"cpu_hours": 1.0}) + assert response.status_code == 422 + + +def test_malformed_json_body_rejected(client): + tc, _mock_db = client + response = tc.post( + "/quota/check", + content="{not valid json", + headers={"content-type": "application/json"}, + ) + assert response.status_code == 422 + + +def test_roles_wrong_type_rejected(client): + tc, _mock_db = client + response = tc.post("/quota/check", json={"user_id": "u7", "roles": "gpu_user"}) + assert response.status_code == 422 + + +def test_negative_cpu_hours_accepted_without_validation(client): + """Characterizes current behavior: cpu_hours has no ge=0 constraint on + QuotaCheck, so a negative request is accepted at the API boundary.""" + tc, _mock_db = client + usage = MagicMock(cpu_hours=0.0, gpu_hours=0.0) + with patch.object(routes_quota_mod.UsageService, "get_or_create_user_usage", return_value=usage): + response = tc.post("/quota/check", json={"user_id": "u8", "cpu_hours": -10.0}) + assert response.status_code == 200 + assert response.json()["allow"] is True + + +def test_db_session_from_dependency_override_is_used(client): + tc, mock_db = client + usage = MagicMock(cpu_hours=0.0, gpu_hours=0.0) + with patch.object(routes_quota_mod.UsageService, "get_or_create_user_usage", return_value=usage) as mock_svc, \ + patch.object(routes_quota_mod.QuotaService, "evaluate", return_value=_allow()): + tc.post("/quota/check", json={"user_id": "u9"}) + assert mock_svc.call_args[0][0] is mock_db diff --git a/tests/test_scheduler_service_source.py b/tests/test_scheduler_service_source.py new file mode 100644 index 0000000..2797c11 --- /dev/null +++ b/tests/test_scheduler_service_source.py @@ -0,0 +1,34 @@ +""" +Direct-source unit tests for app/services/scheduler_service.py::SchedulerService. + +Loaded via tests/_srcload.py (see that module's docstring) so its lines are +measured even where the checked-in .so shadows the .py import. + +Audit note: SchedulerService is not referenced anywhere under app/api/, so +it is not reachable through any HTTP endpoint -- these are plain unit tests +of the class itself. See test_core_scheduler_source.py for why there is no +scheduler-failure path to test (SchedulerAdapter is a hardcoded stub). +""" +import asyncio + +from tests._srcload import load + +scheduler_service_mod = load("app/services/scheduler_service.py") +SchedulerService = scheduler_service_mod.SchedulerService + + +def test_init_creates_a_scheduler_adapter(): + svc = SchedulerService() + assert svc.scheduler.__class__.__name__ == "SchedulerAdapter" + + +def test_cluster_status_delegates_to_adapter(): + svc = SchedulerService() + result = asyncio.run(svc.cluster_status()) + assert result == {"cpu_load": 0.45, "gpu_load": 0.60, "running_jobs": 21} + + +def test_each_instance_gets_its_own_adapter(): + a = SchedulerService() + b = SchedulerService() + assert a.scheduler is not b.scheduler