Implement testing suite: pytest scaffolding, unit tests for existing pure logic, CI workflow

[zachtheyek]

Closes #113

Summary

Adds the pytest test suite that every later PR builds on: scaffolding, unit tests for the existing pure logic, cluster-marked end-to-end smokes, and a CI workflow.

Scaffolding

• tests/conftest.py — autouse isolation fixture for every non-integration test: points AETHERSCAN_{DATA,MODEL,OUTPUT}_PATH at tmp_path, resets all five singletons (Config, ResourceManager, Database, Logger, ResourceMonitor) via their _reset() hooks, re-runs init_config(), and on teardown stops leaked pools/SHM/threads (mirroring cleanup_all() ordering), unregisters the per-test atexit hook, and restores signal handlers. Also provides synthetic data factories: tiny .npy background plates ((n, 6, 16, width) float32), tiny filterbank-style .h5 files (data dset (16, 1, n_chans) + fch1/foff/nchans attrs), and tiny cadence-grouping CSVs (column names read from InferenceConfig).
• pyproject.toml — [tool.pytest.ini_options]: pythonpath = ["src"], testpaths, --strict-markers, and the slow / gpu / cluster / integration markers.
• tests/integration/conftest.py — shared cluster_paths + run_pipeline fixtures so the two smokes can't drift apart.

Unit tests (tests/unit/, 187 tests)

File	Covers
test_config.py	singleton semantics, _reset(), env-var paths, to_dict() drift guard (section keys vs dataclass fields), JSON serializability
test_cli_validation.py	_TAG_PATTERN accept/reject matrix, cross-replica divisibility matrix (defaults valid for 4/6 replicas but not 5; blpc3 smoke config the inverse), semantic checks (SNR ordering, step-schedule sum, enums, file-exists), _check_cross_constraints / _solve_cross_param_constraints (incl. L1-minimality on a patched grid + candidate budget), apply_saved_config precedence (defaults < saved < CLI), mode-scoped flag routing, infer_start_round
test_data_generation.py	log_norm bounds/ordering/idempotence, check_valid_intersection (ON-region boundaries inclusive), new_cadence signal_info keys + forced slope-clamp path, create_false/create_true_single/create_true_double shapes, prefixes, OFF-observation purity, stage A/B/C stats, _compute_intensity_stats NaN handling
test_preprocessing.py	_deduplicate_hits (greedy merge, strict boundary), group_observations_from_csv (valid/flagged/missing-col), _cadence_npy_filename sanitization, _to_json_safe, _remove_dc_spike (exact asymmetric interpolation), _fit_channel_bandpass on a synthetic bandpass, _read_coarse_channel_worker
test_train_utils.py	get_latest_tag priority ladder (final > round > timestamp > test, numeric ties), _calculate_curriculum_snr all 3 schedules incl. endpoints + error cases, archive_directory move/copy/delete-≥round semantics, compute_expected_std / check_encoder_trained, _select_positive_class_shap shape matrix
test_models.py	prepare_latent_features row-major layout, RandomForestModel train/predict/proba/verbose/threshold/save-load on toy data, Sampling layer, encoder/decoder symmetry + forward pass (marked slow)
test_db.py	writer thread lifecycle (start idempotence, stop, restart), flush sentinel protocol, executemany batching across all 5 tables, is_finite sanitization + slope_clamped round-trip + stability aggregation, query filters (scalar/list/range) and column whitelists (SQL-injection rejection)
test_manager.py	pool/SHM tracking + stats, close idempotence, cleanup_all idempotence (Linux-only: PSS accounting)

Integration smokes (tests/integration/, marked integration + gpu + cluster)

• test_train_smoke.py — the known-good blpc3 5-GPU smoke config (batch geometry divisible for exactly 5 replicas) via a python -m aetherscan.main train subprocess with a fresh auto-incremented test_vNN tag; asserts clean exit + final encoder/decoder/RF artifacts + per-round checkpoints + config snapshot.
• test_inference_smoke.py — subset CSV inference against the persisted test_v17 dummy model; asserts clean exit + config snapshot.

