Test modules must not build state while being collected

[lmoresi]

Closes#505. Closes#575. Closes one of the gaps in #570.

pytest imports a test module in order to collect it, so anything a module does at
module level runs before any test, any fixture, and any isolation
tests/conftest.py provides. Two defects have reached development that way:
#567, where a module switched the units system on at import and the first
module-scoped fixture in that worker built its mesh under dimensional
coordinates; and #505, where a module ran two Stokes solves at import, so
--collect-only sat inside SNESSolve looking like a silent death.

PR #574 fixed the first instance. This fixes the practice.

The three files that had no tests in them

test_0050_utils.py, test_0120_data_property_access.py and
test_0130_field_creation.py were converted scripts. Between them they built two
meshes, a swarm, two Stokes solvers, and solved both — all at module level — and
contained no test function at all. 0120 and 0130 wrapped every statement in a
try/except that printed the exception, so a broken .data property or a
duplicate field_id printed a cross and the run stayed green. 0050's only
assertion was disabled by the name dont_test_auditor.

Each is now written as fixtures and tests, and each keeps its original subject:

• 0050 — the installation auditor, and a second Stokes solver built over the
  fields the first already owns. The auditor check is a delta rather than the
  uw_object_count == 7 it used to assert: the counter is process-wide and
  monotonic, so an absolute count is only true in a fresh process running that
  file alone, which is why the assertion could not survive being enabled. Its
  control is that reading the auditor does not itself move the count.
• 0120 — a write through a swarm variable's .data is read back and compared
  with what was written, twice, which is what exercises the cache.
• 0130 — three variables on one mesh get three distinct field_ids, and
  .array is shaped by the variable's own component count.

Importing test_0050_utils.py cost 2.0 s of the 5.2 s import with warm JIT and
mesh caches; the 20+ minutes recorded on #505 was that work on a cold cache.

The guard

tests/conftest.py fingerprints three pieces of process-global state —
the count of uw_objects created, the active model's reference quantities, and
the strict-units flag — before and after each module is collected, and reports
the difference as a collection error against that module. The fingerprint is
skipped when underworld3 is not importable, since conftest is loaded before the
package is necessarily installed in CI.

A collection error rather than a session abort, because the first form aborted
from pytest_collection_finish and that leaves an xdist worker part-collected:
the controller reports INTERNALERROR> assert not crashitem at return code 3,
which is the mode CI runs in. pytest.exit(returncode=4) behaves the same. The
check now lives in a pytest_make_collect_report wrapper, which sees the report
before it is dispatched. Measured on a module that builds a mesh at import:

	offending module	clean tree
serial	RC=2, ERROR tests/test_offender.py	RC=0
-n 2	RC=1, 10 passed, 1 error	RC=0

tests/test_0051_collection_state_guard.py runs pytest on a module that offends
and on one that does not, loading the hooks out of the real conftest.py by
path. The second run is the control: without it the first proves only that
something failed.

UW_TEST_COLLECTION_GUARD=off turns the guard off. test_0742 needs it: that
test copies the live conftest into a pytester sub-run together with a module
that leaks units at import, because what it pins is that the module-scoped reset
survives exactly that. The guard refused the module and ended the sub-run before
it reached its assertion — correct behaviour on a deliberately generated
offender. test_0742 now sets the variable for its sub-run only, and
test_0051 covers both settings so the off switch cannot become the default
without a test failing.

The ratchet

Eighteen modules already move that state, so they are exempted by name in
_KNOWN_COLLECTION_TIME_WORK. The guard therefore stops the practice spreading
today rather than after eighteen files have been rewritten. The list is #587,
with test_0601_mesh_vector_calc.py named first because it is the only entry
that moves the units state — use_strict_units(False) at import, which is the
#567 mechanism itself, reaching every module collected after it.

scripts/test.sh disables tests/test_06*py wholesale, which is why 0601's
import-time flip has never been noticed by our own CI: it is collected by any
whole-directory run and executed by none of ours.

scripts/test.sh

tests/test_0050*py is no longer commented out. It was disabled because the
auditor assertion could not pass, which the delta form fixes.

Verified

• pytest tests/ --collect-only exits 0 with 2741 tests collected; before, it
  reported eighteen modules and, with the three files above, ran two Stokes
  solves.
• The guard was confirmed to fire by adding a module-level mesh to a scratch
  file: it named that file and no other.
• Full ./uw test: 1504 passed, 32 skipped, 2 xfailed.

The fingerprint is not a general leak detector. It sees objects built and the
units state; a module that writes an environment variable, a PETSc option, or a
monkeypatched attribute at import passes it. The rule it enforces is the
stricter and simpler one — build nothing while being collected.

Underworld development team with AI support from Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[lmoresi]

Adversarial review

Reviewed against the branch head (18e3833). Four findings, none blocking; the
fifth item is a property we checked rather than a finding.

1. The ratchet matches by suffix, so it exempts more than the eighteen files.
_is_known_offender compares nodeid.endswith(known). Any path ending in a
listed basename is exempt, in any directory — a future
tests/parallel/test_1010_stokesCart.py inherits the exemption without anyone
deciding it should. The reason it is a suffix match is that pytest.ini lives
in tests/, so a run under scripts/test.sh produces nodeids without the
tests/ prefix while a run from the repo root produces them with it, and an
exact match would have to know which. We are accepting this because the list is
closed and only ever shrinks (#587), but it is a hole while it exists.

2. The auditor delta could pass for the wrong reason.after - before >= 2
is a lower bound over a mesh and a variable, and mesh construction alone may
account for both. If MeshVariable stopped registering as a uw_object the
assertion would very likely still hold. We chose the bound over an exact count
deliberately — an exact count is what made the old == 7 unrunnable — and the
control that reading the auditor does not itself move the count is what stops
the test being vacuous. It is a weaker test than the file used to intend.

3. test_second_solver_over_the_same_fields does not check either answer.
It asserts both fields are finite, the first is non-zero, and the two differ.
A regression that made the second solve wrong in a way that differed from the
first would pass. That is the scope we want here — Stokes accuracy is
test_1010's subject, and this file's is that a second solver over the same
MeshVariables is not served the first one's setup — but the assertion is
weaker than it reads.

4. The off switch is process-wide and unqualified.UW_TEST_COLLECTION_GUARD=off
exported in a shell disables the guard for every run from that shell, silently.
Nothing prints when the guard is off. test_0742 needs the switch and sets it
with monkeypatch for its sub-run only, which is the right scope, but the
mechanism does not enforce that scope on anyone else.

5. Checked: the fingerprint does nothing collective. It reads
uw_object._obj_count, model._default_model and _STRICT_UNITS_MODE — all
rank-local. This matters because the fingerprint runs twice per module inside a
collection hook, and the auditor's own get_runtime_data does an MPI.bcast;
calling that here would have put a collective in the collection path of every
mpirun test job. get_runtime_data is used only inside test_0050's own
test body.

Underworld development team with AI support from Claude Code