This is an automated email from the ASF dual-hosted git repository. tqchen pushed a commit to branch tvm-further-cleanup-python-tests-followup in repository https://gitbox.apache.org/repos/asf/tvm.git
commit 034831ecff17fec6bbcd229555c4db259c46bdd6 Author: Tianqi Chen <[email protected]> AuthorDate: Sun Jul 5 15:53:27 2026 +0000 [CI] Remove Python collection bookkeeping Keep cached fixtures process-local and independently copied without relying on pytest collection internals. Remove the corresponding ordering/load-group coupling, while hardening the request hook and optional-suite collection paths identified in review. --- docs/contribute/pull_request.rst | 2 +- pyproject.toml | 5 +- python/tvm/testing/utils.py | 87 ++++++++--------------- tests/python/conftest.py | 48 ++----------- tests/python/disco/test_custom_allreduce.py | 5 +- tests/python/request_hook.py | 2 + tests/python/testing/test_tvm_testing_features.py | 72 ++++--------------- tests/python/tirx/conftest.py | 11 ++- tests/scripts/task_python_unittest.sh | 3 +- 9 files changed, 67 insertions(+), 168 deletions(-) diff --git a/docs/contribute/pull_request.rst b/docs/contribute/pull_request.rst index 969bb6f7d8..9150838e76 100644 --- a/docs/contribute/pull_request.rst +++ b/docs/contribute/pull_request.rst @@ -251,7 +251,7 @@ If you want to run all tests: # build tvm (see install-from-source for CMake build instructions) cd build && cmake .. && cmake --build . --parallel $(nproc) && cd .. - python -m pytest -vvs -n auto --dist=loadgroup tests/python + python -m pytest -vvs -n auto tests/python If you want to run a single test: diff --git a/pyproject.toml b/pyproject.toml index 2c38d1f9d8..a4f80182cf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -167,10 +167,7 @@ addopts = "-v --tb=short" python_files = ["test_*.py", "*_test.py"] python_classes = ["Test*"] python_functions = ["test_*"] -markers = [ - "gpu: Mark a test as requiring a GPU", - "xdist_group(name): Run related tests in one pytest-xdist load group", -] +markers = ["gpu: Mark a test as requiring a GPU"] [tool.ruff] include = [ diff --git a/python/tvm/testing/utils.py b/python/tvm/testing/utils.py index 2e1e04a7f0..86cfb66dc7 100644 --- a/python/tvm/testing/utils.py +++ b/python/tvm/testing/utils.py @@ -24,7 +24,7 @@ Organization This file contains functions expected to be called directly by a user while writing unit tests. Integrations with the pytest framework -are in plugin.py. +for TVM's own test suite are in ``tests/python/conftest.py``. Testing Markers *************** @@ -73,6 +73,7 @@ import logging import os import pickle import platform +import runpy import sys import time from pathlib import Path @@ -91,6 +92,7 @@ from tvm.support import nvcc SKIP_SLOW_TESTS = os.getenv("SKIP_SLOW_TESTS", "").lower() in {"true", "1", "yes"} IS_IN_CI = os.getenv("CI", "") == "true" +_REQUEST_HOOK_INITIALIZERS = {} skip_if_wheel_test = pytest.mark.skipif( os.getenv("WHEEL_TEST", "").lower() in {"true", "1", "yes"}, @@ -679,10 +681,11 @@ def fixture(func=None, *, cache_return_value=False): If the setup is expensive to perform, then the cache_return_value=True argument can be passed to cache the setup. The fixture function will be run only once (or once per parameter, - if used with tvm.testing.parameter), and the same return value - will be passed to all tests that use it. If the environment - variable TVM_TEST_DISABLE_CACHE is set to a non-zero value, it - will disable this feature and no caching will be performed. + if used with tvm.testing.parameter). The cached setup value is + retained for the lifetime of the test process, and each test receives + an independent copy. If the environment variable TVM_TEST_DISABLE_CACHE + is set to a non-zero value, it will disable this feature and no caching + will be performed. Example ------- @@ -719,15 +722,10 @@ def fixture(func=None, *, cache_return_value=False): force_disable_cache = bool(int(os.environ.get("TVM_TEST_DISABLE_CACHE", "0"))) cache_return_value = cache_return_value and not force_disable_cache - # Deliberately at function scope, so that caching can track how - # many times the fixture has been used. If used, the cache gets - # cleared after the fixture is no longer needed. - scope = "function" - def wraps(func): if cache_return_value: func = _fixture_cache(func) - func = pytest.fixture(func, scope=scope) + func = pytest.fixture(func, scope="function") return func if func is None: @@ -791,13 +789,6 @@ class _DeepCopyAllowedClasses(dict): def _fixture_cache(func): cache = {} - # Can't use += on a bound method's property. Therefore, this is a - # list rather than a variable so that it can be accessed from the - # pytest_collection_modifyitems(). - num_tests_use_this_fixture = [0] - - num_times_fixture_used = 0 - # Using functools.lru_cache would require the function arguments # to be hashable, which wouldn't allow caching fixtures that # depend on numpy arrays. For example, a fixture that takes a @@ -821,40 +812,21 @@ def _fixture_cache(func): @functools.wraps(func) def wrapper(*args, **kwargs): - if num_tests_use_this_fixture[0] == 0: - raise RuntimeError( - "Fixture use count is 0. " - "Cached tvm.testing fixtures must be collected from tests/python " - "so its conftest can count fixture uses." - ) + cache_key = get_cache_key(*args, **kwargs) try: - cache_key = get_cache_key(*args, **kwargs) - - try: - cached_value = cache[cache_key] - except KeyError: - cached_value = cache[cache_key] = func(*args, **kwargs) - - yield copy.deepcopy( - cached_value, - # allowed_class_list should be a list of classes that - # are safe to copy using copy.deepcopy, but do not - # implement __deepcopy__, __reduce__, or - # __reduce_ex__. - _DeepCopyAllowedClasses(allowed_class_list=[]), - ) - - finally: - # Clear the cache once all tests that use a particular fixture - # have completed. - nonlocal num_times_fixture_used - num_times_fixture_used += 1 - if num_times_fixture_used >= num_tests_use_this_fixture[0]: - cache.clear() - - # Set in the pytest_collection_modifyitems(), by _count_num_fixture_uses - wrapper.num_tests_use_this_fixture = num_tests_use_this_fixture + cached_value = cache[cache_key] + except KeyError: + cached_value = cache[cache_key] = func(*args, **kwargs) + + return copy.deepcopy( + cached_value, + # allowed_class_list should be a list of classes that + # are safe to copy using copy.deepcopy, but do not + # implement __deepcopy__, __reduce__, or + # __reduce_ex__. + _DeepCopyAllowedClasses(allowed_class_list=[]), + ) return wrapper @@ -929,13 +901,14 @@ def install_request_hook(depth: int) -> None: if not hook_script.is_file(): raise RuntimeError(f"File {hook_script} does not exist:\n{msg}") - # Import the hook and start it up (it's not included here directly to avoid - # keeping a database of URLs inside the tvm Python package - sys.path.append(str(hook_script_dir)) - # This import is intentionally delayed since it should only happen in CI - import request_hook # pylint: disable=import-outside-toplevel - - request_hook.init() + # Load the exact hook file without exposing the test root as an import path. + # Cache its initializer because Sphinx invokes this once per gallery example. + hook_script = hook_script.resolve() + try: + init = _REQUEST_HOOK_INITIALIZERS[hook_script] + except KeyError: + init = _REQUEST_HOOK_INITIALIZERS[hook_script] = runpy.run_path(str(hook_script))["init"] + init() def strtobool(val): diff --git a/tests/python/conftest.py b/tests/python/conftest.py index bf105ae97c..9046f9d083 100644 --- a/tests/python/conftest.py +++ b/tests/python/conftest.py @@ -18,51 +18,11 @@ import os -import _pytest - - -def pytest_collection_modifyitems(items): - """Maintain the ordering and cache bookkeeping required by TVM fixtures.""" - _count_num_fixture_uses(items) - _remove_global_fixture_definitions(items) - _sort_tests(items) - - -def _count_num_fixture_uses(items): - for item in items: - is_skipped = item.get_closest_marker("skip") or any( - mark.args[0] for mark in item.iter_markers("skipif") - ) - if is_skipped: - continue - - for fixturedefs in item._fixtureinfo.name2fixturedefs.values(): - fixturedef = fixturedefs[-1] - if hasattr(fixturedef.func, "num_tests_use_this_fixture"): - fixturedef.func.num_tests_use_this_fixture[0] += 1 - - -def _remove_global_fixture_definitions(items): - modules = {item.module for item in items} - for module in modules: - for name in dir(module): - obj = getattr(module, name) - if hasattr(obj, "_pytestfixturefunction") and isinstance( - obj._pytestfixturefunction, _pytest.fixtures.FixtureFunctionMarker - ): - delattr(module, name) - - -def _sort_tests(items): - def sort_key(item): - filename, lineno, test_name = item.location - return filename, lineno, test_name.split("[")[0] - - items.sort(key=sort_key) - def pytest_sessionstart(): if os.getenv("CI", "") == "true": - from request_hook import init # pylint: disable=import-outside-toplevel + from tvm.testing.utils import ( + install_request_hook, # pylint: disable=import-outside-toplevel + ) - init() + install_request_hook(3) diff --git a/tests/python/disco/test_custom_allreduce.py b/tests/python/disco/test_custom_allreduce.py index 6728e3115d..db23257703 100644 --- a/tests/python/disco/test_custom_allreduce.py +++ b/tests/python/disco/test_custom_allreduce.py @@ -27,6 +27,9 @@ import tvm import tvm.testing from tvm.runtime import DataType, disco +if disco is None: + pytest.skip("disco runtime is not available", allow_module_level=True) + class AllReduceStrategyType(enum.IntEnum): RING = 0 @@ -55,7 +58,7 @@ _ccl = [ccl for ccl in _compiled_ccl() if ccl == "nccl"] @pytest.mark.parametrize("strategy", _strategies) def test_allreduce(shape, ccl, strategy): devices = [0, 1] - sess: disco.Session = disco.ProcessSession(num_workers=len(devices)) + sess = disco.ProcessSession(num_workers=len(devices)) sess.init_ccl(ccl, *devices) num_elements = reduce(lambda x, y: x * y, shape) diff --git a/tests/python/request_hook.py b/tests/python/request_hook.py index 5a00a11cdd..f3faf77a19 100644 --- a/tests/python/request_hook.py +++ b/tests/python/request_hook.py @@ -46,6 +46,8 @@ class TvmRequestHook(urllib.request.Request): def init(): global LOGGER + if urllib.request.Request is TvmRequestHook: + return urllib.request.Request = TvmRequestHook LOGGER = logging.getLogger("tvm_request_hook") LOGGER.setLevel(logging.DEBUG) diff --git a/tests/python/testing/test_tvm_testing_features.py b/tests/python/testing/test_tvm_testing_features.py index 2f4c798947..a3eaab3656 100644 --- a/tests/python/testing/test_tvm_testing_features.py +++ b/tests/python/testing/test_tvm_testing_features.py @@ -22,30 +22,17 @@ import pytest import tvm.testing -pytestmark = pytest.mark.xdist_group(name="tvm-testing-features") - -# This file tests features in tvm.testing, such as verifying that -# cached fixtures are run an appropriate number of times. As a -# result, the order of the tests is important. Use of --last-failed -# or --failed-first while debugging this file is not advised. If -# these tests are distributed/parallelized using pytest-xdist or -# similar, all tests in this file should run sequentially on the same -# node. (See https://stackoverflow.com/a/59504228) - class TestParameter: param1_vals = [1, 2, 3] param2_vals = ["a", "b", "c"] - independent_usages = 0 param1 = tvm.testing.parameter(*param1_vals) param2 = tvm.testing.parameter(*param2_vals) def test_using_independent(self, param1, param2): - type(self).independent_usages += 1 - - def test_independent(self): - assert self.independent_usages == len(self.param1_vals) * len(self.param2_vals) + assert param1 in self.param1_vals + assert param2 in self.param2_vals class TestFixtureCaching: @@ -55,50 +42,35 @@ class TestFixtureCaching: param1 = tvm.testing.parameter(*param1_vals) param2 = tvm.testing.parameter(*param2_vals) - uncached_calls = 0 - cached_calls = 0 - @tvm.testing.fixture def uncached_fixture(self, param1): - type(self).uncached_calls += 1 return 2 * param1 def test_use_uncached(self, param1, param2, uncached_fixture): assert 2 * param1 == uncached_fixture - def test_uncached_count(self): - assert self.uncached_calls == len(self.param1_vals) * len(self.param2_vals) - @tvm.testing.fixture(cache_return_value=True) def cached_fixture(self, param1): - type(self).cached_calls += 1 return 3 * param1 def test_use_cached(self, param1, param2, cached_fixture): assert 3 * param1 == cached_fixture - def test_cached_count(self): - cache_disabled = bool(int(os.environ.get("TVM_TEST_DISABLE_CACHE", "0"))) - if cache_disabled: - assert self.cached_calls == len(self.param1_vals) * len(self.param2_vals) - else: - assert self.cached_calls == len(self.param1_vals) - -class TestCachedFixtureIsCopy: - param = tvm.testing.parameter(1, 2, 3, 4) +def test_fixture_cache_reuses_setup_and_returns_copies(): + setup_calls = [] - @tvm.testing.fixture(cache_return_value=True) - def cached_mutable_fixture(self): - return {"val": 0} + def setup(value): + setup_calls.append(value) + return {"value": value} - def test_modifies_fixture(self, param, cached_mutable_fixture): - assert cached_mutable_fixture["val"] == 0 + cached_setup = tvm.testing.utils._fixture_cache(setup) + first = cached_setup(1) + first["value"] = 0 - # The tests should receive a copy of the fixture value. If - # the test receives the original and not a copy, then this - # will cause the next parametrization to fail. - cached_mutable_fixture["val"] = param + assert cached_setup(1) == {"value": 1} + assert cached_setup(2) == {"value": 2} + assert setup_calls == [1, 2] class TestBrokenFixture: @@ -107,19 +79,13 @@ class TestBrokenFixture: # This behavior should be the same whether or not the fixture # results are cached. - num_uses_broken_uncached_fixture = 0 - num_uses_broken_cached_fixture = 0 - @tvm.testing.fixture def broken_uncached_fixture(self): raise RuntimeError("Intentionally broken fixture") @pytest.mark.xfail(True, reason="Broken fixtures should result in a failing setup", strict=True) def test_uses_broken_uncached_fixture(self, broken_uncached_fixture): - type(self).num_uses_broken_fixture += 1 - - def test_num_uses_uncached(self): - assert self.num_uses_broken_uncached_fixture == 0 + pass @tvm.testing.fixture(cache_return_value=True) def broken_cached_fixture(self): @@ -127,10 +93,7 @@ class TestBrokenFixture: @pytest.mark.xfail(True, reason="Broken fixtures should result in a failing setup", strict=True) def test_uses_broken_cached_fixture(self, broken_cached_fixture): - type(self).num_uses_broken_cached_fixture += 1 - - def test_num_uses_cached(self): - assert self.num_uses_broken_cached_fixture == 0 + pass @pytest.mark.skipif( @@ -146,11 +109,6 @@ class TestCacheableTypes: return self.EmptyClass() def test_uses_uncacheable(self, request): - # Normally the num_tests_use_this_fixture would be set before - # anything runs. For this test case only, because we are - # delaying the use of the fixture, we need to manually - # increment it. - self.uncacheable_fixture.num_tests_use_this_fixture[0] += 1 with pytest.raises(TypeError): request.getfixturevalue("uncacheable_fixture") diff --git a/tests/python/tirx/conftest.py b/tests/python/tirx/conftest.py index c2a7a28d28..2653a1f05a 100644 --- a/tests/python/tirx/conftest.py +++ b/tests/python/tirx/conftest.py @@ -35,10 +35,17 @@ from tvm.testing import env def pytest_collection_modifyitems(config, items): if env.has_cuda_compute(10): return - suite_root = Path(__file__).parent + suite_root = Path(__file__).resolve().parent skip = pytest.mark.skip( reason="tirx suite requires a CUDA compute capability 10.0 (sm_100a) device" ) for item in items: - if item.path.is_relative_to(suite_root): + path = getattr(item, "path", None) + if path is None: + continue + try: + path = Path(path).resolve() + except TypeError: + continue + if path.is_relative_to(suite_root): item.add_marker(skip) diff --git a/tests/scripts/task_python_unittest.sh b/tests/scripts/task_python_unittest.sh index 1cf297fc28..02d468257c 100755 --- a/tests/scripts/task_python_unittest.sh +++ b/tests/scripts/task_python_unittest.sh @@ -24,5 +24,4 @@ export PYTEST_ADDOPTS="${CI_PYTEST_ADD_OPTIONS:-} ${PYTEST_ADDOPTS:-}" # setup tvm-ffi into python folder uv pip install -v --target=python ./3rdparty/tvm-ffi/ -# Load-group distribution keeps the order-dependent fixture tests on one worker. -python3 -m pytest -vvs -n auto --dist=loadgroup tests/python +python3 -m pytest -vvs -n auto tests/python