CI (.github/workflows/tests.yml)

PR + push-to-master, ubuntu-latest, Python 3.10 and 3.12 matrix, tensorflow-cpu==2.17.* + the pins from requirements-container.txt + h5py hdf5plugin pandas psutil pytest, pip cache. Runs pytest -m "not gpu and not cluster" -q. This also gives claude-flaky-test-tracker real runs to read.

Docs

CLAUDE.md, CONTRIBUTING.md (new Testing section: commands, marker table, fixture contract, test-writing expectations), and the repo-context skill now describe the suite instead of "tests/ is a placeholder"; tests/placeholder removed (its two TODO obligations are carried into tests/conftest.py).

No src/ changes — the _reset() hooks the suite depends on already existed on all five singletons.

Verification

Local (macOS arm64, CPU-only venv: Python 3.12.13, tensorflow 2.17.1 / Keras 3.15, pins from requirements-container.txt):

$ pytest -m "not gpu and not cluster" -q
184 passed, 3 skipped, 2 deselected

(3 skipped = Linux-only cleanup_all PSS tests, which run in CI; 2 deselected = the gpu/cluster smokes.)

Cluster (blpc3, NGC 25.02 container, 5x RTX PRO 6000, branch @ 9b379eb):

Integration smokes (GPUs confirmed idle via nvidia-smi before launch):

$ SINGULARITY_BIND=/datag ./utils/run_container.sh python -m pytest tests/ -m "gpu or cluster" -q
2 passed, 187 deselected, 913 warnings in 1798.18s (0:29:58)

• test_inference_smoke: subset CSV inference against the test_v17 dummy model (resumed from the existing preprocessed subset stamps), clean exit + config snapshot.
• test_train_smoke: full end-to-end train under the fresh auto-incremented test_v18 tag — final vae_encoder/vae_decoder/random_forest_test_v18 artifacts, round_01/round_02 checkpoints, and config_test_v18.json all verified on disk. The test_v18 artifacts were left in place as reusable dummies (like test_v17).

Full default (CPU) selection also run inside the NGC container, exercising the tf-keras shim + pytest 9.1.1 path and the 3 Linux-only tests that self-skip on macOS:

$ CUDA_VISIBLE_DEVICES='' ./utils/run_container.sh python -m pytest -m "not gpu and not cluster" -q
187 passed, 2 deselected, 927 warnings in 46.65s

Cluster hygiene: /dev/shm diffed against a pre-run snapshot — no leaked segments; temp logs removed; cluster repo restored to master.

pre-commit run --all-files passes (ruff lint+format, yaml/toml checks, gitleaks).

Deferred validation

• bla0 (6-GPU) divisibility case in real training: the 4-vs-6-replica side of the divisibility matrix is unit-tested against collect_validation_errors, but not exercised end-to-end on hardware in this PR (bla0 not accessible this session). The default-config train path on 6 GPUs is the existing, previously-validated configuration.
• Clean-shutdown-under-signal integration tests and release build/deploy tests (carried as TODOs in tests/conftest.py) belong to the retry-overhaul and release-engineering work respectively.

AI assistance disclosure

Implemented with Claude Code (Anthropic). Extent: test-suite implementation (conftest, unit + integration tests), CI workflow, documentation updates, multi-angle code review, and local + cluster verification runs. All work reviewed against the existing source before submission; no src/ behavior was modified.

🤖 Generated with Claude Code

[claude]

Claude finished @zachtheyek's task in 1m 21s —— View job

---

PR Review: Testing Suite Implementation

• Read and analyze all changed files
• Review test scaffolding (conftest.py, pyproject.toml)
• Review unit tests (8 test modules)
• Review integration tests
• Review CI workflow
• Review documentation changes
• Post review feedback

---

Overall Assessment

