Skip to content

Add a pytest plugin for testflo-style MPI test execution - #131

Open
robfalck wants to merge 28 commits into
OpenMDAO:masterfrom
robfalck:pytest_plugin2
Open

robfalck wants to merge 28 commits into
OpenMDAO:masterfrom
robfalck:pytest_plugin2

Conversation

@robfalck

@robfalck robfalck commented Oct 1, 2026 •

Copy link
Copy Markdown

Summary

Adds testflo.pytest_plugin, a pytest plugin (registered via the pytest11 entry point, no conftest.py needed) that brings testflo's MPI execution model — spawn mpirun -n N, run the test on every rank, gather results — into pytest suites, including full pytest-xdist support. Ships alongside testflo rather than replacing it; testflo itself takes no new dependency since pytest/mpi4py/psutil are all optional extras (testflo[pytest]).

Why

testflo's own runner handles MPI tests well pytest has become the standard. Teams that have already standardized on pytest (fixtures, markers, -k/-m selection, xdist, JUnit XML, pytest-cov, IDE integration) had no way to get testflo's "just spawn mpirun and run the test on every rank" model without leaving pytest. This plugin gives them that, while staying optional and backward compatible with the existing testflo runner.

Now that we're also looking into multiprocessing, we needed a tool that could schedule tests and dedicate a given number of processors to them, either under an MPI environment or just a python environment with multiprocessing used within.

A few existing mpi-related pytest plugins either relied on the pytest invocation itself to be done under MPI, or spawned tests in a monolithic MPI environment.

Marking tests for parallelization

Two independent markers, so they can be selected separately:

import pytest

@pytest.mark.mpi(4)
def test_foo(comm):
    assert comm.size == 4

@pytest.mark.mpi([2, 4])          # parametrizes into test_bar[nprocs=2], test_bar[nprocs=4]
def test_bar(comm):
    assert comm.allreduce(1) == comm.size

@pytest.mark.multiprocessing(4)   # no mpirun spawned; runs in-process on a size-1 FakeComm
def test_pool():
    with multiprocessing.Pool(4) as pool:
        ...

@pytest.mark.mpi(N) is canonical; nprocs=N is an accepted alias. A bare @pytest.mark.mpi defaults to 2 ranks. multiprocessing has no default — a bare @pytest.mark.multiprocessing is a UsageError, since there's no sensible pool size to assume. The two compose: mpi(4) + multiprocessing(2) costs 4 * 2 = 8 cores (each rank spawns its own pool).

Existing testflo-style N_PROCS unittest.TestCase classes work unchanged — N_PROCS is treated as an alias for mpi(nprocs=N_PROCS), including for -m mpi selection.

pytest -m 'not mpi' runs everything that doesn't need mpirun (multiprocessing-only tests included) — useful for a fast pass that still exercises pool scaling without MPI's overhead or hang risk.

Execution modes

The plugin detects which of three contexts it's running in purely from environment variables (never by importing mpi4py in the host process — that would initialize MPI and make the host unsafe to fork mpirun from):

flowchart TD
    start([pytest process starts]) --> childcheck{TESTFLO_PYTEST_CHILD=1?}
    childcheck -- yes --> child["<b>Child mode</b><br/>test runs normally on this rank;<br/>results gathered on COMM_WORLD<br/>at sessionfinish, rank 0 writes JSON"]
    childcheck -- no --> outercheck{"OMPI_COMM_WORLD_SIZE / PMI_SIZE /<br/>MV2_COMM_WORLD_SIZE / MSMPI_RANK_SIZE<br/>&gt; 1?"}
    outercheck -- yes --> outer["<b>Outer-mpirun mode</b><br/>user ran `mpirun -n N pytest ...` themselves;<br/>tests whose mpi size == N run in place,<br/>everything else is skipped;<br/>xdist is rejected (UsageError)"]
    outercheck -- no --> launcher["<b>Launcher mode</b><br/>(the normal case)"]
    launcher --> spec{"item's mpi size &gt; 1?"}
    spec -- yes --> mpi["reserve cores, then spawn<br/>`mpirun -n N python -m pytest &lt;nodeid&gt;`,<br/>synthesize TestReports from the result"]
    spec -- no --> serial["reserve cores (if under xdist),<br/>run normally in this process<br/>(FakeComm size 1, or real COMM_WORLD<br/>if MPI happens to already be initialized)"]
