Tests review

[ian-ross]

Clanker test review and fixes. Fixes #115.

Some notes on the process here:

• I got Claude Code to generate a test review document, classifying issues by severity, type and region of the code.
• I then used a mixture of models (Claude Sonnet and Opus, OpenAI ChatGPT 5.5) to fix all the issues, in order from most severe to least.
• I performed spot checks on some of the fixes to check them for sanity.
• I did cross-model reviews using Claude Opus 4.7, OpenAI ChatGPT 5.5 and GitHub Copilot, fixing issues using agents (again!). No agent found any significant problems with the test changes made by any other agent. There were a few documentation cleanups found by Copilot at the end, but that was all.

Overall, my confidence in this process is much higher than if the work was done by a human (that human being me or anyone else). This work is tedious, boring and fiddly, and the agents did a great job. They don't get bored, they're good at tracing the consequences of decisions through logically, they don't get lost in a long list of issues, etc.

One other thing: I needed to rebase this branch at one point. There were only a very small number of apparent conflicts, but there was a conceptual change to how performance models were represented on the rebase target branch, so the required changes to testing were much more extensive than a naive analysis would reveal. ChatGPT 5.5 discovered the "rebase rot", identified the locations where changes were needed, made an interactive rebase plan to surgically correct the relevant commits while preserving the overall branch history, and performed the rebase without any drama at all. That would have taken me days, or I would have fallen back on a much less principled approach.

Coding agent PR description follows below the line.

---

Summary

Multi-session test-suite audit of AEIC plus the full remediation sweep that followed. The audit log and per-finding detail live in test-quality.md; this branch lands the resulting fixes.

By the numbers:

• 115 audit findings closed: 11 High / 52 Medium / 52 Low.
• Test count: ~225 → 316 passing (+1 skipped slow test gated on --run-slow).
• ~108 commits, organised one finding per commit so each item is bisectable.
• Suite still runs cleanly via uv run pytest from the repo root.

What changed, at a glance