This is a very well-structured, thorough first test suite. The scaffolding design (autouse isolation fixture, singleton reset ordering, signal handler restoration) is production-quality and will make future test writing low-friction. 187 unit tests covering the pure-logic surface area of 8 modules, plus properly gated cluster integration smokes — strong foundation.

A few observations below, roughly ordered by importance.

---

Findings

1. np.random.seed deprecation path (tests/unit/test_data_generation.py:49)

@pytest.fixture(autouse=True)
def _seed_rngs():
    random.seed(11)
    np.random.seed(11)

np.random.seed() seeds the legacy global RandomState. NumPy docs recommend np.random.default_rng() for new code. This is fine here since the production code under test (data_generation.py) likely uses the legacy API, but worth a # NOTE: comment explaining why so future contributors don't "fix" it and break test determinism.

2. _PipelineStub coupling (tests/unit/test_train_utils.py:89-98)

class _PipelineStub:
    """Just enough of TrainingPipeline to drive _calculate_curriculum_snr."""
    def __init__(self, config):
        self.config = config

def _curriculum(round_idx):
    return TrainingPipeline._calculate_curriculum_snr(_PipelineStub(get_config()), round_idx)

This works because _calculate_curriculum_snr only reads self.config. If that method ever touches another attribute (e.g. self.logger), the stub will silently fail with an AttributeError. This is acceptable for a unit test — just noting the coupling surface.

3. Empty-list filter behavior documentation (tests/unit/test_db.py:211-218)

def test_empty_list_filter_matches_everything(self):
    # NOTE: this documents CURRENT behavior, not desired behavior — ...
    assert len(self.db.query_training_stat(tag=[])) == 6

Excellent use of a test to document a known semantic quirk. The inline comment is exactly the right approach — it prevents someone from "fixing" the test without understanding the implication.

4. CI workflow: no PYTHONPATH needed (tests.yml:57)

- name: Run test suite
  run: pytest -m "not gpu and not cluster" -q

pyproject.toml sets pythonpath = ["src"] for pytest, so no PYTHONPATH=src prefix is needed. Good — the CI command matches CLAUDE.md's documented command exactly.

5. Flush sentinel test race window (tests/unit/test_db.py:74-90)

The test_flush_during_shutdown_returns_false test uses a careful _AliveStub to avoid a TOCTOU race between stop() and flush(). The comment and implementation are clear — nice attention to determinism in concurrent code tests.

6. Integration test tag auto-increment (tests/integration/test_train_smoke.py:46-57)

_next_test_tag scans both model_path and model_path/checkpoints for existing test_vNN tags. This avoids collisions when multiple smoke runs share a cluster, but the scan is not atomic — two concurrent pytest runs could race to the same tag. Since integration smokes are run manually and sequentially on the cluster, this is fine in practice. Worth a brief comment if this ever moves to a CI runner with shared storage.

7. Inference smoke hardcodes test_v17 (tests/integration/test_inference_smoke.py:25)

_MODEL_TAG = "test_v17"  # persisted dummy model on blpc3

This couples the test to a specific artifact that must exist on the cluster. The pytest.skip guards handle the missing-artifact case gracefully, so this is safe. Just noting this is a known external dependency — the PR body documents it clearly.

8. conftest.py teardown: contextlib.suppress(Exception) breadth (tests/conftest.py:67-71)

for managed_pool in list(manager._pools):
    with contextlib.suppress(Exception):
        managed_pool.close(timeout=5.0)

Suppressing all exceptions during teardown is the right call here — the alternative is a test that passes but then blows up in cleanup, masking the real failure. The broad Exception catch is warranted since close() on a corrupted pool can raise anything from OSError to ValueError.

9. Minor: smoke flag sync comment (test_train_smoke.py:23, test_cli_validation.py:23)

Both files note that their smoke flags must stay in sync with the other. This is a manual invariant — consider whether a shared constant in a tests/_shared.py or the integration conftest.py would eliminate the drift risk. Not blocking — the current approach is fine for two files, and the comments are clear.

