From 3b1a0b44d67509b80ca731e323a1584723489c32 Mon Sep 17 00:00:00 2001
From: wcwxy <26245345+ChaoWao@users.noreply.github.com>
Date: Fri, 8 May 2026 17:14:57 +0800
Subject: [PATCH] Refactor: hoist conftest lazy imports that have no rationale
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Seven `from simpler_setup.* import ...` / `import faulthandler` lines
inside individual functions were carrying `# noqa: PLC0415` for no
technical reason — the modules they imported are pure Python with no
side effects, no circular import risk, and no significant import cost.
Move them to the top of the file:

- `simpler_setup.parallel_scheduler` (used in 4 spots)
- `simpler_setup.pto_isa.ensure_pto_isa_root`
- `simpler_setup.scene_test.clear_compile_cache`
- stdlib `faulthandler`

Lazy imports that *do* have a reason are kept:

- `simpler.worker.Worker` (2 spots) — pulls in the nanobind C extension
  `_task_interface`; hoisting would slow every pytest startup including
  paths that never need a ChipWorker.
- `xdist` — `try`/`except ImportError` probe for an optional dep; the
  whole point of importing inline is to handle the import failure.

A side benefit: the SIGALRM handler no longer does a
`from simpler_setup import parallel_scheduler` inside a signal context,
which can deadlock on Python's import lock when the module has not been
loaded yet. The handler now only references an already-loaded module
attribute. Comment added at the handler explaining the constraint.
---
 conftest.py | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/conftest.py b/conftest.py
index 7019b788e..c36cc1c11 100644
--- a/conftest.py
+++ b/conftest.py
@@ -16,6 +16,7 @@
 
 from __future__ import annotations
 
+import faulthandler
 import logging
 import os
 import signal
@@ -47,7 +48,10 @@
 
 import pytest  # noqa: E402
 
+from simpler_setup import parallel_scheduler as _ps  # noqa: E402
 from simpler_setup.log_config import configure_logging  # noqa: E402
+from simpler_setup.pto_isa import ensure_pto_isa_root  # noqa: E402
+from simpler_setup.scene_test import clear_compile_cache  # noqa: E402
 
 # Exit code used when the session watchdog fires. Matches the GNU `timeout`
 # convention so shell wrappers (e.g. CI) can distinguish timeout from other
@@ -62,9 +66,7 @@ def _parse_device_range(s: str) -> list[int]:
     so both conftest and standalone share the same parser (supports ``0``,
     ``0-7``, ``0,2,5``, and mixed ``0,2-4,7``).
     """
-    from simpler_setup.parallel_scheduler import device_range_to_list  # noqa: PLC0415
-
-    return device_range_to_list(s)
+    return _ps.device_range_to_list(s)
 
 
 class DevicePool:
@@ -177,9 +179,12 @@ def pytest_addoption(parser):
 
 
 def _install_session_timeout(timeout_s: int) -> None:
+    # Module-level `_ps` import is intentional (rather than a function-local
+    # one): doing `from simpler_setup import parallel_scheduler` inside a
+    # signal handler can deadlock on the import lock if the module hasn't
+    # been imported yet. Hoisting it to the top guarantees the handler only
+    # touches an already-loaded module.
     def _handler(signum, frame):
-        from simpler_setup import parallel_scheduler as _ps  # noqa: PLC0415
-
         print(
             f"\n{'=' * 40}\n[pytest] TIMEOUT: session exceeded {timeout_s}s ({timeout_s // 60}min) limit\n{'=' * 40}",
             flush=True,
@@ -246,8 +251,6 @@ def _install_child_faulthandler() -> None:
     Always-on ``faulthandler.enable()`` also gives us a stack on real crashes
     (SIGSEGV/SIGABRT) instead of a silent exit.
     """
-    import faulthandler  # noqa: PLC0415
-
     faulthandler.enable()
     if hasattr(signal, "SIGUSR1"):
         try:
@@ -285,8 +288,6 @@ def pytest_configure(config):
     # need PTO-ISA (e.g. pytest tests/ut on a runner without SSH keys) must not
     # be aborted when the eager clone fails. If an actual scene test later needs
     # PTO-ISA, scene_test.py's lazy path will re-raise the original error.
-    from simpler_setup.pto_isa import ensure_pto_isa_root  # noqa: PLC0415
-
     try:
         root = ensure_pto_isa_root(
             verbose=True,
@@ -571,11 +572,9 @@ def _base_pytest_argv(session):
 
 def _resolve_max_parallel(cfg, platform: str, device_ids: list[int]) -> int:
     """Parse the -j/--max-parallel CLI value; 'auto' → platform-aware default."""
-    from simpler_setup.parallel_scheduler import default_max_parallel  # noqa: PLC0415
-
     raw = cfg.getoption("--max-parallel", default="auto")
     if raw in (None, "", "auto"):
-        return default_max_parallel(platform or "", device_ids)
+        return _ps.default_max_parallel(platform or "", device_ids)
     try:
         val = int(raw)
     except (TypeError, ValueError) as e:
@@ -611,8 +610,6 @@ def _dispatch_test_phases(session, resource_specs):  # noqa: PLR0912
     already has to inspect the list to decide whether to dispatch) so
     this function does not walk ``session.items`` a second time.
     """
-    from simpler_setup import parallel_scheduler as _ps  # noqa: PLC0415
-
     cfg = session.config
     device_spec = cfg.getoption("--device", default="0")
     device_ids = _parse_device_range(device_spec)
@@ -808,8 +805,6 @@ def pytest_sessionfinish(session, exitstatus):  # noqa: ARG001
     worker pool) lets those instances die while nanobind is still
     available.
     """
-    from simpler_setup.scene_test import clear_compile_cache  # noqa: PLC0415
-
     clear_compile_cache()