• Logical errors: fixed cases where a test passed without verifying its stated invariant (e.g. a no-op .all truthiness check, a @patch on the wrong target, mocked thrust-categorization that ran the real SUT).
• Suspicious data: replaced SUT-self-generated expected values with notebook-cited rounded results, ISA-formula derivations, or honest smoke + physical-envelope checks; marked irreducible cases explicitly as regression guards.
• Coverage gaps: added focused tests for previously unreached branches (PerformanceTable.__post_init__ validators, EDBEntry.get_engine per-sheet absence, Config.escape, default_data_file_location's FileNotFoundError, etc.).
• Hygiene: parametrized monolithic tests, decorated NetCDF-touching tests with @pytest.mark.forked, gated long-running tests behind a --run-slow flag, seeded previously-unseeded RNGs, and stripped traffic lights from the audit document.
• Bonus SUT observations (separate from the test-quality findings): swapped Config.escape's decorator order; pinned pytest to testpaths = ["tests"] (and migrated [tool.pytest] → [tool.pytest.ini_options], which had been silently ignored); replaced print() in Builder.fly()'s exception path with a module logger; replaced isinstance(x, str | None | …) with concrete tuples in Container equality; documented calculate_nvPM_scope11_LTO's SN ∈ {-1, 0} sentinel handling.

See test-quality.md for the full findings list, the rubric used to triage them, the prioritized remediation tiers, and per-finding remediation notes (each annotated *[DONE]*, with *Actual remediation:* clauses where the fix diverged from the original suggestion).

Test plan

• uv run pytest from repo root: 316 passing, 1 skipped (the gated slow test).
• uv run pytest --run-slow tests/test_storage.py::test_create_reopen_large collects and runs the previously-permanently-skipped scaling test.
• No 🟢 / 🟡 / 🔴 traffic lights remain in test-quality.md.
• Reviewer spot-check: pick any commit whose subject names a finding ID (Phase N Low #M, Headline #N) and cross-reference the matching bullet in test-quality.md for context.

[Copilot]

Pull request overview

This PR is a broad test-suite audit/remediation for AEIC ahead of the V1 release, tightening assertions, removing self-referential expected values, expanding branch coverage, and making a few small SUT fixes discovered during the audit (config/pytest setup, logging, docstrings, and runtime type guards).

Changes:

• Strengthens and expands tests across weather, trajectories/storage, missions DB/OAG ingestion, performance models/tables, emissions, and verification; adds deterministic seeding and clearer invariants.
• Adds pytest suite hygiene: --run-slow gating, @pytest.mark.forked for NetCDF-touching tests, and fixes pytest configuration in pyproject.toml so collection is constrained to tests/.
• Includes small SUT adjustments observed during the audit (logger usage in builders, Config.escape decorator order, docstring fixes, Container isinstance guards, nvPM sentinel documentation).

Reviewed changes

Copilot reviewed 36 out of 37 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/test_weather.py	Makes ground-speed test a real-fixture smoke test + adds targeted unit tests for azimuth override and domain errors.
tests/test_trajectory_simulation.py	Tightens trajectory invariants, seeds RNG, verifies NetCDF field round-trip, adds weather-effect assertion, and adds mass-iteration boundary + stub-builder tests.
tests/test_trajectories.py	Adds round-trip spot checks for point fields and pins out-of-bounds behavior for interpolation/copy.
tests/test_thrust_modes.py	Expands coverage of ThrustModeValues operations, constructor shapes, immutability, broadcasting, and ThrustModeArray validation.
tests/test_storage.py	Strengthens TrajectoryStore argument validation tests, adds numeric equality checks on reopen, gates slow test behind --run-slow, improves threading guard test, and improves file access recorder test realism.
tests/test_performance_table.py	Adds thorough PerformanceTable validation/negative-path coverage and interpolation behavior checks using the sample model.
tests/test_performance_model.py	Adds deeper content checks for initialized models, selector behavior (including warnings), and selector caching behavior.
tests/test_model_utilities.py	Adds CI model normalization edge-case coverage including top-level list normalization with RootModel.
tests/test_mission_db_creation.py	Makes OAG conversion tests assert concrete DB row content and adds warning-category coverage.
tests/test_mission_db.py	Refactors filter tests into parametrized form, pins deterministic sampling SQL, expands query/result coverage, and adds Mission/QueryResult conversion tests.
tests/test_matlab_verification.py	Documents final-point skipping rationale and adds unit tests for process_matlab_csvs splitting/consistency checks.
tests/test_legacy_verification.py	Removes legacy script-style verification test file from the suite.
tests/test_golden.py	Clarifies golden snapshot semantics, adds precondition for golden length, and adds unit-envelope tripwires.
tests/test_emissions_storage.py	Verifies emissions totals/indices round-trip through storage.
tests/test_emissions.py	Strengthens species set checks, adds ISA-independent reference helper, and expands coverage for APU disabled path and LTO/climb-descent mode behavior.
tests/test_emission_functions.py	Fixes no-op assertion, improves tolerance handling, removes ineffective patching, adds branch coverage and clearer provenance notes.
tests/test_edb.py	Parameterizes sheet-absence error paths and makes engine-attribute checks granular and fixture-backed.
tests/test_dimensions.py	Adds coverage for netCDF dim-name ordering, removal semantics, and dim-name parsing; splits str/repr into its own test.
tests/test_config.py	Improves clarity around fixture override, pins concrete defaults, adds Config.escape behavior test, and pins missing default-data file branch.
tests/data/weather/README.md	Documents the ERA5 weather fixture provenance and coverage.
tests/data/verification/legacy/README.md	Documents the MATLAB legacy verification dataset layout/provenance for the verification tests.
tests/data/oag/README.md	Documents the OAG CSV extract used for conversion tests.
tests/data/missions/oag-2019-test-subset.README.md	Documents the SQLite test DB fixture contents and expected query counts.
tests/conftest.py	Adds --run-slow flag, registers slow marker, and skips slow tests by default; clarifies AEIC_PATH import-time behavior.
test-quality-notebook-index.md	Adds an index mapping notebook sections to tests and provenance expectations.
src/AEIC/weather.py	Fixes docstring typos for azimuth parameter.
src/AEIC/trajectories/builders/base.py	Replaces print() on exception path with module logger + stacktrace (logger.exception).
src/AEIC/storage/container.py	Replaces PEP604 union usage in isinstance checks with tuples for runtime correctness.
src/AEIC/emissions/ei/nvpm.py	Documents -1/0 smoke number sentinel behavior and its effect on outputs.
src/AEIC/data/engines/README.md	Documents provenance/intent for the sample EDB workbook fixture.
src/AEIC/config/core.py	Fixes decorator order on Config.escape so @contextmanager wraps the underlying function correctly.
pyproject.toml	Moves pytest config to [tool.pytest.ini_options] and restricts collection to tests/.
notebooks/test-cases.ipynb	Adds “Rounded results for use in test” blocks for multiple sections to improve provenance.
tests/data/performance/bad_performance_model_2.toml	Removes large legacy bad-model fixture.
tests/data/performance/bad_performance_model_3.toml	Removes large legacy bad-model fixture.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.