Add two-stage pytest retry to SlangPy CI test jobs - #1123
Draft
nv-slang-bot[bot] wants to merge 1 commit into
Draft
Add two-stage pytest retry to SlangPy CI test jobs#1123nv-slang-bot[bot] wants to merge 1 commit into
nv-slang-bot[bot] wants to merge 1 commit into
Conversation
Contributor
|
Automated notice (PR board sync) — do not reply to this comment. Auto-assigned @jkiviluoto-nv as shepherd for this Bot PR. FYI for maintainers: committer signal on the changed files is highest for skallweitNV among collaborators other than the assignee. They were not auto-requested; a human may optionally add them as a reviewer. |
Contributor
|
Automated notice (PR board sync) — do not reply to this comment. Auto-assigned @jkiviluoto-nv as shepherd for this Bot PR. FYI for maintainers: committer signal on the changed files is highest for skallweitNV among collaborators other than the assignee. They were not auto-requested; a human may optionally add them as a reviewer. |
Parallel CI test jobs (pytest -n auto) can go red from GPU device contention between xdist workers rather than a real defect. Mirror the retry that Slang's old test-slangpy job had (removed in slang#9900): on a parallel-run failure, re-run only the failed tests once in a single process (-n 0 --lf) to distinguish contention flakiness from a real bug. Wrap unit_test_python and test_examples in tools/ci.py through a shared run_pytest_with_retry helper. The retry is gated on --parallel: only parallel runs suffer xdist contention, and the sanitizer jobs run without --parallel, so an intermittent ASan/UBSan finding is never hidden by a rerun. --cache-clear on the first attempt drops any stale last-failed record so the rerun can only select this run's failures; if the first attempt crashes before writing the cache, the empty --lf selection makes pytest exit 5, which run_command treats as a failure, so a real failure cannot be masked (cf. slang#11911). Add GPU-free tests in slangpy/tests/test_ci_retry.py covering the control flow and, via real throwaway pytest suites, the end-to-end invariants: a flaky test recovers green on the rerun, a deterministic failure stays red, and neither a stale cache nor an empty rerun selection can mask a real failure. Fixes #829
nv-slang-bot
Bot
force-pushed
the
dev/slangpy-fixer/829
branch
from
August 24, 2026 08:17
86b9da5 to
d7528d1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary (5-bullet)
tools/ci.py(pytest slangpy/tests/test_ci_retry.py, author's machine — not yet run on a built checkout / in CI); peer review dispatched to internal reviewer (verdict outstanding at open time).Motivation
SlangPy's parallel CI test jobs run
pytest -n auto(xdist). A test can go red from GPU device contention between xdist workers rather than a real defect, turning an entire job red with no second chance. Slang's oldtest-slangpyjob had a retry for exactly this (removed in shader-slang/slang#9900 when SlangPy CI was decoupled); the review comment that removed it called the missing SlangPy-side retry a "pre-existing gap … add the retries later in separate change." This is that change.Proposed solution
Mirror the old Slang pattern: on a parallel run failure, re-run only the failed tests, once, sequentially (
-n 0 --lf). A single-process rerun escapes flakiness caused by contention between xdist workers; a deterministic failure that reproduces sequentially still fails on the rerun and keeps the job red.The retry is added once, in
tools/ci.py, so it covers every parallel CI call site that routes throughunit_test_python/test_examples(ci.yml,ci-gcp.yml, and the composite action); the non-parallel sanitizer calls insanitizers.ymlroute through the same code but take the single-run branch (see the--parallelgate below). No new dependency —--lfis core pytest andpytest-xdistis already a dev requirement.Gated on
--parallel. The sequential rerun only earns its keep against parallel-only contention. The sanitizer jobs (sanitizers.yml) run without--parallel; auto-retrying an intermittent ASan/UBSan finding would mask it, so the non-parallel path runs exactly once.Change summary
tools/ci.pyrun_pytest_with_retry(test_path, env, parallel);unit_test_pythonandtest_examplesroute through it. Parallel: first attempt-n auto --maxprocesses=4 --cache-clear, on failure rerun-n 0 --lf --lfnf=all. Non-parallel: single run.slangpy/tests/test_ci_retry.pyrun_commandstub, 4 real-pytest integration) covering the retry flow and the false-green and re-execution safeguards.Concepts and vocabulary
--lf/ last-failed cache: pytest records failed node ids in<rootdir>/.pytest_cache;--lfreselects them on the next run. This cache lives at the pytest rootdir and is independent of--basetemp(which relocates the temporary-directory fixtures — for exampletmp_pathandtmpdir— not the cache).--lfnf=all(last-failed-no-failures): when the last-failed set is empty,--lfnf=all(the pytest default) reruns the whole suite rather than selecting nothing.EXIT_NOTESTSCOLLECTED;run_commandtreats any nonzero exit (including 5) as a failure.Process report
Why gate on
--parallel(input-shape check). The rerun's purpose is to distinguish contention between xdist workers from real failures. That specific contention exists only under-n auto; the non-parallel sanitizer runs don't spawn competing workers, and an intermittent sanitizer finding must not be retried away. So the non-parallel input is handled by not retrying — a deliberate, not incidental, choice.False-green and re-execution safeguards (cf. shader-slang/slang#11911). A retry that re-runs zero tests but reports success would convert a real failure into a green. The base guarantee is exit-code propagation; two flags address separate failure modes, each covered by a test that fails if the flag is removed:
--cache-clearon the first attempt (false-green guard). pytest merges into the existing last-failed mapping, so a stale entry for a test not collected this run can survive. Clearing up front guarantees the rerun's--lfset contains only this run's failures — without it, a rerun could select a now-passing stale set and go green. (test_integration_stale_cache_cannot_mask_failurefails without it.)--lfnf=allon the rerun (re-execution guard). If the first attempt dies before writing the cache (crash/OOM at session start), the rerun's last-failed set is empty. Note the run is already kept red in that case by exit-code propagation — an empty--lfselection exits 5 (EXIT_NOTESTSCOLLECTED), whichrun_commandtreats as a failure, even under an inheritedPYTEST_ADDOPTS=--lfnf=none.--lfnf=allmakes the rerun re-run the whole suite so the real failure is actually re-executed and re-observed rather than merely inferred from an empty-collection exit code.test_integration_crash_rerun_reexecutes_failing_testasserts (via a marker file) that the failing test truly re-ran.Exit-code propagation (the base guarantee).
run_commandraisesRuntimeErroron any nonzero exit (including exit 5 for an empty collection), and the rerun's exception is never swallowed, so a still-failing rerun keeps the job red.Scoped out (stated so a reviewer needn't ask): the raw inline
python -m pytest slang/tests/integration/slangpy/at.github/actions/build-and-test-with-slang/action.yml:181(not routed throughci.py, Windows-only, non-parallel — lowest contention risk) andbenchmark_python(its current CI invocations already continue after per-device failures).🤖 Generated by an automated SlangPy coworker — may be inaccurate. A human maintainer should verify.