Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .github/workflows/native-tests.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,8 +5,15 @@ name: native-tests
# Until #38 NOTHING in CI built these tests. Every workflow that compiles the
# project passes BNGSIM_BUILD_TESTS=OFF (mir.yml, windows-nfsim.yml,
# windows-tail.yml), and no job ever invoked the binaries — so the suite's only
# gate was a developer remembering to build it by hand. The pre-push hook
# covers python/tests; the C++ side had no equivalent at any stage.
# gate was a developer remembering to build it by hand. The C++ side had no
# equivalent at any stage of CI.
#
# The original wording here was "the pre-push hook covers python/tests", offered
# as the contrast that made the C++ gap worse. Issue #169 showed that assumption
# was itself the other half of the problem: a local hook covers one platform and
# `--no-verify` removes it, so python/tests had no cross-platform CI gate either.
# python-tests.yml (ubuntu + macos-14, whole suite, default build) is that gate
# now; this job remains the C++ half.
#
# That is not hypothetical. #28 flipped SteadyStateOptions::method from "newton"
# to "integration" and updated the Python side, leaving the C++ assertion in
Expand Down
227 changes: 227 additions & 0 deletions .github/workflows/python-tests.yml
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
name: python-tests

# The cross-platform Python gate (issue #169).
#
# Before this job, GitHub CI ran 533 of the suite's ~3490 Python tests on Linux
# or macOS — and every one of them under BNGSIM_CODEGEN_JIT=mir on a
# -DBNGSIM_ENABLE_MIR=ON build. The number exercised in the DEFAULT configuration
# on any non-Windows host was zero. lint.yml runs no pytest by design;
# native-tests.yml is the C++ suite; wheels.yml runs only the cibuildwheel import
# smoke test; windows-tail.yml and windows-nfsim.yml are Windows-only. So a
# regression in SBML loading, .net parsing, events, steady state, SSA, conversion
# or coupling could only be caught on Windows, and only if the file happened to
# be named in one of two hand-maintained lists.
#
# The gap was structural, not a bug. Every job that runs pytest names a curated
# file list, so a NEW test file defaults to running nowhere, and the backstop was
# assumed to be the local pre-push hook — which covers one platform (whichever
# the developer is on, here macOS arm64) and which `git push --no-verify`
# removes. native-tests.yml stated that assumption outright.
#
# Three properties of this job are therefore deliberate, and each one is the
# thing that closes a specific half of the gap:
#
# * NO paths filter. A gate that fires selectively reintroduces the per-file
# opt-in through the trigger instead of through the run list. lint.yml takes
# the same position for the same reason.
# * NO file list. It runs `python/tests` as a directory, so a file added
# tomorrow runs here with no workflow edit — the property native-tests.yml
# gets from driving ctest rather than one named target.
# * NO -D overrides. Every other workflow disables something (KLU off in
# mir/windows-tail/windows-nfsim/native-tests, MIR on in mir.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.
#
# It is the pre-push hook, on two more platforms. `uv sync --extra dev` provisions
# from uv.lock and the pytest call is the hook's own (.pre-commit-config.yaml),
# so green here means what a clean `git push` means locally — on a host the
# developer does not have.
#
# SuiteSparse/KLU is provisioned differently per leg, and the asymmetry is
# deliberate rather than an inconsistency:
#
# * macOS installs nothing. Accelerate supplies the BLAS, so
# BNGSIM_KLU_AUTOBUILD (GH #209) builds the pinned KLU subset from source —
# which makes this the ONLY job anywhere that exercises that path. Every
# wheel leg resolves a prebuilt SuiteSparse through SUITESPARSE_ROOT and
# every other job sets ENABLE_KLU=OFF, so the fallback an sdist install on a
# bare box depends on had no CI at all.
# * Linux installs libsuitesparse-dev, because on a bare ubuntu-latest the
# autobuild does NOT work: SuiteSparse's own CMake calls find_package(BLAS)
# and the runner image ships no BLAS, so the build dies in SuiteSparse_config
# before KLU is reached. Measured here, not assumed — the first run of this
# job failed exactly that way, which means GH #209's self-sufficiency claim
# holds on macOS but not on a bare Linux host. Installing the system package
# is what cibuildwheel's Linux leg already does (`dnf install
# suitesparse-devel`), so this leg matches the shipped wheel either way.
#
# The HAS_KLU assertion below is what keeps both honest — without it a build that
# quietly lost KLU would skip the sparse tests and still report green.
#
# Two coverage axes are deliberately NOT taken here, so they are not mistaken for
# oversights:
#
# * macOS x86_64. mir.yml already runs a macos-15-intel leg, and a
# macOS-x86_64-specific *Python-level* regression is the least likely of the
# three. Adding it is one line in the matrix.
# * The Python floor. requires-python is >=3.10 and every job here runs 3.12,
# so nothing tests the floor at runtime (ruff's target-version=py310 catches
# syntax, not stdlib API). Adding it is one more matrix entry.
#
# One known skip is environmental rather than structural: $BNGPATH is unset, so
# the 14 BNG2.pl round-trip tests in test_sbml_to_bngl.py skip. Closing that needs
# the perl toolchain plus the `parity` dependency group (a git+https reference to
# PyBioNetGen), which is a heavier provisioning step than this gate warrants; the
# skip is declared in conftest's _DECLARED_SKIPS and shows up in the audit table.

on:
push:
branches: [main]
pull_request:
workflow_dispatch:
inputs:
pytest_args:
description: "Extra pytest args (e.g. -x, -k some_pattern, or a single file path)"
required: false
default: ""

concurrency:
group: python-tests-${{ github.ref }}
cancel-in-progress: true

jobs:
python-tests:
name: Python suite · ${{ matrix.os }}
runs-on: ${{ matrix.os }}
timeout-minutes: 90
strategy:
fail-fast: false
matrix:
include:
- os: ubuntu-latest # x86_64 Linux — no Python coverage at all before #169
- os: macos-14 # macOS arm64 — covered only by one developer's pre-push hook
steps:
- uses: actions/checkout@v4

- name: Install uv
uses: astral-sh/setup-uv@v5
with:
enable-cache: true
cache-dependency-glob: "uv.lock"

# See the SuiteSparse note in the header: Linux only, because the runner
# image has no BLAS for the from-source autobuild to link against. macOS
# deliberately gets nothing so its leg exercises BNGSIM_KLU_AUTOBUILD.
- name: Install SuiteSparse (Linux)
if: runner.os == 'Linux'
run: |
set -euxo pipefail
sudo apt-get update
sudo apt-get install -y libsuitesparse-dev

# --extra dev rather than an enumerated extras list: `dev` is the set a
# developer's `uv sync` installs, so this job and the pre-push hook see the
# same environment, and an extra added to `dev` later reaches CI without a
# workflow edit. It is what un-skips the antimony / roadrunner / xarray /
# h5py / pandas / jax / vivarium guarded tests — ~50 items that skip on a
# bare install. Building bngsim itself is the slow part (SUNDIALS is
# FetchContent'd and the KLU subset is built from source).
- name: Provision from uv.lock (default build config)
run: uv sync --extra dev --python 3.12

- name: Show build / capability info
run: |
set -euxo pipefail
uv run --no-sync python -c "import platform, sys; print(platform.platform()); print(sys.version)"
uv run --no-sync python -c "import bngsim; print('bngsim', bngsim.__version__); print('HAS_KLU', bngsim.HAS_KLU); print('HAS_NFSIM', bngsim.HAS_NFSIM); print('HAS_MIR', bngsim.HAS_MIR)"
# The point of this job is the DEFAULT configuration, and KLU is the
# part of it nothing else in CI builds. A build that silently lost KLU
# would skip the sparse-solver tests and still be green, so fail here
# instead — the same false-green guard mir.yml puts on HAS_MIR.
uv run --no-sync python -c "import bngsim, sys; sys.exit(0 if bngsim.HAS_KLU else 'HAS_KLU is False — the default build did not link SuiteSparse/KLU')"
# The runtime codegen path shells out to this compiler at simulate time.
uv run --no-sync python -c "from bngsim import _codegen; print('C compiler:', _codegen._find_c_compiler())"

# BNGSIM_SKIP_AUDIT=strict turns any skip whose reason is not in conftest's
# _DECLARED_SKIPS into a failure. That is the second half of the false-green
# guard below: the count floor catches tests that vanish from collection,
# strict catches tests that quietly turn into skips for a reason nobody
# signed off on.
- name: Run the full Python suite
id: pytest
env:
BNGSIM_SKIP_AUDIT: strict
BNGSIM_TEST_DATA: ${{ github.workspace }}/tests/data
run: |
set -o pipefail
uv run --no-sync python -m pytest -p no:cacheprovider python/tests \
-q --tb=short --durations=15 ${{ github.event.inputs.pytest_args }} \
2>&1 | tee python-tests-${{ matrix.os }}.log

- name: Guard against a false green
if: ${{ !github.event.inputs.pytest_args }}
run: |
set -euo pipefail
# A directory-scoped pytest cannot drift out of sync with a file list,
# but it can still shrink: 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,
# which is the #28/#36 shape native-tests.yml guards with its own
# floors. Two denominators here, for the same reason it needs two:
#
# 1. COLLECTED (passed + skipped + xfailed + xpassed). This is the
# structural one, and it is nearly environment-INDEPENDENT: 3490 on
# ubuntu-latest, on macos-14, and on a developer's macOS box, all at
# the commit that added this job. Every skip in this suite is
# function- or class-level, so a corpus being absent moves tests
# between the columns without changing the total. A drop here means
# tests stopped being collected. Floor sits ~10 below.
# 2. PASSED. This one IS environment-sensitive (3415 on macOS, 3411 on
# Linux — the difference is lanl/bngsim#176's four xfails), so its
# floor is looser. It exists to catch the case COLLECTED cannot: a
# whole group converting to skips for a *declared* reason, e.g. an
# extra silently failing to install and taking the 13 roadrunner
# tests with it. Floor sits ~30 below, so it trips on a group and
# not on one test.
#
# Both floors only rise as tests are added; additions never break them,
# only silent removals do.
COLLECTED_FLOOR=3480
PASS_FLOOR=3380

LOG="python-tests-${{ matrix.os }}.log"
summary="$(grep -E '[0-9]+ (passed|failed).* in [0-9.]+s' "$LOG" | tail -1 || true)"
if [ -z "$summary" ]; then
echo "::error::No pytest summary line — the suite did not run to completion."
exit 1
fi
echo "summary: $summary"

# "3 xpassed" cannot match "[0-9]+ passed", so the categories stay disjoint.
count() { echo "$summary" | grep -oE "[0-9]+ $1" | grep -oE '^[0-9]+' || echo 0; }
passed="$(count passed)"
collected=$(( passed + $(count skipped) + $(count xfailed) + $(count xpassed) ))

echo "collected $collected (floor $COLLECTED_FLOOR); passed $passed (floor $PASS_FLOOR)."
if [ "$collected" -lt "$COLLECTED_FLOOR" ]; then
echo "::error::Only $collected tests collected, expected >= $COLLECTED_FLOOR — tests stopped being collected rather than failing."
exit 1
fi
if [ "$passed" -lt "$PASS_FLOOR" ]; then
echo "::error::Only $passed tests passed, expected >= $PASS_FLOOR — a group of tests turned into skips."
exit 1
fi

# Conditioned on the pytest step having been reached, not just on always():
# if the build fails there is no log to upload, and if-no-files-found=error
# would then report a second, misleading failure on top of the real one.
# Keeping `error` (rather than downgrading to `warn`) means a pytest step
# that ran but produced no log is still caught.
- name: Upload pytest log
if: always() && steps.pytest.conclusion != 'skipped'
uses: actions/upload-artifact@v4
with:
name: python-tests-log-${{ matrix.os }}
path: python-tests-${{ matrix.os }}.log
if-no-files-found: error
76 changes: 76 additions & 0 deletions CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,6 +15,20 @@ in `CMakeLists.txt`) is derived from it.
## [Unreleased]

### Fixed
- **A LAPACK-dense skip that no `_DECLARED_SKIPS` entry matched (found while
wiring issue #169).** Two files skip for the same build-variant condition and
phrase it differently: `test_engine_choice_accessors.py` says `"LAPACK-dense
not built in this configuration"`, `test_lapack_dense_solver.py` says `"build
links no BLAS dense backend (Accelerate / LAPACK)"`. Only the first was
declared, so the second read as an *undeclared* skip — the audit's signal for
"a test stopped running and nobody decided it should".

It could not be seen on macOS: `find_package` always resolves Accelerate there,
so neither test skips at all. It shows up only where CMake finds no BLAS, and
until #169 no CI job ran the full suite anywhere but a developer's macOS box.
The second phrasing is declared now, so `BNGSIM_SKIP_AUDIT=strict` does not
fail a Linux leg for a legitimate build-variant skip.

- **The #161 analytic sensitivity RHS was a net regression on the model it
targeted, and the cost was in the build, not in the emitted code (issue
#165).** `Smith_BMCSystBiol2013` (133 species, 16 sensitivity columns,
Expand DownExpand Up@@ -153,6 +167,68 @@ in `CMakeLists.txt`) is derived from it.
refusal and turns the sensitivity column into a real one.

### Added
- **A cross-platform Python test gate (issue #169).**
`.github/workflows/python-tests.yml` runs the whole of `python/tests` on
`ubuntu-latest` and `macos-14` in the **default** build configuration. Before
it, 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, so the count exercised in the configuration a
wheel actually ships was **zero** on Linux and macOS. A regression in SBML
loading, `.net` parsing, events, steady state, SSA, conversion or coupling
could be caught only on Windows, and only if the file happened to be named in
one of two hand-maintained lists.

The gap was structural, not a bug. Every job that runs pytest names a curated
file list, so a *new* test file defaults to running nowhere, and the assumed
backstop was the local pre-push hook — which covers whichever platform the
developer happens to be on (here macOS arm64) and which `git push --no-verify`
removes. `native-tests.yml` stated that assumption in its header as fact.

The new job therefore carries no `paths:` filter (a selectively-firing gate
reintroduces the per-file opt-in through the trigger instead of the run list),
no file list (it runs the directory, the way `native-tests.yml` drives `ctest`
rather than one named target), and no `-D` overrides (every other workflow
disables something — KLU off in four of them, MIR *on* in `mir.yml` — which is
how the shipped configuration ended up untested). Provisioning is `uv sync
--extra dev` off `uv.lock` and the pytest call is the pre-push hook's own, so a
green run means what a clean `git push` means locally, on a host the developer
does not have.

Two false-green guards, mirroring the ones `native-tests.yml` added for the C++
suite: a floor on the passed count, because a module that stops importing skips
at collection and shrinks the denominator without failing anything; and
`BNGSIM_SKIP_AUDIT=strict`, because a test that quietly turns into a skip for
an undeclared reason is the same invisibility in a different form. `HAS_KLU` is
asserted after the build for the reason `mir.yml` asserts `HAS_MIR` — a build
that silently lost KLU would skip the sparse-solver tests and still be green.

Side effect worth naming: the macOS leg is the only place anywhere that
exercises `BNGSIM_KLU_AUTOBUILD` (GH #209). The wheel legs all resolve a
prebuilt SuiteSparse through `SUITESPARSE_ROOT` and every other job sets
`ENABLE_KLU=OFF`, so the from-source KLU subset that an sdist install on a bare
box falls back to had no CI at all. Wiring it up immediately showed why that
matters: the same autobuild **cannot** complete on a bare `ubuntu-latest`,
because SuiteSparse's own CMake calls `find_package(BLAS)` and the runner image
ships none, so the configure dies in `SuiteSparse_config` before KLU is
reached. So GH #209's self-sufficiency claim holds on macOS but not on a bare
Linux host. The Linux leg therefore installs `libsuitesparse-dev`, the same
system-package route cibuildwheel's Linux leg takes.

And the gate earned itself on its first real run: **four tests fail on Linux
that pass on macOS**, in two unrelated subsystems, both pre-existing on `main`
and both exactly the class #169 said nothing could see. GH #176's
finite-difference retry fires correctly on
`ltype_calcium_discontinuous_jacobian.net` and then dies at a *second*
threshold crossing (t≈34.6) the fixture's own header does not mention; and
`nested_derived_rate_const.net`'s reduced Jacobian is exactly singular under
Linux's reference LAPACK where Accelerate leaves it merely ill-conditioned, so
it takes the refusal branch its sibling test exists to assert rather than the
warning branch its own test asserts. Both are quarantined under
`xfail(sys.platform.startswith("linux"), strict=True, raises=SimulationError)`
and reported in lanl/bngsim#176 — `strict` so they retire themselves, and
quarantined at all so the new gate lands green rather than permanently red,
which is the distinction `native-tests.yml`'s header spells out.

- **`compartment_sizes=` at load, the supported way to change a volume (issue
#164).** `Model.from_sbml`, `from_sbml_string`, `from_antimony`,
`from_antimony_string`, and `Model.load` take
Expand Down
8 changes: 8 additions & 0 deletions SUPPORT_MATRIX.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -77,6 +77,14 @@ environment. `lint.yml`, `native-tests.yml`, `mir.yml`, `windows-nfsim.yml` and
`windows-tail.yml` cover the pre-commit hooks, the C++ unit suite, the MIR JIT
backend, and the Windows NFsim/RuleMonkey paths respectively.

`python-tests.yml` is the Python suite's own gate: the whole of `python/tests`,
on `ubuntu-latest` and `macos-14`, in the **default** build configuration — no
`-D` overrides, so KLU, NFsim and RuleMonkey are all on and the MIR backend is
off, which is what a wheel ships. It has no `paths:` filter and names no test
files, so it fires on every change and picks up new tests without a workflow
edit. Every other job above disables something or runs a curated list, which is
how the shipped configuration went untested on Linux and macOS until issue #169.

Check the latest results before trusting this table:

```bash
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
ci(python-tests): run the whole Python suite on Linux and macOS (#169) by wshlavacek · Pull Request #175 · lanl/bngsim · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .github/workflows/native-tests.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,8 +5,15 @@ name: native-tests
# Until #38 NOTHING in CI built these tests. Every workflow that compiles the
# project passes BNGSIM_BUILD_TESTS=OFF (mir.yml, windows-nfsim.yml,
# windows-tail.yml), and no job ever invoked the binaries — so the suite's only
# gate was a developer remembering to build it by hand. The pre-push hook
# covers python/tests; the C++ side had no equivalent at any stage.
# gate was a developer remembering to build it by hand. The C++ side had no
# equivalent at any stage of CI.
#
# The original wording here was "the pre-push hook covers python/tests", offered
# as the contrast that made the C++ gap worse. Issue #169 showed that assumption
# was itself the other half of the problem: a local hook covers one platform and
# `--no-verify` removes it, so python/tests had no cross-platform CI gate either.
# python-tests.yml (ubuntu + macos-14, whole suite, default build) is that gate
# now; this job remains the C++ half.
#
# That is not hypothetical. #28 flipped SteadyStateOptions::method from "newton"
# to "integration" and updated the Python side, leaving the C++ assertion in
Expand Down
227 changes: 227 additions & 0 deletions .github/workflows/python-tests.yml
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
name: python-tests

# The cross-platform Python gate (issue #169).
#
# Before this job, GitHub CI ran 533 of the suite's ~3490 Python tests on Linux
# or macOS — and every one of them under BNGSIM_CODEGEN_JIT=mir on a
# -DBNGSIM_ENABLE_MIR=ON build. The number exercised in the DEFAULT configuration
# on any non-Windows host was zero. lint.yml runs no pytest by design;
# native-tests.yml is the C++ suite; wheels.yml runs only the cibuildwheel import
# smoke test; windows-tail.yml and windows-nfsim.yml are Windows-only. So a
# regression in SBML loading, .net parsing, events, steady state, SSA, conversion
# or coupling could only be caught on Windows, and only if the file happened to
# be named in one of two hand-maintained lists.
#
# The gap was structural, not a bug. Every job that runs pytest names a curated
# file list, so a NEW test file defaults to running nowhere, and the backstop was
# assumed to be the local pre-push hook — which covers one platform (whichever
# the developer is on, here macOS arm64) and which `git push --no-verify`
# removes. native-tests.yml stated that assumption outright.
#
# Three properties of this job are therefore deliberate, and each one is the
# thing that closes a specific half of the gap:
#
# * NO paths filter. A gate that fires selectively reintroduces the per-file
# opt-in through the trigger instead of through the run list. lint.yml takes
# the same position for the same reason.
# * NO file list. It runs `python/tests` as a directory, so a file added
# tomorrow runs here with no workflow edit — the property native-tests.yml
# gets from driving ctest rather than one named target.
# * NO -D overrides. Every other workflow disables something (KLU off in
# mir/windows-tail/windows-nfsim/native-tests, MIR on in mir.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.
#
# It is the pre-push hook, on two more platforms. `uv sync --extra dev` provisions
# from uv.lock and the pytest call is the hook's own (.pre-commit-config.yaml),
# so green here means what a clean `git push` means locally — on a host the
# developer does not have.
#
# SuiteSparse/KLU is provisioned differently per leg, and the asymmetry is
# deliberate rather than an inconsistency:
#
# * macOS installs nothing. Accelerate supplies the BLAS, so
# BNGSIM_KLU_AUTOBUILD (GH #209) builds the pinned KLU subset from source —
# which makes this the ONLY job anywhere that exercises that path. Every
# wheel leg resolves a prebuilt SuiteSparse through SUITESPARSE_ROOT and
# every other job sets ENABLE_KLU=OFF, so the fallback an sdist install on a
# bare box depends on had no CI at all.
# * Linux installs libsuitesparse-dev, because on a bare ubuntu-latest the
# autobuild does NOT work: SuiteSparse's own CMake calls find_package(BLAS)
# and the runner image ships no BLAS, so the build dies in SuiteSparse_config
# before KLU is reached. Measured here, not assumed — the first run of this
# job failed exactly that way, which means GH #209's self-sufficiency claim
# holds on macOS but not on a bare Linux host. Installing the system package
# is what cibuildwheel's Linux leg already does (`dnf install
# suitesparse-devel`), so this leg matches the shipped wheel either way.
#
# The HAS_KLU assertion below is what keeps both honest — without it a build that
# quietly lost KLU would skip the sparse tests and still report green.
#
# Two coverage axes are deliberately NOT taken here, so they are not mistaken for
# oversights:
#
# * macOS x86_64. mir.yml already runs a macos-15-intel leg, and a
# macOS-x86_64-specific *Python-level* regression is the least likely of the
# three. Adding it is one line in the matrix.
# * The Python floor. requires-python is >=3.10 and every job here runs 3.12,
# so nothing tests the floor at runtime (ruff's target-version=py310 catches
# syntax, not stdlib API). Adding it is one more matrix entry.
#
# One known skip is environmental rather than structural: $BNGPATH is unset, so
# the 14 BNG2.pl round-trip tests in test_sbml_to_bngl.py skip. Closing that needs
# the perl toolchain plus the `parity` dependency group (a git+https reference to
# PyBioNetGen), which is a heavier provisioning step than this gate warrants; the
# skip is declared in conftest's _DECLARED_SKIPS and shows up in the audit table.

on:
push:
branches: [main]
pull_request:
workflow_dispatch:
inputs:
pytest_args:
description: "Extra pytest args (e.g. -x, -k some_pattern, or a single file path)"
required: false
default: ""

concurrency:
group: python-tests-${{ github.ref }}
cancel-in-progress: true

jobs:
python-tests:
name: Python suite · ${{ matrix.os }}
runs-on: ${{ matrix.os }}
timeout-minutes: 90
strategy:
fail-fast: false
matrix:
include:
- os: ubuntu-latest # x86_64 Linux — no Python coverage at all before #169
- os: macos-14 # macOS arm64 — covered only by one developer's pre-push hook
steps:
- uses: actions/checkout@v4

- name: Install uv
uses: astral-sh/setup-uv@v5
with:
enable-cache: true
cache-dependency-glob: "uv.lock"

# See the SuiteSparse note in the header: Linux only, because the runner
# image has no BLAS for the from-source autobuild to link against. macOS
# deliberately gets nothing so its leg exercises BNGSIM_KLU_AUTOBUILD.
- name: Install SuiteSparse (Linux)
if: runner.os == 'Linux'
run: |
set -euxo pipefail
sudo apt-get update
sudo apt-get install -y libsuitesparse-dev

# --extra dev rather than an enumerated extras list: `dev` is the set a
# developer's `uv sync` installs, so this job and the pre-push hook see the
# same environment, and an extra added to `dev` later reaches CI without a
# workflow edit. It is what un-skips the antimony / roadrunner / xarray /
# h5py / pandas / jax / vivarium guarded tests — ~50 items that skip on a
# bare install. Building bngsim itself is the slow part (SUNDIALS is
# FetchContent'd and the KLU subset is built from source).
- name: Provision from uv.lock (default build config)
run: uv sync --extra dev --python 3.12

- name: Show build / capability info
run: |
set -euxo pipefail
uv run --no-sync python -c "import platform, sys; print(platform.platform()); print(sys.version)"
uv run --no-sync python -c "import bngsim; print('bngsim', bngsim.__version__); print('HAS_KLU', bngsim.HAS_KLU); print('HAS_NFSIM', bngsim.HAS_NFSIM); print('HAS_MIR', bngsim.HAS_MIR)"
# The point of this job is the DEFAULT configuration, and KLU is the
# part of it nothing else in CI builds. A build that silently lost KLU
# would skip the sparse-solver tests and still be green, so fail here
# instead — the same false-green guard mir.yml puts on HAS_MIR.
uv run --no-sync python -c "import bngsim, sys; sys.exit(0 if bngsim.HAS_KLU else 'HAS_KLU is False — the default build did not link SuiteSparse/KLU')"
# The runtime codegen path shells out to this compiler at simulate time.
uv run --no-sync python -c "from bngsim import _codegen; print('C compiler:', _codegen._find_c_compiler())"

# BNGSIM_SKIP_AUDIT=strict turns any skip whose reason is not in conftest's
# _DECLARED_SKIPS into a failure. That is the second half of the false-green
# guard below: the count floor catches tests that vanish from collection,
# strict catches tests that quietly turn into skips for a reason nobody
# signed off on.
- name: Run the full Python suite
id: pytest
env:
BNGSIM_SKIP_AUDIT: strict
BNGSIM_TEST_DATA: ${{ github.workspace }}/tests/data
run: |
set -o pipefail
uv run --no-sync python -m pytest -p no:cacheprovider python/tests \
-q --tb=short --durations=15 ${{ github.event.inputs.pytest_args }} \
2>&1 | tee python-tests-${{ matrix.os }}.log

- name: Guard against a false green
if: ${{ !github.event.inputs.pytest_args }}
run: |
set -euo pipefail
# A directory-scoped pytest cannot drift out of sync with a file list,
# but it can still shrink: 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,
# which is the #28/#36 shape native-tests.yml guards with its own
# floors. Two denominators here, for the same reason it needs two:
#
# 1. COLLECTED (passed + skipped + xfailed + xpassed). This is the
# structural one, and it is nearly environment-INDEPENDENT: 3490 on
# ubuntu-latest, on macos-14, and on a developer's macOS box, all at
# the commit that added this job. Every skip in this suite is
# function- or class-level, so a corpus being absent moves tests
# between the columns without changing the total. A drop here means
# tests stopped being collected. Floor sits ~10 below.
# 2. PASSED. This one IS environment-sensitive (3415 on macOS, 3411 on
# Linux — the difference is lanl/bngsim#176's four xfails), so its
# floor is looser. It exists to catch the case COLLECTED cannot: a
# whole group converting to skips for a *declared* reason, e.g. an
# extra silently failing to install and taking the 13 roadrunner
# tests with it. Floor sits ~30 below, so it trips on a group and
# not on one test.
#
# Both floors only rise as tests are added; additions never break them,
# only silent removals do.
COLLECTED_FLOOR=3480
PASS_FLOOR=3380

LOG="python-tests-${{ matrix.os }}.log"
summary="$(grep -E '[0-9]+ (passed|failed).* in [0-9.]+s' "$LOG" | tail -1 || true)"
if [ -z "$summary" ]; then
echo "::error::No pytest summary line — the suite did not run to completion."
exit 1
fi
echo "summary: $summary"

# "3 xpassed" cannot match "[0-9]+ passed", so the categories stay disjoint.
count() { echo "$summary" | grep -oE "[0-9]+ $1" | grep -oE '^[0-9]+' || echo 0; }
passed="$(count passed)"
collected=$(( passed + $(count skipped) + $(count xfailed) + $(count xpassed) ))

echo "collected $collected (floor $COLLECTED_FLOOR); passed $passed (floor $PASS_FLOOR)."
if [ "$collected" -lt "$COLLECTED_FLOOR" ]; then
echo "::error::Only $collected tests collected, expected >= $COLLECTED_FLOOR — tests stopped being collected rather than failing."
exit 1
fi
if [ "$passed" -lt "$PASS_FLOOR" ]; then
echo "::error::Only $passed tests passed, expected >= $PASS_FLOOR — a group of tests turned into skips."
exit 1
fi

# Conditioned on the pytest step having been reached, not just on always():
# if the build fails there is no log to upload, and if-no-files-found=error
# would then report a second, misleading failure on top of the real one.
# Keeping `error` (rather than downgrading to `warn`) means a pytest step
# that ran but produced no log is still caught.
- name: Upload pytest log
if: always() && steps.pytest.conclusion != 'skipped'
uses: actions/upload-artifact@v4
with:
name: python-tests-log-${{ matrix.os }}
path: python-tests-${{ matrix.os }}.log
if-no-files-found: error
76 changes: 76 additions & 0 deletions CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,6 +15,20 @@ in `CMakeLists.txt`) is derived from it.
## [Unreleased]

### Fixed
- **A LAPACK-dense skip that no `_DECLARED_SKIPS` entry matched (found while
wiring issue #169).** Two files skip for the same build-variant condition and
phrase it differently: `test_engine_choice_accessors.py` says `"LAPACK-dense
not built in this configuration"`, `test_lapack_dense_solver.py` says `"build
links no BLAS dense backend (Accelerate / LAPACK)"`. Only the first was
declared, so the second read as an *undeclared* skip — the audit's signal for
"a test stopped running and nobody decided it should".

It could not be seen on macOS: `find_package` always resolves Accelerate there,
so neither test skips at all. It shows up only where CMake finds no BLAS, and
until #169 no CI job ran the full suite anywhere but a developer's macOS box.
The second phrasing is declared now, so `BNGSIM_SKIP_AUDIT=strict` does not
fail a Linux leg for a legitimate build-variant skip.

- **The #161 analytic sensitivity RHS was a net regression on the model it
targeted, and the cost was in the build, not in the emitted code (issue
#165).** `Smith_BMCSystBiol2013` (133 species, 16 sensitivity columns,
Expand DownExpand Up@@ -153,6 +167,68 @@ in `CMakeLists.txt`) is derived from it.
refusal and turns the sensitivity column into a real one.

### Added
- **A cross-platform Python test gate (issue #169).**
`.github/workflows/python-tests.yml` runs the whole of `python/tests` on
`ubuntu-latest` and `macos-14` in the **default** build configuration. Before
it, 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, so the count exercised in the configuration a
wheel actually ships was **zero** on Linux and macOS. A regression in SBML
loading, `.net` parsing, events, steady state, SSA, conversion or coupling
could be caught only on Windows, and only if the file happened to be named in
one of two hand-maintained lists.

The gap was structural, not a bug. Every job that runs pytest names a curated
file list, so a *new* test file defaults to running nowhere, and the assumed
backstop was the local pre-push hook — which covers whichever platform the
developer happens to be on (here macOS arm64) and which `git push --no-verify`
removes. `native-tests.yml` stated that assumption in its header as fact.

The new job therefore carries no `paths:` filter (a selectively-firing gate
reintroduces the per-file opt-in through the trigger instead of the run list),
no file list (it runs the directory, the way `native-tests.yml` drives `ctest`
rather than one named target), and no `-D` overrides (every other workflow
disables something — KLU off in four of them, MIR *on* in `mir.yml` — which is
how the shipped configuration ended up untested). Provisioning is `uv sync
--extra dev` off `uv.lock` and the pytest call is the pre-push hook's own, so a
green run means what a clean `git push` means locally, on a host the developer
does not have.

Two false-green guards, mirroring the ones `native-tests.yml` added for the C++
suite: a floor on the passed count, because a module that stops importing skips
at collection and shrinks the denominator without failing anything; and
`BNGSIM_SKIP_AUDIT=strict`, because a test that quietly turns into a skip for
an undeclared reason is the same invisibility in a different form. `HAS_KLU` is
asserted after the build for the reason `mir.yml` asserts `HAS_MIR` — a build
that silently lost KLU would skip the sparse-solver tests and still be green.

Side effect worth naming: the macOS leg is the only place anywhere that
exercises `BNGSIM_KLU_AUTOBUILD` (GH #209). The wheel legs all resolve a
prebuilt SuiteSparse through `SUITESPARSE_ROOT` and every other job sets
`ENABLE_KLU=OFF`, so the from-source KLU subset that an sdist install on a bare
box falls back to had no CI at all. Wiring it up immediately showed why that
matters: the same autobuild **cannot** complete on a bare `ubuntu-latest`,
because SuiteSparse's own CMake calls `find_package(BLAS)` and the runner image
ships none, so the configure dies in `SuiteSparse_config` before KLU is
reached. So GH #209's self-sufficiency claim holds on macOS but not on a bare
Linux host. The Linux leg therefore installs `libsuitesparse-dev`, the same
system-package route cibuildwheel's Linux leg takes.

And the gate earned itself on its first real run: **four tests fail on Linux
that pass on macOS**, in two unrelated subsystems, both pre-existing on `main`
and both exactly the class #169 said nothing could see. GH #176's
finite-difference retry fires correctly on
`ltype_calcium_discontinuous_jacobian.net` and then dies at a *second*
threshold crossing (t≈34.6) the fixture's own header does not mention; and
`nested_derived_rate_const.net`'s reduced Jacobian is exactly singular under
Linux's reference LAPACK where Accelerate leaves it merely ill-conditioned, so
it takes the refusal branch its sibling test exists to assert rather than the
warning branch its own test asserts. Both are quarantined under
`xfail(sys.platform.startswith("linux"), strict=True, raises=SimulationError)`
and reported in lanl/bngsim#176 — `strict` so they retire themselves, and
quarantined at all so the new gate lands green rather than permanently red,
which is the distinction `native-tests.yml`'s header spells out.

- **`compartment_sizes=` at load, the supported way to change a volume (issue
#164).** `Model.from_sbml`, `from_sbml_string`, `from_antimony`,
`from_antimony_string`, and `Model.load` take
Expand Down
8 changes: 8 additions & 0 deletions SUPPORT_MATRIX.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -77,6 +77,14 @@ environment. `lint.yml`, `native-tests.yml`, `mir.yml`, `windows-nfsim.yml` and
`windows-tail.yml` cover the pre-commit hooks, the C++ unit suite, the MIR JIT
backend, and the Windows NFsim/RuleMonkey paths respectively.

`python-tests.yml` is the Python suite's own gate: the whole of `python/tests`,
on `ubuntu-latest` and `macos-14`, in the **default** build configuration — no
`-D` overrides, so KLU, NFsim and RuleMonkey are all on and the MIR backend is
off, which is what a wheel ships. It has no `paths:` filter and names no test
files, so it fires on every change and picks up new tests without a workflow
edit. Every other job above disables something or runs a curated list, which is
how the shipped configuration went untested on Linux and macOS until issue #169.

Check the latest results before trusting this table:

```bash
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' ci(python-tests): run the whole Python suite on Linux and macOS (#169) by wshlavacek · Pull Request #175 · lanl/bngsim · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .github/workflows/native-tests.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,8 +5,15 @@ name: native-tests
# Until #38 NOTHING in CI built these tests. Every workflow that compiles the
# project passes BNGSIM_BUILD_TESTS=OFF (mir.yml, windows-nfsim.yml,
# windows-tail.yml), and no job ever invoked the binaries — so the suite's only
# gate was a developer remembering to build it by hand. The pre-push hook
# covers python/tests; the C++ side had no equivalent at any stage.
# gate was a developer remembering to build it by hand. The C++ side had no
# equivalent at any stage of CI.
#
# The original wording here was "the pre-push hook covers python/tests", offered
# as the contrast that made the C++ gap worse. Issue #169 showed that assumption
# was itself the other half of the problem: a local hook covers one platform and
# `--no-verify` removes it, so python/tests had no cross-platform CI gate either.
# python-tests.yml (ubuntu + macos-14, whole suite, default build) is that gate
# now; this job remains the C++ half.
#
# That is not hypothetical. #28 flipped SteadyStateOptions::method from "newton"
# to "integration" and updated the Python side, leaving the C++ assertion in
Expand Down
227 changes: 227 additions & 0 deletions .github/workflows/python-tests.yml
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
name: python-tests

# The cross-platform Python gate (issue #169).
#
# Before this job, GitHub CI ran 533 of the suite's ~3490 Python tests on Linux
# or macOS — and every one of them under BNGSIM_CODEGEN_JIT=mir on a
# -DBNGSIM_ENABLE_MIR=ON build. The number exercised in the DEFAULT configuration
# on any non-Windows host was zero. lint.yml runs no pytest by design;
# native-tests.yml is the C++ suite; wheels.yml runs only the cibuildwheel import
# smoke test; windows-tail.yml and windows-nfsim.yml are Windows-only. So a
# regression in SBML loading, .net parsing, events, steady state, SSA, conversion
# or coupling could only be caught on Windows, and only if the file happened to
# be named in one of two hand-maintained lists.
#
# The gap was structural, not a bug. Every job that runs pytest names a curated
# file list, so a NEW test file defaults to running nowhere, and the backstop was
# assumed to be the local pre-push hook — which covers one platform (whichever
# the developer is on, here macOS arm64) and which `git push --no-verify`
# removes. native-tests.yml stated that assumption outright.
#
# Three properties of this job are therefore deliberate, and each one is the
# thing that closes a specific half of the gap:
#
# * NO paths filter. A gate that fires selectively reintroduces the per-file
# opt-in through the trigger instead of through the run list. lint.yml takes
# the same position for the same reason.
# * NO file list. It runs `python/tests` as a directory, so a file added
# tomorrow runs here with no workflow edit — the property native-tests.yml
# gets from driving ctest rather than one named target.
# * NO -D overrides. Every other workflow disables something (KLU off in
# mir/windows-tail/windows-nfsim/native-tests, MIR on in mir.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.
#
# It is the pre-push hook, on two more platforms. `uv sync --extra dev` provisions
# from uv.lock and the pytest call is the hook's own (.pre-commit-config.yaml),
# so green here means what a clean `git push` means locally — on a host the
# developer does not have.
#
# SuiteSparse/KLU is provisioned differently per leg, and the asymmetry is
# deliberate rather than an inconsistency:
#
# * macOS installs nothing. Accelerate supplies the BLAS, so
# BNGSIM_KLU_AUTOBUILD (GH #209) builds the pinned KLU subset from source —
# which makes this the ONLY job anywhere that exercises that path. Every
# wheel leg resolves a prebuilt SuiteSparse through SUITESPARSE_ROOT and
# every other job sets ENABLE_KLU=OFF, so the fallback an sdist install on a
# bare box depends on had no CI at all.
# * Linux installs libsuitesparse-dev, because on a bare ubuntu-latest the
# autobuild does NOT work: SuiteSparse's own CMake calls find_package(BLAS)
# and the runner image ships no BLAS, so the build dies in SuiteSparse_config
# before KLU is reached. Measured here, not assumed — the first run of this
# job failed exactly that way, which means GH #209's self-sufficiency claim
# holds on macOS but not on a bare Linux host. Installing the system package
# is what cibuildwheel's Linux leg already does (`dnf install
# suitesparse-devel`), so this leg matches the shipped wheel either way.
#
# The HAS_KLU assertion below is what keeps both honest — without it a build that
# quietly lost KLU would skip the sparse tests and still report green.
#
# Two coverage axes are deliberately NOT taken here, so they are not mistaken for
# oversights:
#
# * macOS x86_64. mir.yml already runs a macos-15-intel leg, and a
# macOS-x86_64-specific *Python-level* regression is the least likely of the
# three. Adding it is one line in the matrix.
# * The Python floor. requires-python is >=3.10 and every job here runs 3.12,
# so nothing tests the floor at runtime (ruff's target-version=py310 catches
# syntax, not stdlib API). Adding it is one more matrix entry.
#
# One known skip is environmental rather than structural: $BNGPATH is unset, so
# the 14 BNG2.pl round-trip tests in test_sbml_to_bngl.py skip. Closing that needs
# the perl toolchain plus the `parity` dependency group (a git+https reference to
# PyBioNetGen), which is a heavier provisioning step than this gate warrants; the
# skip is declared in conftest's _DECLARED_SKIPS and shows up in the audit table.

on:
push:
branches: [main]
pull_request:
workflow_dispatch:
inputs:
pytest_args:
description: "Extra pytest args (e.g. -x, -k some_pattern, or a single file path)"
required: false
default: ""

concurrency:
group: python-tests-${{ github.ref }}
cancel-in-progress: true

jobs:
python-tests:
name: Python suite · ${{ matrix.os }}
runs-on: ${{ matrix.os }}
timeout-minutes: 90
strategy:
fail-fast: false
matrix:
include:
- os: ubuntu-latest # x86_64 Linux — no Python coverage at all before #169
- os: macos-14 # macOS arm64 — covered only by one developer's pre-push hook
steps:
- uses: actions/checkout@v4

- name: Install uv
uses: astral-sh/setup-uv@v5
with:
enable-cache: true
cache-dependency-glob: "uv.lock"

# See the SuiteSparse note in the header: Linux only, because the runner
# image has no BLAS for the from-source autobuild to link against. macOS
# deliberately gets nothing so its leg exercises BNGSIM_KLU_AUTOBUILD.
- name: Install SuiteSparse (Linux)
if: runner.os == 'Linux'
run: |
set -euxo pipefail
sudo apt-get update
sudo apt-get install -y libsuitesparse-dev

# --extra dev rather than an enumerated extras list: `dev` is the set a
# developer's `uv sync` installs, so this job and the pre-push hook see the
# same environment, and an extra added to `dev` later reaches CI without a
# workflow edit. It is what un-skips the antimony / roadrunner / xarray /
# h5py / pandas / jax / vivarium guarded tests — ~50 items that skip on a
# bare install. Building bngsim itself is the slow part (SUNDIALS is
# FetchContent'd and the KLU subset is built from source).
- name: Provision from uv.lock (default build config)
run: uv sync --extra dev --python 3.12

- name: Show build / capability info
run: |
set -euxo pipefail
uv run --no-sync python -c "import platform, sys; print(platform.platform()); print(sys.version)"
uv run --no-sync python -c "import bngsim; print('bngsim', bngsim.__version__); print('HAS_KLU', bngsim.HAS_KLU); print('HAS_NFSIM', bngsim.HAS_NFSIM); print('HAS_MIR', bngsim.HAS_MIR)"
# The point of this job is the DEFAULT configuration, and KLU is the
# part of it nothing else in CI builds. A build that silently lost KLU
# would skip the sparse-solver tests and still be green, so fail here
# instead — the same false-green guard mir.yml puts on HAS_MIR.
uv run --no-sync python -c "import bngsim, sys; sys.exit(0 if bngsim.HAS_KLU else 'HAS_KLU is False — the default build did not link SuiteSparse/KLU')"
# The runtime codegen path shells out to this compiler at simulate time.
uv run --no-sync python -c "from bngsim import _codegen; print('C compiler:', _codegen._find_c_compiler())"

# BNGSIM_SKIP_AUDIT=strict turns any skip whose reason is not in conftest's
# _DECLARED_SKIPS into a failure. That is the second half of the false-green
# guard below: the count floor catches tests that vanish from collection,
# strict catches tests that quietly turn into skips for a reason nobody
# signed off on.
- name: Run the full Python suite
id: pytest
env:
BNGSIM_SKIP_AUDIT: strict
BNGSIM_TEST_DATA: ${{ github.workspace }}/tests/data
run: |
set -o pipefail
uv run --no-sync python -m pytest -p no:cacheprovider python/tests \
-q --tb=short --durations=15 ${{ github.event.inputs.pytest_args }} \
2>&1 | tee python-tests-${{ matrix.os }}.log

- name: Guard against a false green
if: ${{ !github.event.inputs.pytest_args }}
run: |
set -euo pipefail
# A directory-scoped pytest cannot drift out of sync with a file list,
# but it can still shrink: 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,
# which is the #28/#36 shape native-tests.yml guards with its own
# floors. Two denominators here, for the same reason it needs two:
#
# 1. COLLECTED (passed + skipped + xfailed + xpassed). This is the
# structural one, and it is nearly environment-INDEPENDENT: 3490 on
# ubuntu-latest, on macos-14, and on a developer's macOS box, all at
# the commit that added this job. Every skip in this suite is
# function- or class-level, so a corpus being absent moves tests
# between the columns without changing the total. A drop here means
# tests stopped being collected. Floor sits ~10 below.
# 2. PASSED. This one IS environment-sensitive (3415 on macOS, 3411 on
# Linux — the difference is lanl/bngsim#176's four xfails), so its
# floor is looser. It exists to catch the case COLLECTED cannot: a
# whole group converting to skips for a *declared* reason, e.g. an
# extra silently failing to install and taking the 13 roadrunner
# tests with it. Floor sits ~30 below, so it trips on a group and
# not on one test.
#
# Both floors only rise as tests are added; additions never break them,
# only silent removals do.
COLLECTED_FLOOR=3480
PASS_FLOOR=3380

LOG="python-tests-${{ matrix.os }}.log"
summary="$(grep -E '[0-9]+ (passed|failed).* in [0-9.]+s' "$LOG" | tail -1 || true)"
if [ -z "$summary" ]; then
echo "::error::No pytest summary line — the suite did not run to completion."
exit 1
fi
echo "summary: $summary"

# "3 xpassed" cannot match "[0-9]+ passed", so the categories stay disjoint.
count() { echo "$summary" | grep -oE "[0-9]+ $1" | grep -oE '^[0-9]+' || echo 0; }
passed="$(count passed)"
collected=$(( passed + $(count skipped) + $(count xfailed) + $(count xpassed) ))

echo "collected $collected (floor $COLLECTED_FLOOR); passed $passed (floor $PASS_FLOOR)."
if [ "$collected" -lt "$COLLECTED_FLOOR" ]; then
echo "::error::Only $collected tests collected, expected >= $COLLECTED_FLOOR — tests stopped being collected rather than failing."
exit 1
fi
if [ "$passed" -lt "$PASS_FLOOR" ]; then
echo "::error::Only $passed tests passed, expected >= $PASS_FLOOR — a group of tests turned into skips."
exit 1
fi

# Conditioned on the pytest step having been reached, not just on always():
# if the build fails there is no log to upload, and if-no-files-found=error
# would then report a second, misleading failure on top of the real one.
# Keeping `error` (rather than downgrading to `warn`) means a pytest step
# that ran but produced no log is still caught.
- name: Upload pytest log
if: always() && steps.pytest.conclusion != 'skipped'
uses: actions/upload-artifact@v4
with:
name: python-tests-log-${{ matrix.os }}
path: python-tests-${{ matrix.os }}.log
if-no-files-found: error
76 changes: 76 additions & 0 deletions CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,6 +15,20 @@ in `CMakeLists.txt`) is derived from it.
## [Unreleased]

### Fixed
- **A LAPACK-dense skip that no `_DECLARED_SKIPS` entry matched (found while
wiring issue #169).** Two files skip for the same build-variant condition and
phrase it differently: `test_engine_choice_accessors.py` says `"LAPACK-dense
not built in this configuration"`, `test_lapack_dense_solver.py` says `"build
links no BLAS dense backend (Accelerate / LAPACK)"`. Only the first was
declared, so the second read as an *undeclared* skip — the audit's signal for
"a test stopped running and nobody decided it should".

It could not be seen on macOS: `find_package` always resolves Accelerate there,
so neither test skips at all. It shows up only where CMake finds no BLAS, and
until #169 no CI job ran the full suite anywhere but a developer's macOS box.
The second phrasing is declared now, so `BNGSIM_SKIP_AUDIT=strict` does not
fail a Linux leg for a legitimate build-variant skip.

- **The #161 analytic sensitivity RHS was a net regression on the model it
targeted, and the cost was in the build, not in the emitted code (issue
#165).** `Smith_BMCSystBiol2013` (133 species, 16 sensitivity columns,
Expand DownExpand Up@@ -153,6 +167,68 @@ in `CMakeLists.txt`) is derived from it.
refusal and turns the sensitivity column into a real one.

### Added
- **A cross-platform Python test gate (issue #169).**
`.github/workflows/python-tests.yml` runs the whole of `python/tests` on
`ubuntu-latest` and `macos-14` in the **default** build configuration. Before
it, 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, so the count exercised in the configuration a
wheel actually ships was **zero** on Linux and macOS. A regression in SBML
loading, `.net` parsing, events, steady state, SSA, conversion or coupling
could be caught only on Windows, and only if the file happened to be named in
one of two hand-maintained lists.

The gap was structural, not a bug. Every job that runs pytest names a curated
file list, so a *new* test file defaults to running nowhere, and the assumed
backstop was the local pre-push hook — which covers whichever platform the
developer happens to be on (here macOS arm64) and which `git push --no-verify`
removes. `native-tests.yml` stated that assumption in its header as fact.

The new job therefore carries no `paths:` filter (a selectively-firing gate
reintroduces the per-file opt-in through the trigger instead of the run list),
no file list (it runs the directory, the way `native-tests.yml` drives `ctest`
rather than one named target), and no `-D` overrides (every other workflow
disables something — KLU off in four of them, MIR *on* in `mir.yml` — which is
how the shipped configuration ended up untested). Provisioning is `uv sync
--extra dev` off `uv.lock` and the pytest call is the pre-push hook's own, so a
green run means what a clean `git push` means locally, on a host the developer
does not have.

Two false-green guards, mirroring the ones `native-tests.yml` added for the C++
suite: a floor on the passed count, because a module that stops importing skips
at collection and shrinks the denominator without failing anything; and
`BNGSIM_SKIP_AUDIT=strict`, because a test that quietly turns into a skip for
an undeclared reason is the same invisibility in a different form. `HAS_KLU` is
asserted after the build for the reason `mir.yml` asserts `HAS_MIR` — a build
that silently lost KLU would skip the sparse-solver tests and still be green.

Side effect worth naming: the macOS leg is the only place anywhere that
exercises `BNGSIM_KLU_AUTOBUILD` (GH #209). The wheel legs all resolve a
prebuilt SuiteSparse through `SUITESPARSE_ROOT` and every other job sets
`ENABLE_KLU=OFF`, so the from-source KLU subset that an sdist install on a bare
box falls back to had no CI at all. Wiring it up immediately showed why that
matters: the same autobuild **cannot** complete on a bare `ubuntu-latest`,
because SuiteSparse's own CMake calls `find_package(BLAS)` and the runner image
ships none, so the configure dies in `SuiteSparse_config` before KLU is
reached. So GH #209's self-sufficiency claim holds on macOS but not on a bare
Linux host. The Linux leg therefore installs `libsuitesparse-dev`, the same
system-package route cibuildwheel's Linux leg takes.

And the gate earned itself on its first real run: **four tests fail on Linux
that pass on macOS**, in two unrelated subsystems, both pre-existing on `main`
and both exactly the class #169 said nothing could see. GH #176's
finite-difference retry fires correctly on
`ltype_calcium_discontinuous_jacobian.net` and then dies at a *second*
threshold crossing (t≈34.6) the fixture's own header does not mention; and
`nested_derived_rate_const.net`'s reduced Jacobian is exactly singular under
Linux's reference LAPACK where Accelerate leaves it merely ill-conditioned, so
it takes the refusal branch its sibling test exists to assert rather than the
warning branch its own test asserts. Both are quarantined under
`xfail(sys.platform.startswith("linux"), strict=True, raises=SimulationError)`
and reported in lanl/bngsim#176 — `strict` so they retire themselves, and
quarantined at all so the new gate lands green rather than permanently red,
which is the distinction `native-tests.yml`'s header spells out.

- **`compartment_sizes=` at load, the supported way to change a volume (issue
#164).** `Model.from_sbml`, `from_sbml_string`, `from_antimony`,
`from_antimony_string`, and `Model.load` take
Expand Down
8 changes: 8 additions & 0 deletions SUPPORT_MATRIX.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -77,6 +77,14 @@ environment. `lint.yml`, `native-tests.yml`, `mir.yml`, `windows-nfsim.yml` and
`windows-tail.yml` cover the pre-commit hooks, the C++ unit suite, the MIR JIT
backend, and the Windows NFsim/RuleMonkey paths respectively.

`python-tests.yml` is the Python suite's own gate: the whole of `python/tests`,
on `ubuntu-latest` and `macos-14`, in the **default** build configuration — no
`-D` overrides, so KLU, NFsim and RuleMonkey are all on and the MIR backend is
off, which is what a wheel ships. It has no `paths:` filter and names no test
files, so it fires on every change and picks up new tests without a workflow
edit. Every other job above disables something or runs a curated list, which is
how the shipped configuration went untested on Linux and macOS until issue #169.

Check the latest results before trusting this table:

```bash
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' ci(python-tests): run the whole Python suite on Linux and macOS (#169) by wshlavacek · Pull Request #175 · lanl/bngsim · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .github/workflows/native-tests.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,8 +5,15 @@ name: native-tests
# Until #38 NOTHING in CI built these tests. Every workflow that compiles the
# project passes BNGSIM_BUILD_TESTS=OFF (mir.yml, windows-nfsim.yml,
# windows-tail.yml), and no job ever invoked the binaries — so the suite's only
# gate was a developer remembering to build it by hand. The pre-push hook
# covers python/tests; the C++ side had no equivalent at any stage.
# gate was a developer remembering to build it by hand. The C++ side had no
# equivalent at any stage of CI.
#
# The original wording here was "the pre-push hook covers python/tests", offered
# as the contrast that made the C++ gap worse. Issue #169 showed that assumption
# was itself the other half of the problem: a local hook covers one platform and
# `--no-verify` removes it, so python/tests had no cross-platform CI gate either.
# python-tests.yml (ubuntu + macos-14, whole suite, default build) is that gate
# now; this job remains the C++ half.
#
# That is not hypothetical. #28 flipped SteadyStateOptions::method from "newton"
# to "integration" and updated the Python side, leaving the C++ assertion in
Expand Down
227 changes: 227 additions & 0 deletions .github/workflows/python-tests.yml
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
name: python-tests

# The cross-platform Python gate (issue #169).
#
# Before this job, GitHub CI ran 533 of the suite's ~3490 Python tests on Linux
# or macOS — and every one of them under BNGSIM_CODEGEN_JIT=mir on a
# -DBNGSIM_ENABLE_MIR=ON build. The number exercised in the DEFAULT configuration
# on any non-Windows host was zero. lint.yml runs no pytest by design;
# native-tests.yml is the C++ suite; wheels.yml runs only the cibuildwheel import
# smoke test; windows-tail.yml and windows-nfsim.yml are Windows-only. So a
# regression in SBML loading, .net parsing, events, steady state, SSA, conversion
# or coupling could only be caught on Windows, and only if the file happened to
# be named in one of two hand-maintained lists.
#
# The gap was structural, not a bug. Every job that runs pytest names a curated
# file list, so a NEW test file defaults to running nowhere, and the backstop was
# assumed to be the local pre-push hook — which covers one platform (whichever
# the developer is on, here macOS arm64) and which `git push --no-verify`
# removes. native-tests.yml stated that assumption outright.
#
# Three properties of this job are therefore deliberate, and each one is the
# thing that closes a specific half of the gap:
#
# * NO paths filter. A gate that fires selectively reintroduces the per-file
# opt-in through the trigger instead of through the run list. lint.yml takes
# the same position for the same reason.
# * NO file list. It runs `python/tests` as a directory, so a file added
# tomorrow runs here with no workflow edit — the property native-tests.yml
# gets from driving ctest rather than one named target.
# * NO -D overrides. Every other workflow disables something (KLU off in
# mir/windows-tail/windows-nfsim/native-tests, MIR on in mir.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.
#
# It is the pre-push hook, on two more platforms. `uv sync --extra dev` provisions
# from uv.lock and the pytest call is the hook's own (.pre-commit-config.yaml),
# so green here means what a clean `git push` means locally — on a host the
# developer does not have.
#
# SuiteSparse/KLU is provisioned differently per leg, and the asymmetry is
# deliberate rather than an inconsistency:
#
# * macOS installs nothing. Accelerate supplies the BLAS, so
# BNGSIM_KLU_AUTOBUILD (GH #209) builds the pinned KLU subset from source —
# which makes this the ONLY job anywhere that exercises that path. Every
# wheel leg resolves a prebuilt SuiteSparse through SUITESPARSE_ROOT and
# every other job sets ENABLE_KLU=OFF, so the fallback an sdist install on a
# bare box depends on had no CI at all.
# * Linux installs libsuitesparse-dev, because on a bare ubuntu-latest the
# autobuild does NOT work: SuiteSparse's own CMake calls find_package(BLAS)
# and the runner image ships no BLAS, so the build dies in SuiteSparse_config
# before KLU is reached. Measured here, not assumed — the first run of this
# job failed exactly that way, which means GH #209's self-sufficiency claim
# holds on macOS but not on a bare Linux host. Installing the system package
# is what cibuildwheel's Linux leg already does (`dnf install
# suitesparse-devel`), so this leg matches the shipped wheel either way.
#
# The HAS_KLU assertion below is what keeps both honest — without it a build that
# quietly lost KLU would skip the sparse tests and still report green.
#
# Two coverage axes are deliberately NOT taken here, so they are not mistaken for
# oversights:
#
# * macOS x86_64. mir.yml already runs a macos-15-intel leg, and a
# macOS-x86_64-specific *Python-level* regression is the least likely of the
# three. Adding it is one line in the matrix.
# * The Python floor. requires-python is >=3.10 and every job here runs 3.12,
# so nothing tests the floor at runtime (ruff's target-version=py310 catches
# syntax, not stdlib API). Adding it is one more matrix entry.
#
# One known skip is environmental rather than structural: $BNGPATH is unset, so
# the 14 BNG2.pl round-trip tests in test_sbml_to_bngl.py skip. Closing that needs
# the perl toolchain plus the `parity` dependency group (a git+https reference to
# PyBioNetGen), which is a heavier provisioning step than this gate warrants; the
# skip is declared in conftest's _DECLARED_SKIPS and shows up in the audit table.

on:
push:
branches: [main]
pull_request:
workflow_dispatch:
inputs:
pytest_args:
description: "Extra pytest args (e.g. -x, -k some_pattern, or a single file path)"
required: false
default: ""

concurrency:
group: python-tests-${{ github.ref }}
cancel-in-progress: true

jobs:
python-tests:
name: Python suite · ${{ matrix.os }}
runs-on: ${{ matrix.os }}
timeout-minutes: 90
strategy:
fail-fast: false
matrix:
include:
- os: ubuntu-latest # x86_64 Linux — no Python coverage at all before #169
- os: macos-14 # macOS arm64 — covered only by one developer's pre-push hook
steps:
- uses: actions/checkout@v4

- name: Install uv
uses: astral-sh/setup-uv@v5
with:
enable-cache: true
cache-dependency-glob: "uv.lock"

# See the SuiteSparse note in the header: Linux only, because the runner
# image has no BLAS for the from-source autobuild to link against. macOS
# deliberately gets nothing so its leg exercises BNGSIM_KLU_AUTOBUILD.
- name: Install SuiteSparse (Linux)
if: runner.os == 'Linux'
run: |
set -euxo pipefail
sudo apt-get update
sudo apt-get install -y libsuitesparse-dev

# --extra dev rather than an enumerated extras list: `dev` is the set a
# developer's `uv sync` installs, so this job and the pre-push hook see the
# same environment, and an extra added to `dev` later reaches CI without a
# workflow edit. It is what un-skips the antimony / roadrunner / xarray /
# h5py / pandas / jax / vivarium guarded tests — ~50 items that skip on a
# bare install. Building bngsim itself is the slow part (SUNDIALS is
# FetchContent'd and the KLU subset is built from source).
- name: Provision from uv.lock (default build config)
run: uv sync --extra dev --python 3.12

- name: Show build / capability info
run: |
set -euxo pipefail
uv run --no-sync python -c "import platform, sys; print(platform.platform()); print(sys.version)"
uv run --no-sync python -c "import bngsim; print('bngsim', bngsim.__version__); print('HAS_KLU', bngsim.HAS_KLU); print('HAS_NFSIM', bngsim.HAS_NFSIM); print('HAS_MIR', bngsim.HAS_MIR)"
# The point of this job is the DEFAULT configuration, and KLU is the
# part of it nothing else in CI builds. A build that silently lost KLU
# would skip the sparse-solver tests and still be green, so fail here
# instead — the same false-green guard mir.yml puts on HAS_MIR.
uv run --no-sync python -c "import bngsim, sys; sys.exit(0 if bngsim.HAS_KLU else 'HAS_KLU is False — the default build did not link SuiteSparse/KLU')"
# The runtime codegen path shells out to this compiler at simulate time.
uv run --no-sync python -c "from bngsim import _codegen; print('C compiler:', _codegen._find_c_compiler())"

# BNGSIM_SKIP_AUDIT=strict turns any skip whose reason is not in conftest's
# _DECLARED_SKIPS into a failure. That is the second half of the false-green
# guard below: the count floor catches tests that vanish from collection,
# strict catches tests that quietly turn into skips for a reason nobody
# signed off on.
- name: Run the full Python suite
id: pytest
env:
BNGSIM_SKIP_AUDIT: strict
BNGSIM_TEST_DATA: ${{ github.workspace }}/tests/data
run: |
set -o pipefail
uv run --no-sync python -m pytest -p no:cacheprovider python/tests \
-q --tb=short --durations=15 ${{ github.event.inputs.pytest_args }} \
2>&1 | tee python-tests-${{ matrix.os }}.log

- name: Guard against a false green
if: ${{ !github.event.inputs.pytest_args }}
run: |
set -euo pipefail
# A directory-scoped pytest cannot drift out of sync with a file list,
# but it can still shrink: 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,
# which is the #28/#36 shape native-tests.yml guards with its own
# floors. Two denominators here, for the same reason it needs two:
#
# 1. COLLECTED (passed + skipped + xfailed + xpassed). This is the
# structural one, and it is nearly environment-INDEPENDENT: 3490 on
# ubuntu-latest, on macos-14, and on a developer's macOS box, all at
# the commit that added this job. Every skip in this suite is
# function- or class-level, so a corpus being absent moves tests
# between the columns without changing the total. A drop here means
# tests stopped being collected. Floor sits ~10 below.
# 2. PASSED. This one IS environment-sensitive (3415 on macOS, 3411 on
# Linux — the difference is lanl/bngsim#176's four xfails), so its
# floor is looser. It exists to catch the case COLLECTED cannot: a
# whole group converting to skips for a *declared* reason, e.g. an
# extra silently failing to install and taking the 13 roadrunner
# tests with it. Floor sits ~30 below, so it trips on a group and
# not on one test.
#
# Both floors only rise as tests are added; additions never break them,
# only silent removals do.
COLLECTED_FLOOR=3480
PASS_FLOOR=3380

LOG="python-tests-${{ matrix.os }}.log"
summary="$(grep -E '[0-9]+ (passed|failed).* in [0-9.]+s' "$LOG" | tail -1 || true)"
if [ -z "$summary" ]; then
echo "::error::No pytest summary line — the suite did not run to completion."
exit 1
fi
echo "summary: $summary"

# "3 xpassed" cannot match "[0-9]+ passed", so the categories stay disjoint.
count() { echo "$summary" | grep -oE "[0-9]+ $1" | grep -oE '^[0-9]+' || echo 0; }
passed="$(count passed)"
collected=$(( passed + $(count skipped) + $(count xfailed) + $(count xpassed) ))

echo "collected $collected (floor $COLLECTED_FLOOR); passed $passed (floor $PASS_FLOOR)."
if [ "$collected" -lt "$COLLECTED_FLOOR" ]; then
echo "::error::Only $collected tests collected, expected >= $COLLECTED_FLOOR — tests stopped being collected rather than failing."
exit 1
fi
if [ "$passed" -lt "$PASS_FLOOR" ]; then
echo "::error::Only $passed tests passed, expected >= $PASS_FLOOR — a group of tests turned into skips."
exit 1
fi

# Conditioned on the pytest step having been reached, not just on always():
# if the build fails there is no log to upload, and if-no-files-found=error
# would then report a second, misleading failure on top of the real one.
# Keeping `error` (rather than downgrading to `warn`) means a pytest step
# that ran but produced no log is still caught.
- name: Upload pytest log
if: always() && steps.pytest.conclusion != 'skipped'
uses: actions/upload-artifact@v4
with:
name: python-tests-log-${{ matrix.os }}
path: python-tests-${{ matrix.os }}.log
if-no-files-found: error
76 changes: 76 additions & 0 deletions CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,6 +15,20 @@ in `CMakeLists.txt`) is derived from it.
## [Unreleased]

### Fixed
- **A LAPACK-dense skip that no `_DECLARED_SKIPS` entry matched (found while
wiring issue #169).** Two files skip for the same build-variant condition and
phrase it differently: `test_engine_choice_accessors.py` says `"LAPACK-dense
not built in this configuration"`, `test_lapack_dense_solver.py` says `"build
links no BLAS dense backend (Accelerate / LAPACK)"`. Only the first was
declared, so the second read as an *undeclared* skip — the audit's signal for
"a test stopped running and nobody decided it should".

It could not be seen on macOS: `find_package` always resolves Accelerate there,
so neither test skips at all. It shows up only where CMake finds no BLAS, and
until #169 no CI job ran the full suite anywhere but a developer's macOS box.
The second phrasing is declared now, so `BNGSIM_SKIP_AUDIT=strict` does not
fail a Linux leg for a legitimate build-variant skip.

- **The #161 analytic sensitivity RHS was a net regression on the model it
targeted, and the cost was in the build, not in the emitted code (issue
#165).** `Smith_BMCSystBiol2013` (133 species, 16 sensitivity columns,
Expand DownExpand Up@@ -153,6 +167,68 @@ in `CMakeLists.txt`) is derived from it.
refusal and turns the sensitivity column into a real one.

### Added
- **A cross-platform Python test gate (issue #169).**
`.github/workflows/python-tests.yml` runs the whole of `python/tests` on
`ubuntu-latest` and `macos-14` in the **default** build configuration. Before
it, 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, so the count exercised in the configuration a
wheel actually ships was **zero** on Linux and macOS. A regression in SBML
loading, `.net` parsing, events, steady state, SSA, conversion or coupling
could be caught only on Windows, and only if the file happened to be named in
one of two hand-maintained lists.

The gap was structural, not a bug. Every job that runs pytest names a curated
file list, so a *new* test file defaults to running nowhere, and the assumed
backstop was the local pre-push hook — which covers whichever platform the
developer happens to be on (here macOS arm64) and which `git push --no-verify`
removes. `native-tests.yml` stated that assumption in its header as fact.

The new job therefore carries no `paths:` filter (a selectively-firing gate
reintroduces the per-file opt-in through the trigger instead of the run list),
no file list (it runs the directory, the way `native-tests.yml` drives `ctest`
rather than one named target), and no `-D` overrides (every other workflow
disables something — KLU off in four of them, MIR *on* in `mir.yml` — which is
how the shipped configuration ended up untested). Provisioning is `uv sync
--extra dev` off `uv.lock` and the pytest call is the pre-push hook's own, so a
green run means what a clean `git push` means locally, on a host the developer
does not have.

Two false-green guards, mirroring the ones `native-tests.yml` added for the C++
suite: a floor on the passed count, because a module that stops importing skips
at collection and shrinks the denominator without failing anything; and
`BNGSIM_SKIP_AUDIT=strict`, because a test that quietly turns into a skip for
an undeclared reason is the same invisibility in a different form. `HAS_KLU` is
asserted after the build for the reason `mir.yml` asserts `HAS_MIR` — a build
that silently lost KLU would skip the sparse-solver tests and still be green.

Side effect worth naming: the macOS leg is the only place anywhere that
exercises `BNGSIM_KLU_AUTOBUILD` (GH #209). The wheel legs all resolve a
prebuilt SuiteSparse through `SUITESPARSE_ROOT` and every other job sets
`ENABLE_KLU=OFF`, so the from-source KLU subset that an sdist install on a bare
box falls back to had no CI at all. Wiring it up immediately showed why that
matters: the same autobuild **cannot** complete on a bare `ubuntu-latest`,
because SuiteSparse's own CMake calls `find_package(BLAS)` and the runner image
ships none, so the configure dies in `SuiteSparse_config` before KLU is
reached. So GH #209's self-sufficiency claim holds on macOS but not on a bare
Linux host. The Linux leg therefore installs `libsuitesparse-dev`, the same
system-package route cibuildwheel's Linux leg takes.

And the gate earned itself on its first real run: **four tests fail on Linux
that pass on macOS**, in two unrelated subsystems, both pre-existing on `main`
and both exactly the class #169 said nothing could see. GH #176's
finite-difference retry fires correctly on
`ltype_calcium_discontinuous_jacobian.net` and then dies at a *second*
threshold crossing (t≈34.6) the fixture's own header does not mention; and
`nested_derived_rate_const.net`'s reduced Jacobian is exactly singular under
Linux's reference LAPACK where Accelerate leaves it merely ill-conditioned, so
it takes the refusal branch its sibling test exists to assert rather than the
warning branch its own test asserts. Both are quarantined under
`xfail(sys.platform.startswith("linux"), strict=True, raises=SimulationError)`
and reported in lanl/bngsim#176 — `strict` so they retire themselves, and
quarantined at all so the new gate lands green rather than permanently red,
which is the distinction `native-tests.yml`'s header spells out.

- **`compartment_sizes=` at load, the supported way to change a volume (issue
#164).** `Model.from_sbml`, `from_sbml_string`, `from_antimony`,
`from_antimony_string`, and `Model.load` take
Expand Down
8 changes: 8 additions & 0 deletions SUPPORT_MATRIX.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -77,6 +77,14 @@ environment. `lint.yml`, `native-tests.yml`, `mir.yml`, `windows-nfsim.yml` and
`windows-tail.yml` cover the pre-commit hooks, the C++ unit suite, the MIR JIT
backend, and the Windows NFsim/RuleMonkey paths respectively.

`python-tests.yml` is the Python suite's own gate: the whole of `python/tests`,
on `ubuntu-latest` and `macos-14`, in the **default** build configuration — no
`-D` overrides, so KLU, NFsim and RuleMonkey are all on and the MIR backend is
off, which is what a wheel ships. It has no `paths:` filter and names no test
files, so it fires on every change and picks up new tests without a workflow
edit. Every other job above disables something or runs a curated list, which is
how the shipped configuration went untested on Linux and macOS until issue #169.

Check the latest results before trusting this table:

```bash
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' ci(python-tests): run the whole Python suite on Linux and macOS (#169) by wshlavacek · Pull Request #175 · lanl/bngsim · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .github/workflows/native-tests.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,8 +5,15 @@ name: native-tests
# Until #38 NOTHING in CI built these tests. Every workflow that compiles the
# project passes BNGSIM_BUILD_TESTS=OFF (mir.yml, windows-nfsim.yml,
# windows-tail.yml), and no job ever invoked the binaries — so the suite's only
# gate was a developer remembering to build it by hand. The pre-push hook
# covers python/tests; the C++ side had no equivalent at any stage.
# gate was a developer remembering to build it by hand. The C++ side had no
# equivalent at any stage of CI.
#
# The original wording here was "the pre-push hook covers python/tests", offered
# as the contrast that made the C++ gap worse. Issue #169 showed that assumption
# was itself the other half of the problem: a local hook covers one platform and
# `--no-verify` removes it, so python/tests had no cross-platform CI gate either.
# python-tests.yml (ubuntu + macos-14, whole suite, default build) is that gate
# now; this job remains the C++ half.
#
# That is not hypothetical. #28 flipped SteadyStateOptions::method from "newton"
# to "integration" and updated the Python side, leaving the C++ assertion in
Expand Down
227 changes: 227 additions & 0 deletions .github/workflows/python-tests.yml
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
name: python-tests

# The cross-platform Python gate (issue #169).
#
# Before this job, GitHub CI ran 533 of the suite's ~3490 Python tests on Linux
# or macOS — and every one of them under BNGSIM_CODEGEN_JIT=mir on a
# -DBNGSIM_ENABLE_MIR=ON build. The number exercised in the DEFAULT configuration
# on any non-Windows host was zero. lint.yml runs no pytest by design;
# native-tests.yml is the C++ suite; wheels.yml runs only the cibuildwheel import
# smoke test; windows-tail.yml and windows-nfsim.yml are Windows-only. So a
# regression in SBML loading, .net parsing, events, steady state, SSA, conversion
# or coupling could only be caught on Windows, and only if the file happened to
# be named in one of two hand-maintained lists.
#
# The gap was structural, not a bug. Every job that runs pytest names a curated
# file list, so a NEW test file defaults to running nowhere, and the backstop was
# assumed to be the local pre-push hook — which covers one platform (whichever
# the developer is on, here macOS arm64) and which `git push --no-verify`
# removes. native-tests.yml stated that assumption outright.
#
# Three properties of this job are therefore deliberate, and each one is the
# thing that closes a specific half of the gap:
#
# * NO paths filter. A gate that fires selectively reintroduces the per-file
# opt-in through the trigger instead of through the run list. lint.yml takes
# the same position for the same reason.
# * NO file list. It runs `python/tests` as a directory, so a file added
# tomorrow runs here with no workflow edit — the property native-tests.yml
# gets from driving ctest rather than one named target.
# * NO -D overrides. Every other workflow disables something (KLU off in
# mir/windows-tail/windows-nfsim/native-tests, MIR on in mir.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.
#
# It is the pre-push hook, on two more platforms. `uv sync --extra dev` provisions
# from uv.lock and the pytest call is the hook's own (.pre-commit-config.yaml),
# so green here means what a clean `git push` means locally — on a host the
# developer does not have.
#
# SuiteSparse/KLU is provisioned differently per leg, and the asymmetry is
# deliberate rather than an inconsistency:
#
# * macOS installs nothing. Accelerate supplies the BLAS, so
# BNGSIM_KLU_AUTOBUILD (GH #209) builds the pinned KLU subset from source —
# which makes this the ONLY job anywhere that exercises that path. Every
# wheel leg resolves a prebuilt SuiteSparse through SUITESPARSE_ROOT and
# every other job sets ENABLE_KLU=OFF, so the fallback an sdist install on a
# bare box depends on had no CI at all.
# * Linux installs libsuitesparse-dev, because on a bare ubuntu-latest the
# autobuild does NOT work: SuiteSparse's own CMake calls find_package(BLAS)
# and the runner image ships no BLAS, so the build dies in SuiteSparse_config
# before KLU is reached. Measured here, not assumed — the first run of this
# job failed exactly that way, which means GH #209's self-sufficiency claim
# holds on macOS but not on a bare Linux host. Installing the system package
# is what cibuildwheel's Linux leg already does (`dnf install
# suitesparse-devel`), so this leg matches the shipped wheel either way.
#
# The HAS_KLU assertion below is what keeps both honest — without it a build that
# quietly lost KLU would skip the sparse tests and still report green.
#
# Two coverage axes are deliberately NOT taken here, so they are not mistaken for
# oversights:
#
# * macOS x86_64. mir.yml already runs a macos-15-intel leg, and a
# macOS-x86_64-specific *Python-level* regression is the least likely of the
# three. Adding it is one line in the matrix.
# * The Python floor. requires-python is >=3.10 and every job here runs 3.12,
# so nothing tests the floor at runtime (ruff's target-version=py310 catches
# syntax, not stdlib API). Adding it is one more matrix entry.
#
# One known skip is environmental rather than structural: $BNGPATH is unset, so
# the 14 BNG2.pl round-trip tests in test_sbml_to_bngl.py skip. Closing that needs
# the perl toolchain plus the `parity` dependency group (a git+https reference to
# PyBioNetGen), which is a heavier provisioning step than this gate warrants; the
# skip is declared in conftest's _DECLARED_SKIPS and shows up in the audit table.

on:
push:
branches: [main]
pull_request:
workflow_dispatch:
inputs:
pytest_args:
description: "Extra pytest args (e.g. -x, -k some_pattern, or a single file path)"
required: false
default: ""

concurrency:
group: python-tests-${{ github.ref }}
cancel-in-progress: true

jobs:
python-tests:
name: Python suite · ${{ matrix.os }}
runs-on: ${{ matrix.os }}
timeout-minutes: 90
strategy:
fail-fast: false
matrix:
include:
- os: ubuntu-latest # x86_64 Linux — no Python coverage at all before #169
- os: macos-14 # macOS arm64 — covered only by one developer's pre-push hook
steps:
- uses: actions/checkout@v4

- name: Install uv
uses: astral-sh/setup-uv@v5
with:
enable-cache: true
cache-dependency-glob: "uv.lock"

# See the SuiteSparse note in the header: Linux only, because the runner
# image has no BLAS for the from-source autobuild to link against. macOS
# deliberately gets nothing so its leg exercises BNGSIM_KLU_AUTOBUILD.
- name: Install SuiteSparse (Linux)
if: runner.os == 'Linux'
run: |
set -euxo pipefail
sudo apt-get update
sudo apt-get install -y libsuitesparse-dev

# --extra dev rather than an enumerated extras list: `dev` is the set a
# developer's `uv sync` installs, so this job and the pre-push hook see the
# same environment, and an extra added to `dev` later reaches CI without a
# workflow edit. It is what un-skips the antimony / roadrunner / xarray /
# h5py / pandas / jax / vivarium guarded tests — ~50 items that skip on a
# bare install. Building bngsim itself is the slow part (SUNDIALS is
# FetchContent'd and the KLU subset is built from source).
- name: Provision from uv.lock (default build config)
run: uv sync --extra dev --python 3.12

- name: Show build / capability info
run: |
set -euxo pipefail
uv run --no-sync python -c "import platform, sys; print(platform.platform()); print(sys.version)"
uv run --no-sync python -c "import bngsim; print('bngsim', bngsim.__version__); print('HAS_KLU', bngsim.HAS_KLU); print('HAS_NFSIM', bngsim.HAS_NFSIM); print('HAS_MIR', bngsim.HAS_MIR)"
# The point of this job is the DEFAULT configuration, and KLU is the
# part of it nothing else in CI builds. A build that silently lost KLU
# would skip the sparse-solver tests and still be green, so fail here
# instead — the same false-green guard mir.yml puts on HAS_MIR.
uv run --no-sync python -c "import bngsim, sys; sys.exit(0 if bngsim.HAS_KLU else 'HAS_KLU is False — the default build did not link SuiteSparse/KLU')"
# The runtime codegen path shells out to this compiler at simulate time.
uv run --no-sync python -c "from bngsim import _codegen; print('C compiler:', _codegen._find_c_compiler())"

# BNGSIM_SKIP_AUDIT=strict turns any skip whose reason is not in conftest's
# _DECLARED_SKIPS into a failure. That is the second half of the false-green
# guard below: the count floor catches tests that vanish from collection,
# strict catches tests that quietly turn into skips for a reason nobody
# signed off on.
- name: Run the full Python suite
id: pytest
env:
BNGSIM_SKIP_AUDIT: strict
BNGSIM_TEST_DATA: ${{ github.workspace }}/tests/data
run: |
set -o pipefail
uv run --no-sync python -m pytest -p no:cacheprovider python/tests \
-q --tb=short --durations=15 ${{ github.event.inputs.pytest_args }} \
2>&1 | tee python-tests-${{ matrix.os }}.log

- name: Guard against a false green
if: ${{ !github.event.inputs.pytest_args }}
run: |
set -euo pipefail
# A directory-scoped pytest cannot drift out of sync with a file list,
# but it can still shrink: 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,
# which is the #28/#36 shape native-tests.yml guards with its own
# floors. Two denominators here, for the same reason it needs two:
#
# 1. COLLECTED (passed + skipped + xfailed + xpassed). This is the
# structural one, and it is nearly environment-INDEPENDENT: 3490 on
# ubuntu-latest, on macos-14, and on a developer's macOS box, all at
# the commit that added this job. Every skip in this suite is
# function- or class-level, so a corpus being absent moves tests
# between the columns without changing the total. A drop here means
# tests stopped being collected. Floor sits ~10 below.
# 2. PASSED. This one IS environment-sensitive (3415 on macOS, 3411 on
# Linux — the difference is lanl/bngsim#176's four xfails), so its
# floor is looser. It exists to catch the case COLLECTED cannot: a
# whole group converting to skips for a *declared* reason, e.g. an
# extra silently failing to install and taking the 13 roadrunner
# tests with it. Floor sits ~30 below, so it trips on a group and
# not on one test.
#
# Both floors only rise as tests are added; additions never break them,
# only silent removals do.
COLLECTED_FLOOR=3480
PASS_FLOOR=3380

LOG="python-tests-${{ matrix.os }}.log"
summary="$(grep -E '[0-9]+ (passed|failed).* in [0-9.]+s' "$LOG" | tail -1 || true)"
if [ -z "$summary" ]; then
echo "::error::No pytest summary line — the suite did not run to completion."
exit 1
fi
echo "summary: $summary"

# "3 xpassed" cannot match "[0-9]+ passed", so the categories stay disjoint.
count() { echo "$summary" | grep -oE "[0-9]+ $1" | grep -oE '^[0-9]+' || echo 0; }
passed="$(count passed)"
collected=$(( passed + $(count skipped) + $(count xfailed) + $(count xpassed) ))

echo "collected $collected (floor $COLLECTED_FLOOR); passed $passed (floor $PASS_FLOOR)."
if [ "$collected" -lt "$COLLECTED_FLOOR" ]; then
echo "::error::Only $collected tests collected, expected >= $COLLECTED_FLOOR — tests stopped being collected rather than failing."
exit 1
fi
if [ "$passed" -lt "$PASS_FLOOR" ]; then
echo "::error::Only $passed tests passed, expected >= $PASS_FLOOR — a group of tests turned into skips."
exit 1
fi

# Conditioned on the pytest step having been reached, not just on always():
# if the build fails there is no log to upload, and if-no-files-found=error
# would then report a second, misleading failure on top of the real one.
# Keeping `error` (rather than downgrading to `warn`) means a pytest step
# that ran but produced no log is still caught.
- name: Upload pytest log
if: always() && steps.pytest.conclusion != 'skipped'
uses: actions/upload-artifact@v4
with:
name: python-tests-log-${{ matrix.os }}
path: python-tests-${{ matrix.os }}.log
if-no-files-found: error
76 changes: 76 additions & 0 deletions CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,6 +15,20 @@ in `CMakeLists.txt`) is derived from it.
## [Unreleased]

### Fixed
- **A LAPACK-dense skip that no `_DECLARED_SKIPS` entry matched (found while
wiring issue #169).** Two files skip for the same build-variant condition and
phrase it differently: `test_engine_choice_accessors.py` says `"LAPACK-dense
not built in this configuration"`, `test_lapack_dense_solver.py` says `"build
links no BLAS dense backend (Accelerate / LAPACK)"`. Only the first was
declared, so the second read as an *undeclared* skip — the audit's signal for
"a test stopped running and nobody decided it should".

It could not be seen on macOS: `find_package` always resolves Accelerate there,
so neither test skips at all. It shows up only where CMake finds no BLAS, and
until #169 no CI job ran the full suite anywhere but a developer's macOS box.
The second phrasing is declared now, so `BNGSIM_SKIP_AUDIT=strict` does not
fail a Linux leg for a legitimate build-variant skip.

- **The #161 analytic sensitivity RHS was a net regression on the model it
targeted, and the cost was in the build, not in the emitted code (issue
#165).** `Smith_BMCSystBiol2013` (133 species, 16 sensitivity columns,
Expand DownExpand Up@@ -153,6 +167,68 @@ in `CMakeLists.txt`) is derived from it.
refusal and turns the sensitivity column into a real one.

### Added
- **A cross-platform Python test gate (issue #169).**
`.github/workflows/python-tests.yml` runs the whole of `python/tests` on
`ubuntu-latest` and `macos-14` in the **default** build configuration. Before
it, 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, so the count exercised in the configuration a
wheel actually ships was **zero** on Linux and macOS. A regression in SBML
loading, `.net` parsing, events, steady state, SSA, conversion or coupling
could be caught only on Windows, and only if the file happened to be named in
one of two hand-maintained lists.

The gap was structural, not a bug. Every job that runs pytest names a curated
file list, so a *new* test file defaults to running nowhere, and the assumed
backstop was the local pre-push hook — which covers whichever platform the
developer happens to be on (here macOS arm64) and which `git push --no-verify`
removes. `native-tests.yml` stated that assumption in its header as fact.

The new job therefore carries no `paths:` filter (a selectively-firing gate
reintroduces the per-file opt-in through the trigger instead of the run list),
no file list (it runs the directory, the way `native-tests.yml` drives `ctest`
rather than one named target), and no `-D` overrides (every other workflow
disables something — KLU off in four of them, MIR *on* in `mir.yml` — which is
how the shipped configuration ended up untested). Provisioning is `uv sync
--extra dev` off `uv.lock` and the pytest call is the pre-push hook's own, so a
green run means what a clean `git push` means locally, on a host the developer
does not have.

Two false-green guards, mirroring the ones `native-tests.yml` added for the C++
suite: a floor on the passed count, because a module that stops importing skips
at collection and shrinks the denominator without failing anything; and
`BNGSIM_SKIP_AUDIT=strict`, because a test that quietly turns into a skip for
an undeclared reason is the same invisibility in a different form. `HAS_KLU` is
asserted after the build for the reason `mir.yml` asserts `HAS_MIR` — a build
that silently lost KLU would skip the sparse-solver tests and still be green.

Side effect worth naming: the macOS leg is the only place anywhere that
exercises `BNGSIM_KLU_AUTOBUILD` (GH #209). The wheel legs all resolve a
prebuilt SuiteSparse through `SUITESPARSE_ROOT` and every other job sets
`ENABLE_KLU=OFF`, so the from-source KLU subset that an sdist install on a bare
box falls back to had no CI at all. Wiring it up immediately showed why that
matters: the same autobuild **cannot** complete on a bare `ubuntu-latest`,
because SuiteSparse's own CMake calls `find_package(BLAS)` and the runner image
ships none, so the configure dies in `SuiteSparse_config` before KLU is
reached. So GH #209's self-sufficiency claim holds on macOS but not on a bare
Linux host. The Linux leg therefore installs `libsuitesparse-dev`, the same
system-package route cibuildwheel's Linux leg takes.

And the gate earned itself on its first real run: **four tests fail on Linux
that pass on macOS**, in two unrelated subsystems, both pre-existing on `main`
and both exactly the class #169 said nothing could see. GH #176's
finite-difference retry fires correctly on
`ltype_calcium_discontinuous_jacobian.net` and then dies at a *second*
threshold crossing (t≈34.6) the fixture's own header does not mention; and
`nested_derived_rate_const.net`'s reduced Jacobian is exactly singular under
Linux's reference LAPACK where Accelerate leaves it merely ill-conditioned, so
it takes the refusal branch its sibling test exists to assert rather than the
warning branch its own test asserts. Both are quarantined under
`xfail(sys.platform.startswith("linux"), strict=True, raises=SimulationError)`
and reported in lanl/bngsim#176 — `strict` so they retire themselves, and
quarantined at all so the new gate lands green rather than permanently red,
which is the distinction `native-tests.yml`'s header spells out.

- **`compartment_sizes=` at load, the supported way to change a volume (issue
#164).** `Model.from_sbml`, `from_sbml_string`, `from_antimony`,
`from_antimony_string`, and `Model.load` take
Expand Down
8 changes: 8 additions & 0 deletions SUPPORT_MATRIX.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -77,6 +77,14 @@ environment. `lint.yml`, `native-tests.yml`, `mir.yml`, `windows-nfsim.yml` and
`windows-tail.yml` cover the pre-commit hooks, the C++ unit suite, the MIR JIT
backend, and the Windows NFsim/RuleMonkey paths respectively.

`python-tests.yml` is the Python suite's own gate: the whole of `python/tests`,
on `ubuntu-latest` and `macos-14`, in the **default** build configuration — no
`-D` overrides, so KLU, NFsim and RuleMonkey are all on and the MIR backend is
off, which is what a wheel ships. It has no `paths:` filter and names no test
files, so it fires on every change and picks up new tests without a workflow
edit. Every other job above disables something or runs a curated list, which is
how the shipped configuration went untested on Linux and macOS until issue #169.

Check the latest results before trusting this table:

```bash
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' ci(python-tests): run the whole Python suite on Linux and macOS (#169) by wshlavacek · Pull Request #175 · lanl/bngsim · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .github/workflows/native-tests.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,8 +5,15 @@ name: native-tests
# Until #38 NOTHING in CI built these tests. Every workflow that compiles the
# project passes BNGSIM_BUILD_TESTS=OFF (mir.yml, windows-nfsim.yml,
# windows-tail.yml), and no job ever invoked the binaries — so the suite's only
# gate was a developer remembering to build it by hand. The pre-push hook
# covers python/tests; the C++ side had no equivalent at any stage.
# gate was a developer remembering to build it by hand. The C++ side had no
# equivalent at any stage of CI.
#
# The original wording here was "the pre-push hook covers python/tests", offered
# as the contrast that made the C++ gap worse. Issue #169 showed that assumption
# was itself the other half of the problem: a local hook covers one platform and
# `--no-verify` removes it, so python/tests had no cross-platform CI gate either.
# python-tests.yml (ubuntu + macos-14, whole suite, default build) is that gate
# now; this job remains the C++ half.
#
# That is not hypothetical. #28 flipped SteadyStateOptions::method from "newton"
# to "integration" and updated the Python side, leaving the C++ assertion in
Expand Down
227 changes: 227 additions & 0 deletions .github/workflows/python-tests.yml
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
name: python-tests

# The cross-platform Python gate (issue #169).
#
# Before this job, GitHub CI ran 533 of the suite's ~3490 Python tests on Linux
# or macOS — and every one of them under BNGSIM_CODEGEN_JIT=mir on a
# -DBNGSIM_ENABLE_MIR=ON build. The number exercised in the DEFAULT configuration
# on any non-Windows host was zero. lint.yml runs no pytest by design;
# native-tests.yml is the C++ suite; wheels.yml runs only the cibuildwheel import
# smoke test; windows-tail.yml and windows-nfsim.yml are Windows-only. So a
# regression in SBML loading, .net parsing, events, steady state, SSA, conversion
# or coupling could only be caught on Windows, and only if the file happened to
# be named in one of two hand-maintained lists.
#
# The gap was structural, not a bug. Every job that runs pytest names a curated
# file list, so a NEW test file defaults to running nowhere, and the backstop was
# assumed to be the local pre-push hook — which covers one platform (whichever
# the developer is on, here macOS arm64) and which `git push --no-verify`
# removes. native-tests.yml stated that assumption outright.
#
# Three properties of this job are therefore deliberate, and each one is the
# thing that closes a specific half of the gap:
#
# * NO paths filter. A gate that fires selectively reintroduces the per-file
# opt-in through the trigger instead of through the run list. lint.yml takes
# the same position for the same reason.
# * NO file list. It runs `python/tests` as a directory, so a file added
# tomorrow runs here with no workflow edit — the property native-tests.yml
# gets from driving ctest rather than one named target.
# * NO -D overrides. Every other workflow disables something (KLU off in
# mir/windows-tail/windows-nfsim/native-tests, MIR on in mir.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.
#
# It is the pre-push hook, on two more platforms. `uv sync --extra dev` provisions
# from uv.lock and the pytest call is the hook's own (.pre-commit-config.yaml),
# so green here means what a clean `git push` means locally — on a host the
# developer does not have.
#
# SuiteSparse/KLU is provisioned differently per leg, and the asymmetry is
# deliberate rather than an inconsistency:
#
# * macOS installs nothing. Accelerate supplies the BLAS, so
# BNGSIM_KLU_AUTOBUILD (GH #209) builds the pinned KLU subset from source —
# which makes this the ONLY job anywhere that exercises that path. Every
# wheel leg resolves a prebuilt SuiteSparse through SUITESPARSE_ROOT and
# every other job sets ENABLE_KLU=OFF, so the fallback an sdist install on a
# bare box depends on had no CI at all.
# * Linux installs libsuitesparse-dev, because on a bare ubuntu-latest the
# autobuild does NOT work: SuiteSparse's own CMake calls find_package(BLAS)
# and the runner image ships no BLAS, so the build dies in SuiteSparse_config
# before KLU is reached. Measured here, not assumed — the first run of this
# job failed exactly that way, which means GH #209's self-sufficiency claim
# holds on macOS but not on a bare Linux host. Installing the system package
# is what cibuildwheel's Linux leg already does (`dnf install
# suitesparse-devel`), so this leg matches the shipped wheel either way.
#
# The HAS_KLU assertion below is what keeps both honest — without it a build that
# quietly lost KLU would skip the sparse tests and still report green.
#
# Two coverage axes are deliberately NOT taken here, so they are not mistaken for
# oversights:
#
# * macOS x86_64. mir.yml already runs a macos-15-intel leg, and a
# macOS-x86_64-specific *Python-level* regression is the least likely of the
# three. Adding it is one line in the matrix.
# * The Python floor. requires-python is >=3.10 and every job here runs 3.12,
# so nothing tests the floor at runtime (ruff's target-version=py310 catches
# syntax, not stdlib API). Adding it is one more matrix entry.
#
# One known skip is environmental rather than structural: $BNGPATH is unset, so
# the 14 BNG2.pl round-trip tests in test_sbml_to_bngl.py skip. Closing that needs
# the perl toolchain plus the `parity` dependency group (a git+https reference to
# PyBioNetGen), which is a heavier provisioning step than this gate warrants; the
# skip is declared in conftest's _DECLARED_SKIPS and shows up in the audit table.

on:
push:
branches: [main]
pull_request:
workflow_dispatch:
inputs:
pytest_args:
description: "Extra pytest args (e.g. -x, -k some_pattern, or a single file path)"
required: false
default: ""

concurrency:
group: python-tests-${{ github.ref }}
cancel-in-progress: true

jobs:
python-tests:
name: Python suite · ${{ matrix.os }}
runs-on: ${{ matrix.os }}
timeout-minutes: 90
strategy:
fail-fast: false
matrix:
include:
- os: ubuntu-latest # x86_64 Linux — no Python coverage at all before #169
- os: macos-14 # macOS arm64 — covered only by one developer's pre-push hook
steps:
- uses: actions/checkout@v4

- name: Install uv
uses: astral-sh/setup-uv@v5
with:
enable-cache: true
cache-dependency-glob: "uv.lock"

# See the SuiteSparse note in the header: Linux only, because the runner
# image has no BLAS for the from-source autobuild to link against. macOS
# deliberately gets nothing so its leg exercises BNGSIM_KLU_AUTOBUILD.
- name: Install SuiteSparse (Linux)
if: runner.os == 'Linux'
run: |
set -euxo pipefail
sudo apt-get update
sudo apt-get install -y libsuitesparse-dev

# --extra dev rather than an enumerated extras list: `dev` is the set a
# developer's `uv sync` installs, so this job and the pre-push hook see the
# same environment, and an extra added to `dev` later reaches CI without a
# workflow edit. It is what un-skips the antimony / roadrunner / xarray /
# h5py / pandas / jax / vivarium guarded tests — ~50 items that skip on a
# bare install. Building bngsim itself is the slow part (SUNDIALS is
# FetchContent'd and the KLU subset is built from source).
- name: Provision from uv.lock (default build config)
run: uv sync --extra dev --python 3.12

- name: Show build / capability info
run: |
set -euxo pipefail
uv run --no-sync python -c "import platform, sys; print(platform.platform()); print(sys.version)"
uv run --no-sync python -c "import bngsim; print('bngsim', bngsim.__version__); print('HAS_KLU', bngsim.HAS_KLU); print('HAS_NFSIM', bngsim.HAS_NFSIM); print('HAS_MIR', bngsim.HAS_MIR)"
# The point of this job is the DEFAULT configuration, and KLU is the
# part of it nothing else in CI builds. A build that silently lost KLU
# would skip the sparse-solver tests and still be green, so fail here
# instead — the same false-green guard mir.yml puts on HAS_MIR.
uv run --no-sync python -c "import bngsim, sys; sys.exit(0 if bngsim.HAS_KLU else 'HAS_KLU is False — the default build did not link SuiteSparse/KLU')"
# The runtime codegen path shells out to this compiler at simulate time.
uv run --no-sync python -c "from bngsim import _codegen; print('C compiler:', _codegen._find_c_compiler())"

# BNGSIM_SKIP_AUDIT=strict turns any skip whose reason is not in conftest's
# _DECLARED_SKIPS into a failure. That is the second half of the false-green
# guard below: the count floor catches tests that vanish from collection,
# strict catches tests that quietly turn into skips for a reason nobody
# signed off on.
- name: Run the full Python suite
id: pytest
env:
BNGSIM_SKIP_AUDIT: strict
BNGSIM_TEST_DATA: ${{ github.workspace }}/tests/data
run: |
set -o pipefail
uv run --no-sync python -m pytest -p no:cacheprovider python/tests \
-q --tb=short --durations=15 ${{ github.event.inputs.pytest_args }} \
2>&1 | tee python-tests-${{ matrix.os }}.log

- name: Guard against a false green
if: ${{ !github.event.inputs.pytest_args }}
run: |
set -euo pipefail
# A directory-scoped pytest cannot drift out of sync with a file list,
# but it can still shrink: 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,
# which is the #28/#36 shape native-tests.yml guards with its own
# floors. Two denominators here, for the same reason it needs two:
#
# 1. COLLECTED (passed + skipped + xfailed + xpassed). This is the
# structural one, and it is nearly environment-INDEPENDENT: 3490 on
# ubuntu-latest, on macos-14, and on a developer's macOS box, all at
# the commit that added this job. Every skip in this suite is
# function- or class-level, so a corpus being absent moves tests
# between the columns without changing the total. A drop here means
# tests stopped being collected. Floor sits ~10 below.
# 2. PASSED. This one IS environment-sensitive (3415 on macOS, 3411 on
# Linux — the difference is lanl/bngsim#176's four xfails), so its
# floor is looser. It exists to catch the case COLLECTED cannot: a
# whole group converting to skips for a *declared* reason, e.g. an
# extra silently failing to install and taking the 13 roadrunner
# tests with it. Floor sits ~30 below, so it trips on a group and
# not on one test.
#
# Both floors only rise as tests are added; additions never break them,
# only silent removals do.
COLLECTED_FLOOR=3480
PASS_FLOOR=3380

LOG="python-tests-${{ matrix.os }}.log"
summary="$(grep -E '[0-9]+ (passed|failed).* in [0-9.]+s' "$LOG" | tail -1 || true)"
if [ -z "$summary" ]; then
echo "::error::No pytest summary line — the suite did not run to completion."
exit 1
fi
echo "summary: $summary"

# "3 xpassed" cannot match "[0-9]+ passed", so the categories stay disjoint.
count() { echo "$summary" | grep -oE "[0-9]+ $1" | grep -oE '^[0-9]+' || echo 0; }
passed="$(count passed)"
collected=$(( passed + $(count skipped) + $(count xfailed) + $(count xpassed) ))

echo "collected $collected (floor $COLLECTED_FLOOR); passed $passed (floor $PASS_FLOOR)."
if [ "$collected" -lt "$COLLECTED_FLOOR" ]; then
echo "::error::Only $collected tests collected, expected >= $COLLECTED_FLOOR — tests stopped being collected rather than failing."
exit 1
fi
if [ "$passed" -lt "$PASS_FLOOR" ]; then
echo "::error::Only $passed tests passed, expected >= $PASS_FLOOR — a group of tests turned into skips."
exit 1
fi

# Conditioned on the pytest step having been reached, not just on always():
# if the build fails there is no log to upload, and if-no-files-found=error
# would then report a second, misleading failure on top of the real one.
# Keeping `error` (rather than downgrading to `warn`) means a pytest step
# that ran but produced no log is still caught.
- name: Upload pytest log
if: always() && steps.pytest.conclusion != 'skipped'
uses: actions/upload-artifact@v4
with:
name: python-tests-log-${{ matrix.os }}
path: python-tests-${{ matrix.os }}.log
if-no-files-found: error
76 changes: 76 additions & 0 deletions CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,6 +15,20 @@ in `CMakeLists.txt`) is derived from it.
## [Unreleased]

### Fixed
- **A LAPACK-dense skip that no `_DECLARED_SKIPS` entry matched (found while
wiring issue #169).** Two files skip for the same build-variant condition and
phrase it differently: `test_engine_choice_accessors.py` says `"LAPACK-dense
not built in this configuration"`, `test_lapack_dense_solver.py` says `"build
links no BLAS dense backend (Accelerate / LAPACK)"`. Only the first was
declared, so the second read as an *undeclared* skip — the audit's signal for
"a test stopped running and nobody decided it should".

It could not be seen on macOS: `find_package` always resolves Accelerate there,
so neither test skips at all. It shows up only where CMake finds no BLAS, and
until #169 no CI job ran the full suite anywhere but a developer's macOS box.
The second phrasing is declared now, so `BNGSIM_SKIP_AUDIT=strict` does not
fail a Linux leg for a legitimate build-variant skip.

- **The #161 analytic sensitivity RHS was a net regression on the model it
targeted, and the cost was in the build, not in the emitted code (issue
#165).** `Smith_BMCSystBiol2013` (133 species, 16 sensitivity columns,
Expand DownExpand Up@@ -153,6 +167,68 @@ in `CMakeLists.txt`) is derived from it.
refusal and turns the sensitivity column into a real one.

### Added
- **A cross-platform Python test gate (issue #169).**
`.github/workflows/python-tests.yml` runs the whole of `python/tests` on
`ubuntu-latest` and `macos-14` in the **default** build configuration. Before
it, 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, so the count exercised in the configuration a
wheel actually ships was **zero** on Linux and macOS. A regression in SBML
loading, `.net` parsing, events, steady state, SSA, conversion or coupling
could be caught only on Windows, and only if the file happened to be named in
one of two hand-maintained lists.

The gap was structural, not a bug. Every job that runs pytest names a curated
file list, so a *new* test file defaults to running nowhere, and the assumed
backstop was the local pre-push hook — which covers whichever platform the
developer happens to be on (here macOS arm64) and which `git push --no-verify`
removes. `native-tests.yml` stated that assumption in its header as fact.

The new job therefore carries no `paths:` filter (a selectively-firing gate
reintroduces the per-file opt-in through the trigger instead of the run list),
no file list (it runs the directory, the way `native-tests.yml` drives `ctest`
rather than one named target), and no `-D` overrides (every other workflow
disables something — KLU off in four of them, MIR *on* in `mir.yml` — which is
how the shipped configuration ended up untested). Provisioning is `uv sync
--extra dev` off `uv.lock` and the pytest call is the pre-push hook's own, so a
green run means what a clean `git push` means locally, on a host the developer
does not have.

Two false-green guards, mirroring the ones `native-tests.yml` added for the C++
suite: a floor on the passed count, because a module that stops importing skips
at collection and shrinks the denominator without failing anything; and
`BNGSIM_SKIP_AUDIT=strict`, because a test that quietly turns into a skip for
an undeclared reason is the same invisibility in a different form. `HAS_KLU` is
asserted after the build for the reason `mir.yml` asserts `HAS_MIR` — a build
that silently lost KLU would skip the sparse-solver tests and still be green.

Side effect worth naming: the macOS leg is the only place anywhere that
exercises `BNGSIM_KLU_AUTOBUILD` (GH #209). The wheel legs all resolve a
prebuilt SuiteSparse through `SUITESPARSE_ROOT` and every other job sets
`ENABLE_KLU=OFF`, so the from-source KLU subset that an sdist install on a bare
box falls back to had no CI at all. Wiring it up immediately showed why that
matters: the same autobuild **cannot** complete on a bare `ubuntu-latest`,
because SuiteSparse's own CMake calls `find_package(BLAS)` and the runner image
ships none, so the configure dies in `SuiteSparse_config` before KLU is
reached. So GH #209's self-sufficiency claim holds on macOS but not on a bare
Linux host. The Linux leg therefore installs `libsuitesparse-dev`, the same
system-package route cibuildwheel's Linux leg takes.

And the gate earned itself on its first real run: **four tests fail on Linux
that pass on macOS**, in two unrelated subsystems, both pre-existing on `main`
and both exactly the class #169 said nothing could see. GH #176's
finite-difference retry fires correctly on
`ltype_calcium_discontinuous_jacobian.net` and then dies at a *second*
threshold crossing (t≈34.6) the fixture's own header does not mention; and
`nested_derived_rate_const.net`'s reduced Jacobian is exactly singular under
Linux's reference LAPACK where Accelerate leaves it merely ill-conditioned, so
it takes the refusal branch its sibling test exists to assert rather than the
warning branch its own test asserts. Both are quarantined under
`xfail(sys.platform.startswith("linux"), strict=True, raises=SimulationError)`
and reported in lanl/bngsim#176 — `strict` so they retire themselves, and
quarantined at all so the new gate lands green rather than permanently red,
which is the distinction `native-tests.yml`'s header spells out.

- **`compartment_sizes=` at load, the supported way to change a volume (issue
#164).** `Model.from_sbml`, `from_sbml_string`, `from_antimony`,
`from_antimony_string`, and `Model.load` take
Expand Down
8 changes: 8 additions & 0 deletions SUPPORT_MATRIX.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -77,6 +77,14 @@ environment. `lint.yml`, `native-tests.yml`, `mir.yml`, `windows-nfsim.yml` and
`windows-tail.yml` cover the pre-commit hooks, the C++ unit suite, the MIR JIT
backend, and the Windows NFsim/RuleMonkey paths respectively.

`python-tests.yml` is the Python suite's own gate: the whole of `python/tests`,
on `ubuntu-latest` and `macos-14`, in the **default** build configuration — no
`-D` overrides, so KLU, NFsim and RuleMonkey are all on and the MIR backend is
off, which is what a wheel ships. It has no `paths:` filter and names no test
files, so it fires on every change and picks up new tests without a workflow
edit. Every other job above disables something or runs a curated list, which is
how the shipped configuration went untested on Linux and macOS until issue #169.

Check the latest results before trusting this table:

```bash
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' ci(python-tests): run the whole Python suite on Linux and macOS (#169) by wshlavacek · Pull Request #175 · lanl/bngsim · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .github/workflows/native-tests.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,8 +5,15 @@ name: native-tests
# Until #38 NOTHING in CI built these tests. Every workflow that compiles the
# project passes BNGSIM_BUILD_TESTS=OFF (mir.yml, windows-nfsim.yml,
# windows-tail.yml), and no job ever invoked the binaries — so the suite's only
# gate was a developer remembering to build it by hand. The pre-push hook
# covers python/tests; the C++ side had no equivalent at any stage.
# gate was a developer remembering to build it by hand. The C++ side had no
# equivalent at any stage of CI.
#
# The original wording here was "the pre-push hook covers python/tests", offered
# as the contrast that made the C++ gap worse. Issue #169 showed that assumption
# was itself the other half of the problem: a local hook covers one platform and
# `--no-verify` removes it, so python/tests had no cross-platform CI gate either.
# python-tests.yml (ubuntu + macos-14, whole suite, default build) is that gate
# now; this job remains the C++ half.
#
# That is not hypothetical. #28 flipped SteadyStateOptions::method from "newton"
# to "integration" and updated the Python side, leaving the C++ assertion in
Expand Down
227 changes: 227 additions & 0 deletions .github/workflows/python-tests.yml
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
name: python-tests

# The cross-platform Python gate (issue #169).
#
# Before this job, GitHub CI ran 533 of the suite's ~3490 Python tests on Linux
# or macOS — and every one of them under BNGSIM_CODEGEN_JIT=mir on a
# -DBNGSIM_ENABLE_MIR=ON build. The number exercised in the DEFAULT configuration
# on any non-Windows host was zero. lint.yml runs no pytest by design;
# native-tests.yml is the C++ suite; wheels.yml runs only the cibuildwheel import
# smoke test; windows-tail.yml and windows-nfsim.yml are Windows-only. So a
# regression in SBML loading, .net parsing, events, steady state, SSA, conversion
# or coupling could only be caught on Windows, and only if the file happened to
# be named in one of two hand-maintained lists.
#
# The gap was structural, not a bug. Every job that runs pytest names a curated
# file list, so a NEW test file defaults to running nowhere, and the backstop was
# assumed to be the local pre-push hook — which covers one platform (whichever
# the developer is on, here macOS arm64) and which `git push --no-verify`
# removes. native-tests.yml stated that assumption outright.
#
# Three properties of this job are therefore deliberate, and each one is the
# thing that closes a specific half of the gap:
#
# * NO paths filter. A gate that fires selectively reintroduces the per-file
# opt-in through the trigger instead of through the run list. lint.yml takes
# the same position for the same reason.
# * NO file list. It runs `python/tests` as a directory, so a file added
# tomorrow runs here with no workflow edit — the property native-tests.yml
# gets from driving ctest rather than one named target.
# * NO -D overrides. Every other workflow disables something (KLU off in
# mir/windows-tail/windows-nfsim/native-tests, MIR on in mir.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.
#
# It is the pre-push hook, on two more platforms. `uv sync --extra dev` provisions
# from uv.lock and the pytest call is the hook's own (.pre-commit-config.yaml),
# so green here means what a clean `git push` means locally — on a host the
# developer does not have.
#
# SuiteSparse/KLU is provisioned differently per leg, and the asymmetry is
# deliberate rather than an inconsistency:
#
# * macOS installs nothing. Accelerate supplies the BLAS, so
# BNGSIM_KLU_AUTOBUILD (GH #209) builds the pinned KLU subset from source —
# which makes this the ONLY job anywhere that exercises that path. Every
# wheel leg resolves a prebuilt SuiteSparse through SUITESPARSE_ROOT and
# every other job sets ENABLE_KLU=OFF, so the fallback an sdist install on a
# bare box depends on had no CI at all.
# * Linux installs libsuitesparse-dev, because on a bare ubuntu-latest the
# autobuild does NOT work: SuiteSparse's own CMake calls find_package(BLAS)
# and the runner image ships no BLAS, so the build dies in SuiteSparse_config
# before KLU is reached. Measured here, not assumed — the first run of this
# job failed exactly that way, which means GH #209's self-sufficiency claim
# holds on macOS but not on a bare Linux host. Installing the system package
# is what cibuildwheel's Linux leg already does (`dnf install
# suitesparse-devel`), so this leg matches the shipped wheel either way.
#
# The HAS_KLU assertion below is what keeps both honest — without it a build that
# quietly lost KLU would skip the sparse tests and still report green.
#
# Two coverage axes are deliberately NOT taken here, so they are not mistaken for
# oversights:
#
# * macOS x86_64. mir.yml already runs a macos-15-intel leg, and a
# macOS-x86_64-specific *Python-level* regression is the least likely of the
# three. Adding it is one line in the matrix.
# * The Python floor. requires-python is >=3.10 and every job here runs 3.12,
# so nothing tests the floor at runtime (ruff's target-version=py310 catches
# syntax, not stdlib API). Adding it is one more matrix entry.
#
# One known skip is environmental rather than structural: $BNGPATH is unset, so
# the 14 BNG2.pl round-trip tests in test_sbml_to_bngl.py skip. Closing that needs
# the perl toolchain plus the `parity` dependency group (a git+https reference to
# PyBioNetGen), which is a heavier provisioning step than this gate warrants; the
# skip is declared in conftest's _DECLARED_SKIPS and shows up in the audit table.

on:
push:
branches: [main]
pull_request:
workflow_dispatch:
inputs:
pytest_args:
description: "Extra pytest args (e.g. -x, -k some_pattern, or a single file path)"
required: false
default: ""

concurrency:
group: python-tests-${{ github.ref }}
cancel-in-progress: true

jobs:
python-tests:
name: Python suite · ${{ matrix.os }}
runs-on: ${{ matrix.os }}
timeout-minutes: 90
strategy:
fail-fast: false
matrix:
include:
- os: ubuntu-latest # x86_64 Linux — no Python coverage at all before #169
- os: macos-14 # macOS arm64 — covered only by one developer's pre-push hook
steps:
- uses: actions/checkout@v4

- name: Install uv
uses: astral-sh/setup-uv@v5
with:
enable-cache: true
cache-dependency-glob: "uv.lock"

# See the SuiteSparse note in the header: Linux only, because the runner
# image has no BLAS for the from-source autobuild to link against. macOS
# deliberately gets nothing so its leg exercises BNGSIM_KLU_AUTOBUILD.
- name: Install SuiteSparse (Linux)
if: runner.os == 'Linux'
run: |
set -euxo pipefail
sudo apt-get update
sudo apt-get install -y libsuitesparse-dev

# --extra dev rather than an enumerated extras list: `dev` is the set a
# developer's `uv sync` installs, so this job and the pre-push hook see the
# same environment, and an extra added to `dev` later reaches CI without a
# workflow edit. It is what un-skips the antimony / roadrunner / xarray /
# h5py / pandas / jax / vivarium guarded tests — ~50 items that skip on a
# bare install. Building bngsim itself is the slow part (SUNDIALS is
# FetchContent'd and the KLU subset is built from source).
- name: Provision from uv.lock (default build config)
run: uv sync --extra dev --python 3.12

- name: Show build / capability info
run: |
set -euxo pipefail
uv run --no-sync python -c "import platform, sys; print(platform.platform()); print(sys.version)"
uv run --no-sync python -c "import bngsim; print('bngsim', bngsim.__version__); print('HAS_KLU', bngsim.HAS_KLU); print('HAS_NFSIM', bngsim.HAS_NFSIM); print('HAS_MIR', bngsim.HAS_MIR)"
# The point of this job is the DEFAULT configuration, and KLU is the
# part of it nothing else in CI builds. A build that silently lost KLU
# would skip the sparse-solver tests and still be green, so fail here
# instead — the same false-green guard mir.yml puts on HAS_MIR.
uv run --no-sync python -c "import bngsim, sys; sys.exit(0 if bngsim.HAS_KLU else 'HAS_KLU is False — the default build did not link SuiteSparse/KLU')"
# The runtime codegen path shells out to this compiler at simulate time.
uv run --no-sync python -c "from bngsim import _codegen; print('C compiler:', _codegen._find_c_compiler())"

# BNGSIM_SKIP_AUDIT=strict turns any skip whose reason is not in conftest's
# _DECLARED_SKIPS into a failure. That is the second half of the false-green
# guard below: the count floor catches tests that vanish from collection,
# strict catches tests that quietly turn into skips for a reason nobody
# signed off on.
- name: Run the full Python suite
id: pytest
env:
BNGSIM_SKIP_AUDIT: strict
BNGSIM_TEST_DATA: ${{ github.workspace }}/tests/data
run: |
set -o pipefail
uv run --no-sync python -m pytest -p no:cacheprovider python/tests \
-q --tb=short --durations=15 ${{ github.event.inputs.pytest_args }} \
2>&1 | tee python-tests-${{ matrix.os }}.log

- name: Guard against a false green
if: ${{ !github.event.inputs.pytest_args }}
run: |
set -euo pipefail
# A directory-scoped pytest cannot drift out of sync with a file list,
# but it can still shrink: 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,
# which is the #28/#36 shape native-tests.yml guards with its own
# floors. Two denominators here, for the same reason it needs two:
#
# 1. COLLECTED (passed + skipped + xfailed + xpassed). This is the
# structural one, and it is nearly environment-INDEPENDENT: 3490 on
# ubuntu-latest, on macos-14, and on a developer's macOS box, all at
# the commit that added this job. Every skip in this suite is
# function- or class-level, so a corpus being absent moves tests
# between the columns without changing the total. A drop here means
# tests stopped being collected. Floor sits ~10 below.
# 2. PASSED. This one IS environment-sensitive (3415 on macOS, 3411 on
# Linux — the difference is lanl/bngsim#176's four xfails), so its
# floor is looser. It exists to catch the case COLLECTED cannot: a
# whole group converting to skips for a *declared* reason, e.g. an
# extra silently failing to install and taking the 13 roadrunner
# tests with it. Floor sits ~30 below, so it trips on a group and
# not on one test.
#
# Both floors only rise as tests are added; additions never break them,
# only silent removals do.
COLLECTED_FLOOR=3480
PASS_FLOOR=3380

LOG="python-tests-${{ matrix.os }}.log"
summary="$(grep -E '[0-9]+ (passed|failed).* in [0-9.]+s' "$LOG" | tail -1 || true)"
if [ -z "$summary" ]; then
echo "::error::No pytest summary line — the suite did not run to completion."
exit 1
fi
echo "summary: $summary"

# "3 xpassed" cannot match "[0-9]+ passed", so the categories stay disjoint.
count() { echo "$summary" | grep -oE "[0-9]+ $1" | grep -oE '^[0-9]+' || echo 0; }
passed="$(count passed)"
collected=$(( passed + $(count skipped) + $(count xfailed) + $(count xpassed) ))

echo "collected $collected (floor $COLLECTED_FLOOR); passed $passed (floor $PASS_FLOOR)."
if [ "$collected" -lt "$COLLECTED_FLOOR" ]; then
echo "::error::Only $collected tests collected, expected >= $COLLECTED_FLOOR — tests stopped being collected rather than failing."
exit 1
fi
if [ "$passed" -lt "$PASS_FLOOR" ]; then
echo "::error::Only $passed tests passed, expected >= $PASS_FLOOR — a group of tests turned into skips."
exit 1
fi

# Conditioned on the pytest step having been reached, not just on always():
# if the build fails there is no log to upload, and if-no-files-found=error
# would then report a second, misleading failure on top of the real one.
# Keeping `error` (rather than downgrading to `warn`) means a pytest step
# that ran but produced no log is still caught.
- name: Upload pytest log
if: always() && steps.pytest.conclusion != 'skipped'
uses: actions/upload-artifact@v4
with:
name: python-tests-log-${{ matrix.os }}
path: python-tests-${{ matrix.os }}.log
if-no-files-found: error
76 changes: 76 additions & 0 deletions CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,6 +15,20 @@ in `CMakeLists.txt`) is derived from it.
## [Unreleased]

### Fixed
- **A LAPACK-dense skip that no `_DECLARED_SKIPS` entry matched (found while
wiring issue #169).** Two files skip for the same build-variant condition and
phrase it differently: `test_engine_choice_accessors.py` says `"LAPACK-dense
not built in this configuration"`, `test_lapack_dense_solver.py` says `"build
links no BLAS dense backend (Accelerate / LAPACK)"`. Only the first was
declared, so the second read as an *undeclared* skip — the audit's signal for
"a test stopped running and nobody decided it should".

It could not be seen on macOS: `find_package` always resolves Accelerate there,
so neither test skips at all. It shows up only where CMake finds no BLAS, and
until #169 no CI job ran the full suite anywhere but a developer's macOS box.
The second phrasing is declared now, so `BNGSIM_SKIP_AUDIT=strict` does not
fail a Linux leg for a legitimate build-variant skip.

- **The #161 analytic sensitivity RHS was a net regression on the model it
targeted, and the cost was in the build, not in the emitted code (issue
#165).** `Smith_BMCSystBiol2013` (133 species, 16 sensitivity columns,
Expand DownExpand Up@@ -153,6 +167,68 @@ in `CMakeLists.txt`) is derived from it.
refusal and turns the sensitivity column into a real one.

### Added
- **A cross-platform Python test gate (issue #169).**
`.github/workflows/python-tests.yml` runs the whole of `python/tests` on
`ubuntu-latest` and `macos-14` in the **default** build configuration. Before
it, 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, so the count exercised in the configuration a
wheel actually ships was **zero** on Linux and macOS. A regression in SBML
loading, `.net` parsing, events, steady state, SSA, conversion or coupling
could be caught only on Windows, and only if the file happened to be named in
one of two hand-maintained lists.

The gap was structural, not a bug. Every job that runs pytest names a curated
file list, so a *new* test file defaults to running nowhere, and the assumed
backstop was the local pre-push hook — which covers whichever platform the
developer happens to be on (here macOS arm64) and which `git push --no-verify`
removes. `native-tests.yml` stated that assumption in its header as fact.

The new job therefore carries no `paths:` filter (a selectively-firing gate
reintroduces the per-file opt-in through the trigger instead of the run list),
no file list (it runs the directory, the way `native-tests.yml` drives `ctest`
rather than one named target), and no `-D` overrides (every other workflow
disables something — KLU off in four of them, MIR *on* in `mir.yml` — which is
how the shipped configuration ended up untested). Provisioning is `uv sync
--extra dev` off `uv.lock` and the pytest call is the pre-push hook's own, so a
green run means what a clean `git push` means locally, on a host the developer
does not have.

Two false-green guards, mirroring the ones `native-tests.yml` added for the C++
suite: a floor on the passed count, because a module that stops importing skips
at collection and shrinks the denominator without failing anything; and
`BNGSIM_SKIP_AUDIT=strict`, because a test that quietly turns into a skip for
an undeclared reason is the same invisibility in a different form. `HAS_KLU` is
asserted after the build for the reason `mir.yml` asserts `HAS_MIR` — a build
that silently lost KLU would skip the sparse-solver tests and still be green.

Side effect worth naming: the macOS leg is the only place anywhere that
exercises `BNGSIM_KLU_AUTOBUILD` (GH #209). The wheel legs all resolve a
prebuilt SuiteSparse through `SUITESPARSE_ROOT` and every other job sets
`ENABLE_KLU=OFF`, so the from-source KLU subset that an sdist install on a bare
box falls back to had no CI at all. Wiring it up immediately showed why that
matters: the same autobuild **cannot** complete on a bare `ubuntu-latest`,
because SuiteSparse's own CMake calls `find_package(BLAS)` and the runner image
ships none, so the configure dies in `SuiteSparse_config` before KLU is
reached. So GH #209's self-sufficiency claim holds on macOS but not on a bare
Linux host. The Linux leg therefore installs `libsuitesparse-dev`, the same
system-package route cibuildwheel's Linux leg takes.

And the gate earned itself on its first real run: **four tests fail on Linux
that pass on macOS**, in two unrelated subsystems, both pre-existing on `main`
and both exactly the class #169 said nothing could see. GH #176's
finite-difference retry fires correctly on
`ltype_calcium_discontinuous_jacobian.net` and then dies at a *second*
threshold crossing (t≈34.6) the fixture's own header does not mention; and
`nested_derived_rate_const.net`'s reduced Jacobian is exactly singular under
Linux's reference LAPACK where Accelerate leaves it merely ill-conditioned, so
it takes the refusal branch its sibling test exists to assert rather than the
warning branch its own test asserts. Both are quarantined under
`xfail(sys.platform.startswith("linux"), strict=True, raises=SimulationError)`
and reported in lanl/bngsim#176 — `strict` so they retire themselves, and
quarantined at all so the new gate lands green rather than permanently red,
which is the distinction `native-tests.yml`'s header spells out.

- **`compartment_sizes=` at load, the supported way to change a volume (issue
#164).** `Model.from_sbml`, `from_sbml_string`, `from_antimony`,
`from_antimony_string`, and `Model.load` take
Expand Down
8 changes: 8 additions & 0 deletions SUPPORT_MATRIX.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -77,6 +77,14 @@ environment. `lint.yml`, `native-tests.yml`, `mir.yml`, `windows-nfsim.yml` and
`windows-tail.yml` cover the pre-commit hooks, the C++ unit suite, the MIR JIT
backend, and the Windows NFsim/RuleMonkey paths respectively.

`python-tests.yml` is the Python suite's own gate: the whole of `python/tests`,
on `ubuntu-latest` and `macos-14`, in the **default** build configuration — no
`-D` overrides, so KLU, NFsim and RuleMonkey are all on and the MIR backend is
off, which is what a wheel ships. It has no `paths:` filter and names no test
files, so it fires on every change and picks up new tests without a workflow
edit. Every other job above disables something or runs a curated list, which is
how the shipped configuration went untested on Linux and macOS until issue #169.

Check the latest results before trusting this table:

```bash
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); ci(python-tests): run the whole Python suite on Linux and macOS (#169) by wshlavacek · Pull Request #175 · lanl/bngsim · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .github/workflows/native-tests.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,8 +5,15 @@ name: native-tests
# Until #38 NOTHING in CI built these tests. Every workflow that compiles the
# project passes BNGSIM_BUILD_TESTS=OFF (mir.yml, windows-nfsim.yml,
# windows-tail.yml), and no job ever invoked the binaries — so the suite's only
# gate was a developer remembering to build it by hand. The pre-push hook
# covers python/tests; the C++ side had no equivalent at any stage.
# gate was a developer remembering to build it by hand. The C++ side had no
# equivalent at any stage of CI.
#
# The original wording here was "the pre-push hook covers python/tests", offered
# as the contrast that made the C++ gap worse. Issue #169 showed that assumption
# was itself the other half of the problem: a local hook covers one platform and
# `--no-verify` removes it, so python/tests had no cross-platform CI gate either.
# python-tests.yml (ubuntu + macos-14, whole suite, default build) is that gate
# now; this job remains the C++ half.
#
# That is not hypothetical. #28 flipped SteadyStateOptions::method from "newton"
# to "integration" and updated the Python side, leaving the C++ assertion in
Expand Down
227 changes: 227 additions & 0 deletions .github/workflows/python-tests.yml
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
name: python-tests

# The cross-platform Python gate (issue #169).
#
# Before this job, GitHub CI ran 533 of the suite's ~3490 Python tests on Linux
# or macOS — and every one of them under BNGSIM_CODEGEN_JIT=mir on a
# -DBNGSIM_ENABLE_MIR=ON build. The number exercised in the DEFAULT configuration
# on any non-Windows host was zero. lint.yml runs no pytest by design;
# native-tests.yml is the C++ suite; wheels.yml runs only the cibuildwheel import
# smoke test; windows-tail.yml and windows-nfsim.yml are Windows-only. So a
# regression in SBML loading, .net parsing, events, steady state, SSA, conversion
# or coupling could only be caught on Windows, and only if the file happened to
# be named in one of two hand-maintained lists.
#
# The gap was structural, not a bug. Every job that runs pytest names a curated
# file list, so a NEW test file defaults to running nowhere, and the backstop was
# assumed to be the local pre-push hook — which covers one platform (whichever
# the developer is on, here macOS arm64) and which `git push --no-verify`
# removes. native-tests.yml stated that assumption outright.
#
# Three properties of this job are therefore deliberate, and each one is the
# thing that closes a specific half of the gap:
#
# * NO paths filter. A gate that fires selectively reintroduces the per-file
# opt-in through the trigger instead of through the run list. lint.yml takes
# the same position for the same reason.
# * NO file list. It runs `python/tests` as a directory, so a file added
# tomorrow runs here with no workflow edit — the property native-tests.yml
# gets from driving ctest rather than one named target.
# * NO -D overrides. Every other workflow disables something (KLU off in
# mir/windows-tail/windows-nfsim/native-tests, MIR on in mir.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.
#
# It is the pre-push hook, on two more platforms. `uv sync --extra dev` provisions
# from uv.lock and the pytest call is the hook's own (.pre-commit-config.yaml),
# so green here means what a clean `git push` means locally — on a host the
# developer does not have.
#
# SuiteSparse/KLU is provisioned differently per leg, and the asymmetry is
# deliberate rather than an inconsistency:
#
# * macOS installs nothing. Accelerate supplies the BLAS, so
# BNGSIM_KLU_AUTOBUILD (GH #209) builds the pinned KLU subset from source —
# which makes this the ONLY job anywhere that exercises that path. Every
# wheel leg resolves a prebuilt SuiteSparse through SUITESPARSE_ROOT and
# every other job sets ENABLE_KLU=OFF, so the fallback an sdist install on a
# bare box depends on had no CI at all.
# * Linux installs libsuitesparse-dev, because on a bare ubuntu-latest the
# autobuild does NOT work: SuiteSparse's own CMake calls find_package(BLAS)
# and the runner image ships no BLAS, so the build dies in SuiteSparse_config
# before KLU is reached. Measured here, not assumed — the first run of this
# job failed exactly that way, which means GH #209's self-sufficiency claim
# holds on macOS but not on a bare Linux host. Installing the system package
# is what cibuildwheel's Linux leg already does (`dnf install
# suitesparse-devel`), so this leg matches the shipped wheel either way.
#
# The HAS_KLU assertion below is what keeps both honest — without it a build that
# quietly lost KLU would skip the sparse tests and still report green.
#
# Two coverage axes are deliberately NOT taken here, so they are not mistaken for
# oversights:
#
# * macOS x86_64. mir.yml already runs a macos-15-intel leg, and a
# macOS-x86_64-specific *Python-level* regression is the least likely of the
# three. Adding it is one line in the matrix.
# * The Python floor. requires-python is >=3.10 and every job here runs 3.12,
# so nothing tests the floor at runtime (ruff's target-version=py310 catches
# syntax, not stdlib API). Adding it is one more matrix entry.
#
# One known skip is environmental rather than structural: $BNGPATH is unset, so
# the 14 BNG2.pl round-trip tests in test_sbml_to_bngl.py skip. Closing that needs
# the perl toolchain plus the `parity` dependency group (a git+https reference to
# PyBioNetGen), which is a heavier provisioning step than this gate warrants; the
# skip is declared in conftest's _DECLARED_SKIPS and shows up in the audit table.

on:
push:
branches: [main]
pull_request:
workflow_dispatch:
inputs:
pytest_args:
description: "Extra pytest args (e.g. -x, -k some_pattern, or a single file path)"
required: false
default: ""

concurrency:
group: python-tests-${{ github.ref }}
cancel-in-progress: true

jobs:
python-tests:
name: Python suite · ${{ matrix.os }}
runs-on: ${{ matrix.os }}
timeout-minutes: 90
strategy:
fail-fast: false
matrix:
include:
- os: ubuntu-latest # x86_64 Linux — no Python coverage at all before #169
- os: macos-14 # macOS arm64 — covered only by one developer's pre-push hook
steps:
- uses: actions/checkout@v4

- name: Install uv
uses: astral-sh/setup-uv@v5
with:
enable-cache: true
cache-dependency-glob: "uv.lock"

# See the SuiteSparse note in the header: Linux only, because the runner
# image has no BLAS for the from-source autobuild to link against. macOS
# deliberately gets nothing so its leg exercises BNGSIM_KLU_AUTOBUILD.
- name: Install SuiteSparse (Linux)
if: runner.os == 'Linux'
run: |
set -euxo pipefail
sudo apt-get update
sudo apt-get install -y libsuitesparse-dev

# --extra dev rather than an enumerated extras list: `dev` is the set a
# developer's `uv sync` installs, so this job and the pre-push hook see the
# same environment, and an extra added to `dev` later reaches CI without a
# workflow edit. It is what un-skips the antimony / roadrunner / xarray /
# h5py / pandas / jax / vivarium guarded tests — ~50 items that skip on a
# bare install. Building bngsim itself is the slow part (SUNDIALS is
# FetchContent'd and the KLU subset is built from source).
- name: Provision from uv.lock (default build config)
run: uv sync --extra dev --python 3.12

- name: Show build / capability info
run: |
set -euxo pipefail
uv run --no-sync python -c "import platform, sys; print(platform.platform()); print(sys.version)"
uv run --no-sync python -c "import bngsim; print('bngsim', bngsim.__version__); print('HAS_KLU', bngsim.HAS_KLU); print('HAS_NFSIM', bngsim.HAS_NFSIM); print('HAS_MIR', bngsim.HAS_MIR)"
# The point of this job is the DEFAULT configuration, and KLU is the
# part of it nothing else in CI builds. A build that silently lost KLU
# would skip the sparse-solver tests and still be green, so fail here
# instead — the same false-green guard mir.yml puts on HAS_MIR.
uv run --no-sync python -c "import bngsim, sys; sys.exit(0 if bngsim.HAS_KLU else 'HAS_KLU is False — the default build did not link SuiteSparse/KLU')"
# The runtime codegen path shells out to this compiler at simulate time.
uv run --no-sync python -c "from bngsim import _codegen; print('C compiler:', _codegen._find_c_compiler())"

# BNGSIM_SKIP_AUDIT=strict turns any skip whose reason is not in conftest's
# _DECLARED_SKIPS into a failure. That is the second half of the false-green
# guard below: the count floor catches tests that vanish from collection,
# strict catches tests that quietly turn into skips for a reason nobody
# signed off on.
- name: Run the full Python suite
id: pytest
env:
BNGSIM_SKIP_AUDIT: strict
BNGSIM_TEST_DATA: ${{ github.workspace }}/tests/data
run: |
set -o pipefail
uv run --no-sync python -m pytest -p no:cacheprovider python/tests \
-q --tb=short --durations=15 ${{ github.event.inputs.pytest_args }} \
2>&1 | tee python-tests-${{ matrix.os }}.log

- name: Guard against a false green
if: ${{ !github.event.inputs.pytest_args }}
run: |
set -euo pipefail
# A directory-scoped pytest cannot drift out of sync with a file list,
# but it can still shrink: 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,
# which is the #28/#36 shape native-tests.yml guards with its own
# floors. Two denominators here, for the same reason it needs two:
#
# 1. COLLECTED (passed + skipped + xfailed + xpassed). This is the
# structural one, and it is nearly environment-INDEPENDENT: 3490 on
# ubuntu-latest, on macos-14, and on a developer's macOS box, all at
# the commit that added this job. Every skip in this suite is
# function- or class-level, so a corpus being absent moves tests
# between the columns without changing the total. A drop here means
# tests stopped being collected. Floor sits ~10 below.
# 2. PASSED. This one IS environment-sensitive (3415 on macOS, 3411 on
# Linux — the difference is lanl/bngsim#176's four xfails), so its
# floor is looser. It exists to catch the case COLLECTED cannot: a
# whole group converting to skips for a *declared* reason, e.g. an
# extra silently failing to install and taking the 13 roadrunner
# tests with it. Floor sits ~30 below, so it trips on a group and
# not on one test.
#
# Both floors only rise as tests are added; additions never break them,
# only silent removals do.
COLLECTED_FLOOR=3480
PASS_FLOOR=3380

LOG="python-tests-${{ matrix.os }}.log"
summary="$(grep -E '[0-9]+ (passed|failed).* in [0-9.]+s' "$LOG" | tail -1 || true)"
if [ -z "$summary" ]; then
echo "::error::No pytest summary line — the suite did not run to completion."
exit 1
fi
echo "summary: $summary"

# "3 xpassed" cannot match "[0-9]+ passed", so the categories stay disjoint.
count() { echo "$summary" | grep -oE "[0-9]+ $1" | grep -oE '^[0-9]+' || echo 0; }
passed="$(count passed)"
collected=$(( passed + $(count skipped) + $(count xfailed) + $(count xpassed) ))

echo "collected $collected (floor $COLLECTED_FLOOR); passed $passed (floor $PASS_FLOOR)."
if [ "$collected" -lt "$COLLECTED_FLOOR" ]; then
echo "::error::Only $collected tests collected, expected >= $COLLECTED_FLOOR — tests stopped being collected rather than failing."
exit 1
fi
if [ "$passed" -lt "$PASS_FLOOR" ]; then
echo "::error::Only $passed tests passed, expected >= $PASS_FLOOR — a group of tests turned into skips."
exit 1
fi

# Conditioned on the pytest step having been reached, not just on always():
# if the build fails there is no log to upload, and if-no-files-found=error
# would then report a second, misleading failure on top of the real one.
# Keeping `error` (rather than downgrading to `warn`) means a pytest step
# that ran but produced no log is still caught.
- name: Upload pytest log
if: always() && steps.pytest.conclusion != 'skipped'
uses: actions/upload-artifact@v4
with:
name: python-tests-log-${{ matrix.os }}
path: python-tests-${{ matrix.os }}.log
if-no-files-found: error
76 changes: 76 additions & 0 deletions CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,6 +15,20 @@ in `CMakeLists.txt`) is derived from it.
## [Unreleased]

### Fixed
- **A LAPACK-dense skip that no `_DECLARED_SKIPS` entry matched (found while
wiring issue #169).** Two files skip for the same build-variant condition and
phrase it differently: `test_engine_choice_accessors.py` says `"LAPACK-dense
not built in this configuration"`, `test_lapack_dense_solver.py` says `"build
links no BLAS dense backend (Accelerate / LAPACK)"`. Only the first was
declared, so the second read as an *undeclared* skip — the audit's signal for
"a test stopped running and nobody decided it should".

It could not be seen on macOS: `find_package` always resolves Accelerate there,
so neither test skips at all. It shows up only where CMake finds no BLAS, and
until #169 no CI job ran the full suite anywhere but a developer's macOS box.
The second phrasing is declared now, so `BNGSIM_SKIP_AUDIT=strict` does not
fail a Linux leg for a legitimate build-variant skip.

- **The #161 analytic sensitivity RHS was a net regression on the model it
targeted, and the cost was in the build, not in the emitted code (issue
#165).** `Smith_BMCSystBiol2013` (133 species, 16 sensitivity columns,
Expand DownExpand Up@@ -153,6 +167,68 @@ in `CMakeLists.txt`) is derived from it.
refusal and turns the sensitivity column into a real one.

### Added
- **A cross-platform Python test gate (issue #169).**
`.github/workflows/python-tests.yml` runs the whole of `python/tests` on
`ubuntu-latest` and `macos-14` in the **default** build configuration. Before
it, 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, so the count exercised in the configuration a
wheel actually ships was **zero** on Linux and macOS. A regression in SBML
loading, `.net` parsing, events, steady state, SSA, conversion or coupling
could be caught only on Windows, and only if the file happened to be named in
one of two hand-maintained lists.

The gap was structural, not a bug. Every job that runs pytest names a curated
file list, so a *new* test file defaults to running nowhere, and the assumed
backstop was the local pre-push hook — which covers whichever platform the
developer happens to be on (here macOS arm64) and which `git push --no-verify`
removes. `native-tests.yml` stated that assumption in its header as fact.

The new job therefore carries no `paths:` filter (a selectively-firing gate
reintroduces the per-file opt-in through the trigger instead of the run list),
no file list (it runs the directory, the way `native-tests.yml` drives `ctest`
rather than one named target), and no `-D` overrides (every other workflow
disables something — KLU off in four of them, MIR *on* in `mir.yml` — which is
how the shipped configuration ended up untested). Provisioning is `uv sync
--extra dev` off `uv.lock` and the pytest call is the pre-push hook's own, so a
green run means what a clean `git push` means locally, on a host the developer
does not have.

Two false-green guards, mirroring the ones `native-tests.yml` added for the C++
suite: a floor on the passed count, because a module that stops importing skips
at collection and shrinks the denominator without failing anything; and
`BNGSIM_SKIP_AUDIT=strict`, because a test that quietly turns into a skip for
an undeclared reason is the same invisibility in a different form. `HAS_KLU` is
asserted after the build for the reason `mir.yml` asserts `HAS_MIR` — a build
that silently lost KLU would skip the sparse-solver tests and still be green.

Side effect worth naming: the macOS leg is the only place anywhere that
exercises `BNGSIM_KLU_AUTOBUILD` (GH #209). The wheel legs all resolve a
prebuilt SuiteSparse through `SUITESPARSE_ROOT` and every other job sets
`ENABLE_KLU=OFF`, so the from-source KLU subset that an sdist install on a bare
box falls back to had no CI at all. Wiring it up immediately showed why that
matters: the same autobuild **cannot** complete on a bare `ubuntu-latest`,
because SuiteSparse's own CMake calls `find_package(BLAS)` and the runner image
ships none, so the configure dies in `SuiteSparse_config` before KLU is
reached. So GH #209's self-sufficiency claim holds on macOS but not on a bare
Linux host. The Linux leg therefore installs `libsuitesparse-dev`, the same
system-package route cibuildwheel's Linux leg takes.

And the gate earned itself on its first real run: **four tests fail on Linux
that pass on macOS**, in two unrelated subsystems, both pre-existing on `main`
and both exactly the class #169 said nothing could see. GH #176's
finite-difference retry fires correctly on
`ltype_calcium_discontinuous_jacobian.net` and then dies at a *second*
threshold crossing (t≈34.6) the fixture's own header does not mention; and
`nested_derived_rate_const.net`'s reduced Jacobian is exactly singular under
Linux's reference LAPACK where Accelerate leaves it merely ill-conditioned, so
it takes the refusal branch its sibling test exists to assert rather than the
warning branch its own test asserts. Both are quarantined under
`xfail(sys.platform.startswith("linux"), strict=True, raises=SimulationError)`
and reported in lanl/bngsim#176 — `strict` so they retire themselves, and
quarantined at all so the new gate lands green rather than permanently red,
which is the distinction `native-tests.yml`'s header spells out.

- **`compartment_sizes=` at load, the supported way to change a volume (issue
#164).** `Model.from_sbml`, `from_sbml_string`, `from_antimony`,
`from_antimony_string`, and `Model.load` take
Expand Down
8 changes: 8 additions & 0 deletions SUPPORT_MATRIX.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -77,6 +77,14 @@ environment. `lint.yml`, `native-tests.yml`, `mir.yml`, `windows-nfsim.yml` and
`windows-tail.yml` cover the pre-commit hooks, the C++ unit suite, the MIR JIT
backend, and the Windows NFsim/RuleMonkey paths respectively.

`python-tests.yml` is the Python suite's own gate: the whole of `python/tests`,
on `ubuntu-latest` and `macos-14`, in the **default** build configuration — no
`-D` overrides, so KLU, NFsim and RuleMonkey are all on and the MIR backend is
off, which is what a wheel ships. It has no `paths:` filter and names no test
files, so it fires on every change and picks up new tests without a workflow
edit. Every other job above disables something or runs a curated list, which is
how the shipped configuration went untested on Linux and macOS until issue #169.

Check the latest results before trusting this table:

```bash
Expand Down
Loading
Loading