ci(python-tests): run the whole Python suite on Linux and macOS (#169) - #175
Merged
Conversation
Before this, GitHub CI ran 533 of the suite's ~3490 Python tests on any non-Windows host, and every one of them under BNGSIM_CODEGEN_JIT=mir on a -DBNGSIM_ENABLE_MIR=ON build. The count exercised in the DEFAULT configuration on Linux or macOS was zero. The gap was structural: every job that runs pytest names a hand-maintained file list, so a new test file defaults to running nowhere, and the assumed backstop was the local pre-push hook — one platform, and `--no-verify` removes it. python-tests.yml therefore has no paths filter, no file list, and no -D overrides: it runs `python/tests` as a directory on ubuntu-latest and macos-14 against the pyproject defaults (KLU on, NFsim on, RuleMonkey on, MIR off). It is the pre-push hook's own pytest call over a uv.lock-provisioned env, so green here means what a clean `git push` means locally. False-green guards, mirroring native-tests.yml: a floor on the passed count (a module that stops importing shrinks the denominator without failing) and BNGSIM_SKIP_AUDIT=strict (a skip whose reason nobody declared fails the run). HAS_KLU is asserted after the build for the same reason. Side effect: this is the only job that exercises BNGSIM_KLU_AUTOBUILD (GH #209). Every wheel leg resolves a prebuilt SuiteSparse via SUITESPARSE_ROOT and every other job sets ENABLE_KLU=OFF, so the from-source fallback had no CI anywhere. Also corrects three places that documented the old assumption as fact: native-tests.yml's header, conftest's skip-audit rationale, and SUPPORT_MATRIX.
…169) test_engine_choice_accessors.py skips with "LAPACK-dense not built in this configuration"; test_lapack_dense_solver.py skips with "build links no BLAS dense backend" for the same reason. Only the first was in _DECLARED_SKIPS, so the second reads as an undeclared skip wherever find_package(LAPACK) comes up empty. On macOS it never does (Accelerate), which is why nobody saw it — and until #169 no CI job ran the full suite anywhere else.
…ds a BLAS First run of the new job died in ~12s on ubuntu-latest: BNGSIM_KLU_AUTOBUILD started building the pinned SuiteSparse subset, and SuiteSparse_config's own CMakeLists calls find_package(BLAS), which the runner image cannot satisfy. So REQUIRE_KLU=ON turned into a hard configure failure. The Linux leg now installs libsuitesparse-dev, the same route cibuildwheel's Linux leg takes (dnf install suitesparse-devel), so it matches the shipped wheel. macOS deliberately still installs nothing: Accelerate supplies the BLAS, the autobuild completes, and that leg stays the only CI coverage the autobuild path has anywhere. Also stops the artifact upload reporting a second, misleading failure when the build fails before pytest ever writes a log.
The first whole-suite Linux run turned up 4 failures that pass on macOS, in two unrelated subsystems. Both are pre-existing on main and both are exactly the class #169 said no CI job could see. Neither is caused by this PR. 1. test_jacobian_discontinuous_fallback.py (3 tests). The GH #176 retry machinery is not what breaks: the analytical attempt dies at the t~25 crossing the file's header documents, the warning fires, FD engages — and FD then dies at t~34.6, a SECOND crossing the header does not mention. So the header's premise, that FD straddles the step and integrates the model cleanly, holds under Accelerate and not under Linux's reference LAPACK. The four steady-state tests on the same fixture pass, because #127's march never reaches t~34.6; that contrast is left visible by marking per-test. 2. test_steady_state_codegen.py::test_degenerate_steady_state_is_flagged. Its docstring already said the finite numbers survive "only because the pivots stay nonzero" — it just did not know that was platform-decided. Under reference LAPACK they do not: rcond is 0.00e+00, the solve returns NaN, and the code takes the refusal branch its own sibling test exists to assert. Quarantined with xfail(sys.platform.startswith("linux"), strict=True, raises=SimulationError), so they retire themselves the moment either is fixed. Quarantined at all so the new gate lands green: native-tests.yml's header explains why a permanently-red suite is worse than a smaller green one. Also sets the false-green floors from the measurements. COLLECTED (passed + skipped + xfailed + xpassed) is 3490 on ubuntu-latest, macos-14 and a local macOS box alike — every skip in this suite is function- or class-level, so an absent corpus moves tests between columns without changing the total. That is the structural floor. PASSED differs by leg (3415 vs 3411), so it gets a looser floor and a different job: catching a whole group converting to skips.
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
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.
Closes #169.
Takes option (1) from the issue: a
python-testsjob on an OS matrix runningthe whole suite. Option (3) is discussed below — it turns out to be either
vacuous or wrong once (1) exists, so what it was reaching for is implemented
differently.
What lands
.github/workflows/python-tests.yml— the entirepython/testssuite onubuntu-latestandmacos-14, in the default build configuration.Three properties are deliberate, and each closes a specific half of the gap:
paths:filter. A gate that fires selectively reintroduces the per-fileopt-in through the trigger instead of through the run list.
lint.ymlalreadytakes this position for the same reason.
python/testsas a directory, so a file addedtomorrow runs here with no workflow edit — the property
native-tests.ymlgetsfrom driving
ctestrather than one named target.-Doverrides. Every other workflow disables something (KLU off inmir/windows-tail/windows-nfsim/native-tests, MIR on inmir.yml),which is exactly how the shipped configuration ended up untested. Here the
build takes the pyproject defaults: KLU on (
REQUIRE_KLU=ON), NFsim on,RuleMonkey on, MIR off.
Provisioning is
uv sync --extra devagainstuv.lock, and the pytest call isthe pre-push hook's own. So the job is the hook, on two platforms the developer
does not have — which is the honest fix for "a local hook was treated as the
cross-platform gate".
--extra devrather than an enumerated extras list:devis what a developer'suv syncinstalls, so CI and the hook see the same environment, and an extraadded to
devlater reaches CI without a workflow edit. It is what un-skips theantimony / roadrunner / xarray / h5py / pandas / jax / vivarium guarded tests.
What it found on its first run
Four tests fail on Linux that pass on macOS, in two unrelated subsystems.
Both are pre-existing on
main, neither is caused by this PR, and both areexactly the class #169 said nothing in CI could see. Reported as
#176 and quarantined here so the gate lands green —
native-tests.yml'sheader explains why a permanently-red suite is worse than a smaller green one.
GH #176's finite-difference retry does not rescue its own fixture. The retry
machinery is not what breaks — the traceback shows it working:
The analytical attempt dies at the t≈25 crossing the file's header documents, FD
engages, and FD then dies at t≈34.6 — a second crossing the header does not
mention. Its stated premise ("the finite-difference Jacobian straddles the step
… so it integrates the model cleanly") holds under Accelerate and not under
Linux's reference LAPACK. The four steady-state tests on the same fixture pass,
because #127's march never reaches t≈34.6; the markers are per-test so that
contrast stays visible.
nested_derived_rate_const.netis exactly singular under reference LAPACK.test_degenerate_steady_state_is_flaggedasserts the ill-conditioned warningbranch; on Linux
rcondis0.00e+00, the solve returns non-finite values, andthe code takes the refusal branch that its own sibling test exists to assert.
The docstring already said the finite numbers survive "only because the pivots
stay nonzero" — it just did not know that was platform-decided.
Both use
xfail(sys.platform.startswith("linux"), strict=True, raises=SimulationError), so they retire themselves: the day either is fixed, thetest xpasses and the run goes red until the marker is deleted.
False-green guards
A directory-scoped pytest cannot drift out of sync with a file list, but the
suite can still shrink silently — a module that stops importing skips at
collection, a rename orphans a file, a bad conftest deselects. None of those is
a failure; the run stays green with a smaller denominator. That is the #28/#36
shape
native-tests.ymlguards with its own floors, so this job gets two, withtwo different jobs to do:
Structural, and nearly environment-independent: 3490 on
ubuntu-latest, onmacos-14, and on a local macOS box alike. Every skip in this suite isfunction- or class-level, so an absent corpus moves tests between columns
without changing the total. A drop here means tests stopped being collected.
macOS vs 3411 on Linux), so it sits looser. It catches what the collected floor
cannot: a whole group converting to skips for a declared reason — an extra
silently failing to install and taking the 13 roadrunner tests with it.
BNGSIM_SKIP_AUDIT=strict. Catches tests that turn into skips for a reasonnobody signed off on at all.
Plus
HAS_KLUasserted after the build — the same false-green guardmir.ymlputs on
HAS_MIR. A build that silently lost KLU would skip the sparse-solvertests and still report green, which would defeat the point of running the
default configuration in the first place.
Free coverage this picks up — and what it immediately found
The macOS leg is the only place anywhere that exercises
BNGSIM_KLU_AUTOBUILD(GH #209). Every wheel leg resolves a prebuilt SuiteSparsethrough
SUITESPARSE_ROOT, and every other job setsENABLE_KLU=OFF— so thefrom-source KLU subset that an sdist install on a bare box falls back to had no
CI anywhere.
Turning it on found the thing straight away: the autobuild cannot complete on a
bare
ubuntu-latest. SuiteSparse's ownSuiteSparse_config/CMakeLists.txtcalls
find_package(BLAS), the runner image ships none, and withREQUIRE_KLU=ON(the pyproject default) that is a hard configure failure ~12 sin — before KLU is reached at all:
So GH #209's self-sufficiency claim holds on macOS (Accelerate supplies the BLAS)
but not on a bare Linux host. The Linux leg therefore installs
libsuitesparse-dev— the same system-package route cibuildwheel's Linux legalready takes (
dnf install suitesparse-devel), so it matches the shipped wheeleither way. macOS still installs nothing, so the autobuild keeps its one leg of
coverage. That gap is worth its own issue and is not fixed here.
A defect the job found before it ran
_DECLARED_SKIPSdeclared"LAPACK-dense not built", whichtest_engine_choice_accessors.pyemits.test_lapack_dense_solver.pyskips forthe same build-variant condition but phrases it
"build links no BLAS dense backend (Accelerate / LAPACK)"— undeclared, so it would failstrictanywherefind_package(LAPACK)comes up empty. That is invisible on macOS (Accelerate isalways found, so neither test skips) and nothing ran the full suite anywhere
else. Declared in the second commit; the audit table still reports the skip and
its count, so the Linux leg's log says whether it actually fires.
On option (3), the drift check
The issue proposed it as worth adding regardless. It isn't, in this form:
python/tests/test_*.pyappears in at least one workflowlist" — it becomes vacuous: this job satisfies it for every file without
naming any.
paths:filter matches is inthat workflow's run list" — would be wrong.
mir.yml's filter istest_codegen*.py, which matches 25 files; it runs 18. The 7 it skips(
cache_key,chunking,compile_reaping,gen_scaling,jacobian_sparse,sens_obs_func,shard_compile) are all about the cc backend — caching a.so, shardedcc -c+ link, process-group reaping of a compiler, aKLU-gated sparse Jacobian. Under
BNGSIM_CODEGEN_JIT=mirthere is no.soatall. That list is curated for a real reason, and a check demanding those files
join it would be pressure in the wrong direction.
What (3) was reaching for is the backstop, and that is what the count floor plus
strict audit provide — without asserting that two hand-maintained lists agree.
Also corrected
Three places documented the old assumption as fact:
native-tests.yml's header ("The pre-push hook covers python/tests"),python/tests/conftest.py's skip-audit rationale ("Nothing in CI runs the fullPython suite"),
SUPPORT_MATRIX.md's validation section, which listed the five workflows andomitted any Python-suite gate because there wasn't one.
Not in scope, named so they are not mistaken for oversights
mir.ymlalready runs amacos-15-intelleg, and amacOS-x86_64-specific Python-level regression is the least likely of the
three. One line in the matrix.
requires-pythonis>=3.10and every job runs 3.12, sonothing tests the floor at runtime (ruff's
target-version=py310catchessyntax, not stdlib API). One more matrix entry.
$BNGPATH. Unset, so the 14 BNG2.pl round-trip tests intest_sbml_to_bngl.pyskip. Closing that needs the perl toolchain plus theparitydependency group (agit+httpsreference to PyBioNetGen) — heavierprovisioning than this gate warrants. The skip is declared and appears in the
audit table.
Verification and cost
Local baseline, macOS arm64, default build,
--extra dev,$BNGPATHunset:3462 passed / 28 skipped in 5m08s, every skip reason declared.
CI, run 31022533801,
default build confirmed on both legs (
HAS_KLU True,HAS_NFSIM True,HAS_MIR False):macos-14ubuntu-latestFor scale, the workflows already in the repo:
mir-jit13m05s,windows-tail12m30s,
wheels51m24s. So this sits inside the existing envelope and needs nobuild caching — the build is 3–4 min, not the 15–20 I had budgeted for.
The 75 skips are all declared (
strictpassed): 14 BNG2.pl, ~40 rr_parity corpusmodels, 9 MIR-off, 3 RuleMonkey-compiled-in, 2 requires-a-KLU-off-build, and a
handful of absent fixtures. The RuleMonkey and KLU groups are inverse-condition
tests that skip because the build is complete, which reads backwards at a
glance.