chore(ci): pass python -u to run_python_test_script.py callsites - #6071
Merged
Merged
Conversation
… os.system run_python_test_script.py uses os.system() to invoke pytest. Python's own stdout buffer (the script's print() calls) and the subprocess's output go to the same fd in CI, but the buffer doesn't flush before os.system spawns the shell, so the script's prints can flush *after* a subprocess that ran *before* them. Visible in a recent macos-build-test log as: ============================= test session starts ============================== platform darwin -- Python 3.10.20, pytest-7.4.4, pluggy-1.2.0 -- /opt/... cachedir: .pytest_cache rootdir: /Users/admin/actions-runner/_work/MeshLib/MeshLib/test_python plugins: check-2.3.1 collecting ... Namespace(cmd=None, multi_cmd=False, create_venv=False, ...) python3.10 -m pytest -s -v --basetemp=../pytest_temp ... ERROR: Some tests failed! `Namespace(...)` is the script's own `print(args)` from line 40 — it should appear FIRST in the output but ends up half-way through pytest's "collecting ..." line because of the buffer mix-up. The actual pytest test results / failure summaries between "collecting ..." and the script's "ERROR" are missing entirely; CI only gets a generic "Some tests failed" with no diagnostic. Fix: introduce a small `run_shell()` helper that calls `sys.stdout.flush()` + `sys.stderr.flush()` before `subprocess.run(cmd, shell=True, check=False)`. Replace every `os.system(...)` with `run_shell(...)`. Add `flush=True` to all `print(...)` calls for belt-and-suspenders. Add a "^^^ pytest exited with code N" line after a failed run so the actual exit code (5 = no tests collected, 2 = collection error, 1 = test failures, …) is in the log when the next person debugs a failure. Behavior is otherwise unchanged — same args, same exit code on failure.
…utput CI captures stdout via a pipe (block-buffered by default), which lets the script's own print() output flush after a child's output and reorder the log — visible in the macos-build-test failure where 'Namespace(...)' (printed at line 40, before the loop) ended up *inside* pytest's 'collecting ...' line. Switch sys.stdout/sys.stderr to line-buffered; keep the rest of the script (os.system + plain print) untouched.
…side
Move the line-buffering knob out of scripts/run_python_test_script.py
into each workflow callsite as the standard `-u` interpreter flag.
Same effect (Python writes stdout/stderr unbuffered, prints stay in
order with subprocess output in CI logs); no changes to the script
itself, so the fix is co-located with where the CI buffering matters.
Touched 8 callsites:
build-test-{linux-vcpkg,macos,ubuntu-arm64,ubuntu-x64}.yml
build-test-windows.yml
pip-build.yml (manylinux + windows + macos pip-build jobs)
Each `python3 …` becomes `python3 -u …`; the Windows `py -3 …` and
`py …` invocations both become `py -3 -u …`.
Grantim
approved these changes
May 8, 2026
Fedr
added a commit
that referenced
this pull request
May 8, 2026
Two follow-ups for the verify-meshlib-python-import step: 1) Add `timeout-minutes: 3` to each of the 8 callsites. The step is cheap (one or a few `import meshlib.mrmeshpy` calls), so 3 minutes is plenty even on slow runners; without it the step could in theory hang on a runner-side issue and inherit the surrounding job's much longer timeout (60–100 min). 2) Pass `-u` to python at the action invocation, same trick as #6071. Lets us drop the half-applied `flush=True` on individual prints in `scripts/ci/verify_meshlib_import.py` — `-u` flushes stdout/stderr unbuffered, so the script's own output stays in chronological order with the wrapping shell's `echo` lines (the per-shim `===== verify with pythonX.Y =====` headers, the final `Summary:` line, etc.) in the GitHub Actions log.
Fedr
added a commit
that referenced
this pull request
May 8, 2026
* ci: verify meshlib.mrmeshpy imports cleanly before Unit Tests
When the Python bindings are built but `mrmeshpy.pyd` / `.so` can't be
loaded — DLL load failure, missing PyInit_*, libpython ABI mismatch,
binding-generation regression that drops a referenced class — MRTest's
embedded-python smoke test surfaces the problem only as CPython's
opaque
ImportError: initialization failed
…with no traceback and a single `<string>(N): <module>` location.
Diagnosing requires reproducing the import out-of-process to get a
real traceback.
Add a small Python script (`scripts/ci/verify_meshlib_import.py`) that
runs `import meshlib` and `import meshlib.mrmeshpy` with PYTHONPATH
pointing at the build's bin dir, exits non-zero with a real Python
traceback on failure, and a composite action
(`.github/actions/verify-meshlib-python-import`) that invokes it with
the right shell on Windows (pwsh + py -3) vs Unix (bash + python3).
Wire the action into all eight build pipelines that run
`MRTest`/`MRTest.exe`, immediately before the Unit Tests step:
* build-test-windows.yml (Windows main matrix)
* build-test-linux-vcpkg.yml (Linux vcpkg)
* build-test-macos.yml (macOS)
* build-test-ubuntu-x64.yml (Ubuntu x64)
* build-test-ubuntu-arm64.yml (Ubuntu arm64)
* pip-build.yml (manylinux + windows + macos
pip-wheel jobs)
Each invocation respects the same `if:` gate the workflow already uses
to decide whether bindings were built (`inputs.mrbind` for the build-test
matrices, the iterator-debug exclusion on Windows, unconditional for
the pip-wheel jobs which always build bindings).
* ci: verify import for every pybind11 non-limited-api shim, not just one
Previous version probed for one shim and used the matching Python.
That works for build-test-* (one shim per build), but pip-build's
wheel jobs (FOR_WHEEL=1) build a shim per Python in
scripts/mrbind-pybind11/python_versions.txt — currently 3.8 .. 3.14.
Verify against all of them.
For each pybind11nonlimitedapi_meshlib_<X.Y>.<ext> in
<build-bin-dir>/meshlib/:
* if `pythonX.Y` (Unix) or `py -X.Y` (Windows) is on the runner,
invoke it on scripts/ci/verify_meshlib_import.py and tally
pass/fail.
* if not on the runner, log "SKIP: pythonX.Y not on PATH" so the
step's output is honest about coverage instead of silently
hiding gaps.
Step exits non-zero if any *attempted* import failed, prints a
summary line of shims/attempted/failed/skipped at the end. The
single-shim build-test-* case still works — the loop runs once
with the one matching Python.
* ci: import every native submodule in meshlib/, not just mrmeshpy
The existing single-import check `import meshlib.mrmeshpy` is too narrow:
`test_python/helper/__init__.py` does both `import meshlib.mrmeshpy as
mrmesh` AND `import meshlib.mrmeshnumpy as mrmeshnumpy` at module
load. If pytest collects a test file that pulls in `helper`, both of
those imports run during collection. A crash in mrmeshnumpy (or any
other shipped submodule) takes pytest's collector down silently —
exactly what we're seeing on Daniil's macOS runner.
Iterate over every native-extension file in `<build>/meshlib/`
(`*.{so,dylib,pyd}`), strip ABI tags like `.cpython-310-darwin`, skip
`libpybind11nonlimitedapi*` shims, and import each with traceback on
failure. Print a per-module OK/FAIL line and a tally summary at the
end. Same exit-code semantics as before (non-zero on any failure).
* ci: mirror exact helper imports — drop mrviewerpy/mrcudapy auto-discovery
A run on the new code (Ubuntu Debug) imported all 4 native submodules
in meshlib/ and exposed an unrelated pre-existing bug: importing
`meshlib.mrviewerpy` and letting Python shut down trips a C++ debug
assertion in MRViewer/MRCommandLoop.cpp (CommandLoop destructor
expecting a drained queue), aborting the process despite all 4
imports having succeeded:
OK: meshlib.mrmeshpy ...
OK: meshlib.mrmeshnumpy ...
OK: meshlib.mrviewerpy ...
Summary: 4/4 submodules imported OK
python3.10: source/MRViewer/MRCommandLoop.cpp:13:
MR::CommandLoop::~CommandLoop(): Assertion `commands_.empty()' failed.
Aborted (core dumped)
That's a real but separate bug from what this script is gating
against (binding load failures pre-pytest), and it makes the verify
step false-positive on Debug builds.
Drop auto-discovery; hardcode the list to exactly what
test_python/helper/__init__.py imports — `mrmeshpy` and `mrmeshnumpy`
— since `helper` is what pytest collection pulls in transitively
through every test file. That's the meaningful set for catching
collection-time silent crashes (the original Daniil failure mode).
Comment links the list to the helper file so the next person knows
to keep it in sync.
* ci: drop mrmeshnumpy from verify step — needs numpy not installed in pip-build env
Last run on pip-build's manylinux x86_64 job hit:
FAIL: import meshlib.mrmeshnumpy
ModuleNotFoundError: No module named 'numpy'
ImportError: initialization failed
That container's bare system Python has no numpy — the actual
Python Tests step uses `uv run --with-requirements requirements/python.txt`
to spawn each interpreter with numpy installed, but the verify step
runs against the bare system Python without that, so mrmeshnumpy's
module-init `import numpy` fails. False positive for the verify step.
Drop mrmeshnumpy from the import list. Keep mrmeshpy: it's the one
whose silent load failure was the original motivation, and it has no
external runtime deps. mrmeshnumpy load failures (in env where numpy
is actually present) still surface downstream in pytest collection.
* ci: add 3-minute timeout + run verify script with python -u
Two follow-ups for the verify-meshlib-python-import step:
1) Add `timeout-minutes: 3` to each of the 8 callsites. The step is
cheap (one or a few `import meshlib.mrmeshpy` calls), so 3 minutes
is plenty even on slow runners; without it the step could in
theory hang on a runner-side issue and inherit the surrounding
job's much longer timeout (60–100 min).
2) Pass `-u` to python at the action invocation, same trick as #6071.
Lets us drop the half-applied `flush=True` on individual prints
in `scripts/ci/verify_meshlib_import.py` — `-u` flushes
stdout/stderr unbuffered, so the script's own output stays in
chronological order with the wrapping shell's `echo` lines (the
per-shim `===== verify with pythonX.Y =====` headers, the final
`Summary:` line, etc.) in the GitHub Actions log.
* ci: shorten verify-step comments per review
Drop the long-winded background paragraphs from the script docstring and
the action comments; keep just enough to explain the per-shim loop.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
scripts/run_python_test_script.pyinvokes pytest viaos.system(...). CI captures stdout via a pipe — block-buffered by default — so the script's ownprint()output and the subprocess's output go to the same fd but at different times: Python's buffer holds the script's prints until the process exits, while pytest's output writes through directly. The result is thatprint()calls flush after a child that ran before them, reordering the log.Concretely visible in this recent macos-build-test failure (job 74880762447):
Namespace(...)is the script's ownprint(args)from line 40 — it should appear first in the output but flushed mid-line into pytest'scollecting .... Worse, all the actual pytest test results / failure summaries betweencollecting ...and the finalERROR:line are missing from the log; CI only sees a generic "Some tests failed" with no diagnostic.Changes
Pass
-u(unbuffered stdout/stderr) to the Python interpreter at every callsite that runsscripts/run_python_test_script.py. The script itself is unchanged.build-test-linux-vcpkg.ymlpython3 …python3 -u …build-test-macos.ymlpython3 …python3 -u …build-test-ubuntu-arm64.ymlpython3 …python3 -u …build-test-ubuntu-x64.ymlpython3 …python3 -u …build-test-windows.ymlpy -3 …py -3 -u …pip-build.ymlpython3 …(insideuv run)python3 -u …pip-build.ymlpy …py -3 -u …pip-build.ymlpython3 …python3 -u …8 lines added, 0 removed. No script change.
Test plan
collecting ... Namespace(...)interleaving. Verified on job 74984990805:Namespace(...)prints at line 10050 beforetest session startsat 10054 andcollecting ... collected 86 itemsat 10059.