test: hermetic unit-test layer + batches 1-10 (utils, datalayer, gaussian, physics, risk, meteorology, GIS, experiment, openFoam/LSM/mlDL/WRF, evaporation/deposition) - #1067
Open
ilayfalach wants to merge 53 commits into
Conversation
Measured the current state (1,333 public callables, 407 test functions, no coverage measurement) and probed the code for what actually blocks unit testing. Key finding: hera.datalayer.document.connectToDatabase is a single seam through which the whole Project/toolkit stack runs on mongomock with no production code changes -- verified end to end. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
requirements.txt is deliberately left untouched so production installs do not change. coverage_floor.txt holds a placeholder until the baseline is measured. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Extracted as an importable module from the proven inline block in test_toolkit_coverage.py, which is left untouched. Tests fail until conftest.py wires install() in (next task). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Substitutes hera.datalayer.document.connectToDatabase so the real datalayer runs against an in-memory store: no production code is modified. Bootstrap order is load-bearing because hera/datalayer/__init__.py builds collection singletons at import time and raises KeyError without a pyhera config. Also rebinds the module-level Measurements/Simulations/Cache/All singletons, which are created before the seam exists and are used directly by abstractcalculator.py:172,190,221. Overrides the parent conftest's _no_trace_guard: a parent autouse session fixture applies to subdirectories, and that one purges projects in the real MongoDB. Leaving it in place cost 62.95s per run against a stopped MongoDB (pymongo's 30s server-selection timeout, twice); the unit layer now runs in under a second. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Network access raises immediately instead of waiting out pymongo's 30s server-selection timeout. Also pins matplotlib to Agg and asserts no test writes a .hera directory. The seam tests still pass with the socket guard active, which confirms mongomock opens no socket at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Test order must never matter; verified by running the write/read pair in both orders. Factories give deterministic control over the edge cases the S3 data set does not contain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four fixtures skip when TEST_HERA is absent or partial, so a failed S3 download makes CI green with dozens of silent skips. The gate is off by default: local runs keep skipping, which is the historical behaviour and stays the documented one. CI arms it with HERA_REQUIRE_TEST_DATA=1. Verified both directions end to end: with TEST_HERA=/nonexistent the suite skips and exits 0; with the gate armed (env var or --require-data) it fails and exits 1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The unit conftest's bootstrap is process-wide: it moves HOME, replaces connectToDatabase and rebinds the datalayer singletons. Collecting it alongside the integration tests made three dynamic_loading tests fail with 'Authentication failed' -- their subprocesses inherited the moved HOME and its placeholder credentials. All three disappear once the directory is not collected. -m 'not unit' cannot prevent this because marker filtering happens after collection, so the integration invocations now pass --ignore=hera/tests/unit, and the unit conftest raises a UsageError naming the correct commands if the two layers are ever mixed again. A silent wrong result becomes an immediate, explanatory failure. Also adds the make targets (test-unit, coverage-unit, coverage) and omits from coverage the six files under hera/ that are not valid Python: they break 'coverage report' outright and would otherwise pad the denominator with lines no test can reach. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The unit job has no MongoDB service and no S3 test data by design: if it needs either, the seam is broken. A logic failure now surfaces in under a minute instead of after service setup and an S3 download. The integration job arms HERA_REQUIRE_TEST_DATA and enforces the coverage floor on the combined measurement of both layers. Coverage plumbing detail that is easy to get wrong: pytest-cov already folds its parallel fragments into COVERAGE_FILE at the end of a run, so a bare 'coverage combine' afterwards has nothing to merge and exits 1, failing the step. Each layer therefore writes a distinctly named file (.coverage.unit, .coverage.integration) and combine runs only in the job that holds both halves -- which also avoids the two jobs colliding on the artifact name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three separate measurements rather than one, because it matters which is
which:
unit layer only ......... 7% (18446 stmts, 16920 missed)
integration only ....... 22% (18568 stmts, 13878 missed)
combined ............... 22% (18568 stmts, 13871 missed)
Combining recovered seven statements. The 26 infrastructure tests are very
nearly a subset of what the integration suite already covers, which is the
honest reading: their value is that they run in 3.5 seconds with no MongoDB,
no S3 data and no network -- not that they add coverage. Coverage comes from
batches 1-9.
The floor is 20, not the measured 22: CI runs Python 3.11 with the pinned
requirements, so its number will differ, and the gate should fail on
regressions rather than on environment drift. coverage_floor.txt now carries
that reasoning inline, and both readers take the first numeric line.
Note for whoever runs CI next: the floor step sits after the integration
pytest step, which currently fails on two pre-existing TestAutoCache errors
(a circular import in hera/datalayer/autocache.py and a TypeError). Both
reproduce on a clean origin/master worktree under pytest 7.2.0 and 9.0.3, so
the gate will not start enforcing until those are fixed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
--ignore=hera/tests/unit is what keeps the new hermetic layer out of the integration process; -m 'not unit' on top of it was wrong. 147 pre-existing tests inside the integration files carry @pytest.mark.unit (most of test_toolkit_coverage.py), and excluding the marker meant they ran in neither job -- the unit job only collects hera/tests/unit. Verified on one tree: 1444 collected without --ignore, 1418 with it, 26 in the unit directory alone. 1418 + 26 = 1444, so --ignore removes exactly the new layer and nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Scopes the 14 importable modules (912 statements, 23% covered) and defers five with reasons -- rag/* and SALibUtils cannot even be imported, because httpx, chromadb, sentence_transformers, llama_index and SALib are absent and none of them appear in requirements.txt. Records eight defects verified by running the code before any test was written, each to be pinned with xfail(strict=True) so that fixing one makes the test demand its own removal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Assertions derive from the conventions CLAUDE.md states (meteorological 0=North clockwise, mathematical 0=East counter-clockwise) and from the docstrings in query.py, not from what the code returns. angle.py reported 100% line coverage before this commit while nothing tested its behaviour -- the three lambdas are executed at import. A useful demonstration that coverage alone measures the wrong thing. Two defects pinned with xfail(strict=True): toAzimuthAngle does not normalise input outside one cycle, and dictToMongoQuery indexes list-valued mongoengine operator suffixes so __in silently matches nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Filter: all ten documented prepositions against the rows they keep, the inplace/copy contract in both directions, and the asymmetric outsideInterval boundary (lower inclusive, upper exclusive) which is easy to get backwards. _LazyModule: that construction imports nothing, that failure is deferred to first access too, and that the loaded proxy is transparent. A proxy whose import is not actually deferred would pass a naive test. calcDist2d: a three-point fixture small enough to verify by hand, so the assertions state what the normalisations mean rather than snapshotting them -- including that y_normalized leaves an empty row at zero instead of NaN. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
compareDataframeConfigurations: the three documented input shapes are checked to agree with each other, and absence of a parameter in one set is asserted to count as a difference -- that branch is easy to lose in a refactor because it looks redundant next to the value-differs branch. prepareSlurmScriptExecution: the generated directives are checked against Slurm's syntax, not against current output. That is what surfaces B7, where the memory directive is written '-mem=' instead of '--mem=' and sbatch would reject the file. Also pins that invalid argument combinations write no file at all rather than a half-formed one. Two defects pinned with xfail(strict=True): B4 and B7. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
latex.py had no coverage at all. Testing convert() against the rules stated
in its own comments surfaced B9: the math-mode branch compares a single
character to the string 'DOLLAR SIGN', so it can never fire, and the class
docstring's own example -- $4\frac{m}{s}$ -- comes out as
$$4\$\L{frac}{\L{m}}{\L{s}}$. Two more in the parser: items[0] is
always an empty bibItem, and an empty file raises IndexError.
zipUtils: the arcname conventions differ between the file branch (basename)
and the directory branch (relative to the parent, so the directory name is
kept) -- both pinned, since a refactor that unified them would break callers
silently. Also pins that JSON selection is a substring match, not an
extension check.
Five defects pinned with xfail(strict=True): B5, B6, B9, B10, B11.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Units are checked against their definitions -- a dunam is 1000 m2, mmH2O is
atm/10197.162129779, 0 degC is 273.15 K -- which is what surfaced B12: tounit
constructs Quantity(x, unit) from the bare pint import instead of
ureg.Quantity, so its result lives in pint's default application registry.
Verified consequences: the value cannot be combined with any hera quantity
('Cannot operate with Quantity of different registries'), and
tounit(1, 'dunam') raises UndefinedUnitError -- the one custom unit hera's
registry exists to provide is exactly the one this public helper cannot make.
mmH2O only appears to work because pint ships a millimeter_H2O of its own.
Contours are checked geometrically: a level-1 contour of x^2+y^2 is the unit
circle, so the resulting polygon must enclose an area of pi, and switching the
input unit to kilometres must scale that area by exactly 1e6.
Three defects pinned with xfail(strict=True): B8 and B12 (twice).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jsonutils is the largest module in hera/utils (224 statements, 21% covered). The tests are written as round trips, because that is the property callers depend on: a config with units has to come back out of JSON unchanged. Testing the documented meaning of returnStandardize surfaced B13. It is described as 'return the units in MKS', but the decoder reads the 'units' field, which only holds base units when the ENCODER was given standardize=True. Asking the decoder for MKS on a document encoded without it returns the original units and reports nothing. Also pins that setJSONPath copies by default (a variation sweep that mutated its base would corrupt every later variation) and that a path matching nothing raises rather than being ignored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
hera/utils/__init__.py exists so that `import hera` does not pull in pint, pandas, shapely and geopandas. Its __all__ is the package's contract, so the test resolves every entry in one assertion -- a stale name is a broken import for somebody. Also pins that a resolved name is cached into globals(), which is what makes the second lookup free, and that underscore names short-circuit before any submodule import is attempted. Writing these found B14: with_logger is annotated "-> (str, dict)" but returns a 3-tuple (name, config, 'loggers'). Unpacking it the way the annotation says raises ValueError -- which is exactly how the first draft of this test failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Combined measurement (unit + integration, Python 3.12 / pytest 9.0.3):
whole package .......... 22% -> 28%
hera/utils ............. 23% -> 43%
hera/utils in scope .... 23% -> 74% (912 of 2397 statements)
The in-scope figure is the meaningful one for this batch. The remaining 1485
statements in hera/utils are rag/*, data/CLI.py, data/toolkit*, SALibUtils and
freeCAD, deferred with reasons recorded in the batch 1 plan -- rag/* and
SALibUtils cannot be imported at all, because httpx, chromadb,
sentence_transformers, llama_index and SALib are absent and none of them are
declared in requirements.txt.
Floor keeps two points of headroom below the measured value: CI runs Python
3.11 with the pinned requirements, so the gate should fail on regressions
rather than on environment drift.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sixteen defects were scattered across commit messages and xfail reasons, which is too fragile a home for material that becomes a single issue at the end of the effort. Each entry records the file, the test that pins it, and the verification that was actually run -- none of them come from reading code alone. Notable: P6 (two TestAutoCache failures) is verified identical on a clean origin/master worktree under both pytest 7.2.0 and 9.0.3, so it predates this work and currently keeps the CI coverage gate from ever executing. B12 and B16 are the two worth acting on first: a public helper that cannot produce hera's own custom unit, and 473 statements that cannot be imported at all because five dependencies are undeclared. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
hera/toolkit.py was the largest single gap in this area at 30% coverage.
These run the real datalayer -- query construction, desc filtering, version
resolution, document tagging -- against the in-memory store, so they exercise
hera rather than a mock.
Version resolution is checked componentwise, including that [0,0,10] beats
[0,0,9] (string ordering would not) and that a tuple and a list version are
interchangeable even though dictToMongoQuery sends them through two different
query shapes. Also pins that the getter does not write to the database: its
docstring still promises to persist the latest version as the default, but the
code removed that with a comment calling it a hidden side effect. The code is
right and the docstring is stale.
Two defects pinned:
B18: getDataSourceList is documented as returning "data source names" but
yields one entry per document, so a source with three versions appears
three times. getDataSourceMap is the one documented as covering
versions.
B19: the document-adding overrides call desc.setdefault on the CALLER's
dict. The verified consequence is cross-toolkit misattribution -- a
metadata dict reused across two toolkits keeps the first toolkit's tag,
so the second toolkit's document is stored under the wrong name, is
invisible to its own queries, and appears in the other toolkit's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The registry decides which storage format each Python object gets, so it decides whether hera follows its own policy in CLAUDE.md -- tabular data as parquet, never CSV. All eighteen declared format constants are checked to resolve to a handler exposing both saveData and getData; a constant with no handler is a trap that only fails when someone tries to save. Round trips for string, JSON_dict, parquet, numpy_array and pickle write to tmp_path, so none of them touch the test data set. B20: typeDatatypeMap is keyed on third-party INTERNAL module paths, such as 'pandas.core.frame.DataFrame'. pandas 3 reports 'pandas.DataFrame', which the map does not know, so every DataFrame silently degrades to pickle instead of parquet -- no error, worse interoperability, and against the stated policy. requirements.txt pins pandas 2.2.3, where the key still matches, so CI is unaffected and only a drifted environment shows it. The two tests therefore carry an xfail condition computed at collection time from the installed pandas, so the expectation is correct in both environments rather than flaky in one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Counter semantics are documented but surprising, so they get pinned explicitly: the first getCounterAndAdd call DEFINES the counter at 0 and adds nothing, and only later calls increment. N calls therefore leave the counter at N-1. The first draft of these tests assumed post-increment and failed -- the code was right and the assumption was wrong, which is exactly the case where a test earns its keep. Also pins that addDocumentFromDict drops an incoming projectName, so an imported document joins the project doing the importing rather than the one it came from, and that the three collections stay separate. Note on an earlier suspicion: setConfig's long "default project" error looked like it ended in a bare raise, which would have produced an opaque RuntimeError instead of the message. It does not -- project.py:247 is raise ValueError(err). Verified, no defect. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three new defects (B18-B20), plus a section for suspicions that were checked and did NOT hold, so nobody spends time re-investigating them: setConfig does raise its long error properly, every format constant has a handler, and the counter semantics are documented and correct -- my test assumption was the thing that was wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
toolkit.py rises from 36% to 47% under the unit layer alone; the remaining gap
is the JSON import helpers. CLAUDE.md forbids constructing toolkits directly,
so what this registry returns for a given constant is a public contract -- the
save-mode constants get their own test because those strings are persisted,
which makes changing one a data migration rather than a rename.
Three defects found, and together they mean the default-repository feature has
never worked:
B21: _get_data_toolkit declares a projectName parameter and ignores it,
returning dataToolkit(), which always operates on the default project.
So a "per project" default is actually global.
B22: because the write therefore lands on defaultProject, which Project
guards as read-only (project.py:757), setDefaultRepository always
raises RuntimeError. registerToolkit sets
_allowWritingToDefaultProject before writing (toolkit.py:1271);
setDefaultRepository does not.
B23: its data-format lookup tries datatypes.JSON, .json and .TEXT, none of
which exist -- the constants are JSON_DICT, JSON_PANDAS and
JSON_GEOPANDAS. Dead code rather than a wrong result, since the payload
lives in desc; the test fails if one of those names is ever added, so
the lookup gets revisited.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
combined ............... 28% -> 28% (12937 -> 12850 missed)
toolkit.py + datalayer . 61% -> 68%
hera/toolkit.py ........ 47% from the unit layer alone (was 30%)
The floor does NOT move. 87 covered statements is a real gain inside the
group but a 0.4-point gain across the package, because the datalayer was
already well covered by the integration suite -- unlike hera/utils, which
batch 1 took from 23% to 74% in scope. Raising the floor on that would leave
no headroom for the CI-versus-local drift the floor exists to tolerate.
What batch 2 delivered instead: behaviour that previously needed a live
MongoDB now runs hermetically in under two seconds, and six defects surfaced --
including B21/B22, which together mean the default-repository feature has
never worked in any environment.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ed table Every sigma assertion evaluates the Briggs (1973) open-country formula independently and compares -- 36 direct comparisons over six stability classes and six distances. That distinction matters: an assertion copied from the implementation's own output would pass even with a wrong coefficient table, so the table is also checked entry by entry against the published values. The physics comes out right. sigma_y and sigma_z match to 1e-9 relative across the whole grid, both grow monotonically with distance, and both order correctly from A (most unstable, widest plume) to F. The virtual-source construction is self-consistent too: with sigma0 = 10 m the spread at x = 0 is exactly 10 m, which is the property that makes the whole shifted-origin trick valid. B24: getSigma does not validate the distance. x = -100 m under stability D returns sigma_y = -8.04 m -- a negative standard deviation -- and a large negative x returns NaN with a numpy RuntimeWarning, because (1 + b x) goes negative under a fractional power. Either input should be refused; silently returning a negative sigma or NaN lets a bad distance propagate into a concentration field. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each quantity is checked against a value that can be looked up independently: sea-level pressure is 1.0000 atm and falls to exactly 1/e at 1/1.186e-4 m; the lapse rate is 6.5 K/km and linear; density at 20 C is 1.2046 kg/m3 against a textbook 1.204; viscosity lands within 2.4% of 1.81e-5 Pa s and rises with temperature as a gas must; the log profile reproduces u_ref exactly at the reference height and vanishes at z0; and the hot-spot exponents match the published 0.07 to 0.55 across stability classes. Two behaviours are pinned as deliberate rather than accidental: getTKE ignores its height argument (documented as neutral-only), and the wind profile clamps height into [0, 300] m, which the docstring does not mention -- a caller asking for 1 km silently gets the 300 m value. B25: five docstrings under gaussian/ carry the r prefix on the CLOSING triple quotes, so the LaTeX backslashes are invalid escapes and every import emits SyntaxWarning. B26 is the more interesting one, and it is about a test rather than the code. test_no_invalid_escapes.py compiles each file with SyntaxWarning escalated to an error -- but Python surfaces an escalated SyntaxWarning as SyntaxError, and that test's handler for genuinely-unparseable legacy files catches SyntaxError and calls pytest.skip. So the failure it exists to detect can only ever become a skip, and its message claims "file is not importable" about a module that imports fine. Two passing tests here demonstrate the mechanism. Not fixed: it is an existing working test and the standing instruction is to report. One assumption of mine was wrong and the code was right: I expected rougher terrain to slow the wind aloft. With u10 pinned it does the opposite, because ln(z/z0)/ln(10/z0) grows with z0 above the reference height. The test now states that with the numbers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mass conservation is the property worth holding onto, so the tests recover
sigma and total mass from the produced field rather than snapshotting an array:
a normalised puff integrates to Q to within 1e-6, scales linearly with Q, and
the recovered alongwind sigma matches the input across three widths.
Four defects, all verified numerically:
B27: GaussianToMesh.__init__ sets sigmaYName to "sigmaXCorrected" -- the X
column. An input sigmaYCorrected of 50 yields a recovered sigma_y of
5.000: the crosswind data is ignored and the plume is forced isotropic.
For a dispersion model where sigma_x and sigma_y differ by design that
is a physics error, not a rounding one. A passing test shows the kernel
itself is fine -- point sigmaYName at its own column and anisotropy
works -- so only the constructor default is at fault.
B28: _defineCoordinates takes a LABEL from .index[0] and feeds it to .iloc,
which wants a POSITION. Any frame that came out of a groupby, filter or
concat raises IndexError.
B29: GaussianIntegrationToMesh narrows its parent's input. The base accepts
a plain float or a pint quantity for Q; the subclass accepts only pint,
because it calls unumToPint on the value itself. Code written against
the base breaks on exactly the substitution the subclass docstring
invites by calling itself "more accurate".
B30: gaussianToolkit.getMeteorologyFromU10 defaults to powerLaw while
MeteorologyFactory.getMeteorologyFromU10 defaults to log, so the same
call through two entry points gives two different wind profiles.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
combined ............... 28% -> 30% (12850 -> 12513 missed)
gaussian ............... 33%
gaussian in scope ...... 66% (470 of 1061 statements)
Sigma.py ............ 100%
Meteorology.py ....... 95%
MeshUtils.py ......... 88%
337 covered statements, which is enough to move the package figure -- unlike
batch 2, where the datalayer was already covered by the integration suite. The
gain here is close to the 311 statements the unit layer covers inside gaussian,
which suggests the integration suite contributed little there; I could not
measure that separately, because coverage combine consumed the integration data
file, so it is stated as an inference rather than a number.
Deferred with reasons: gasCloud.py (291), FallingNonEvaporatingDroplets.py
(217) and DropletCloud.py (83). The gas-cloud solver and the falling-droplet
physics build multi-dimensional xarray fields and need whole meteorology and
source objects; they deserve a batch rather than the tail of this one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rpolation
Half of this batch's nominal scope could not be tested at all, and finding out
why was the most valuable part of it.
B31: evaporation and deposition are unimportable. All three of their modules
do `from ..utils import tonumber, tounit`, which resolves to
hera.simulations.utils -- an empty package. The names are in hera.utils,
three levels up, and a sibling in the same tree
(gaussian/DropletCloud.py) already writes it with three dots. Two whole
physics packages are dead in any installation; the fix is one dot in three
files.
B32: calculateR mixes normalisations. Its numerator divides by N while its
denominator uses pandas' .std(), which divides by N-1, so every
correlation comes back short by exactly (N-1)/N. Verified across five
sample sizes: a perfect model scores 0.667 at N=3, 0.8 at N=5, 0.98 at
N=50. It can never reach 1.
B33: skin_friction contains `numpy.log()` with no argument, in BOTH channelFlow
and couetteFlow. Neither method can ever have run.
B34: ReynoldsUm returns 80000/meter. The height property strips units with
m_as(m) while the viscosity keeps them, so a dimensionless group comes out
with dimensions. Re_tau in the same class gets it right by using
_channelHeight directly.
B35: ReynoldsUm never reads the Um it is documented and named for.
What does work is checked against its definitions: the Chang and Hanna metrics
all take their defining values for a perfect model, the technical roughness is
3.5 Ra per Schlichting, C+ hits 5 and 8 at its two limits and follows the
documented log law between them, and the interpolant is a genuine convex
combination -- exact at stations, bracketed between them, monotonic along the
line joining two.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
combined ............... 30% -> 31%
targeted files ......... 70% (199 statements)
errorCalculation.py . 100%
nearWallFlow.py ...... 77%
interpolations.py .... 53%
The batch's nominal scope was about 91 public callables; 312 statements of it
cannot be covered at all, because evaporation and deposition are unimportable
(B31). That is a defect, not a testing shortfall, so the floor reflects what
was actually reachable.
A further 425 statements -- canopyWindProfile, coordinateHandler and
windProfile/toolkit -- were not targeted. They need xarray/dask structures or
an external IMS token, which makes them integration-shaped rather than
unit-shaped; taking them on properly belongs in a batch with the data set
available.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The physics that works is verified against its definitions: Haber's law on an
xarray field gives C x t exactly, ten Berge with n = 1 reduces to Haber, and
the log-normal dose response puts exactly half the population at TL_50 with
getToxicLoad as its precise inverse across seven fractions. A decade either
side of TL_50 gives 0.02275 and 0.97725 -- exactly +-2 sigma for sigma = 0.5
in base 10, which pins both the logarithm base and the meaning of sigma.
Four defects:
B36: the pandas default-units branch tests hasattr(field, "attrs"), which
modern pandas satisfies, so it takes the xarray path and evaluates
df.attrs[None] -> KeyError. The documented mg/m3 default is
unreachable.
B37: and with inUnits supplied it returns all NaN, because
concentrationField[:-1] keeps a DatetimeIndex while dt_min[1:] carries
the RangeIndex from reset_index(). CalculatorHaber cannot produce a
number for pandas input at all, despite a docstring that describes the
path in detail.
B38: InjuryLevelThreshold passes its threshold straight to ureg(), which
parses strings only, so a plain number dies inside pint with
"'float' object has no attribute 'replace'". Its sibling accepts a
plain TL_50 through tounit().
B39: and getPercent compares tounit(x, units) against that threshold, so a
numeric toxic load raises "Cannot operate with Quantity of different
registries". This is B12 from batch 1 with a concrete victim: the
ordinary calling form cannot be evaluated. Fixing B12 fixes this.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lence cache
Correcting my own claim. P1 in the findings log asserted that the cache lookup
in abstractcalculator "never finds a match". Running it says otherwise, and
the truth is more interesting.
mongoengine recognises `all` as an operator even in the middle of the
flattened key, so params__all=["u","v"] becomes
{"desc.params.0": {"$all": ["u"]}, "desc.params.1": {"$all": ["v"]}}
$all is defined as order-independent set containment; this is a positional
PREFIX match. Against a stored params of [u, v, w] the requests ["u"],
["u","v"] and ["u","v","w"] all match, while ["v"], ["w"] and ["v","u"] miss --
every element present, wrong position.
So the cache is partially and order-dependently effective rather than dead.
_AllCalculatedParams is built by extend() in request order, so a hit depends on
the caller happening to ask for a prefix. The miss is silent either way.
Both halves are pinned: the prefixes as passing tests, the order-independent
cases as xfail. The findings log has been corrected to match.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
riskassessment reaches 39% and the package holds at 31%. The floor does not move: 38 covered statements is inside the rounding, and the batch's value was four defects rather than a number -- in particular proof that B12's registry split is not latent but actively breaks InjuryLevelThreshold.getPercent for the ordinary numeric calling form. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The season assignment is checked against seasonsdict -- the module's own
declaration of DJF/MAM/JJA/SON -- rather than against the pd.cut edges, so
changing either has to be deliberate. All twelve months land correctly,
including the wrap-around that puts December and January in the same season,
and the HHMM encoding holds across the day.
Two modules cannot be imported, neither for want of an optional dependency:
B40: radiosonde.py subclasses datalayer.ProjectMultiDBPublic, which no
longer exists -- a leftover from an unfinished refactor.
B41: highfreqdata/__main__.py reads
/home/ilay/hera_unittest_data/.../slicedYamim_sonic.parquet at import
time. A personal absolute path, which CLAUDE.md forbids outright, so
the module raises FileNotFoundError on every other machine.
GFS.py also fails to import here, but scikit-learn is declared in
requirements.txt; that one is the environment, not the code, and its test uses
importorskip so it is not miscounted as a defect.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
meteorology reaches 35%, the package holds at 31%. The substance was two unimportable modules and a correction: P1 does not disable the turbulence cache outright, it degrades $all into a positional-prefix match. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The conversion itself is checked against independently known geography: Tel Aviv at 34.78E 32.08N lands inside the ITM easting and northing ranges, the round trip recovers the original to 1e-6 degrees, one degree of latitude comes out at the expected ~111 km, and direction is preserved on both axes. B42: importing hera.measurements.GIS.raster.hill2stl runs a demo at module level -- under a comment that literally reads "# Run the function" -- printing to stdout and writing an 8.1 MB test1.stl into whatever directory the process happens to be in. Mitigating detail, verified: importing the GIS package alone does not reach it, only a direct import of that module. Also pinned as a passing test rather than a defect: convertCRS returns a list of shapely Points while TopographyToolkit.convertPointsCRS returns a GeoDataFrame. Both are documented, but two conversion entry points with different return types is worth unifying. My first draft of these tests assumed the GeoDataFrame form for both. The docstring says list; the tests now say list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_handleType1 cites Floors et al. (2021), WES 6, 1379, table a2, and its seventeen IGBP entries all match it. The physical ordering holds too: open water is the floor at 1e-4 m, closed forest the ceiling at 1 m, urban rougher than grassland, snow and ice nearly as smooth as water. B43: getRoughnessAtPoint carries a SECOND table for the same codes, labelled in the source as "# Example values". It is an arithmetic ramp -- constant steps of 0.05 or 0.1 from code 2 to 16 -- not a physical lookup, and it disagrees with the published one by a factor of 100 for water and 1300 for snow and ice. It also inverts the ordering, making forest smoother than snow. z0 feeds the wind profile and from there every dispersion result, so a caller reaching the type_name="IGBP" branch does not get a slightly different answer; they get a different simulation. Both branches are reachable and the choice is an argument. This is the most scientifically consequential finding of the effort so far. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GIS reaches 43% and the package holds at 31%. The floor does not move on 70 statements; the batch's substance was B43, two contradictory IGBP roughness tables in one file, one cited to Floors et al. (2021) and one labelled 'Example values', differing by a factor of 1300 for snow and ice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CLAUDE.md requires Calculator subclasses to carry pydoc.locate-compatible
names because Injury.py builds the class path from a string. That contract is
exercised the way the factory uses it: the three calculators resolve, an
unknown name returns None rather than raising -- which is what the caller
checks -- and the resolved class is constructible.
B44: Parser_TOA5.parse has an empty body, so it returns None for any input,
including a path that does not exist. It neither parses nor refuses, and a
real TOA5 parser already exists at
meteorology/highfreqdata/parsers/TOA5.ASCIIParser.
The module around it is orphaned: a scan of every .py file in the tree finds
nothing that imports hera.measurements.experiment.parsers, and the dispatch in
lowfreqdata/toolkit.py that would have built "Parser_{name}" paths is
commented out. Recorded rather than acted on -- deleting it is a call for
someone who knows whether an external consumer exists.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deliberately a small batch: experiment/parsers.py is orphaned, so covering its 41 definitions would raise a number without protecting anything anyone runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…o end
The stub layer delivers what Phase 0 promised: openFoam.toolkit, OFWorkflow,
both LSM modules, hermesWorkflowToolkit and WRF all import under it, which is
the precondition for every future test of those modules.
prepareParams is checked against the conversions it documents -- a template
declaring metres turns 1 km into 1000, duration is exempted from
standardisation so 30 minutes does not silently become 1800 seconds, and grid
counts are cast to int.
B45: prepareParams does params = template_desc.get('params', {}) followed by
params.update(...), writing the caller's overrides into the template's own
dict. An LSM template is meant to be reused across simulations, so after the
first run it carries that run's parameters and every later run inherits them.
Same shape as B19 in abstractToolkit.
Correcting myself: Phase 0 recorded that torch is unstubbed because
modelContainer.py subclasses torch.nn.Module and a MagicMock cannot be
inherited from. That is wrong -- it imports LightningModuleHera, a plain
class, and the nn.Module reference is in a docstring. The real reason,
verified: a leaf MagicMock has no __path__, so `import torch.utils` fails with
"'torch' is not a package". Stubbing torch needs the namespace-package form
PyFoam gets. _stubs.py now says that, and a test pins the mechanism instead
of asserting the claim.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
combined ............... 31% -> 32% (12221 -> 12150 missed) Final position across Phase 0 and nine batches: 1000 hermetic tests running in about four seconds with no MongoDB, no S3 data and no network; floor 20 -> 30; package coverage 22% -> 32%; 48 defects recorded, every one pinned by an xfail(strict=True) so that fixing it makes the test demand the marker's removal. Also adds CONSOLIDATED-ISSUE-DRAFT.md -- a draft only, nothing opened or posted. It groups the 48 findings by what they are rather than by batch: code that cannot run at all, scientific errors that change simulation output, cross-context data corruption, infrastructure, and unmet API contracts. It ends with the six one-line fixes worth doing first, and with the two claims I made during the work that turned out to be wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…import side effects
First production changes of this effort. Every one of them removes something
that stopped tests or code from running; no behaviour was redesigned.
The integration suite now passes: 704 passed, 0 failed. It had two failures
before, which meant the CI coverage gate never got far enough to execute.
autocache.py -- `from hera import Project` at module level created a cycle.
Importing autocache directly triggered hera.__getattr__("Project") ->
_load_deferred(), whose step 3 re-imports autocache while it is still
half-executed, so cacheFunction did not exist yet. Project is used in
three functions, so the import is now local to them and the cycle is
gone at its source.
autocache.py -- `return ret, doc if self.returnDoc else ret` parses as
`return (ret, (doc if ... else ret))`, a 2-tuple either way, because the
conditional binds tighter than the comma. Callers got (result, result)
instead of result. Parenthesised; both returnDoc settings verified.
evaporation, deposition, simulations/CLI.py -- nine `from ..utils import`
sites resolved to hera.simulations.utils, whose __init__ is empty; the
names live in hera.utils. The two module-level ones made evaporation and
deposition unimportable outright; the six inside CLI.py functions failed
only when called. A test now asserts no such import remains.
hill2stl.py -- a demo ran at module level, printing to stdout and writing an
8.1 MB test1.stl into the working directory on every import. Guarded
under __main__; the generator it demonstrates is still importable.
highfreqdata/__main__.py -- read /home/ilay/hera_unittest_data/... at import
time, so it raised FileNotFoundError on any other machine. The path is
an argument now, the read is under __main__.
test_no_invalid_escapes.py -- it compiled each file with SyntaxWarning
escalated to an error, but Python surfaces an escalated SyntaxWarning AS
a SyntaxError, and the handler for genuinely unparseable legacy files
caught it and called pytest.skip. The failure it exists to detect could
only ever become a skip. Now two passes: parseability first with no
filter, then escapes on a file known to parse.
Meteorology.py, gasCloud.py -- with that test able to fail, it immediately
found seven docstrings carrying the r prefix on the CLOSING triple
quotes, making the LaTeX backslashes invalid escapes. Moved to the
opening quotes. The tree now compiles warning-free apart from the three
files that never parsed at all.
Fourteen xfail(strict=True) markers turned into real assertions, which is the
mechanism working as intended: fixing a pinned defect makes its test demand
the marker's removal.
Not fixed, deliberately. B40 (radiosonde subclasses the removed
ProjectMultiDBPublic) needs the multi-database semantics redesigned, not a
mechanical edit. monaghan.py needs pyriskassessment, a private package
declared in requirements.txt:372; it is skipped the way GFS.py is for sklearn.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…port fix
Neither module could be imported before B31 was fixed, so nothing in them had
ever run. Opening them to testing produced six defects, four of which mean
the code cannot work at all.
B46: the diameter setter is named `ustar`, so @diameter.setter builds a new
property under that name and replaces the real one. obj.ustar returns
_diameter, obj.diameter has no setter, and depositionRate_Petroff reads
`ustar = self.ustar` -- it computes dry deposition with a particle
diameter substituted for a friction velocity. Passing ustar to the
constructor changes nothing.
B47: the heatFlux setter assigns self._ustar, so heat flux never changes and
the friction velocity is silently overwritten.
B48/B49: the deposition rate is bit-identically insensitive to surface
roughness (z0 0.001 vs 0.5) and to particle density (500 vs 5000), both
checked at a realistic u* so they are not artefacts of B46.
B50: evaporationModels calls RiskToolkit.getAgent on the class, but it is an
instance method, so every construction raises TypeError. The class has
never been instantiable.
B51: flux_US strips the units off its temperature and then passes the bare
number to vaporPressure, which requires units. Every call raises
DimensionalityError, so the module's central calculation cannot run.
The correlations themselves are right, and are checked against their published
forms: FSG and EPA diffusion match term for term, the FSG temperature exponent
is recovered as 1.75 from a log-log slope, air viscosity returns its reference
value exactly at 293 K and scales as sqrt(T), Reynolds is linear in velocity
and length, and Schmidt equals an independently computed nu/D.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A separate dead-code-cleanup effort landed on master and deleted hill2stl.py (zero references at the time it ran). Accepting that deletion rather than re-adding the file just to keep TestImportSideEffects alive -- drop the test class and mark B42 resolved via deletion instead of the __main__ guard it originally documented. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resolves two collisions with unrelated concurrent work: - hera/datalayer/autocache.py: our unblock-the-test-suite fix already supersedes master's circular-import fix (same root cause, plus an additional tuple-return bug); kept ours. - hera/measurements/GIS/raster/hill2stl.py: deleted by a separate dead-code-cleanup effort as unreferenced; accepted the deletion and dropped TestImportSideEffects, which was its only remaining reference. Also drops hera.simulations.LSM.hermesWorkflowToolkit from the stub-layer import smoke test's module list -- that file was deleted by the same dead-code cleanup as a stale diverged fork of hera.simulations.hermesWorkflowToolkit. Full unit suite verified green after the merge: 1076 passed, 3 skipped, 80 xfailed, 0 failed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Collaborator
|
@ilayfalach is this ready for review? it seems that the tests are failing |
…xt pins
All three were verified against a fresh venv installed with the exact
pins from requirements.txt (Pint==0.24.4 etc), not just the shared dev
venv, which had silently drifted (e.g. Pint 0.25.3 instead of 0.24.4).
- test_gaussian_meteorology.py: escalate DeprecationWarning alongside
SyntaxWarning. Python <=3.11 emits invalid-escape warnings as
DeprecationWarning; only >=3.12 uses SyntaxWarning. CI runs 3.11.
- test_meteorology_module_imports.py: also importorskip("osgeo"). GFS.py
imports both sklearn and osgeo at module level; the test only guarded
the first.
- test_risk_injurylevel.py: under Pint==0.24.4, ureg("50") returns a bare
int (not a Quantity), so InjuryLevelThreshold("T", threshold="50")
raises AttributeError at .to(), not DimensionalityError. Verified
directly against 0.24.4 in an isolated venv.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…portable Discovered while validating the full unit suite against a fresh venv pinned to requirements.txt: without a real local argos install (the case CI's hermetic unit job is actually in), experiment.py's module-level `class experimentSetupWithData(argosDataObjects.ExperimentZipFile, ...)` crashes with NameError, because the old stub only covered a bare `import argos` and left argosDataObjects undefined after the caught ImportError -- no unit test currently imports experiment.py under a truly argos-less environment, so this was never exercised. argos.experimentSetup.dataObjects now gets the same namespace-package treatment PyFoam has, plus real (if empty) placeholder classes for ExperimentZipFile/TrialSet/Trial/EntityType/Entity, since a class statement's bases must be actual types -- a MagicMock does not work as a base class the way it works for attribute access or calls. Verified against both a real local argos (unaffected, real one still wins) and a venv with no argos at all (now imports cleanly). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The integration job ("Run full test suite") only installed
requirements.txt, so its `pytest --cov=hera` and the later `coverage
combine`/`coverage report --fail-under` steps had no pytest-cov/coverage
available -- the job failed on its very first pytest invocation with
"unrecognized arguments: --cov=hera --cov-report=".
This job was always previously skipped (the unit job, gating it, used to
fail first), so this is the first time it has actually run for any branch
in this test-expansion effort. Can't be verified locally -- no MongoDB
service, S3 test data, or real hermes/pyargos checkout here -- so this is
reasoned from the error message and the working unit-job config, not
tested end-to-end before push.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Collaborator
Author
|
done, now you can look @lior-antonov |
ilayfalach
pushed a commit
that referenced
this pull request
Sep 1, 2026
…tream test_gis_hill2stl.py (dedicated coverage for compute_normal/write_triangle/ function/generate_solid_stl) collides with the same upstream deletion already handled for test_gis_utils.py in batch10/PR #1067 -- hill2stl.py no longer exists. Removed the file and updated the findings log. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Discovered while cascading the pinned-venv validation into batch 22: subclassing a bare MagicMock() attribute (hermes.workflow, when the real hermes package is absent) does not raise -- it silently produces another MagicMock as the "class". OFWorkflow.py's abstractWorkflow(hermes.workflow) then becomes a MagicMock too, and every isinstance() check against a workflow_Eulerian subclass elsewhere fails with a confusing "TypeError: isinstance() arg 2 must be a type, a tuple of types, or a union" instead of exercising the intended logic. Generalizes the argos.experimentSetup.dataObjects fix: hermes now gets a real module with workflow set to a real (if empty) placeholder class when genuinely absent, same as before real installs still win. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Collaborator
|
@ilayfalach please remove claude's residual files by adding the folders to .gitignore |
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
2 participants
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
First installment of the hermetic unit-test layer described in #1061, plus ten batches of new unit tests against
hera/. Everything here is additive test coverage underhera/tests/unit/, backed by a mongomock reroute (no real MongoDB, no network) and a stub layer for CI-unavailable packages (PyFoam/paraview/FreeCAD/hermes/argos/evtk).conftest.pyisolation guards (no network, no real.herahome writes, DB reset between tests), CI wiring (ci.ymlruns the unit marker before the integration suite), and a monotonically-increasingcoverage_floor.txtgate.hera/utils,toolkit.py+datalayer,simulations/gaussian,evaporation/deposition/hydrodynamics/windProfile/simulations/utils,riskassessment+presentation,measurements/meteorology,measurements/GIS,measurements/experiment,openFoam/LSM/mlDL/WRF, and the evaporation/deposition code unblocked by the batch-10 import fix.docs/superpowers/findings/2026-08-24-test-expansion-findings.md), each pinned with@pytest.mark.xfail(strict=True)rather than fixed -- so a future fix is forced to remove the marker. A handful of suspected bugs that turned out to be correct behavior are recorded too, to save the next person from re-investigating them.8eaa510a, batch 10): unblocked the integration suite by removing a circular import inautocache.py, an operator-precedence bug in the same file, sixwrong-levelimports, an import-time side effect inhill2stl.py, and a test-detection gap intest_no_invalid_escapes.py. Full rationale in the commit message.Note on this branch's history
This branch (and each
tests/batchN-*branch after it) is a linear stack -- batch2 is built on batch1, batch3 on batch2, and so on. This PR is therefore the right unit to merge as a whole; opening separate PRs per batch againstmasterwould show overlapping, duplicated diffs. The next chunk (batches 11-19) will follow as its own PR once this one is in, and so on through batch 28.Conflict resolution against current master
This branch's fork point predates ~21 commits that have since landed on
master, including a separate dead-code-cleanup effort. Two collisions were resolved in the merge commit (2a918afc):hera/datalayer/autocache.py: this branch's own fix already supersedes master's (same root cause, plus an additional tuple-return bug) -- kept ours.hera/measurements/GIS/raster/hill2stl.py: deleted on master as dead code; accepted the deletion and dropped the now-orphanedTestImportSideEffectstest class (its only remaining reference), and removed the now-deletedhera.simulations.LSM.hermesWorkflowToolkitfrom a stub-layer import smoke test's module list for the same reason.Test plan
hera/tests/unit -m unit -q: 1076 passed, 3 skipped, 80 xfailed, 0 failed (verified after merging current master and resolving the two collisions above)hera/tests/, excludingunit/) -- left to CI; not runnable locally in this environment (no localhermespackage)🤖 Generated with Claude Code