From b3177a093f5e749770aa024c257bdcf35a72b111 Mon Sep 17 00:00:00 2001 From: Paul Nechifor Date: Mon, 29 Jun 2026 18:49:32 +0300 Subject: [PATCH] importor skip --- .github/workflows/ci.yml | 12 +-- dimos/codebase_checks/test_no_importorskip.py | 97 +++++++++++++++++++ .../manipulators/openarm/test_driver.py | 3 +- .../visualization/viser/conftest.py | 32 ++++++ .../visualization/viser/test_gui_status.py | 2 - .../viser/test_operation_worker.py | 2 - .../viser/test_viser_visualization.py | 2 - .../viser/test_visualizer_lifecycle.py | 2 - dimos/mapping/ray_tracing/test_transformer.py | 2 - dimos/mapping/ray_tracing/test_voxel_map.py | 2 - dimos/memory2/codecs/test_codecs.py | 3 - .../nav_3d/mls_planner/test_mls_planner.py | 2 - .../nav_3d/mls_planner/test_transformer.py | 3 - .../fiducial/test_fixture_verification.py | 2 - .../test_marker_detection_stream_module.py | 2 - dimos/perception/fiducial/test_marker_pose.py | 3 - .../fiducial/test_marker_transformer.py | 2 - dimos/protocol/pubsub/test_registry.py | 8 +- .../engines/test_robot_sim_binding.py | 3 +- 19 files changed, 141 insertions(+), 43 deletions(-) create mode 100644 dimos/codebase_checks/test_no_importorskip.py create mode 100644 dimos/manipulation/visualization/viser/conftest.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4fd7497c48..7eac0f250d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -281,13 +281,13 @@ jobs: - name: Install dependencies run: uv sync --group tests --frozen - name: Run tests - run: uv run pytest --numprocesses=3 --cov=dimos/ --junitxml=junit.xml -m 'not (tool or self_hosted or mujoco or self_hosted_large)' + run: uv run pytest --ignore=dimos/mapping/ray_tracing --ignore=dimos/navigation/nav_3d/mls_planner --numprocesses=3 --cov=dimos/ --junitxml=junit.xml -m 'not (tool or self_hosted or mujoco or self_hosted_large)' - name: Re-run the failing tests with maximum verbosity if: failure() env: COLOR: yes run: >- # `exit 1` makes sure that the job remains red with flaky runs - uv run pytest --no-cov -vvvvv --lf -m 'not (tool or self_hosted or mujoco or self_hosted_large)' && exit 1 + uv run pytest --ignore=dimos/mapping/ray_tracing --ignore=dimos/navigation/nav_3d/mls_planner --no-cov -vvvvv --lf -m 'not (tool or self_hosted or mujoco or self_hosted_large)' && exit 1 shell: bash - name: Turn coverage into xml run: uv run python -m coverage xml @@ -397,13 +397,13 @@ jobs: echo "ROS_PYTHON_VERSION=$ROS_PYTHON_VERSION" } >> "$GITHUB_ENV" - name: Run tests - run: uv run pytest --cov=dimos/ --junitxml=junit.xml -m '(${{ matrix.markers }}) and not (tool or mujoco)' + run: uv run pytest --ignore=dimos/mapping/ray_tracing --ignore=dimos/navigation/nav_3d/mls_planner --cov=dimos/ --junitxml=junit.xml -m '(${{ matrix.markers }}) and not (tool or mujoco)' - name: Re-run the failing tests with maximum verbosity if: failure() env: COLOR: yes run: >- # `exit 1` makes sure that the job remains red with flaky runs - uv run pytest --no-cov -vvvvv --lf -m '(${{ matrix.markers }}) and not (tool or mujoco)' && exit 1 + uv run pytest --ignore=dimos/mapping/ray_tracing --ignore=dimos/navigation/nav_3d/mls_planner --no-cov -vvvvv --lf -m '(${{ matrix.markers }}) and not (tool or mujoco)' && exit 1 shell: bash - name: Turn coverage into xml run: uv run python -m coverage xml @@ -499,14 +499,14 @@ jobs: # heavier scenes (`apt`). Software X via xvfb is not enough. env: DISPLAY: ":0" - run: uv run pytest --cov=dimos/ --junitxml=junit.xml -m self_hosted_large + run: uv run pytest --ignore=dimos/mapping/ray_tracing --ignore=dimos/navigation/nav_3d/mls_planner --cov=dimos/ --junitxml=junit.xml -m self_hosted_large - name: Re-run the failing tests with maximum verbosity if: failure() env: COLOR: yes DISPLAY: ":0" run: >- # `exit 1` makes sure that the job remains red with flaky runs - uv run pytest --no-cov -vvvvv --lf -m self_hosted_large && exit 1 + uv run pytest --ignore=dimos/mapping/ray_tracing --ignore=dimos/navigation/nav_3d/mls_planner --no-cov -vvvvv --lf -m self_hosted_large && exit 1 shell: bash - name: Turn coverage into xml run: uv run python -m coverage xml diff --git a/dimos/codebase_checks/test_no_importorskip.py b/dimos/codebase_checks/test_no_importorskip.py new file mode 100644 index 0000000000..36e1e8f97c --- /dev/null +++ b/dimos/codebase_checks/test_no_importorskip.py @@ -0,0 +1,97 @@ +# Copyright 2026 Dimensional Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os + +from dimos.constants import DIMOS_PROJECT_ROOT + +PATTERN = "importorskip" + +IGNORED_DIRS = { + ".venv", + "venv", + "__pycache__", + "node_modules", + ".git", + "dist", + "build", + ".egg-info", + ".tox", +} + + +def _is_ignored_dir(dirpath: str) -> bool: + parts = dirpath.split(os.sep) + return bool(IGNORED_DIRS.intersection(parts)) + + +def find_importorskip_usages() -> list[tuple[str, int, str]]: + """Return a list of (rel_path, line_number, line_text) for every violation.""" + dimos_dir = DIMOS_PROJECT_ROOT / "dimos" + violations: list[tuple[str, int, str]] = [] + # Skip this test file: it necessarily contains the forbidden pattern. + self_path = os.path.realpath(__file__) + + for dirpath, dirnames, filenames in os.walk(dimos_dir): + dirnames[:] = [d for d in dirnames if d not in IGNORED_DIRS] + + if _is_ignored_dir(dirpath): + continue + + for fname in filenames: + if not fname.endswith(".py"): + continue + + full_path = os.path.join(dirpath, fname) + if os.path.realpath(full_path) == self_path: + continue + rel_path = os.path.relpath(full_path, DIMOS_PROJECT_ROOT) + + try: + with open(full_path, encoding="utf-8", errors="replace") as f: + for lineno, line in enumerate(f, start=1): + stripped = line.rstrip("\n") + if PATTERN not in stripped: + continue + violations.append((rel_path, lineno, stripped)) + except (OSError, UnicodeDecodeError): + continue + + return violations + + +def test_no_importorskip(): + """ + Fail if any file uses `pytest.importorskip`. + + `importorskip` silently skips a test (or a whole module) when an optional + dependency is missing. That means a broken test, or a dependency missing + from the dev/CI environment, goes unnoticed because the test never runs. + Install all dependencies locally so the tests actually execute. If a + dependency genuinely cannot be installed in some environment, exclude those + tests explicitly (e.g. a CI `--ignore`, or a platform-conditioned + `collect_ignore` in a conftest) rather than silently skipping per import. + """ + violations = find_importorskip_usages() + if violations: + report_lines = [ + f"Found {len(violations)} forbidden use(s) of `pytest.importorskip`. " + "Don't silently skip tests when a dependency is missing -- install all " + "dependencies so the tests run, or exclude the module explicitly " + "(CI `--ignore` / a platform `collect_ignore`):", + "", + ] + for path, lineno, text in violations: + report_lines.append(f" {path}:{lineno}: {text.strip()}") + raise AssertionError("\n".join(report_lines)) diff --git a/dimos/hardware/manipulators/openarm/test_driver.py b/dimos/hardware/manipulators/openarm/test_driver.py index c65a972bd6..3271cf68d9 100644 --- a/dimos/hardware/manipulators/openarm/test_driver.py +++ b/dimos/hardware/manipulators/openarm/test_driver.py @@ -22,10 +22,9 @@ import struct import time +import can import pytest -can = pytest.importorskip("can") - from dimos.hardware.manipulators.openarm.driver import ( CTRL_MODE_MIT, KD_MAX, diff --git a/dimos/manipulation/visualization/viser/conftest.py b/dimos/manipulation/visualization/viser/conftest.py new file mode 100644 index 0000000000..576d3d0186 --- /dev/null +++ b/dimos/manipulation/visualization/viser/conftest.py @@ -0,0 +1,32 @@ +# Copyright 2026 Dimensional Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import platform + +# viser[urdf] has no installable wheel set on linux-aarch64 (see the `tests` +# dependency group marker in pyproject.toml), so these tests cannot be collected +# there: their module-level imports pull in viser. Mirror the skipif_aarch64 +# convention (dimos/conftest.py) at collection time. test_state.py is +# intentionally NOT listed -- it imports only viser.state (no viser dependency) +# and runs on every platform. +collect_ignore = ( + [ + "test_gui_status.py", + "test_operation_worker.py", + "test_viser_visualization.py", + "test_visualizer_lifecycle.py", + ] + if platform.machine() == "aarch64" + else [] +) diff --git a/dimos/manipulation/visualization/viser/test_gui_status.py b/dimos/manipulation/visualization/viser/test_gui_status.py index f461565bdf..ece4443df2 100644 --- a/dimos/manipulation/visualization/viser/test_gui_status.py +++ b/dimos/manipulation/visualization/viser/test_gui_status.py @@ -16,8 +16,6 @@ import pytest -pytest.importorskip("viser", reason="Viser optional dependency is not installed") - from dimos.manipulation.visualization.types import TargetEvaluation from dimos.manipulation.visualization.viser.adapter import InProcessViserAdapter from dimos.manipulation.visualization.viser.config import ViserVisualizationConfig diff --git a/dimos/manipulation/visualization/viser/test_operation_worker.py b/dimos/manipulation/visualization/viser/test_operation_worker.py index 7b7724ce2d..7c56c496da 100644 --- a/dimos/manipulation/visualization/viser/test_operation_worker.py +++ b/dimos/manipulation/visualization/viser/test_operation_worker.py @@ -20,8 +20,6 @@ import pytest -pytest.importorskip("viser", reason="Viser optional dependency is not installed") - from dimos.manipulation.visualization.types import RobotInfo from dimos.manipulation.visualization.viser.adapter import InProcessViserAdapter from dimos.manipulation.visualization.viser.config import ViserVisualizationConfig diff --git a/dimos/manipulation/visualization/viser/test_viser_visualization.py b/dimos/manipulation/visualization/viser/test_viser_visualization.py index 2994158022..e934f6fac4 100644 --- a/dimos/manipulation/visualization/viser/test_viser_visualization.py +++ b/dimos/manipulation/visualization/viser/test_viser_visualization.py @@ -23,8 +23,6 @@ import numpy as np import pytest -pytest.importorskip("viser", reason="Viser optional dependency is not installed") - from dimos.manipulation.visualization.types import RobotInfo, TargetEvaluation from dimos.manipulation.visualization.viser.adapter import InProcessViserAdapter from dimos.manipulation.visualization.viser.animation import ( diff --git a/dimos/manipulation/visualization/viser/test_visualizer_lifecycle.py b/dimos/manipulation/visualization/viser/test_visualizer_lifecycle.py index 19f1f2da60..a00183a3d8 100644 --- a/dimos/manipulation/visualization/viser/test_visualizer_lifecycle.py +++ b/dimos/manipulation/visualization/viser/test_visualizer_lifecycle.py @@ -19,8 +19,6 @@ import pytest -pytest.importorskip("viser", reason="Viser optional dependency is not installed") - from dimos.manipulation.planning.spec.config import RobotModelConfig from dimos.manipulation.planning.spec.models import PlanningSceneInfo from dimos.manipulation.visualization.viser import ( diff --git a/dimos/mapping/ray_tracing/test_transformer.py b/dimos/mapping/ray_tracing/test_transformer.py index c633c519e9..4eb44a7225 100644 --- a/dimos/mapping/ray_tracing/test_transformer.py +++ b/dimos/mapping/ray_tracing/test_transformer.py @@ -18,8 +18,6 @@ from numpy.typing import NDArray import pytest -pytest.importorskip("dimos_voxel_ray_tracing") - from dimos.mapping.ray_tracing.transformer import RayTraceMap from dimos.memory2.type.observation import Observation from dimos.msgs.sensor_msgs.PointCloud2 import PointCloud2 diff --git a/dimos/mapping/ray_tracing/test_voxel_map.py b/dimos/mapping/ray_tracing/test_voxel_map.py index 467c8affc3..3ed93052ec 100644 --- a/dimos/mapping/ray_tracing/test_voxel_map.py +++ b/dimos/mapping/ray_tracing/test_voxel_map.py @@ -15,8 +15,6 @@ import numpy as np import pytest -pytest.importorskip("dimos_voxel_ray_tracing") - from dimos.mapping.ray_tracing.voxel_map import VoxelRayMapper ORIGIN = (0.0, 0.0, 0.0) diff --git a/dimos/memory2/codecs/test_codecs.py b/dimos/memory2/codecs/test_codecs.py index fde55cc837..0f8fc51573 100644 --- a/dimos/memory2/codecs/test_codecs.py +++ b/dimos/memory2/codecs/test_codecs.py @@ -193,7 +193,4 @@ def test_lcm_type_returns_lcm(self) -> None: assert isinstance(codec_for(PoseStamped), LcmCodec) def test_image_type_returns_jpeg(self) -> None: - pytest.importorskip("turbojpeg") - from dimos.memory2.codecs.jpeg import JpegCodec - assert isinstance(codec_for(Image), JpegCodec) diff --git a/dimos/navigation/nav_3d/mls_planner/test_mls_planner.py b/dimos/navigation/nav_3d/mls_planner/test_mls_planner.py index 8d62a61968..655b591950 100644 --- a/dimos/navigation/nav_3d/mls_planner/test_mls_planner.py +++ b/dimos/navigation/nav_3d/mls_planner/test_mls_planner.py @@ -15,8 +15,6 @@ import numpy as np import pytest -pytest.importorskip("dimos_mls_planner") - from dimos.navigation.nav_3d.mls_planner.mls_planner import MLSPlanner diff --git a/dimos/navigation/nav_3d/mls_planner/test_transformer.py b/dimos/navigation/nav_3d/mls_planner/test_transformer.py index 2cd84e03bb..a10ed29a56 100644 --- a/dimos/navigation/nav_3d/mls_planner/test_transformer.py +++ b/dimos/navigation/nav_3d/mls_planner/test_transformer.py @@ -16,9 +16,6 @@ import numpy as np from numpy.typing import NDArray -import pytest - -pytest.importorskip("dimos_mls_planner") from dimos.memory2.type.observation import Observation from dimos.msgs.sensor_msgs.PointCloud2 import PointCloud2 diff --git a/dimos/perception/fiducial/test_fixture_verification.py b/dimos/perception/fiducial/test_fixture_verification.py index 3134bd4434..7665ba0acf 100644 --- a/dimos/perception/fiducial/test_fixture_verification.py +++ b/dimos/perception/fiducial/test_fixture_verification.py @@ -38,8 +38,6 @@ from dimos.perception.fiducial.marker_detect import detect_markers_in_image from dimos.perception.fiducial.marker_tf_module import MarkerTfModule -pytest.importorskip("cv2.aruco") - @pytest.fixture(scope="module") def layout() -> BoardLayout: diff --git a/dimos/perception/fiducial/test_marker_detection_stream_module.py b/dimos/perception/fiducial/test_marker_detection_stream_module.py index c89f5e7ae6..5e91ba17ab 100644 --- a/dimos/perception/fiducial/test_marker_detection_stream_module.py +++ b/dimos/perception/fiducial/test_marker_detection_stream_module.py @@ -19,8 +19,6 @@ import numpy as np import pytest -pytest.importorskip("cv2.aruco") - from dimos.memory2.store.memory import MemoryStore from dimos.memory2.type.observation import Observation from dimos.msgs.geometry_msgs.Quaternion import Quaternion diff --git a/dimos/perception/fiducial/test_marker_pose.py b/dimos/perception/fiducial/test_marker_pose.py index f48eeff6a5..78c2275180 100644 --- a/dimos/perception/fiducial/test_marker_pose.py +++ b/dimos/perception/fiducial/test_marker_pose.py @@ -14,7 +14,6 @@ import cv2 import numpy as np -import pytest from dimos.msgs.sensor_msgs.CameraInfo import CameraInfo from dimos.msgs.sensor_msgs.Image import Image, ImageFormat @@ -25,8 +24,6 @@ marker_reprojection_error, ) -pytest.importorskip("cv2.aruco") - def test_camera_optical_frame_id_resolution() -> None: ts = 1.0 diff --git a/dimos/perception/fiducial/test_marker_transformer.py b/dimos/perception/fiducial/test_marker_transformer.py index c90e79f839..0fc84a8d7c 100644 --- a/dimos/perception/fiducial/test_marker_transformer.py +++ b/dimos/perception/fiducial/test_marker_transformer.py @@ -15,8 +15,6 @@ import numpy as np import pytest -pytest.importorskip("cv2.aruco") - from dimos.memory2.type.observation import Observation from dimos.msgs.sensor_msgs.CameraInfo import CameraInfo from dimos.msgs.sensor_msgs.Image import Image, ImageFormat diff --git a/dimos/protocol/pubsub/test_registry.py b/dimos/protocol/pubsub/test_registry.py index 9d7796c2ce..590c1208c5 100644 --- a/dimos/protocol/pubsub/test_registry.py +++ b/dimos/protocol/pubsub/test_registry.py @@ -15,6 +15,7 @@ from __future__ import annotations import pytest +import turbojpeg from dimos.core.transport import ( JpegLcmTransport, @@ -108,10 +109,9 @@ def test_make_pubsub_transport_shm_uses_SHMTransport() -> None: def test_make_pubsub_transport_jpeg_shm_uses_JpegShmTransport() -> None: - # The Python `turbojpeg` package is importable even when the native - # libturbojpeg.so is missing; the RuntimeError only fires when TurboJPEG() - # is actually constructed. Probe by trying to instantiate it. - turbojpeg = pytest.importorskip("turbojpeg") + # The native libturbojpeg.so can be missing even when the `turbojpeg` + # package imports fine; the RuntimeError only fires when TurboJPEG() is + # actually constructed. Probe by trying to instantiate it. try: turbojpeg.TurboJPEG() except RuntimeError as exc: diff --git a/dimos/simulation/engines/test_robot_sim_binding.py b/dimos/simulation/engines/test_robot_sim_binding.py index e06d115f7e..a05b3a368a 100644 --- a/dimos/simulation/engines/test_robot_sim_binding.py +++ b/dimos/simulation/engines/test_robot_sim_binding.py @@ -17,10 +17,9 @@ from pathlib import Path from typing import Any +import mujoco import pytest -mujoco = pytest.importorskip("mujoco") - from dimos.simulation.engines.mujoco_engine import MujocoEngine from dimos.simulation.engines.robot_sim_binding import ( RobotSimSpec,