fix(test-helpers): match package markers on path segments so they apply in CI - #2566
Open
LeSingh1 wants to merge 1 commit into
Open
fix(test-helpers): match package markers on path segments so they apply in CI#2566LeSingh1 wants to merge 1 commit into
LeSingh1 wants to merge 1 commit into
Conversation
… nodeid
pytest_collection_modifyitems tags every collected item by matching its
nodeid against a repo-root-relative prefix:
if nodeid.startswith("cuda_core/tests/") or "/cuda_core/tests/" in nodeid:
item.add_marker(pytest.mark.core)
but item.nodeid is relative to pytest's *rootdir*, and each subpackage ships
its own pytest.ini. ci/tools/run-tests runs
pushd ./cuda_core
pytest -rxXs -v --durations=0 --randomly-dont-reorganize tests/
so rootdir is cuda_core/ and every nodeid starts at "tests/". Confirmed
against real NVIDIA CI job logs (run 31274048063):
job 93145314849: 9543 node ids matching tests/...py::
0 node ids matching cuda_core/tests/...py::
job 93145314844: 9519 / 0
job 93145314853: 10447 / 0, including tests/cython/test_cython.py::test_ccuda_memcpy
So the core/bindings/pathfinder markers are never applied in CI. The same is
true of the cython marker: for "tests/cython/test_cython.py::test_x", none of
"/tests/cython/" in nodeid, nodeid.endswith("/tests/cython"), or
("/cython/" in nodeid and "/tests/" in nodeid) is true, because each needs a
leading slash that a rootdir-relative nodeid does not have.
That last one matters beyond labelling: the CUDA-header gate is nested inside
the cython branch, so
if "core" in item.keywords and not have_headers: ... skip ...
can never fire. Core cython tests are never skipped when CUDA_PATH is unset;
they run and fail on a missing header instead. "core" in item.keywords is
also unreachable for a second reason -- the core marker it depends on is one
of the ones that never got applied.
Match on path segments from item.path, which is absolute and does not move
with rootdir, falling back to the nodeid when an item has no path. A package
marker is applied when "<package>" is immediately followed by "tests", and
cython/smoke when "tests" is immediately followed by "cython"/"integration".
The repo-root invocation keeps working; "toolshed/tests/..." still gets no
package marker.
Contributor
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.
Problem
pytest_collection_modifyitemstags every collected item by matching its nodeid against a repo-root-relative prefix:But
item.nodeidis relative to pytest's rootdir, and each subpackage ships its ownpytest.ini.ci/tools/run-testsruns:pushd ./cuda_core pytest -rxXs -v --durations=0 --randomly-dont-reorganize tests/so rootdir is
cuda_core/and every nodeid starts attests/.Confirmed against real CI logs
From run
31274048063onmain, counting node ids in the raw job logs:tests/…py::cuda_core/tests/…py::Job 93145314853 also contains the cython suite, e.g.
tests/cython/test_cython.py::test_ccuda_memcpy.So the
core/bindings/pathfindermarkers are never applied in CI.The
cythonmarker is in the same position. Fortests/cython/test_cython.py::test_ccuda_memcpy, none of the three conditions holds:"/tests/cython/" in nodeid→ no leading slash → Falsenodeid.endswith("/tests/cython")→ False"/cython/" in nodeid and "/tests/" in nodeid→/cython/matches,/tests/does not → FalseWhy the cython case matters beyond labelling
The CUDA-header gate is nested inside the cython branch:
Since the outer branch never runs, the gate can never fire: core cython tests are never skipped when
CUDA_PATH/CUDA_HOMEis unset — they run and fail on a missing header instead."core" in item.keywordsis unreachable for a second, independent reason too: thecoremarker it depends on is itself one of the ones that never got applied. The skip reason string appears 0 times across all three job logs.Fix
Match on path segments taken from
item.path, which is absolute and does not move with rootdir, falling back to the nodeid for items that have no path:<package>is immediately followed bytests;cython/smokewhentestsis immediately followed bycython/integration.Segment adjacency replaces substring matching, so
toolshed/tests/...still gets no package marker and a straycythondirectory outside atests/tree is not picked up.Tests
cuda_python_test_helpershad no tests for this. Addedcuda_python_test_helpers/tests/test_pytest_plugin.py, drivingpytest_collection_modifyitemswith fake items whose node ids are taken verbatim from the CI logs above:core,bindings,pathfinder);tests/cython/...gets bothcoreandcython;_cuda_headers_available()is False and does not when it is True;tests/integration/...getssmoke;toolshed/tests/...gets no package marker.Verification
Ran all eight cases against the
upstream/mainplugin and the fixed one, withcuda_python_test_helpers.marksstubbed (it importscuda.pathfinder, which cannot load on macOS):The nodeid shape was also reproduced locally against the repo's own
cuda_core/pytest.ini:pushd cuda_core && pytest tests/yieldstests/test_sample.py::test_one, and evenpytest cuda_core/tests/from the repo root yields the same, because rootdir resolves tocuda_core/either way.Not run:
pytest cuda_python_test_helpers/tests/test_pytest_plugin.pyas written — it imports the package, which importscuda.pathfinder(unavailable on macOS). The stub run above exercises the same function with the same inputs. Please treat CI as the first real run.Note on blast radius
This makes
-m core,-m bindings,-m pathfinderand-m cythonactually select tests in CI for the first time, and re-enables the header gate. If any job relies on those selectors currently matching nothing, it will start behaving differently — worth a look before merging. I did not find such a job in.github/workflowsorci/tools/run-tests.