10. CI matrix coverage

Python 3.10 + 3.12 on ubuntu-latest with fail-fast: false is a solid matrix. The cache-dependency-path including both requirements-container.txt and the workflow file itself ensures cache invalidation on pin changes. Well done.

---

Summary

Verdict: Looks good. This is a thorough, well-isolated test suite with strong scaffolding design. The autouse fixture's singleton reset + signal handler restoration pattern will scale well as the suite grows. No blocking issues found — the notes above are informational / minor improvement suggestions for future iterations.

[zachtheyek]

Addressed the two informational review notes in commit aaab689:

• Finding 1 — tests/unit/test_data_generation.py::_seed_rngs now carries a # NOTE: explaining that np.random.seed() intentionally seeds the legacy global RandomState (the generators under test draw from the legacy np.random.* API), so it shouldn't be "modernized" to np.random.default_rng without migrating data_generation.py first.
• Finding 6 — tests/integration/test_train_smoke.py::_next_test_tag now documents that the tag scan-then-pick is not atomic and assumes manual, sequential smoke runs on the cluster.

Finding 9 (sharing the smoke-flag constants between the unit and integration files) is deliberately left as-is per the reviewer's own note — the two copies are cross-referenced in comments instead, since sharing them would require restructuring tests/ into a package for one constant.

[zachtheyek]

@claude please perform a second-pass code review of this PR. Re-review the current diff for any remaining correctness bugs, security issues, or significant quality problems, and post your findings. If the diff is clean, please say so explicitly. (Final verification round before merge.)

[claude]

Second-pass code review — PR #116

I've re-reviewed the full diff (~1800 lines across 14 files). The diff is clean. No correctness bugs, security issues, or significant quality problems found.

What I checked

• tests/conftest.py — singleton reset ordering mirrors cleanup_all(), signal-handler save/restore is correct, atexit.unregister prevents stale hooks, monkeypatch.delenv protects against Slack leaks. The integration marker gate correctly short-circuits before any TF import.
• Unit tests — assertions match the documented contracts of the functions under test. Edge cases (empty arrays, boundary conditions, idempotence) are covered. The _seed_rngs fixture correctly uses the legacy np.random.seed() API to match production code (explained by the inline comment). The test_empty_list_filter_matches_everything test properly documents current-vs-desired semantics.
• test_db.py — the _AliveStub pattern for deterministic shutdown testing is sound; the write_interval=300 / buffer_max=100k trick isolates flush-sentinel behavior cleanly. SQL-injection rejection is tested.
• test_cli_validation.py — the L1-minimality assertion is exhaustive (brute-forces all valid grid points on the monkeypatched search space). The _SMOKE_FLAGS_5_REPLICAS / _SMOKE_FLAGS mirror is called out in comments on both sides.
• Integration smokes — proper skip guards for missing artifacts/GPUs, subprocess isolation, check=False with tail-of-stdout on failure, non-atomic tag-scan race documented.
• CI workflow (.github/workflows/tests.yml) — permissions: contents: read limits blast radius. fail-fast: false is correct for a matrix. Cache key includes the workflow file itself. tensorflow-cpu==2.17.* matches the NGC ABI.
• pyproject.toml — --strict-markers prevents silent deselection from typos.
• Documentation — CLAUDE.md, CONTRIBUTING.md, and the repo-context skill are internally consistent with the new test infrastructure.

Minor observations (non-blocking, no action needed for this PR)

1. actions/checkout@v5 / actions/setup-python@v5 — not pinned to a commit hash (common practice, and contents: read limits the damage surface; just noting for future hardening if desired).
2. test_near_zero_slope_is_clamped — monkeypatches np.random.choice globally; safe here because the test is deterministic and isolated, but would be fragile if ever parallelized with pytest-xdist (not a concern today).

LGTM — ready to merge.