Loading

Node IDs are identical between launcher and child invocations, so -k/-m selection, parametrize IDs, and JUnit XML all look like ordinary pytest output — a 4-rank MPI test just shows up as one test with per-rank tracebacks and captured output attached if it fails.

Scheduling under pytest-xdist

Every test — serial or parallel — is distributed freely by xdist; there's no special grouping. Instead, each test reserves the cores it needs from a budget shared across all workers just before it runs, so a big MPI test simply waits until enough cores are free while serial tests keep flowing around it. The budget is hosted by a multiprocessing.managers server the xdist controller starts (pytest_configure/pytest_configure_node); workers connect to it over loopback, so a waiting acquire() blocks on a condition variable in the server rather than polling.

Requests are served FIFO with one relaxation (a request may pass ones queued ahead of it as long as it still leaves room for the oldest), so a big test can't be starved indefinitely but small tests still flow. Because xdist can't hand a blocked test back once a worker has committed to it, the plugin defaults --dist to worksteal whenever xdist is active and the user hasn't set --dist explicitly — idle workers steal the tests queued behind a blocked one instead of everyone stalling.

A worker that dies mid-test (holding or waiting for cores) is detected and reaped once a second so it can't starve the rest of the run. A test whose cost alone exceeds the budget fails immediately instead of hanging forever.

Running it

# plain run — no setup needed, the plugin registers itself
pytest

# only tests that don't need mpirun (still runs multiprocessing-marked tests)
pytest -m 'not mpi'

# parallel run; defaults to --dist worksteal automatically
pytest -n auto

# cap or remove the core budget
pytest -n 4 --max-concurrent-cores=8
pytest --oversubscribe                      # or TESTFLO_PYTEST_OVERSUBSCRIBE=1

# run mpi-marked tests in-process on a size-1 FakeComm (no MPI needed at all)
pytest --nompi

# guard against deadlocks from a rank-local failure before a collective call
pytest --mpi-timeout=60

# point at an mpirun/mpiexec not on PATH
pytest --mpirun-exe=/opt/openmpi/bin/mpirun

Running with coverage

MPI-marked tests execute inside a spawned mpirun, never in the pytest process that reports them, so coverage has to be collected in the ranks and merged back in. This rides entirely on coverage.py's own subprocess support — nothing here is pytest-cov-specific, so pytest --cov=... and coverage run -m pytest behave the same:

# .coveragerc, or [tool.coverage.run] in pyproject.toml
[run]
patch = subprocess        # measure spawned mpirun ranks; implies parallel = true
source = mypkg

patch = subprocess makes coverage export a serialized config that the .pth file it installs picks up to start tracing in every freshly-spawned rank. parallel = true is forced on by it and is required — without it every rank writes the same data file and they clobber each other.

If a test also spawns its own worker pool (@pytest.mark.multiprocessing(M), including inside an mpi rank), add _exit too — a pool worker exits via os._exit, which skips the atexit hook coverage normally saves from:

[run]
patch = _exit, subprocess
pytest --cov=mypkg --cov-report=term-missing
# or
coverage run -m pytest
coverage combine     # same as you'd already do with xdist
coverage report

If none of this is configured, the plugin falls back to synthesizing the same COVERAGE_PROCESS_CONFIG environment for the ranks from whatever coverage is already running in the launcher, so a bare pytest --cov=mypkg still measures MPI-only code instead of silently reporting 0%. The one case this can't cover: a rank killed by --mpi-timeout or MPI_Abort never reaches its own save, so its coverage is missing (not corrupted) for that run.

