Repository navigation
test: hermetic unit-test layer, batches 31-33 - #1085
Open
ilayfalach wants to merge 3 commits into
Open
ilayfalach wants to merge 3 commits into
ilayfalach wants to merge 3 commits into
Conversation
added 3 commits
September 3, 2026 10:29
…(B142-B211) Fifteen new test files, 1559 new passing tests (1863 -> 3422) and 75 new strict-xfail bug pins. No production code touched; suite stable across repeat runs. New coverage: utils/data CLI + dataToolkit, openFoam CLI/toolkit/OFWorkflow, the Lagrangian LSM toolkit, GIS raster landcover + topography (against real synthesised GeoTIFFs), experiment.py against a genuine argos v3.0.0 zip, the buildings and basicplots presentation layers, inputForModelsCreation and the Petroff canopy branch. Highest-impact findings: * B166 — three openFoam CLI handlers are defined twice; the surviving bodies are bare `pass`, so mesh commands exit 0 doing nothing. * B174/B179/B180 — three commands call names that do not exist anywhere: tk.workflowCompare (one hit in all of hera: the call site), registerToolkit(toolkit_name=...) (undefined local, NameError swallowed by a bare except), and Project.setDataSourceDefaultVersion (lives on abstractToolkit) — the last directly contradicts CLAUDE.md. * B201 — every _readRecord in the LSM toolkit dies on self.logger, which no base class defines, making the hand-written OpenFOAM fallback parser unreachable. * B187 — np.vectorize without otypes infers dtype from cell (0,0), so a landcover grid starting on forest truncates every roughness to int and silently zeroes the rest; z0=0 makes the log wind profile singular. * B183 — urban roughness writes an IGBP class code into z0, giving rural cells 12-14 m instead of ~0.15 m. * B191 — TopographyToolkit.create_xarray's minx/miny mean (lat,lon) for WSG84 but (x,y) otherwise, silently returning a grid in another country. * B160/B163 — experiment.py reads .x off convertCRS's list return, and two initialisers index keys argos itself reads defensively, so hera cannot load argos's own example export. * B153/B156/B159 — a Cache section in a repository JSON can never load (parameter-name mismatch swallowed by a bare except), Function sections target the wrong object, and isRelativePath is forwarded as document data. Also corrects the consolidated summary, whose "needs real data" blocked list was largely stale: most of those files needed only small synthetic fixtures in tmp_path, not real datasets.
…(B212-B309) Fifty new test files, 1605 new passing tests (3422 -> 5027) and 109 new strict-xfail bug pins. No production code touched; suite stable across repeat runs. This batch deliberately targeted the remaining *real logic* (170 reachable functions across 50 files) and stopped short of the paraview/PyFoam mock tail, where tests would only assert against MagicMocks. Whole subsystems turned out to be dead on arrival: * B286 — hera/bin/hera-LSM:41 is `parser.parse_args()()`; the double parens call the returned Namespace, so the LSM CLI cannot dispatch any subcommand at all. * B259/B260 — ToolkitHome.auto_register_and_get and import_toolkits_from_json both call registerToolkit with the wrong signature entirely. Earlier batches deferred these as "complex registry logic"; they are simply dead. * B263 — riskAreaAlgorithm_Sweep calls effectIsopleths.datalayer(); the method with that signature is `project`, so the only implemented risk-area algorithm never completes. * B280 — prepareSlurmLSMExecution passes desc= where the parameter is template_desc, after already writing scripts and stations.parquet. * B212-B226 — of 13 methods tested across experiment/analysis.py and experiment/presentation.py, exactly one runs, and only for one device layout. Correctness defects worth separate attention: * B257 — kb = 1.83E-23 in the Petroff deposition model; Boltzmann's constant is 1.380649e-23 (digits transposed), so Brownian diffusivity is 32.6% too high wherever that model runs. * B261 — addDocumentFromJSON silently drops any document carrying _id, because mongoengine's from_json defaults to created=False. Since exportProject emits _id, re-importing an exported project imports nothing and raises nothing. This closes a question the suite had left open for many batches, and the stale note blaming mongomock in test_datalayer_project_export.py is corrected. * B272 — _getSimulationsList cross-joins descriptions against parameters instead of pairing them, stapling one run's id to another's params, and propagates into three more methods. * B255/B271 — double unit conversion of particle diameter, and a dosage conversion running opposite to its own docstring. * B309 — presentation/basicplots.py runs seaborn.set() at *import* time, rebinding matplotlib's single-letter colour shorthands process-wide, so c="r" elsewhere in hera silently resolves to a muted red depending on import order. Verified experimentally. B243 is filed as an open disagreement rather than a verdict: the new tests argue the fully-rough Cplus branch is wrong (as written, roughening a wall increases momentum), while a pre-existing test records that plateau as intended. Both pass; a human should adjudicate.
…B310-B332) Six new test files, 573 new passing tests (5027 -> 5600) and 25 new strict-xfail bug pins. No production code touched. This batch was coverage-driven by design: paraview/PyFoam/vtkmodules are stubbed or absent, so much of it asserts argument forwarding, branch selection and what the code builds itself rather than physics. Every bug below is proved by reading source, never from stub behaviour. Highlights: * B332 — abstractLagrangianSolver does `from hera.utils.unitHandler import *`, which re-exports unum's unit table whose minute entry is literally named `min`. Module-level `min` is therefore a Unum, not the builtin, and is not callable (verified). _buildTimeMapping's two uses are exactly the code honouring an explicit originalFlow timeStep, so only the first/last-timestep defaults are usable. * B330 — four call sites use toolkit.getCaseListDocumentFromDB, a name defined nowhere in hera (those four are its only occurrences). The real name is getWorkflowListDocumentFromDB, so createDispersionCaseDirectory can never run. * B320 — VTKPipeline.clearCache deletes from the Simulations collection, while the cache it targets is written by addCacheDocument and read by getCacheDocuments, i.e. the Cache collection. It clears nothing. * B319 — _filterCachedTimesteps rebinds one local inside its per-filter loop, so a filter with a warm cache empties the timestep list for every other filter. * B314/B315 — TilesToolkit sizes its mosaic as lr-ul instead of lr-ul+1, so the eastern/southern-most tile row is never fetched; a box inside a single tile yields a silent 0x0 image with no request at all. * B313 — _parsePointSet squeezes the point array and keeps coordinates only when ndim==2, so a single-point probe (ProbeLocation) silently loses its coordinates. Recovered after the previous process died mid-batch: three agents were lost. Their landed work was salvaged, two invented bug numbers (B301, B302) that collided with existing findings were remapped, and test_gis_landcover_last.py was deleted rather than repaired — it asserted on source line numbers, duplicated the B242 note and the existing sandgrain test, and targeted a single function.
This was referenced Sep 20, 2026
Fix: Five plotting defects that drew the wrong thing or nothing (B143, B144, B224, B225, B322)
#1090
Open
Open
Open
Open
Open
Open
Open
lior-antonov
added this pull request to stack #1117
September 23, 2026 10:19
ilayfalach
pushed a commit
that referenced
this pull request
Oct 14, 2026
Batches 11-33 never picked up #1067's review fix, so in CI the unit layer fell back to the placeholder hermes.workflow / argos classes and ~390 tests failed with 'object.__init__() takes exactly one argument' -- on PR #1085 as well as here. That also skipped the integration job (needs: unit), which is the one that runs against MongoDB 9.0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 15, 2026
This branch has not been deployed
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
1 participant
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.
Last in the stack: #1067 is 1-10, #1083 is 11-20, #1084 is 21-30, this is 31-33.
Base is
tests/batch30-coverage60, so this diff is the final three batches alone. Merge #1084 first.These take unit coverage from about 60% to about 90% and surface B310-B332.
With all four merged the unit layer is ~190 test files and 5,589 hermetic tests that run in about a minute with no MongoDB, no S3 and no network, and the coverage floor in
coverage_floor.txtis enforced in CI.Production code is touched only where the suite could not otherwise run. Every defect is pinned by an
xfail(strict=True)test rather than fixed here.🤖 Generated with Claude Code