[REFACTOR] Phase out src/support/ffi_testing.cc#19459
Conversation
This PR removes src/support/ffi_testing.cc, which registered TVM-side testing helpers (TestAttrs, FrontendTestModuleNode, TestingEventLogger, ErrorTest, testing.identity_cpp, testing.sleep_in_ffi, Variant helpers). None of these have equivalents in tvm-ffi and none are needed going forward; tvm-ffi already covers the duplicated names (echo, nop, object_use_count, run_check_signal, test_raise_error). Main changes: - Delete src/support/ffi_testing.cc and the two Python wrapper files that only existed to surface its symbols (python/tvm/testing/attrs.py, python/tvm/testing/popen_pool.py) - Drop deleted-symbol imports from python/tvm/testing/__init__.py and remove FrontendTestModule from python/tvm/support.py - Delete test files whose entire content depended on removed symbols (test_runtime_container.py, test_runtime_packed_func.py) - Surgically remove test functions using removed symbols from test_runtime_error.py, test_ir_container.py, test_ir_attrs.py, test_node_reflection.py, test_tir_structural_equal_hash.py, test_popen_pool.py; keep all unrelated tests intact
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes various testing utilities and associated test cases related to frontend FFI and TestAttrs. The review highlights that several of these removals, such as the deletion of core runtime container tests, stack trace propagation tests, and fundamental IR reflection tests, eliminate critical test coverage. It is recommended to adapt these tests to use alternative mechanisms or move them to permanent locations rather than deleting them entirely.
I am having trouble creating individual review comments. Click here to see my feedback.
tests/python/runtime/test_runtime_container.py (1-175)
This file contains fundamental tests for String and ShapeTuple (e.g., test_string, test_shape_tuple) as well as FFI argument conversions (bool, int, map, array) that do not depend on the deleted symbols in ffi_testing.cc. Deleting this file entirely removes coverage for core runtime containers and the FFI conversion layer. These tests should be retained or moved to a more appropriate location rather than being deleted.
tests/python/runtime/test_runtime_error.py (39-154)
The removal of test_deep_callback and the stack trace tests (test_cpp_frames_in_stack_trace_from_python_error, test_stack_trace_from_cpp_error) eliminates the only verification for stack trace propagation across the FFI boundary. This is a critical feature for debugging TVM. These tests should be adapted to use other available FFI functions or the necessary minimal testing hooks should be moved to a permanent location in the runtime to maintain this coverage.
tests/python/contrib/test_popen_pool.py (114-178)
Several valuable tests for PopenPoolExecutor and PopenWorker are being removed here. Specifically, test_popen_pool_executor_timeout verifies timeout logic and can be easily adapted to use a Python-based sleep (time.sleep) instead of testing.sleep_in_ffi. Additionally, test_popen_ffi and helpers like fast_summation/slow_summation provide important coverage for the pool's ability to handle FFI calls and general task execution, and they do not strictly depend on the deleted C++ symbols.
tests/python/ir/test_node_reflection.py (100-123)
The test_env_func test provides essential coverage for EnvFunc serialization and reflection. While it currently uses TestAttrs (which is being deleted), the core logic it verifies—JSON round-tripping of EnvFunc—is a fundamental IR feature. This test should be adapted to use a different Attrs type or a simpler node to maintain coverage for EnvFunc reflection.
tests/python/ir/test_ir_attrs.py (26-42)
The test_make_attrs test verifies important behavior of tvm.ir.make_node when creating Attrs objects, including error handling for unknown keys and structural equality. This coverage should be maintained by using a different Attrs type instead of deleting the test.
The previous phase-out commit removed popen-pool tests along with the FFI testing helper they depended on (testing.sleep_in_ffi via ffi_testing.cc). These tests cover the popen_pool worker lifecycle and are still useful; rewire them to use Python's time.sleep and keep them in tree. Restore the four previously-removed test functions (test_popen_initializer, test_popen_worker_recycles_with_initializer, test_popen_ffi, test_popen_pool_executor_timeout) and re-introduce python/tvm/testing/popen_pool.py as a pure-Python helper module (no FFI sleep, no identity_cpp) so worker subprocesses can import the fixtures via cloudpickle module resolution.
3 participants
Deletes src/support/ffi_testing.cc (271 lines) and removes TVM-only testing symbols (TestAttrs, FrontendTestModule, TestingEventLogger, ErrorTest). The duplicated testing.echo, testing.nop, testing.object_use_count, and testing.run_check_signal symbols continue to resolve through tvm-ffi which already registers them. Removes test files that depend exclusively on deleted symbols.
Test plan: 185 passed / 77 skipped / 0 failed across covered suites (tests/python/all-platform-minimal-test/, tests/python/ir/, tests/python/runtime/test_runtime_error.py, tests/python/tirx-base/, tests/python/contrib/). Build clean (cmake + ninja); pre-commit clean.