Running with and without pytest-sugar

The plugin reports MPI results by synthesizing normal TestReport objects through the standard pytest_runtest_logreport hook, so any terminal reporter that just listens to that hook — including pytest-sugar — sees ordinary test outcomes and needs no awareness of this plugin:

pip install pytest-sugar
pytest -n auto                 # sugar's progress bar, same synthesized MPI reports
pytest -n auto -p no:sugar     # fall back to plain dot/verbose output for one run (useful for CI)

Known limitations

  • MPICH on macOS: a networking issue in MPICH's OFI provider can surface as OFI poll failed (... Input/output error) during MPI finalization when ranks are forcefully killed (e.g. a --mpi-timeout SIGKILL). Not seen in normal use; switch to Open MPI if it shows up in stress/timeout-heavy runs.
  • A test whose core cost alone exceeds --max-concurrent-cores is reported as a failure rather than run anyway.

Other changes bundled in this branch

  • CI (test_workflow.yml): matrix expanded to Ubuntu × macOS, each against both Open MPI and MPICH, via pixi environments; added a dedicated "Test pytest plugin" step (pytest ... -n auto, run once normally and once with --nompi).
  • testflo's own self-test suite: previously, testflo's self-tests validated its pass/fail/skip counting by mixing genuinely-passing checks with tests designed to fail/skip/xfail in the same files, so a plain testflo . always "failed" even when everything worked. Moved the intentionally-broken fixtures to self_report_fixtures.py (outside testflo's default discovery pattern) and added test_self_test.py, which runs testflo against those fixtures as a subprocess and asserts the counts — so a standard testflo run now legitimately exits 0.
  • Fixed TESTFLO_RUNNING not being set under the pytest plugin, so tests written against that env var behave the same under testflo and pytest.

Test plan

  • pytest testflo/tests/pytest_tests/test_pytest_plugin.py -n auto — 45 tests covering marker parsing/validation, core-budget fairness and starvation-avoidance, xdist worksteal defaulting, child-mode and outer-mpirun-mode result handling, coverage propagation (patch=subprocess and the fallback synthesis, plus multiprocessing + _exit), and --nompi/--mpi-timeout/--oversubscribe.
  • Same suite run again with pytest-sugar installed — no change in outcome.
  • CI matrix: Ubuntu + macOS, Open MPI + MPICH.

Related Issues

  • Resolves #

Backwards incompatibilities

None

New Dependencies

None

robfalck and others added 28 commits July 7, 2026 14:33
  Adds testflo/pytest_plugin.py: a pytest plugin that brings testflo's MPI
  execution model to pytest suites. Tests marked with @pytest.mark.parallel(N)
  or with a N_PROCS class attribute are run under a spawned mpirun subprocess;
  per-rank results are gathered and synthesized into normal pytest TestReports.

  Key features:
  - --nompi flag runs parallel tests in-process on a FakeComm (size 1)
  - --mpi-timeout guards against deadlocks from desynchronized collectives
  - --mpi-workers=N distributes MPI tests across N xdist workers round-robin;
  --dist loadgroup is applied automatically when xdist is active
  - --mpi-concurrent-slots=N caps total MPI ranks in flight across workers
  - Outer-mpirun mode: if the user runs mpirun -n N pytest ... directly,
  tests whose nprocs matches the world size run in place

  Adds testflo/tests/test_pytest_plugin.py: end-to-end pytester tests covering
  pass/fail/skip/xfail propagation, parametrized nprocs, N_PROCS TestCase,
  captured output, timeout/deadlock, xdist mixed suites, loadgroup pinning,
  concurrent slot budgeting, --nompi, and max-nprocs guard.

  Adds pixi workspace config to pyproject.toml (linux-64, osx-arm64) with
  openmpi and mpi4py dependencies.
…t assertions to check scheduler mode rather than node ID suffixes
…from testflo via --skip_dir=pytest_tests in pixi task and CI
…o tests are found when running from /Users/rfalck
Marker
- @pytest.mark.parallel now takes mpi=N and multiprocessing=M keywords.
  parallel(N) and parallel(nprocs=N) remain aliases for mpi=N.
  multiprocessing-only tests run in-process on a FakeComm (no mpirun);
  the two combine as parallel(mpi=N, multiprocessing=M).
- A test's core cost is max(mpi,1) * max(multiprocessing,1); serial
  tests cost 1.

Core budget (replaces --mpi-concurrent-slots, kept as deprecated alias)
- Every test in launcher mode reserves its core cost before it runs.
  The budget defaults to the cores available to the process
  (sched_getaffinity, else cpu_count); --max-concurrent-cores=N
  overrides it and --oversubscribe / TESTFLO_PYTEST_OVERSUBSCRIBE=1
  removes it. A test that can never fit is reported as failed.
- Serial tests count too, so a multi-core test waits for busy xdist
  workers instead of piling on top of them. Waiting requests are served
  FIFO: smaller requests may pass only if they leave room for the oldest
  waiter, so a large test is not starved by a stream of serial ones.
- Dropped the xdist loadgroup pinning (--mpi-workers, testflo_mpi_N
  groups). Tests are distributed freely; --dist worksteal is the default
  when xdist supports it, so a worker waiting for cores does not hold up
  the tests queued behind it.

_SlotLimiter
- Rewritten to use an OS byte-range lock (fcntl.flock / msvcrt.locking)
  on the state file itself instead of an O_EXCL lockfile. The old scheme
  leaked the lock on Windows when os.remove hit a PermissionError while
  another worker was reading it, stalling every worker; it also created
  and deleted a file per cycle. The OS releases the lock if its holder
  dies, so the stale-lock logic is gone. Dead holders/waiters are pruned
  when the budget looks full.

Refactor
- Single pytest_runtest_protocol handles reservation for all tests and
  either spawns mpirun or runs pytest's own runtestprotocol; removed
  the setup/teardown budget hooks and _LIMITER_KEY.
- MPI availability (mpi4py, mpirun) is decided at collection time via a
  skip marker rather than in pytest_runtest_setup.
- ParallelSpec(mpi, multiprocessing, cores) in one stash key replaces
  _NPROCS_KEY/_NCORES_KEY; _under_mpi() helper; _run_mpi_item slimmed
  with _reports/_failed_reports/_read_results.
- Requires pytest >= 7.4 (TestReport start/stop).

pixi / CI
- Added win-64 to the workspace. MPI deps in the openmpi/mpich features
  are targeted at linux-64/osx-arm64, so the default and pytest
  environments solve on Windows without MPI and MPI-dependent tests
  skip there.
- Dropped the --mpi-workers=2 invocation from the workflow.

Tests
- Marker parsing, multiprocessing Pool tests (standalone, under the
  budget with xdist, under --nompi), MPI x multiprocessing combinations,
  default budget and --oversubscribe precedence, serial tests counting
  against the budget, FIFO non-starvation, worksteal default, dead-holder
  pruning. The plugin's own suite sets TESTFLO_PYTEST_OVERSUBSCRIBE=1 and
  scrubs PYTEST_XDIST_TESTRUNUID so nested pytester runs neither depend
  on the runner's core count nor share the outer run's budget.
Core budget: file-locked JSON state -> in-memory tracker in a manager
- The xdist controller now starts a multiprocessing.managers server
  (spawn context, loopback port, random authkey) hosting one
  _CoreTracker. Workers get its address via xdist's workerinput
  (pytest_configure_node), connect lazily, and call acquire/release.
  A waiting acquire blocks on a Condition inside the server: no state
  file, no fcntl/msvcrt locking, no polling. Without xdist no manager
  is started (a single process has nothing to coordinate); with
  --oversubscribe none is started either. Shut down in
  pytest_unconfigure.
- _CoreTracker keeps the FIFO fairness rule (a request may pass those
  queued ahead only if it still leaves room for the oldest) and runs a
  reaper thread that drops dead holders *and dead waiters* -- a worker
  that dies mid-acquire would otherwise leave a blocked server thread
  with its ticket at the head of the queue.
- Liveness probe is psutil.pid_exists (added to the testflo[pytest]
  extra and the pixi pytest feature), falling back to os.kill(pid, 0).
  This replaces the ctypes OpenProcess/WaitForSingleObject block.
- Measured, 1000 serial tests on -n 8 (Windows): 1.77 s with no
  budget, 1.80 s budgeted. The file limiter cost 4.4 s, the original
  O_EXCL lockfile 13 s.

Alternatives considered for the budget, and why not
- Keep the file-locked JSON state (fcntl.flock / msvcrt.locking): it
  worked and was measured at ~2.5 ms/test under 8-way contention, but
  every worker serialises through one file for every test, it needs a
  poll loop while waiting, and it needed OS-specific locking code and
  a pid-liveness probe to recover from crashed workers.
- SQLite (WAL): still a file with the same byte-range locks
  underneath; every budget operation is a write, so WAL's concurrent
  readers buy nothing; multi-writer contention gives "database is
  locked" errors; waiting would still be a poll loop.
- OS named semaphores / shared memory: not portable across the
  Windows + macOS + Linux matrix without platform branches, and a
  semaphore cannot express variable-sized (N-core) requests or FIFO
  fairness.
- Per-worker "always busy" accounting (charge 1 core per worker at
  startup, only parallel tests touch the tracker): zero per-test cost,
  but under -n auto every worker counts as busy for the whole run, so
  a multi-core test could never fit until the serial queue drained.
  Serial tests must be counted dynamically for fairness to work.
- A plain Condition tracker without a queue (the obvious sketch):
  starves large requests exactly the way the pre-FIFO file limiter
  did, and cannot recover from a worker dying while blocked.
- Passing the tracker address through the environment: leaks into
  nested pytest sessions (pytester subprocesses inherited the outer
  run's id and deadlocked against the workers running them);
  workerinput is scoped to exactly this session's workers.

Liveness probe alternatives
- os.kill(pid, 0): on Windows it only does OpenProcess, which still
  succeeds for an *exited* process while any handle to it is held --
  e.g. the controller's handle on a crashed worker. Measured: reports
  such a process as alive. Kept only as the no-psutil fallback (exact
  on POSIX; errs toward "alive" on Windows, the safe direction).
- ctypes OpenProcess + WaitForSingleObject(0): correct, but raw Win32
  calls with hex constants, and OpenProcess failing with access denied
  was treated as "dead" (could prune a live worker).
- psutil.pid_exists: checks the exit code, so exited-but-handle-held
  reads as dead and access-denied reads as alive. Measured both.

Spawned mpirun timeout
- _run_mpirun: on --mpi-timeout, terminate() first (SIGTERM; Open MPI
  and hydra tear down their ranks), wait 10 s, then kill().
  subprocess.run(timeout=) SIGKILLed mpirun alone and could orphan
  ranks. Deliberately *not* a new process group: the child sharing the
  launcher's group/console is what makes Ctrl-C reach mpirun directly.
- mpirun --version probe timeout 30 s -> 10 s.

Kept as-is, after considering
- _clean_child_env blacklist: a whitelist (PATH, PYTHONPATH, ...) would
  silently break suites that depend on CONDA_PREFIX, LD_LIBRARY_PATH,
  HOME, TMPDIR, coverage/OPENMDAO_*/OMPI_MCA_* variables, proxies and
  application config. The scrub is defence in depth; the primary
  guarantee is that the launcher never initialises MPI.

Plugin simplification (no behaviour change unless noted)
- One pytest_runtest_protocol reserves cores for every launcher-mode
  test and then either spawns mpirun or runs pytest's runtestprotocol;
  removed the setup/teardown budget hooks and _LIMITER_KEY. Over-budget
  is now consistently a failed call report (was an error for
  in-process tests).
- MPI availability (mpi4py, mpirun) decided at collection via a skip
  marker instead of in pytest_runtest_setup; a user's own @Skip on an
  MPI test now skips locally instead of spawning mpirun.
- ParallelSpec(mpi, multiprocessing, cores) in one stash key replaces
  _NPROCS_KEY/_NCORES_KEY; _under_mpi() helper; _run_mpi_item slimmed
  with _reports/_failed_reports/_read_results.
- Child mode appends every report per nodeid; _aggregate_rank_results
  does the per-rank reduction (replaces _record_child_report's merge
  rules). Skip reasons travel as the (path, lineno, reason) tuple
  through JSON instead of repr + ast.literal_eval.
- --mpirun-exe resolved once in pytest_configure into config.stash
  (drops env-var hop, lru_cache and cache_clear).
- Removed TESTFLO_PYTEST_MAX_NPROCS (the budget subsumes it).
- Dropped the pytest < 7.4 TestReport fallback; requires pytest>=7.4.
- class Foo(object) -> class Foo.

Tests
- _CoreTracker unit tests: FIFO non-starvation, reaping of dead
  holders and dead waiters (with the process handle deliberately held
  open). Removed the .slots-file tests; the deprecated alias test now
  only checks the alias works.
- Interval-based scheduling tests write one file per test instead of
  a shared append log, which dropped lines under concurrent writers on
  Windows.
- Rank report aggregation tested with real pytest reports through a
  JSON round trip, so the child-mode path is covered without MPI.
- Fixture no longer needs to scrub PYTEST_XDIST_TESTRUNUID.

README
- Sequence diagram of a session: manager start, worker spawn with
  workerinput, worksteal, serial and multi-core acquire/release,
  mpirun spawn, reaper, shutdown.
…pi and multiprocessing markers

The old @pytest.mark.parallel(mpi=N, multiprocessing=M) conflated two orthogonal concerns under pytest's -m selection, so there was no way to select all MPI-only tests separately from all multiprocessing-only tests. This blocked a fast CI workflow for OpenMDAO 4.0 and Dymos 4.0: running pytest -m 'not mpi' to exercise multiprocessing scaling without the overhead or hang risk of spawning mpirun.

mpi(N) and multiprocessing(M) are now fully independent and composable. mpi keeps its DEFAULT_NPROCS default when given bare; multiprocessing has no sensible default pool size, so a bare @pytest.mark.multiprocessing raises UsageError. The legacy N_PROCS class attribute still works, now synthesizing mpi(nprocs=N) so -m mpi/-m 'not mpi' selection covers it too. This is a breaking change with no back-compat shim, since neither om4.git nor dymos4.git use the old parallel marker directly.
pytest_collection_modifyitems now sorts items so the most expensive
tests (mpi ranks * multiprocessing pool) come first, using the
ParallelSpec already stashed on each item. The sort is stable, so
tests of equal cost keep their collection (module) order.

Why: with the budget empty at the start of a run a multi-core test
fits immediately. Left in collection order it tends to reach the
tracker late, and the FIFO head-of-queue rule then has to drain the
other workers to make room for it at the tail of the run, with those
workers idle meanwhile.

Under --dist worksteal the initial distribution is still a contiguous
slice per worker, so this only front-loads the heavy tests into the
first worker's slice until stealing kicks in; the head-of-queue rule
remains what guarantees they eventually get their cores. Whether the
reorder shortens wall time on a real suite is to be measured.

Ordering is deterministic from the collection, so every xdist worker
produces the same list and xdist's same-collection check is
unaffected. In the mpirun child (one nodeid) it is a no-op.

Tests
- test_collection_sorted_by_core_cost: --collect-only --nompi over
  every marker form (bare mpi, mpi(N), mpi([N1, N2]) parametrization,
  multiprocessing(M), mpi+multiprocessing, serial) and asserts the
  cost-descending, stable order. Needs no MPI.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An mpi-marked test executes only inside the spawned `mpirun`, never in the
pytest process that reports it, so every line reachable only under MPI was
missing from coverage reports -- silently, with no error anywhere. The
plugin had no coverage handling at all; the legacy runner does
(testflo/cover.py, called from mpirun.py and isolatedrun.py).

Approach: ride coverage.py's own subprocess mechanism
- coverage.py already ships the whole thing. `[run] patch = subprocess`
  makes it export COVERAGE_PROCESS_CONFIG (a serialized, absolute-path
  config) and force parallel=true; the .pth file coverage installs then
  starts tracing in any fresh interpreter that inherits it. An mpirun rank
  is a fresh interpreter, so passing the variable through is sufficient.
- Nothing here is pytest-cov specific, so `pytest --cov=pkg` and
  `coverage run -m pytest` behave identically. pytest-cov 7.1 ships no .pth
  and sets no COV_CORE_* of its own -- it delegates to coverage.py too.
- parallel=true is not optional: process_startup() builds Coverage() with
  no data_suffix, so without it every rank writes the *same* data file and
  they overwrite each other, which is worse than collecting nothing.

_child_coverage_env(): the safety net
- If coverage is running but nothing arranged for subprocesses (a plain
  `pytest --cov=pkg`, which gives no hint extra config is needed), copy the
  live config, force parallel, serialize it into the child env. Reuses
  CoverageConfig.serialize(); the live config is not mutated.
- Returns {} whenever something else already owns tracing in the child --
  COVERAGE_PROCESS_CONFIG/START already set, or COV_CORE_SOURCE from a
  pytest-cov older than 7.0. Exactly one mechanism may start coverage in a
  process. Failures are swallowed: a coverage problem must not fail a run.

_clean_child_env(): preservation is now a guarantee
- COVERAGE_*, COV_CORE_* and PYTHONPATH were preserved only because they
  happened not to match scrub_prefixes. Documented as deliberate, with a
  test, since scrubbing any of them loses MPI coverage silently.

CHILD_PYTEST_ARGS: stop the child inheriting ini addopts
- Separate latent bug found while investigating. The child is a plain
  pytest run and inherited the project's addopts: `addopts = -n auto` made
  *every rank* spawn its own xdist cluster (pytest_sessionstart's xdist
  guard returns early in child mode, so nothing caught it), and
  `addopts = --cov=pkg` started a second coverage on top of the
  .pth-started one. Now passes `-o addopts=` and `-p no:xdist`.
- Extracted to a module constant so the argv is unit-testable without MPI.

Measured, not assumed (Windows, py3.14, coverage 7.16, pytest-cov 7.1)
- Plain subprocess inheriting COVERAGE_PROCESS_CONFIG: records and saves
  correctly. This is the mpirun-rank case, so the MPI path works.
- multiprocessing.Pool children: coverage *starts* (.pth fires, env
  inherited -- probed directly) but never saves, because
  BaseProcess._bootstrap exits via os._exit and skips the atexit hook.
  Neither `concurrency = multiprocessing` nor `patch = subprocess` alone
  fixes it; `patch = _exit, subprocess` does. Relevant to the composed
  mpi(N) + multiprocessing(M) case, where the pool lives inside a rank.

Considered and rejected
- Forwarding COVERAGE_PROCESS_START: coverage only ever *reads* that
  variable; neither `coverage run` nor pytest-cov sets it, so forwarding it
  alone is a no-op. COVERAGE_PROCESS_CONFIG carries the payload.
- A sitecustomize.py on PYTHONPATH: obsolete. coverage 7.16 ships
  pth_file.py, installed as a1_coverage.pth; there is no sitecustomize
  reference in the package. (PYTHONPATH still must not be scrubbed.)
- pytest-cov specific hooks: unnecessary, and would break `coverage run`.
- Calling setup_coverage() in the child the way mpirun.py does: the .pth
  starts tracing *before* any imports, so it also catches module-level
  lines that testflo's approach misses.

Tests (10; 7 run without MPI)
- Unit: env preservation incl. PYTHONPATH; _child_coverage_env forcing
  parallel, deferring to an existing owner, and no-op without coverage;
  CHILD_PYTEST_ARGS neutralizing addopts.
- test_child_coverage_env_payload_actually_measures: feeds the synthesized
  env to a real subprocess and asserts the child-only line was recorded --
  the safety net is validated end to end, not just shape-checked.
- test_subprocess_child_coverage: end-to-end through pytest-cov; same
  mechanism as an mpirun rank, so it guards the MPI path on every platform.
- test_mpi_rank_coverage / test_mpi_rank_with_pool_coverage: MPI-gated,
  run in CI's pytest/pytest-mpich envs.
- test_multiprocessing_pool_coverage: skipped on Windows per the finding
  above.
- No CI workflow change needed: these live in the file CI already runs.

Also in this commit: order the collection by descending core cost
- pytest_collection_modifyitems sorts items so the most expensive tests
  (mpi ranks * multiprocessing pool) come first, from the ParallelSpec
  already stashed on each item. Stable, so equal-cost tests keep file
  order. With the budget empty at the start of a run a multi-core test
  fits immediately; left in collection order it tends to reach the tracker
  late, and the FIFO head-of-queue rule then has to drain the other workers
  to make room for it at the tail, with those workers idle. Deterministic
  from the collection, so xdist's same-collection check is unaffected, and
  a no-op in the single-nodeid mpirun child. Whether it shortens wall time
  on a real suite is still to be measured.

pytest-cov added to the `test` extra and the pixi pytest feature; it was in
neither environment, which is why this gap went unnoticed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…on-order assertions

Pool.exit calls terminate(), which SIGTERMs idle workers on macOS/spawn before they can save coverage data through atexit. Add sigterm = true to the affected .coveragerc fixtures so coverage saves before re-raising the signal.

test_parametrized_nprocs and test_mpi_sizes_parametrized_with_pool asserted an nprocs=2-before-nprocs=3 execution order that no longer holds now that collection is sorted by descending core cost; update the expected order to nprocs=3 before nprocs=2.
testflo's own tests validated its pass/fail/skip counting by mixing genuinely-passing checks with tests designed to fail/skip/expectedFailure in the same files, so a plain `testflo .` or `pixi run test` always reported several failures even when everything worked correctly.

Move the intentionally-broken fixtures into self_report_fixtures.py, whose filename doesn't match testflo's default test discovery pattern, so it's never picked up by a standard invocation. Add test_self_test.py, a normal discovered test that runs testflo against those fixtures as a subprocess and asserts the resulting counts are correct -- so validating the counting logic is just another passing test rather than a wrapper script or separate CI step. Trim test_testflo.py and test_nested_fixtures.py down to the checks that should always pass, fixing test_subtests' deliberately-corrupted values in the process.

Simplify test_workflow.yml's Run tests steps accordingly, since a healthy run now legitimately exits 0 instead of expecting return code 1 and grepping for hardcoded counts.
Add `patch = fork` so forked Pool workers (Linux's default start method) get their own coverage data file instead of inheriting the parent's. Switch pool teardown in the coverage tests from `with Pool(...) as pool:` to `pool.close(); pool.join()`, avoiding a race between `Pool.terminate()`'s SIGTERM and coverage's sigterm-triggered save that made test_multiprocessing_pool_coverage flaky on macOS. Document both in the README, including a known-limitation note that `patch = fork` hangs when composed with mpi-nested pools due to MPI runtimes forking internally.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant