diff --git a/.github/actions/run-package-tests/action.yml b/.github/actions/run-package-tests/action.yml index 6afdde7cbc7..ccd3fa55b68 100644 --- a/.github/actions/run-package-tests/action.yml +++ b/.github/actions/run-package-tests/action.yml @@ -79,6 +79,13 @@ inputs: description: 'Additional pytest options' default: '' required: false + test-path: + description: >- + Path handed to pytest. Defaults to "tools", which loads tools/conftest.py and runs each + test file in its own subprocess. Point it at a test directory instead to run those files + together in a single pytest process, bypassing the per-file orchestrator. + default: 'tools' + required: false extra-pip-packages: description: 'Space-separated pip packages to install inside the Docker container before pytest starts' default: '' @@ -291,7 +298,7 @@ runs: - name: Run Tests uses: ./.github/actions/run-tests with: - test-path: "tools" + test-path: ${{ inputs.test-path }} result-file: "${{ inputs.result-file != '' && inputs.result-file || format('{0}-report.xml', github.job) }}" container-name: "${{ inputs.container-name }}-${{ github.run_id }}-${{ github.run_attempt }}" image-tag: ${{ inputs.image-tag }} diff --git a/pyproject.toml b/pyproject.toml index d743d6a2d54..b34bf53c547 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -338,6 +338,11 @@ markers = [ "benchmark: test covers the Isaac Lab benchmark framework and infrastructure", "rendering: test exercises the rendering / camera / visualizer pipeline", "smoke: tests for core installation, task, and RL functionality", + "kit: test file needs a booted headless Kit app; it calls isaaclab.test.launch.launch_kit() at module scope rather than constructing AppLauncher", + "kit_cameras: like `kit`, but the app is booted with cameras enabled via launch_kit(cameras=True)", + "kitless: test file runs without Kit; no AppLauncher and no module-scope import of omni/carb/isaacsim", + "kit_solo: keep this file in its own process; it is never grouped with other files", + "newton_ci: mark test to run in the Newton CI lane", ] # Add pypi.nvidia.com so that `uv pip install isaaclab[isaacsim]` works without --extra-index-url. diff --git a/source/isaaclab/changelog.d/mataylor-kit-test-markers.rst b/source/isaaclab/changelog.d/mataylor-kit-test-markers.rst new file mode 100644 index 00000000000..1b2acabc992 --- /dev/null +++ b/source/isaaclab/changelog.d/mataylor-kit-test-markers.rst @@ -0,0 +1,15 @@ +Added +^^^^^ + +* Added :func:`~isaaclab.test.launch.launch_kit` so test modules can share one Kit app per + pytest process instead of each launching their own. It is idempotent: the first module to + call it boots Kit and later modules receive the running app. +* Added the ``kit``, ``kit_cameras``, ``kitless``, and ``kit_solo`` pytest markers so a test + file can declare its Kit launch configuration, plus a test that checks each file's markers + against what it actually does at module scope. + +Fixed +^^^^^ + +* Fixed ``test_operational_space.py`` assigning ``pytestmark`` twice, which silently dropped + its ``arm_ci`` marker and kept the file out of the ARM CI lane. diff --git a/source/isaaclab/isaaclab/test/launch.py b/source/isaaclab/isaaclab/test/launch.py new file mode 100644 index 00000000000..1234ef57501 --- /dev/null +++ b/source/isaaclab/isaaclab/test/launch.py @@ -0,0 +1,94 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""Shared Kit launch helper for Isaac Lab tests. + +Test modules that need Isaac Sim call :func:`launch_kit` at module scope in place of +constructing :class:`~isaaclab.app.AppLauncher` directly:: + + from isaaclab.test.launch import launch_kit + + launch_kit() # or launch_kit(cameras=True) + +The call must stay at module scope: a test module's own imports (``pxr``, ``omni``, +``isaaclab_physx``, ...) run during pytest collection, before any fixture executes, so Kit +must already be running by then. + +:func:`launch_kit` is idempotent within a process. The first test module to call it boots +Kit; every later module gets the running app back. A pytest process covering several test +files therefore pays Kit startup once rather than once per file. + +Declare the matching marker on the module so the test runner can group files that share a +launch configuration into one process:: + + pytestmark = pytest.mark.kit # launch_kit() + pytestmark = pytest.mark.kit_cameras # launch_kit(cameras=True) + +The two groups cannot be merged. Cameras cannot be enabled after startup, so a plain ``kit`` +file cannot run in a process a ``kit_cameras`` file will later join; and a camera-enabled app +is not a drop-in replacement for a plain one either, because some tests assert that offscreen +rendering is off. :func:`launch_kit` therefore raises on any mismatch rather than handing back +an app whose configuration is not the one the caller asked for. +""" + +from __future__ import annotations + +from typing import Any + +_app: Any = None +"""The Kit application booted by :func:`launch_kit`, or None before the first call.""" + +_cameras: bool = False +"""Whether :attr:`_app` was booted with camera and render extensions enabled.""" + + +def launch_kit(*, cameras: bool = False) -> Any: + """Boot the shared Kit app for this process, or return the one already running. + + Args: + cameras: Whether the app must be booted with camera and render extensions enabled. + Passed through to :paramref:`~isaaclab.app.AppLauncher.enable_cameras`. + + Returns: + The running ``SimulationApp``. + + Raises: + RuntimeError: If the running app was booted with a different ``cameras`` setting, or if + Kit was started by something other than this function. Both mean the test files + sharing this process do not share a launch configuration and must be split across + processes. + """ + global _app, _cameras + + if _app is not None: + if cameras != _cameras: + wanted = "with" if cameras else "without" + running = "with" if _cameras else "without" + raise RuntimeError( + f"launch_kit(cameras={cameras}) wants an app {wanted} cameras, but Kit is already" + f" running in this process {running} them, and that cannot be changed after" + " startup. Files marked `kit` and `kit_cameras` need separate processes. A" + " camera-enabled app is not a drop-in replacement for a plain one:" + " test_simulation_context.py::test_headless_mode asserts that offscreen" + " rendering is off." + ) + return _app + + from isaaclab.utils import has_kit + + if has_kit(): + raise RuntimeError( + "Kit is already running but was not started by launch_kit(), so its launch" + " configuration is unknown. Another test file in this process still constructs" + " AppLauncher directly; run that file in its own process." + ) + + from isaaclab.app import AppLauncher + + from .utils import resolve_test_sim_device + + _app = AppLauncher(headless=True, enable_cameras=cameras, device=resolve_test_sim_device()).app + _cameras = cameras + return _app diff --git a/source/isaaclab/test/controllers/test_operational_space.py b/source/isaaclab/test/controllers/test_operational_space.py index 1925c6673a0..8db637450a3 100644 --- a/source/isaaclab/test/controllers/test_operational_space.py +++ b/source/isaaclab/test/controllers/test_operational_space.py @@ -16,8 +16,6 @@ import torch from flaky import flaky -pytestmark = pytest.mark.arm_ci - import isaaclab.envs.mdp as mdp import isaaclab.sim as sim_utils from isaaclab import cloner @@ -51,7 +49,7 @@ from isaaclab_assets import FRANKA_PANDA_CFG, G1_29DOF_CFG # isort:skip -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.arm_ci, pytest.mark.integration] @pytest.fixture diff --git a/source/isaaclab/test/sim/test_articulation_fragments.py b/source/isaaclab/test/sim/test_articulation_fragments.py index 2319363122e..68de1554d21 100644 --- a/source/isaaclab/test/sim/test_articulation_fragments.py +++ b/source/isaaclab/test/sim/test_articulation_fragments.py @@ -3,14 +3,9 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() import os @@ -21,6 +16,8 @@ import isaaclab.sim as sim_utils from isaaclab.sim import SimulationCfg, SimulationContext +pytestmark = pytest.mark.kit + def _make_xform(stage, path="/World/Art"): UsdGeom.Xform.Define(stage, path) diff --git a/source/isaaclab/test/sim/test_build_simulation_context_headless.py b/source/isaaclab/test/sim/test_build_simulation_context_headless.py index cf266f73f4f..cc3e98ebabf 100644 --- a/source/isaaclab/test/sim/test_build_simulation_context_headless.py +++ b/source/isaaclab/test/sim/test_build_simulation_context_headless.py @@ -13,21 +13,16 @@ ``test_build_simulation_context_nonheadless.py``. """ -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() import pytest from isaaclab.sim.simulation_cfg import SimulationCfg from isaaclab.sim.simulation_context import build_simulation_context -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit, pytest.mark.integration] @pytest.mark.parametrize("gravity_enabled", [True, False]) diff --git a/source/isaaclab/test/sim/test_build_simulation_context_nonheadless.py b/source/isaaclab/test/sim/test_build_simulation_context_nonheadless.py index 2ce2345062c..fd5fe7137ab 100644 --- a/source/isaaclab/test/sim/test_build_simulation_context_nonheadless.py +++ b/source/isaaclab/test/sim/test_build_simulation_context_nonheadless.py @@ -12,21 +12,16 @@ ``test_build_simulation_context_headless.py``. """ -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() import pytest from isaaclab.sim.simulation_cfg import SimulationCfg from isaaclab.sim.simulation_context import build_simulation_context -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit, pytest.mark.integration] @pytest.mark.parametrize("gravity_enabled", [True, False]) diff --git a/source/isaaclab/test/sim/test_cloner.py b/source/isaaclab/test/sim/test_cloner.py index 7bf436e7023..485f0ee09ba 100644 --- a/source/isaaclab/test/sim/test_cloner.py +++ b/source/isaaclab/test/sim/test_cloner.py @@ -5,14 +5,9 @@ """Tests for USD cloner utilities (no PhysX dependency).""" -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() from types import SimpleNamespace from unittest.mock import MagicMock @@ -37,7 +32,7 @@ ) from isaaclab.sim import build_simulation_context -pytestmark = [pytest.mark.integration, pytest.mark.isaacsim_ci] +pytestmark = [pytest.mark.kit, pytest.mark.integration, pytest.mark.isaacsim_ci] @pytest.fixture(params=["cpu", "cuda"]) diff --git a/source/isaaclab/test/sim/test_collision_fragments.py b/source/isaaclab/test/sim/test_collision_fragments.py index 712390bc2f5..c3c4005c449 100644 --- a/source/isaaclab/test/sim/test_collision_fragments.py +++ b/source/isaaclab/test/sim/test_collision_fragments.py @@ -3,14 +3,9 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() import pytest @@ -19,7 +14,7 @@ import isaaclab.sim as sim_utils from isaaclab.sim import SimulationCfg, SimulationContext -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit, pytest.mark.integration] def _make_xform(stage, path="/World/Body"): diff --git a/source/isaaclab/test/sim/test_joint_drive_fragments.py b/source/isaaclab/test/sim/test_joint_drive_fragments.py index a9c5534ede3..1a6be46df6a 100644 --- a/source/isaaclab/test/sim/test_joint_drive_fragments.py +++ b/source/isaaclab/test/sim/test_joint_drive_fragments.py @@ -3,14 +3,9 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() import math @@ -21,7 +16,7 @@ import isaaclab.sim as sim_utils from isaaclab.sim import SimulationCfg, SimulationContext -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit, pytest.mark.integration] def _make_revolute_joint(stage, path="/World/Articulation/joint_0"): diff --git a/source/isaaclab/test/sim/test_mass_fragments.py b/source/isaaclab/test/sim/test_mass_fragments.py index f017d9d2d16..f08578ac18d 100644 --- a/source/isaaclab/test/sim/test_mass_fragments.py +++ b/source/isaaclab/test/sim/test_mass_fragments.py @@ -3,14 +3,9 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() import pytest @@ -19,7 +14,7 @@ import isaaclab.sim as sim_utils from isaaclab.sim import SimulationCfg, SimulationContext -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit, pytest.mark.integration] def _make_xform(stage, path="/World/Body"): diff --git a/source/isaaclab/test/sim/test_material_fragments.py b/source/isaaclab/test/sim/test_material_fragments.py index c09362c5efd..c69d51c71e8 100644 --- a/source/isaaclab/test/sim/test_material_fragments.py +++ b/source/isaaclab/test/sim/test_material_fragments.py @@ -3,14 +3,9 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() import pytest @@ -19,7 +14,7 @@ import isaaclab.sim as sim_utils from isaaclab.sim import SimulationCfg, SimulationContext -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit, pytest.mark.integration] # ------------------------------------------------------------------------------------- # RigidBodyMaterialFragment marker + metadata diff --git a/source/isaaclab/test/sim/test_mesh_collision_fragments.py b/source/isaaclab/test/sim/test_mesh_collision_fragments.py index ae33dbb938d..5be29b24ea0 100644 --- a/source/isaaclab/test/sim/test_mesh_collision_fragments.py +++ b/source/isaaclab/test/sim/test_mesh_collision_fragments.py @@ -3,14 +3,9 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() import pytest @@ -19,7 +14,7 @@ import isaaclab.sim as sim_utils from isaaclab.sim import SimulationCfg, SimulationContext -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit, pytest.mark.integration] def _make_xform(stage, path="/World/Mesh"): diff --git a/source/isaaclab/test/sim/test_mesh_converter.py b/source/isaaclab/test/sim/test_mesh_converter.py index f4551b4ba82..2120df259f2 100644 --- a/source/isaaclab/test/sim/test_mesh_converter.py +++ b/source/isaaclab/test/sim/test_mesh_converter.py @@ -3,14 +3,9 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() import math import os @@ -27,7 +22,7 @@ from isaaclab.sim.schemas import MESH_APPROXIMATION_TOKENS, schemas_cfg from isaaclab.utils.assets import ISAACLAB_NUCLEUS_DIR, retrieve_file_path -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit, pytest.mark.integration] def random_quaternion(): diff --git a/source/isaaclab/test/sim/test_schema_fragments.py b/source/isaaclab/test/sim/test_schema_fragments.py index e6ca68c3ddd..c8a00f31cff 100644 --- a/source/isaaclab/test/sim/test_schema_fragments.py +++ b/source/isaaclab/test/sim/test_schema_fragments.py @@ -3,14 +3,9 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() import pytest @@ -19,7 +14,7 @@ import isaaclab.sim as sim_utils from isaaclab.sim import SimulationCfg, SimulationContext -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit, pytest.mark.integration] def _make_xform(stage, path="/World/Body"): diff --git a/source/isaaclab/test/sim/test_schema_writer_nested_targets.py b/source/isaaclab/test/sim/test_schema_writer_nested_targets.py index b1cf4331a04..1aa9146a2c3 100644 --- a/source/isaaclab/test/sim/test_schema_writer_nested_targets.py +++ b/source/isaaclab/test/sim/test_schema_writer_nested_targets.py @@ -3,14 +3,9 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() import os @@ -23,7 +18,7 @@ from isaaclab.sim import SimulationCfg, SimulationContext from isaaclab.sim.schemas import MassCfg -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit, pytest.mark.integration] def _author_robot_usd(path: str) -> None: diff --git a/source/isaaclab/test/sim/test_schemas.py b/source/isaaclab/test/sim/test_schemas.py index 337dd2b6930..92f9adcfbf8 100644 --- a/source/isaaclab/test/sim/test_schemas.py +++ b/source/isaaclab/test/sim/test_schemas.py @@ -3,14 +3,9 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() import math import warnings @@ -45,7 +40,7 @@ from isaaclab.utils.assets import ISAAC_NUCLEUS_DIR from isaaclab.utils.string import to_camel_case -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit, pytest.mark.integration] @pytest.fixture diff --git a/source/isaaclab/test/sim/test_simulation_context.py b/source/isaaclab/test/sim/test_simulation_context.py index 17d9c348f26..dc5c05842eb 100644 --- a/source/isaaclab/test/sim/test_simulation_context.py +++ b/source/isaaclab/test/sim/test_simulation_context.py @@ -3,15 +3,10 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit +from isaaclab.test.utils import test_devices -from isaaclab.app import AppLauncher -from isaaclab.test.utils import resolve_test_sim_device, test_devices - -# launch omniverse app -simulation_app = AppLauncher(headless=True, device=resolve_test_sim_device()).app - -"""Rest everything follows.""" +launch_kit() import weakref @@ -24,7 +19,7 @@ import isaaclab.sim as sim_utils from isaaclab.sim import SimulationCfg, SimulationContext -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit, pytest.mark.integration] @pytest.fixture(autouse=True) diff --git a/source/isaaclab/test/sim/test_simulation_stage_in_memory.py b/source/isaaclab/test/sim/test_simulation_stage_in_memory.py index f91947fc32b..450d893d4ed 100644 --- a/source/isaaclab/test/sim/test_simulation_stage_in_memory.py +++ b/source/isaaclab/test/sim/test_simulation_stage_in_memory.py @@ -5,16 +5,10 @@ """Integration tests for simulation context with stage in memory.""" -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app # FIXME (mmittal): Stage in memory requires cameras to be enabled. -simulation_app = AppLauncher(headless=True, enable_cameras=True).app - -"""Rest everything follows.""" - +launch_kit(cameras=True) import pytest import torch @@ -28,7 +22,12 @@ from isaaclab.utils.assets import ISAACLAB_NUCLEUS_DIR from isaaclab.utils.version import get_isaac_sim_version -pytestmark = pytest.mark.integration +# kit_solo: sharing a Kit app with other test files killed the pytest process here. In the +# kit-reuse-probe-batched CI job this file's first test aborted the interpreter immediately +# after collection, with no Python traceback, while the same test is fine in its own process. +# The cause is not yet understood -- creating the stage in memory is sensitive to what else has +# already touched the stage or the extension set -- so keep the file on its own until it is. +pytestmark = [pytest.mark.kit_cameras, pytest.mark.kit_solo, pytest.mark.integration] @pytest.fixture diff --git a/source/isaaclab/test/sim/test_spawn_from_files.py b/source/isaaclab/test/sim/test_spawn_from_files.py index 0a771c956f2..4515555fb1b 100644 --- a/source/isaaclab/test/sim/test_spawn_from_files.py +++ b/source/isaaclab/test/sim/test_spawn_from_files.py @@ -3,14 +3,9 @@ # # SPDX-License-Identifier: BSD-3-Clause -from isaaclab.app import AppLauncher +from isaaclab.test.launch import launch_kit -"""Launch Isaac Sim Simulator first.""" - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() import pytest @@ -20,7 +15,7 @@ from isaaclab.sim import SimulationCfg, SimulationContext from isaaclab.utils.assets import ISAACLAB_NUCLEUS_DIR -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit, pytest.mark.integration] @pytest.fixture diff --git a/source/isaaclab/test/sim/test_spawn_lights.py b/source/isaaclab/test/sim/test_spawn_lights.py index 59c77188078..bea78e90915 100644 --- a/source/isaaclab/test/sim/test_spawn_lights.py +++ b/source/isaaclab/test/sim/test_spawn_lights.py @@ -3,15 +3,9 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" - -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +from isaaclab.test.launch import launch_kit +launch_kit() import pytest @@ -21,7 +15,7 @@ from isaaclab.sim import SimulationCfg, SimulationContext from isaaclab.utils.string import to_camel_case -pytestmark = [pytest.mark.integration, pytest.mark.isaacsim_ci] +pytestmark = [pytest.mark.kit, pytest.mark.integration, pytest.mark.isaacsim_ci] @pytest.fixture(autouse=True) diff --git a/source/isaaclab/test/sim/test_spawn_materials.py b/source/isaaclab/test/sim/test_spawn_materials.py index d1cb86c8702..93ccd392f7c 100644 --- a/source/isaaclab/test/sim/test_spawn_materials.py +++ b/source/isaaclab/test/sim/test_spawn_materials.py @@ -3,15 +3,9 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" - -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +from isaaclab.test.launch import launch_kit +launch_kit() import pytest @@ -21,7 +15,7 @@ from isaaclab.sim import SimulationCfg, SimulationContext from isaaclab.utils.assets import NVIDIA_NUCLEUS_DIR -pytestmark = [pytest.mark.integration, pytest.mark.isaacsim_ci] +pytestmark = [pytest.mark.kit, pytest.mark.integration, pytest.mark.isaacsim_ci] @pytest.fixture diff --git a/source/isaaclab/test/sim/test_spawn_meshes.py b/source/isaaclab/test/sim/test_spawn_meshes.py index a9ad5158c2f..1a2fc76f296 100644 --- a/source/isaaclab/test/sim/test_spawn_meshes.py +++ b/source/isaaclab/test/sim/test_spawn_meshes.py @@ -3,22 +3,16 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" - -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +from isaaclab.test.launch import launch_kit +launch_kit() import pytest import isaaclab.sim as sim_utils from isaaclab.sim import SimulationCfg, SimulationContext -pytestmark = [pytest.mark.integration, pytest.mark.isaacsim_ci] +pytestmark = [pytest.mark.kit, pytest.mark.integration, pytest.mark.isaacsim_ci] @pytest.fixture diff --git a/source/isaaclab/test/sim/test_spawn_sensors.py b/source/isaaclab/test/sim/test_spawn_sensors.py index 9e50b54496b..af0df8b714a 100644 --- a/source/isaaclab/test/sim/test_spawn_sensors.py +++ b/source/isaaclab/test/sim/test_spawn_sensors.py @@ -3,15 +3,9 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" - -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +from isaaclab.test.launch import launch_kit +launch_kit() import pytest @@ -22,7 +16,7 @@ from isaaclab.sim.spawners.sensors.sensors import CUSTOM_FISHEYE_CAMERA_ATTRIBUTES, CUSTOM_PINHOLE_CAMERA_ATTRIBUTES from isaaclab.utils.string import to_camel_case -pytestmark = [pytest.mark.integration, pytest.mark.isaacsim_ci] +pytestmark = [pytest.mark.kit, pytest.mark.integration, pytest.mark.isaacsim_ci] @pytest.fixture diff --git a/source/isaaclab/test/sim/test_spawn_shapes.py b/source/isaaclab/test/sim/test_spawn_shapes.py index be59ea011d0..def648d5e7e 100644 --- a/source/isaaclab/test/sim/test_spawn_shapes.py +++ b/source/isaaclab/test/sim/test_spawn_shapes.py @@ -3,21 +3,16 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() import pytest import isaaclab.sim as sim_utils from isaaclab.sim import SimulationCfg, SimulationContext -pytestmark = [pytest.mark.integration, pytest.mark.isaacsim_ci] +pytestmark = [pytest.mark.kit, pytest.mark.integration, pytest.mark.isaacsim_ci] @pytest.fixture diff --git a/source/isaaclab/test/sim/test_spawn_wrappers.py b/source/isaaclab/test/sim/test_spawn_wrappers.py index a0be9336a56..c66d9fd7daf 100644 --- a/source/isaaclab/test/sim/test_spawn_wrappers.py +++ b/source/isaaclab/test/sim/test_spawn_wrappers.py @@ -3,15 +3,9 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" - -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +from isaaclab.test.launch import launch_kit +launch_kit() import pytest @@ -19,7 +13,7 @@ from isaaclab.sim import SimulationCfg, SimulationContext from isaaclab.utils.assets import ISAACLAB_NUCLEUS_DIR -pytestmark = [pytest.mark.integration, pytest.mark.isaacsim_ci] +pytestmark = [pytest.mark.kit, pytest.mark.integration, pytest.mark.isaacsim_ci] @pytest.fixture diff --git a/source/isaaclab/test/sim/test_tendon_fragments.py b/source/isaaclab/test/sim/test_tendon_fragments.py index c7569081a16..65e485911db 100644 --- a/source/isaaclab/test/sim/test_tendon_fragments.py +++ b/source/isaaclab/test/sim/test_tendon_fragments.py @@ -3,14 +3,9 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() import pytest @@ -19,7 +14,7 @@ import isaaclab.sim as sim_utils from isaaclab.sim import SimulationCfg, SimulationContext -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit, pytest.mark.integration] def _new_sim(): diff --git a/source/isaaclab/test/sim/test_utils_prims.py b/source/isaaclab/test/sim/test_utils_prims.py index 117aaced160..c1703011b08 100644 --- a/source/isaaclab/test/sim/test_utils_prims.py +++ b/source/isaaclab/test/sim/test_utils_prims.py @@ -3,15 +3,10 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app # note: need to enable cameras to be able to make replicator core available -simulation_app = AppLauncher(headless=True, enable_cameras=True).app - -"""Rest everything follows.""" +launch_kit(cameras=True) import math @@ -25,7 +20,7 @@ from isaaclab.sim.utils.prims import _to_tuple # type: ignore[reportPrivateUsage] from isaaclab.utils.assets import ISAAC_NUCLEUS_DIR, ISAACLAB_NUCLEUS_DIR, retrieve_file_path -pytestmark = [pytest.mark.integration, pytest.mark.isaacsim_ci] +pytestmark = [pytest.mark.kit_cameras, pytest.mark.integration, pytest.mark.isaacsim_ci] @pytest.fixture(autouse=True) diff --git a/source/isaaclab/test/sim/test_utils_queries.py b/source/isaaclab/test/sim/test_utils_queries.py index 973e7e71856..92997d04b09 100644 --- a/source/isaaclab/test/sim/test_utils_queries.py +++ b/source/isaaclab/test/sim/test_utils_queries.py @@ -3,15 +3,10 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app # note: need to enable cameras to be able to make replicator core available -simulation_app = AppLauncher(headless=True, enable_cameras=True).app - -"""Rest everything follows.""" +launch_kit(cameras=True) import pytest @@ -20,7 +15,7 @@ import isaaclab.sim as sim_utils from isaaclab.utils.assets import ISAAC_NUCLEUS_DIR, ISAACLAB_NUCLEUS_DIR -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit_cameras, pytest.mark.integration] @pytest.fixture(autouse=True) diff --git a/source/isaaclab/test/sim/test_utils_semantics.py b/source/isaaclab/test/sim/test_utils_semantics.py index 926a2d0d80a..c88f9e0d8df 100644 --- a/source/isaaclab/test/sim/test_utils_semantics.py +++ b/source/isaaclab/test/sim/test_utils_semantics.py @@ -3,21 +3,16 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app # note: need to enable cameras to be able to make replicator core available -simulation_app = AppLauncher(headless=True, enable_cameras=True).app - -"""Rest everything follows.""" +launch_kit(cameras=True) import pytest import isaaclab.sim as sim_utils -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit_cameras, pytest.mark.integration] @pytest.fixture(autouse=True) diff --git a/source/isaaclab/test/sim/test_utils_stage.py b/source/isaaclab/test/sim/test_utils_stage.py index 39a70a076f7..3bcd26e6636 100644 --- a/source/isaaclab/test/sim/test_utils_stage.py +++ b/source/isaaclab/test/sim/test_utils_stage.py @@ -5,14 +5,9 @@ """Tests for stage utilities.""" -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() import tempfile from pathlib import Path @@ -23,7 +18,7 @@ import isaaclab.sim as sim_utils -pytestmark = [pytest.mark.integration, pytest.mark.isaacsim_ci] +pytestmark = [pytest.mark.kit, pytest.mark.integration, pytest.mark.isaacsim_ci] def test_create_new_stage(): diff --git a/source/isaaclab/test/sim/test_utils_transforms.py b/source/isaaclab/test/sim/test_utils_transforms.py index e7cc178b65d..1af8ce75bea 100644 --- a/source/isaaclab/test/sim/test_utils_transforms.py +++ b/source/isaaclab/test/sim/test_utils_transforms.py @@ -3,14 +3,9 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Launch Isaac Sim Simulator first.""" +from isaaclab.test.launch import launch_kit -from isaaclab.app import AppLauncher - -# launch omniverse app -simulation_app = AppLauncher(headless=True).app - -"""Rest everything follows.""" +launch_kit() import math @@ -23,7 +18,7 @@ import isaaclab.sim as sim_utils import isaaclab.utils.math as math_utils -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit, pytest.mark.integration] @pytest.fixture(autouse=True) diff --git a/source/isaaclab/test/sim/test_views_xform_prim.py b/source/isaaclab/test/sim/test_views_xform_prim.py index 9217ca537d0..8b56f5f39f6 100644 --- a/source/isaaclab/test/sim/test_views_xform_prim.py +++ b/source/isaaclab/test/sim/test_views_xform_prim.py @@ -10,10 +10,10 @@ prim ordering, xformOp standardization, and Isaac Sim comparison. """ -from isaaclab.app import AppLauncher -from isaaclab.test.utils import resolve_test_sim_device, test_devices +from isaaclab.test.launch import launch_kit +from isaaclab.test.utils import test_devices -simulation_app = AppLauncher(headless=True, device=resolve_test_sim_device()).app +launch_kit() import pytest # noqa: E402 import torch # noqa: E402 @@ -24,6 +24,9 @@ try: from isaaclab.sim.utils import enable_extension # noqa: E402 + # NOTE: this runs at import, so in a process shared with other test files it changes the + # running app's extension set during collection, before any test executes. Harmless when + # this file has the process to itself; a hazard once files are batched together. enable_extension("isaacsim.core.experimental.prims") from isaacsim.core.experimental.prims import XformPrim as _IsaacSimXformPrimView except (ModuleNotFoundError, ImportError, RuntimeError): @@ -36,7 +39,13 @@ from isaaclab.sim.views import UsdFrameView as FrameView # noqa: E402 from isaaclab.utils.assets import ISAAC_NUCLEUS_DIR # noqa: E402 -pytestmark = [pytest.mark.integration, pytest.mark.isaacsim_ci] +# kit_solo: test_compare_get_world_poses_with_isaacsim goes through Isaac Sim's +# SimulationManager, a process-global singleton that caches the PhysxScene wrapping +# /physicsScene. In a process shared with other test files that prim belongs to a stage an +# earlier file already tore down, so the cached wrapper is dangling and the test dies with +# "Accessed invalid expired 'PhysicsScene' prim". Nothing in this file owns that state, so the +# file needs a process to itself until SimulationManager can be reset between files. +pytestmark = [pytest.mark.kit, pytest.mark.kit_solo, pytest.mark.integration, pytest.mark.isaacsim_ci] PARENT_POS = (0.0, 0.0, 1.0) diff --git a/source/isaaclab/test/test_kit_batching.py b/source/isaaclab/test/test_kit_batching.py new file mode 100644 index 00000000000..93c698db885 --- /dev/null +++ b/source/isaaclab/test/test_kit_batching.py @@ -0,0 +1,212 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""Tests for the Kit batching grouping and JUnit demultiplexing. + +Both are pure functions over paths and strings, so they run anywhere; the process machinery +they feed is POSIX-only and only exercisable in CI. +""" + +from __future__ import annotations + +import sys +import textwrap +from pathlib import Path + +import pytest +from junitparser import JUnitXml + +sys.path.insert(0, str(Path(__file__).resolve().parents[3] / "tools")) + +from _kit_batching import ( # noqa: E402 + Batch, + batch_size, + batching_enabled, + file_profile, + group_test_files, + split_batch_status, +) + +pytestmark = [pytest.mark.unit, pytest.mark.kitless] + + +KIT = "pytestmark = pytest.mark.kit\n" +CAMERAS = "pytestmark = [pytest.mark.kit_cameras, pytest.mark.integration]\n" +SOLO = "pytestmark = [pytest.mark.kit, pytest.mark.kit_solo]\n" +KITLESS = "pytestmark = pytest.mark.kitless\n" +LEGACY = "simulation_app = AppLauncher(headless=True).app\n" + + +class TestFileProfile: + """`file_profile` classifies a file from its marker text.""" + + @pytest.mark.parametrize( + "source,expected", + [ + (KIT, "kit"), + (CAMERAS, "kit_cameras"), + (SOLO, None), + (KITLESS, None), + (LEGACY, None), + ("", None), + ], + ) + def test_profile_matches_markers(self, source: str, expected: str | None): + assert file_profile(source) == expected + + def test_kit_pattern_does_not_swallow_the_longer_markers(self): + """A bare `kit` match must not claim kit_cameras or kit_solo files.""" + assert file_profile("pytest.mark.kit_cameras") == "kit_cameras" + assert file_profile("pytest.mark.kit_solo") is None + assert file_profile("pytest.mark.kitless") is None + + +class TestGrouping: + """`group_test_files` batches same-profile files and isolates everything else.""" + + def test_same_profile_files_share_one_batch(self): + files = ["a.py", "b.py", "c.py"] + batches = group_test_files(files, dict.fromkeys(files, KIT)) + assert len(batches) == 1 + assert batches[0].profile == "kit" + assert batches[0].files == files + + def test_profiles_never_mix(self): + sources = {"a.py": KIT, "b.py": CAMERAS, "c.py": KIT} + batches = group_test_files(list(sources), sources) + by_profile = {b.profile: b.files for b in batches} + assert by_profile["kit"] == ["a.py", "c.py"] + assert by_profile["kit_cameras"] == ["b.py"] + + @pytest.mark.parametrize("source", [SOLO, KITLESS, LEGACY]) + def test_unbatchable_files_get_their_own_batch(self, source: str): + sources = {"a.py": KIT, "b.py": source, "c.py": KIT} + batches = group_test_files(list(sources), sources) + solo = [b for b in batches if b.files == ["b.py"]] + assert solo and solo[0].profile is None + assert not solo[0].is_batched + + def test_explicit_unbatchable_overrides_the_marker(self): + sources = {"a.py": KIT, "b.py": KIT} + batches = group_test_files(list(sources), sources, unbatchable={"b.py"}) + assert Batch(profile=None, files=["b.py"]) in batches + + def test_missing_source_is_treated_as_unbatchable(self): + """An unreadable file must not be assumed safe to share a process.""" + batches = group_test_files(["a.py", "b.py"], {"a.py": KIT}) + assert any(b.files == ["b.py"] and b.profile is None for b in batches) + + def test_batches_are_capped(self): + files = [f"f{i}.py" for i in range(7)] + batches = group_test_files(files, dict.fromkeys(files, KIT), max_size=3) + assert [len(b.files) for b in batches] == [3, 3, 1] + + def test_labels_are_unique_across_batches(self): + """A label becomes a JUnit report filename, so two batches must never collide. + + Two same-profile batches of equal size are the case that matters: without the index + they would produce the same label and the second would overwrite the first's report. + """ + files = [f"f{i}.py" for i in range(6)] + batches = group_test_files(files, dict.fromkeys(files, KIT), max_size=3) + labels = [b.label for b in batches] + assert len(batches) == 2 + assert len(labels) == len(set(labels)), f"colliding labels: {labels}" + + def test_every_file_appears_exactly_once(self): + sources = {"a.py": KIT, "b.py": CAMERAS, "c.py": SOLO, "d.py": KIT, "e.py": LEGACY} + batches = group_test_files(list(sources), sources) + covered = [f for b in batches for f in b.files] + assert sorted(covered) == sorted(sources) + assert len(covered) == len(set(covered)) + + +def _report(*cases: tuple[str, str, str, float]) -> JUnitXml: + """Build a JUnit report from ``(classname, name, outcome, time)`` tuples.""" + body = "".join( + f'' + + {"pass": "", "fail": "", "error": "", "skip": ""}[ + outcome + ] + + "" + for cls, name, outcome, t in cases + ) + xml = textwrap.dedent(f"""\ + + {body} + """) + return JUnitXml.fromstring(xml.encode("utf-8")) + + +class TestSplitBatchStatus: + """`split_batch_status` attributes a batch's report back to individual files.""" + + def test_counts_are_attributed_per_file(self): + report = _report( + ("source.sim.test_a", "test_one", "pass", 1.0), + ("source.sim.test_a", "test_two", "fail", 2.0), + ("source.sim.test_b", "test_three", "pass", 3.0), + ) + status = split_batch_status( + report, ["source/sim/test_a.py", "source/sim/test_b.py"], wall_time=60.0, batch_result="CRASHED" + ) + a = status["source/sim/test_a.py"] + b = status["source/sim/test_b.py"] + assert (a["tests"], a["failures"], a["result"]) == (2, 1, "FAILED") + assert (b["tests"], b["failures"], b["result"]) == (1, 0, "passed") + assert a["time_elapsed"] == pytest.approx(3.0) + assert b["time_elapsed"] == pytest.approx(3.0) + + def test_files_that_never_ran_take_the_batch_result(self): + """A file with no testcases means the shared process died before reaching it.""" + report = _report(("source.sim.test_a", "test_one", "pass", 1.0)) + status = split_batch_status( + report, ["source/sim/test_a.py", "source/sim/test_b.py"], wall_time=10.0, batch_result="CRASHED" + ) + assert status["source/sim/test_a.py"]["result"] == "passed" + assert status["source/sim/test_b.py"]["result"] == "CRASHED" + assert status["source/sim/test_b.py"]["errors"] == 1 + + def test_wall_time_is_shared_only_between_files_that_ran(self): + report = _report( + ("source.sim.test_a", "t", "pass", 1.0), + ("source.sim.test_b", "t", "pass", 1.0), + ) + files = ["source/sim/test_a.py", "source/sim/test_b.py", "source/sim/test_c.py"] + status = split_batch_status(report, files, wall_time=90.0, batch_result="CRASHED") + assert status["source/sim/test_a.py"]["wall_time"] == pytest.approx(45.0) + assert status["source/sim/test_b.py"]["wall_time"] == pytest.approx(45.0) + assert status["source/sim/test_c.py"]["wall_time"] == 0.0 + + def test_errors_and_skips_are_counted_separately(self): + report = _report( + ("source.sim.test_a", "t1", "error", 0.5), + ("source.sim.test_a", "t2", "skip", 0.0), + ) + status = split_batch_status(report, ["source/sim/test_a.py"], wall_time=5.0, batch_result="CRASHED") + a = status["source/sim/test_a.py"] + assert (a["errors"], a["skipped"], a["result"]) == (1, 1, "FAILED") + + def test_ambiguous_stems_are_not_misattributed(self): + """Two members sharing a basename cannot be told apart, so neither claims the case.""" + report = _report(("pkg.one.test_dup", "t", "pass", 1.0)) + files = ["pkg/one/test_dup.py", "pkg/two/test_dup.py"] + status = split_batch_status(report, files, wall_time=10.0, batch_result="CRASHED") + assert all(status[f]["result"] == "CRASHED" for f in files) + + +class TestEnvironmentToggles: + """Batching stays off unless explicitly enabled.""" + + @pytest.mark.parametrize("value,expected", [("1", True), ("true", True), ("YES", True), ("0", False), ("", False)]) + def test_enable_flag(self, value: str, expected: bool): + assert batching_enabled({"ISAACLAB_TEST_BATCH_KIT": value}) is expected + + def test_disabled_when_unset(self): + assert batching_enabled({}) is False + + @pytest.mark.parametrize("value,expected", [("5", 5), ("", 12), ("nonsense", 12), ("0", 12), ("-3", 12)]) + def test_batch_size_override(self, value: str, expected: int): + assert batch_size({"ISAACLAB_TEST_BATCH_SIZE": value}) == expected diff --git a/source/isaaclab/test/test_kit_marker_contract.py b/source/isaaclab/test/test_kit_marker_contract.py new file mode 100644 index 00000000000..b434090c1b8 --- /dev/null +++ b/source/isaaclab/test/test_kit_marker_contract.py @@ -0,0 +1,392 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""Test that every test file's Kit markers agree with what the file actually does. + +Kit-dependence is a property of *importing* a test module: a module that constructs +:class:`~isaaclab.app.AppLauncher` at module scope boots Isaac Sim during pytest collection, +before any fixture runs. The ``kit`` / ``kit_cameras`` / ``kitless`` markers make that +property declarative so the runner can group files that share a launch configuration into a +single process instead of paying Kit startup once per file. + +A marker is only useful if it cannot drift from reality, which is what this test enforces: + +* ``kit`` / ``kit_cameras`` -- the file calls :func:`~isaaclab.test.launch.launch_kit` at + module scope with the matching ``cameras`` argument, and never constructs ``AppLauncher`` + or ``SimulationApp`` itself. Direct construction would boot a second, unshared app. +* ``kitless`` -- the file never launches Kit and does not import a Kit runtime package at + module scope, so it can run in a process where Kit was never started. +* ``unit`` -- same requirement as ``kitless``, which turns the marker's registered + description ("does not launch the simulator") into a checked invariant. +* At most one module-scope ``pytestmark`` assignment, since a second assignment silently + rebinds the name and discards the markers from the first. + +The checks are AST-based rather than text-based because a source-text search cannot tell an +``AppLauncher`` reference in a docstring from a real call -- several kit-free files mention +``AppLauncher`` only to document that they do not use it. + +Files outside :data:`_ENFORCED_ROOTS` are not yet *required* to carry a marker; the +consistency rules above still apply to them whenever they do. Extend that tuple as each +package is migrated. +""" + +from __future__ import annotations + +import ast +import json +import re +import subprocess +import sys +import textwrap +from pathlib import Path + +import pytest + +pytestmark = [pytest.mark.unit, pytest.mark.kitless] + +_REPO_ROOT = Path(__file__).resolve().parents[3] + +_SCAN_ROOTS = ("source", "scripts") + +_EXCLUDED_PARTS = frozenset( + { + # Own pytest.ini / rootdir; deliberately excluded from the main collector too. + "install_ci", + # Vendored copies of the source tree produced by the wheel builder. + "build", + # Virtual environments and the Isaac Sim symlink. + ".venv", + "env_isaaclab", + "_isaac_sim", + } +) + +# Packages that only exist inside a running Kit application. ``pxr`` is deliberately absent: +# OpenUSD is importable kit-less through the ``usd-core`` wheel, so importing it says nothing +# about whether Kit is running. +_KIT_RUNTIME_PREFIXES = ("omni", "carb", "isaacsim") + +# Directories where a test file is required to declare `kit`, `kit_cameras`, or `kitless`. +# Grows one package at a time as files are migrated off module-scope ``AppLauncher``. +_ENFORCED_ROOTS: tuple[str, ...] = () + +_PROFILE_MARKERS = ("kit", "kit_cameras", "kitless") + + +# --------------------------------------------------------------------------- +# AST helpers +# --------------------------------------------------------------------------- + + +def _module_scope_nodes(tree: ast.Module): + """Yield every node that executes at module import, without entering callables. + + Descends through module-level control flow (``if`` / ``try`` / ``with``) because those + bodies still run at import, but stops at function, class, and lambda boundaries because + those bodies only run when called. + """ + stack = list(tree.body) + while stack: + node = stack.pop() + yield node + if isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef | ast.ClassDef | ast.Lambda): + continue + for child in ast.iter_child_nodes(node): + stack.append(child) + + +def _call_name(node: ast.AST) -> str | None: + """Return the called function's bare name, for ``f()`` and ``mod.f()`` alike.""" + if not isinstance(node, ast.Call): + return None + func = node.func + if isinstance(func, ast.Name): + return func.id + if isinstance(func, ast.Attribute): + return func.attr + return None + + +def _marker_names(node: ast.AST) -> list[str]: + """Return the marker names in a ``pytest.mark.`` expression or a list of them.""" + if isinstance(node, ast.List | ast.Tuple): + return [name for element in node.elts for name in _marker_names(element)] + if isinstance(node, ast.Call): + return _marker_names(node.func) + if isinstance(node, ast.Attribute) and isinstance(node.value, ast.Attribute): + # pytest.mark. + if node.value.attr == "mark": + return [node.attr] + return [] + + +class _FileFacts: + """What a single test file declares and what it actually does at module scope.""" + + def __init__(self, path: Path, tree: ast.Module): + self.path = path + self.pytestmark_assignments: list[int] = [] + self.markers: set[str] = set() + self.launch_kit_cameras: bool | None = None + self.module_scope_launcher: list[tuple[str, int]] = [] + self.launch_kit_anywhere = False + self.kit_runtime_imports: list[tuple[str, int]] = [] + + module_scope = set() + for node in _module_scope_nodes(tree): + module_scope.add(id(node)) + + if isinstance(node, ast.Assign) and any( + isinstance(target, ast.Name) and target.id == "pytestmark" for target in node.targets + ): + self.pytestmark_assignments.append(node.lineno) + self.markers.update(_marker_names(node.value)) + + name = _call_name(node) + if name in ("AppLauncher", "SimulationApp"): + self.module_scope_launcher.append((name, node.lineno)) + elif name == "launch_kit": + self.launch_kit_cameras = any( + keyword.arg == "cameras" and isinstance(keyword.value, ast.Constant) and keyword.value.value + for keyword in node.keywords + ) + + if isinstance(node, ast.Import): + for alias in node.names: + if alias.name.split(".")[0] in _KIT_RUNTIME_PREFIXES: + self.kit_runtime_imports.append((alias.name, node.lineno)) + elif isinstance(node, ast.ImportFrom) and node.module and node.level == 0: + if node.module.split(".")[0] in _KIT_RUNTIME_PREFIXES: + self.kit_runtime_imports.append((node.module, node.lineno)) + + # Decorator markers (e.g. a per-test `@pytest.mark.unit`) count toward the file's + # marker set, and AppLauncher use anywhere -- not just module scope -- disqualifies + # a file from claiming `kitless`. + for node in ast.walk(tree): + if isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef | ast.ClassDef): + for decorator in node.decorator_list: + self.markers.update(_marker_names(decorator)) + name = _call_name(node) + if name == "launch_kit": + self.launch_kit_anywhere = True + elif name in ("AppLauncher", "SimulationApp") and id(node) not in module_scope: + self.module_scope_launcher.append((f"{name} (deferred)", node.lineno)) + + @property + def rel(self) -> str: + return self.path.relative_to(_REPO_ROOT).as_posix() + + @property + def profile_markers(self) -> list[str]: + return [marker for marker in _PROFILE_MARKERS if marker in self.markers] + + @property + def launches_kit_directly(self) -> list[tuple[str, int]]: + return self.module_scope_launcher + + +# --------------------------------------------------------------------------- +# Collection +# --------------------------------------------------------------------------- + + +def _iter_test_files(): + for root in _SCAN_ROOTS: + for path in sorted((_REPO_ROOT / root).rglob("test_*.py")): + if _EXCLUDED_PARTS.isdisjoint(path.parts): + yield path + + +@pytest.fixture(scope="module") +def facts() -> list[_FileFacts]: + """Parse every test file once and return the extracted facts.""" + collected = [] + for path in _iter_test_files(): + try: + tree = ast.parse(path.read_text(encoding="utf-8", errors="replace"), filename=str(path)) + except SyntaxError as exc: + pytest.fail(f"{path.relative_to(_REPO_ROOT).as_posix()} failed to parse: {exc}") + collected.append(_FileFacts(path, tree)) + assert collected, f"no test files discovered under {_SCAN_ROOTS} -- the scan roots are wrong" + return collected + + +# --------------------------------------------------------------------------- +# Rules +# --------------------------------------------------------------------------- + + +def test_pytestmark_is_assigned_at_most_once(facts: list[_FileFacts]): + """A second module-scope ``pytestmark`` rebinds the name and drops the first one's markers.""" + offenders = [ + f"{f.rel}: lines {sorted(f.pytestmark_assignments)}" for f in facts if len(f.pytestmark_assignments) > 1 + ] + assert not offenders, ( + "These files assign `pytestmark` more than once at module scope. The later assignment" + " replaces the earlier one, so the markers declared first are silently lost:\n " + + "\n ".join(offenders) + + "\n\nFix: merge them into a single list, e.g. `pytestmark = [pytest.mark.a, pytest.mark.b]`." + ) + + +def test_profile_markers_are_mutually_exclusive(facts: list[_FileFacts]): + """A file runs in exactly one of the launch configurations, so it declares only one.""" + offenders = [f"{f.rel}: {', '.join(f.profile_markers)}" for f in facts if len(f.profile_markers) > 1] + assert not offenders, "These files declare more than one of `kit`, `kit_cameras`, `kitless`:\n " + "\n ".join( + offenders + ) + + +def test_kit_marked_files_use_launch_kit(facts: list[_FileFacts]): + """`kit` / `kit_cameras` files share the process app; they must not build their own.""" + offenders = [] + for f in facts: + markers = f.profile_markers + if not markers or markers[0] == "kitless": + continue + if f.launches_kit_directly: + where = ", ".join(f"{name} at line {line}" for name, line in f.launches_kit_directly) + offenders.append(f"{f.rel}: declares `{markers[0]}` but constructs {where}") + continue + if f.launch_kit_cameras is None: + offenders.append(f"{f.rel}: declares `{markers[0]}` but never calls launch_kit() at module scope") + continue + wants_cameras = markers[0] == "kit_cameras" + if f.launch_kit_cameras != wants_cameras: + expected = "launch_kit(cameras=True)" if wants_cameras else "launch_kit()" + offenders.append(f"{f.rel}: declares `{markers[0]}` but does not call {expected}") + + assert not offenders, ( + "These files' Kit markers disagree with how they launch Kit:\n " + + "\n ".join(offenders) + + "\n\nFix: call `launch_kit()` (or `launch_kit(cameras=True)`) from" + " `isaaclab.test.launch` at module scope instead of constructing AppLauncher, and make" + " the marker match the `cameras` argument." + ) + + +@pytest.mark.parametrize("marker", ["kitless", "unit"]) +def test_kit_free_files_do_not_touch_kit(marker: str, facts: list[_FileFacts]): + """`kitless` and `unit` files must run in a process where Kit was never started.""" + offenders = [] + for f in facts: + if marker not in f.markers: + continue + if f.launches_kit_directly: + where = ", ".join(f"{name} at line {line}" for name, line in f.launches_kit_directly) + offenders.append(f"{f.rel}: constructs {where}") + if f.launch_kit_anywhere: + offenders.append(f"{f.rel}: calls launch_kit()") + if f.kit_runtime_imports: + where = ", ".join(f"`{name}` at line {line}" for name, line in f.kit_runtime_imports) + offenders.append(f"{f.rel}: imports {where} at module scope") + + assert not offenders, ( + f"These files are marked `{marker}` but depend on a running Kit:\n " + + "\n ".join(offenders) + + f"\n\nKit runtime packages: {_KIT_RUNTIME_PREFIXES}." + f"\nFix: drop the `{marker}` marker and declare `kit`, or move the Kit import inside the" + " test function so it is not paid at collection." + ) + + +def test_migrated_packages_declare_a_marker(facts: list[_FileFacts]): + """Within a migrated package, every test file states its launch configuration.""" + if not _ENFORCED_ROOTS: + pytest.skip("no packages are enforced yet; extend _ENFORCED_ROOTS as files are migrated") + + offenders = [f.rel for f in facts if f.rel.startswith(_ENFORCED_ROOTS) and not f.profile_markers] + assert not offenders, ( + "These files are in a migrated package but declare none of `kit`, `kit_cameras`," + " `kitless`:\n " + "\n ".join(offenders) + ) + + +def test_shareable_file_list_is_derived_from_the_markers(): + """``tools/kit_test_files.py`` must agree with the markers and keep the profiles apart. + + CI batches test files by asking that script which ones can share a Kit app, instead of + carrying a hand-written list that goes stale as files are added or reclassified. These are + the invariants a caller relies on. + """ + sys.path.insert(0, str(_REPO_ROOT / "tools")) + from kit_test_files import shareable_test_files # noqa: PLC0415 + from test_settings import TESTS_TO_SKIP # noqa: PLC0415 + + directory = _REPO_ROOT / "source" / "isaaclab" / "test" / "sim" + sources = {path.name: path.read_text(encoding="utf-8") for path in directory.glob("test_*.py")} + + def declares(source: str, marker: str) -> bool: + # `kit` must not match `kit_cameras` or `kit_solo`. + return re.search(rf"pytest\.mark\.{marker}(?![\w])", source) is not None + + groups = {} + for profile in ("kit", "kit_cameras"): + names = [path.name for path in shareable_test_files(directory, profile)] + assert names, f"no {profile} files found in {directory}" + assert len(names) == len(set(names)), f"duplicate entries for {profile}: {names}" + groups[profile] = set(names) + + expected = { + name + for name, source in sources.items() + if name not in TESTS_TO_SKIP and declares(source, profile) and not declares(source, "kit_solo") + } + assert groups[profile] == expected, ( + f"the derived {profile} list disagrees with the markers:" + f"\n only in list: {sorted(groups[profile] - expected)}" + f"\n only in markers: {sorted(expected - groups[profile])}" + ) + + # The two profiles are separate batches. Cameras cannot be enabled after startup, and a + # camera-enabled app is not a drop-in for a plain one either, so a file appearing in both + # lists would put launch_kit() in a process booted for the other configuration. + overlap = groups["kit"] & groups["kit_cameras"] + assert not overlap, f"these files are in both profile groups and would mix configurations: {sorted(overlap)}" + + +def test_kitless_files_import_without_kit(facts: list[_FileFacts]): + """Importing every `kitless` module must not pull in Kit through a helper module. + + The AST rules only see each file's own imports. A shared test utility that imports Kit + would slip past them, so this imports the real modules in one subprocess and checks that + ``omni.kit.app`` never appears in :data:`sys.modules`. + """ + modules = sorted(f.rel for f in facts if "kitless" in f.markers) + if not modules: + pytest.skip("no files are marked `kitless` yet") + + script = textwrap.dedent(f""" + import importlib.util, json, os, sys + + offenders = [] + for rel in {modules!r}: + # pytest puts a test file's own directory on sys.path (rootdir/conftest handling), + # which is how these modules reach their sibling helpers. Mirror that here. + directory = os.path.dirname(rel) + if directory not in sys.path: + sys.path.insert(0, directory) + + name = "_kitless_probe_" + rel.replace("/", "_")[:-3] + spec = importlib.util.spec_from_file_location(name, rel) + module = importlib.util.module_from_spec(spec) + sys.modules[name] = module + try: + spec.loader.exec_module(module) + except Exception as exc: + offenders.append(f"{{rel}}: import failed: {{type(exc).__name__}}: {{exc}}") + continue + if "omni.kit.app" in sys.modules: + offenders.append(f"{{rel}}: importing it started Kit") + break + print("__RESULTS__" + json.dumps(offenders)) + """) + result = subprocess.run([sys.executable, "-c", script], capture_output=True, text=True, cwd=_REPO_ROOT, timeout=600) + line = next((ln for ln in result.stdout.splitlines() if ln.startswith("__RESULTS__")), None) + assert line is not None, ( + f"kitless import probe did not report results\n--- stdout ---\n{result.stdout}\n--- stderr ---\n{result.stderr}" + ) + offenders = json.loads(line[len("__RESULTS__") :]) + assert not offenders, "These `kitless` files pull in Kit transitively:\n " + "\n ".join(offenders) diff --git a/tools/_kit_batching.py b/tools/_kit_batching.py new file mode 100644 index 00000000000..41229bf73b0 --- /dev/null +++ b/tools/_kit_batching.py @@ -0,0 +1,272 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""Group test files that can share one Kit app into a single pytest invocation. + +A test file that boots Kit at module scope pays Kit startup on its own, and the runner +gives every file its own subprocess, so a directory of 23 such files boots Kit 23 times. +Files migrated to :func:`~isaaclab.test.launch.launch_kit` share the app when they land in +one process, which turns those 23 boots into one. + +Only files carrying the same launch profile may be grouped. ``kit`` and ``kit_cameras`` +cannot share a process in either direction: cameras cannot be enabled after startup, and a +camera-enabled app is not a substitute for a plain one because some tests assert that +offscreen rendering is off. Anything whose behaviour depends on having a process to itself +stays on the per-file path. + +This module is deliberately free of ``os`` and ``subprocess`` calls: the grouping and the +report demultiplexing are pure functions over paths and strings, so they can be exercised on +any platform, unlike the POSIX-only process machinery in ``tools/conftest.py``. +""" + +from __future__ import annotations + +import os +import re +from dataclasses import dataclass, field + +BATCH_ENV_VAR = "ISAACLAB_TEST_BATCH_KIT" +"""Environment variable that opts a run into batching. Unset keeps the per-file path.""" + +BATCH_SIZE_ENV_VAR = "ISAACLAB_TEST_BATCH_SIZE" +"""Environment variable overriding :data:`DEFAULT_BATCH_SIZE`.""" + +DEFAULT_BATCH_SIZE = 12 +"""Files per batch. + +Bounded so that one crash cannot cost a whole lane, and so accumulated GPU memory in a long +shared process does not become its own failure mode. +""" + +BATCH_TIMEOUT_CUTOFF = 2000 +"""Files whose own timeout reaches this stay unbatched. + +A batch's timeout is the sum of its members', so one file hanging consumes the whole budget. +The long-running files are also the ones where Kit startup is a rounding error, so excluding +them removes most of the risk and almost none of the benefit. +""" + +# `kit` must not match `kit_cameras` or `kit_solo`. +_MARK_KIT = re.compile(r"pytest\.mark\.kit(?![\w])") +_MARK_CAMERAS = re.compile(r"pytest\.mark\.kit_cameras\b") +_MARK_SOLO = re.compile(r"pytest\.mark\.kit_solo\b") + + +@dataclass +class Batch: + """One pytest invocation covering one or more test files. + + Attributes: + profile: Launch profile shared by every member, or None for an unbatched file. + files: Test files to hand to pytest, in invocation order. + index: Position among the batches of this profile. Part of :attr:`label`, which + becomes a JUnit report filename, so two batches of the same profile and size + cannot write to the same path. + """ + + profile: str | None + files: list[str] = field(default_factory=list) + index: int = 0 + + @property + def is_batched(self) -> bool: + """Whether this covers more than one file.""" + return len(self.files) > 1 + + @property + def label(self) -> str: + """Short identifier used in logs and JUnit report filenames.""" + return f"batch-{self.profile}-{self.index}-{len(self.files)}files" if self.is_batched else self.files[0] + + +def batching_enabled(env: dict | None = None) -> bool: + """Whether the run opted into batching via :data:`BATCH_ENV_VAR`.""" + env = os.environ if env is None else env + return env.get(BATCH_ENV_VAR, "").strip().lower() in ("1", "true", "yes") + + +def batch_size(env: dict | None = None) -> int: + """Resolve the per-batch file cap, falling back to :data:`DEFAULT_BATCH_SIZE`.""" + env = os.environ if env is None else env + raw = env.get(BATCH_SIZE_ENV_VAR, "").strip() + if not raw: + return DEFAULT_BATCH_SIZE + try: + value = int(raw) + except ValueError: + return DEFAULT_BATCH_SIZE + return value if value > 0 else DEFAULT_BATCH_SIZE + + +def file_profile(source: str) -> str | None: + """Return the launch profile a test file declares, or None if it cannot be batched. + + Args: + source: The test file's text. Markers are matched against the source rather than by + importing the module, because importing a Kit-dependent module boots Kit. + + Returns: + ``"kit_cameras"``, ``"kit"``, or None when the file is unmarked or opts out. + """ + if _MARK_SOLO.search(source): + return None + if _MARK_CAMERAS.search(source): + return "kit_cameras" + if _MARK_KIT.search(source): + return "kit" + return None + + +def group_test_files( + test_files: list[str], + sources: dict[str, str], + *, + unbatchable: set[str] | None = None, + max_size: int = DEFAULT_BATCH_SIZE, +) -> list[Batch]: + """Partition ``test_files`` into batches, preserving the given order. + + Files that cannot be grouped -- unmarked, ``kit_solo``, or listed in ``unbatchable`` -- + each become a batch of one, which is exactly the current per-file behaviour. + + Args: + test_files: Test file paths, in the order the runner would execute them. + sources: Map from a path in ``test_files`` to that file's text. A path missing from + the map is treated as unbatchable rather than assumed safe. + unbatchable: Paths to keep on the per-file path regardless of their markers. + max_size: Maximum files per batch. + + Returns: + Batches covering every input file exactly once, in input order. + """ + unbatchable = unbatchable or set() + batches: list[Batch] = [] + pending: dict[str, Batch] = {} + counts: dict[str, int] = {} + + def flush(profile: str) -> None: + if profile in pending: + batches.append(pending.pop(profile)) + + for path in test_files: + source = sources.get(path) + profile = None if source is None or path in unbatchable else file_profile(source) + + if profile is None: + batches.append(Batch(profile=None, files=[path])) + continue + + current = pending.get(profile) + if current is None: + current = Batch(profile=profile, index=counts.get(profile, 0)) + counts[profile] = current.index + 1 + pending[profile] = current + current.files.append(path) + if len(current.files) >= max_size: + flush(profile) + + # Emit any partially filled batches in a stable order. + for profile in sorted(pending): + batches.append(pending[profile]) + return batches + + +def _testcase_files(report, batch_files: list[str]) -> dict[str, list]: + """Map each batch member to the testcases attributed to it in a JUnit report. + + JUnit ``classname`` encodes the dotted module path, so a file is matched by its stem. + Where two members share a stem the match is ambiguous and those testcases are dropped + from the per-file split rather than assigned to the wrong file. + """ + stems: dict[str, list[str]] = {} + for path in batch_files: + stem = os.path.splitext(os.path.basename(path))[0] + stems.setdefault(stem, []).append(path) + + per_file: dict[str, list] = {path: [] for path in batch_files} + for suite in report: + for case in suite: + classname = getattr(case, "classname", "") or "" + name = getattr(case, "name", "") or "" + for part in reversed(classname.split(".")): + owners = stems.get(part) + if owners and len(owners) == 1: + per_file[owners[0]].append(case) + break + else: + # Fall back to the test name for parametrized ids that carry the module. + for stem, owners in stems.items(): + if len(owners) == 1 and stem in name: + per_file[owners[0]].append(case) + break + return per_file + + +def split_batch_status( + report, + batch_files: list[str], + *, + wall_time: float, + batch_result: str, +) -> dict[str, dict]: + """Attribute a batch's JUnit report back to its individual files. + + The summary table, the failed-file list, and the per-file JUnit artifact are all keyed by + file, so a batch has to be taken apart again before its results are reported. + + A file with no testcases in the report never ran -- the shared process died before + reaching it -- and is marked with ``batch_result`` so the caller can re-run it. + + Args: + report: Parsed JUnit XML for the whole batch. + batch_files: The batch's members. + wall_time: Wall seconds for the whole batch, shared out across members that ran. + batch_result: Result to record for members that produced no testcases. + + Returns: + Map from file path to a status dict of the same shape the per-file path produces. + """ + per_file = _testcase_files(report, batch_files) + ran = [path for path, cases in per_file.items() if cases] + share = wall_time / len(ran) if ran else 0.0 + + statuses: dict[str, dict] = {} + for path in batch_files: + cases = per_file[path] + if not cases: + statuses[path] = { + "errors": 1, + "failures": 0, + "skipped": 0, + "tests": 1, + "result": batch_result, + "time_elapsed": 0.0, + "wall_time": 0.0, + } + continue + + errors = failures = skipped = 0 + elapsed = 0.0 + for case in cases: + elapsed += float(getattr(case, "time", 0.0) or 0.0) + result = getattr(case, "result", None) or [] + kinds = {type(entry).__name__ for entry in result} + if "Error" in kinds: + errors += 1 + elif "Failure" in kinds: + failures += 1 + elif "Skipped" in kinds: + skipped += 1 + + statuses[path] = { + "errors": errors, + "failures": failures, + "skipped": skipped, + "tests": len(cases), + "result": "FAILED" if (errors or failures) else "passed", + "time_elapsed": elapsed, + "wall_time": share, + } + return statuses diff --git a/tools/codemods/kit_launch_migration.py b/tools/codemods/kit_launch_migration.py new file mode 100644 index 00000000000..f9eec8dad2c --- /dev/null +++ b/tools/codemods/kit_launch_migration.py @@ -0,0 +1,325 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""Rewrite test modules from a module-scope ``AppLauncher`` to the shared ``launch_kit()``. + +A test module that constructs :class:`~isaaclab.app.AppLauncher` at module scope boots its +own Kit app during pytest collection, so a process covering several such files pays Kit +startup once per file. :func:`~isaaclab.test.launch.launch_kit` is idempotent, so migrated +files share one app per process. + +The rewrite is deliberately in-place and line-based rather than an ``ast.unparse`` round +trip, which would discard comments, ``# isort:skip`` directives, and docstring formatting. +Each edit replaces a statement's own line range, so import ordering -- which matters here, +because Kit must boot before the Kit-dependent imports below it -- is preserved exactly. + +Usage:: + + uv run python tools/codemods/kit_launch_migration.py source/isaaclab/test/sim + uv run python tools/codemods/kit_launch_migration.py --check source/isaaclab/test/sim + +Files the transform cannot handle safely are reported and left untouched. +""" + +from __future__ import annotations + +import argparse +import ast +import sys +from pathlib import Path + +_LAUNCH_IMPORT = "from isaaclab.test.launch import launch_kit" +_APP_IMPORT_MODULE = "isaaclab.app" + +# Docstrings used purely as section separators around the old launch block. They document a +# launch step that no longer exists in the file once it is migrated. +_BOILERPLATE_DOCSTRINGS = ("Launch Isaac Sim Simulator first.", "Rest everything follows.") + +_BOILERPLATE_COMMENTS = ("# launch omniverse app", "# launch the simulator") + + +class Unsupported(Exception): + """Raised when a file needs manual attention rather than a mechanical rewrite.""" + + +def _module_scope_nodes(tree: ast.Module): + """Yield nodes that execute at import, without descending into callables.""" + stack = list(tree.body) + while stack: + node = stack.pop() + yield node + if isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef | ast.ClassDef | ast.Lambda): + continue + stack.extend(ast.iter_child_nodes(node)) + + +def _call_name(node: ast.AST) -> str | None: + if not isinstance(node, ast.Call): + return None + func = node.func + if isinstance(func, ast.Name): + return func.id + if isinstance(func, ast.Attribute): + return func.attr + return None + + +def _name_usage_count(tree: ast.Module, name: str) -> int: + """Count how many times ``name`` is loaded anywhere in the module.""" + return sum(1 for node in ast.walk(tree) if isinstance(node, ast.Name) and node.id == name) + + +def _find_launcher(tree: ast.Module) -> tuple[ast.stmt, ast.Call]: + """Return the module-scope statement that builds the app, and the ``AppLauncher`` call.""" + found = [] + for statement in tree.body: + for node in ast.walk(statement): + if _call_name(node) == "SimulationApp": + raise Unsupported("constructs SimulationApp directly") + if _call_name(node) == "AppLauncher": + found.append((statement, node)) + + if not found: + raise Unsupported("no module-scope AppLauncher call") + if len(found) > 1: + raise Unsupported(f"{len(found)} module-scope AppLauncher calls") + + statement, call = found[0] + + # The whole statement is replaced by a bare launch_kit() call, so the launch must be + # unconditional. A file that boots Kit only on some branch -- e.g. + # `AppLauncher(...).app if _USE_KIT else None`, used where a standalone wheel lets the + # tests run kitlessly -- would silently become an unconditional boot. Accept only + # ` = AppLauncher(...)`, ` = AppLauncher(...).app`, or a bare call. + value = statement.value if isinstance(statement, ast.Assign | ast.Expr) else None + if isinstance(value, ast.Attribute): + value = value.value + if value is not call: + raise Unsupported(f"AppLauncher launch is conditional or nested: `{ast.unparse(statement).splitlines()[0]}`") + + # `AppLauncher` must not be referenced for anything else, since its import is removed. + if _name_usage_count(tree, "AppLauncher") > 1: + raise Unsupported("`AppLauncher` is referenced beyond the launch call") + + return statement, call + + +def _resolve_cameras(call: ast.Call) -> bool: + """Map the AppLauncher keywords onto the ``cameras`` argument of ``launch_kit``.""" + if call.args: + raise Unsupported("AppLauncher called with positional arguments") + + cameras = False + for keyword in call.keywords: + if keyword.arg is None: + raise Unsupported("AppLauncher called with **kwargs") + value = keyword.value + literal = value.value if isinstance(value, ast.Constant) else None + + if keyword.arg == "headless": + # `headless=True`, or `headless=HEADLESS` where HEADLESS is a True constant. + if literal is not True and not isinstance(value, ast.Name): + raise Unsupported(f"headless={ast.unparse(value)} is not a literal True") + elif keyword.arg == "enable_cameras": + if not isinstance(literal, bool): + raise Unsupported(f"enable_cameras={ast.unparse(value)} is not a literal bool") + cameras = literal + elif keyword.arg == "device": + # launch_kit always applies resolve_test_sim_device(); anything else is a real + # difference in behaviour and must be looked at by hand. + if ast.unparse(value) != "resolve_test_sim_device()": + raise Unsupported(f"device={ast.unparse(value)} is not resolve_test_sim_device()") + else: + raise Unsupported(f"unsupported AppLauncher keyword {keyword.arg}=") + + return cameras + + +def _pytestmark_statement(tree: ast.Module) -> ast.Assign | None: + marks = [ + node + for node in tree.body + if isinstance(node, ast.Assign) + and any(isinstance(target, ast.Name) and target.id == "pytestmark" for target in node.targets) + ] + if len(marks) > 1: + raise Unsupported("multiple module-scope pytestmark assignments; merge them first") + return marks[0] if marks else None + + +def _render_pytestmark(existing: ast.Assign | None, marker: str) -> str: + """Build the new ``pytestmark`` line with the Kit marker in front.""" + new = f"pytest.mark.{marker}" + if existing is None: + return f"pytestmark = {new}" + value = existing.value + if isinstance(value, ast.List | ast.Tuple): + parts = [new] + [ast.unparse(element) for element in value.elts] + else: + parts = [new, ast.unparse(value)] + return f"pytestmark = [{', '.join(parts)}]" + + +def _is_boilerplate_docstring(node: ast.stmt) -> bool: + return ( + isinstance(node, ast.Expr) + and isinstance(node.value, ast.Constant) + and isinstance(node.value.value, str) + and node.value.value.strip() in _BOILERPLATE_DOCSTRINGS + ) + + +def migrate_source(source: str) -> tuple[str, str]: + """Return the rewritten source and the marker it should carry. + + Raises: + Unsupported: If the file needs manual attention. + """ + tree = ast.parse(source) + statement, call = _find_launcher(tree) + cameras = _resolve_cameras(call) + marker = "kit_cameras" if cameras else "kit" + existing_mark = _pytestmark_statement(tree) + + lines = source.splitlines() + # 1-indexed line numbers to drop entirely. + drop: set[int] = set() + # 1-indexed line number -> replacement text. + replace: dict[int, str] = {} + # 1-indexed line number -> text appended after that line. + insert_after: dict[int, list[str]] = {} + + # The launch statement becomes the launch_kit() call, in place, so that the Kit-dependent + # imports below it still run after Kit has started. + replace[statement.lineno] = "launch_kit(cameras=True)" if cameras else "launch_kit()" + drop.update(range(statement.lineno + 1, (statement.end_lineno or statement.lineno) + 1)) + + # `from isaaclab.app import AppLauncher` becomes the launch_kit import, keeping its slot. + app_import_replaced = False + for node in tree.body: + if isinstance(node, ast.ImportFrom) and node.module == _APP_IMPORT_MODULE: + names = [alias.name for alias in node.names] + if names == ["AppLauncher"]: + replace[node.lineno] = _LAUNCH_IMPORT + drop.update(range(node.lineno + 1, (node.end_lineno or node.lineno) + 1)) + app_import_replaced = True + else: + raise Unsupported(f"`from isaaclab.app import {', '.join(names)}` imports more than AppLauncher") + if not app_import_replaced: + raise Unsupported("no `from isaaclab.app import AppLauncher` to replace") + + # Drop `resolve_test_sim_device` imports that only existed to feed AppLauncher, and + # `HEADLESS = True` constants that nothing else reads. launch_kit covers both. + for node in tree.body: + if isinstance(node, ast.ImportFrom) and node.module == "isaaclab.test.utils": + names = [alias.name for alias in node.names] + if "resolve_test_sim_device" not in names or _name_usage_count(tree, "resolve_test_sim_device") != 1: + continue + remaining = [name for name in names if name != "resolve_test_sim_device"] + span = range(node.lineno, (node.end_lineno or node.lineno) + 1) + if remaining: + # Keep the other names; re-emit as a single line, which is how these imports + # are already written and how the formatter would leave them. + replace[node.lineno] = f"from {node.module} import {', '.join(remaining)}" + drop.update(list(span)[1:]) + else: + drop.update(span) + if isinstance(node, ast.Assign) and len(node.targets) == 1: + target = node.targets[0] + if ( + isinstance(target, ast.Name) + and target.id in ("HEADLESS", "headless") + and _name_usage_count(tree, target.id) == 1 + ): + drop.update(range(node.lineno, (node.end_lineno or node.lineno) + 1)) + + # Drop the separator docstrings and comments that described the removed launch block. + for node in tree.body: + if _is_boilerplate_docstring(node): + drop.update(range(node.lineno, (node.end_lineno or node.lineno) + 1)) + for index, line in enumerate(lines, start=1): + if line.strip().lower() in _BOILERPLATE_COMMENTS: + drop.add(index) + + # Attach the marker, either by extending the existing pytestmark or by adding one after + # the last module-scope import (where such a declaration conventionally sits). + marked = _render_pytestmark(existing_mark, marker) + if existing_mark is not None: + replace[existing_mark.lineno] = marked + drop.update(range(existing_mark.lineno + 1, (existing_mark.end_lineno or existing_mark.lineno) + 1)) + else: + import_ends = [ + node.end_lineno or node.lineno for node in tree.body if isinstance(node, ast.Import | ast.ImportFrom) + ] + if not import_ends: + raise Unsupported("no imports to anchor a new pytestmark to") + if _name_usage_count(tree, "pytest") == 0 and not any( + isinstance(node, ast.Import) and any(a.name == "pytest" for a in node.names) for node in tree.body + ): + raise Unsupported("pytest is not imported, so a pytestmark cannot be added") + insert_after.setdefault(max(import_ends), []).append(marked) + + # Only the header is rewritten, so blank-line cleanup is confined to it. Collapsing + # runs across the whole file would also eat the blank lines PEP 8 requires between + # top-level definitions and produce a diff far larger than the change being made. + header_end = max([*drop, *replace, *insert_after, 1]) + + out: list[str] = [] + for index, line in enumerate(lines, start=1): + if index in replace: + emitted = replace[index] + elif index not in drop: + emitted = line + else: + emitted = None + + if emitted is not None: + in_header = index <= header_end + if not (in_header and not emitted.strip() and out and not out[-1].strip()): + out.append(emitted) + + for extra in insert_after.get(index, []): + out.extend(["", extra]) + + result = "\n".join(out).rstrip("\n") + "\n" + ast.parse(result) # refuse to emit anything that does not parse + return result, marker + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("paths", nargs="+", type=Path, help="files or directories to migrate") + parser.add_argument("--check", action="store_true", help="report what would change without writing") + args = parser.parse_args(argv) + + targets: list[Path] = [] + for path in args.paths: + targets.extend(sorted(path.rglob("test_*.py")) if path.is_dir() else [path]) + + changed, skipped = [], [] + for path in targets: + source = path.read_text(encoding="utf-8") + try: + new_source, marker = migrate_source(source) + except Unsupported as exc: + skipped.append((path, str(exc))) + continue + except SyntaxError as exc: + skipped.append((path, f"produced invalid syntax: {exc}")) + continue + if new_source != source and not args.check: + path.write_text(new_source, encoding="utf-8", newline="\n") + changed.append((path, marker)) + + for path, marker in changed: + print(f"{'would migrate' if args.check else 'migrated'}: {path.as_posix()} -> {marker}") + for path, reason in skipped: + print(f"skipped: {path.as_posix()}: {reason}", file=sys.stderr) + print(f"\n{len(changed)} migrated, {len(skipped)} skipped, {len(targets)} scanned") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tools/conftest.py b/tools/conftest.py index b391c8ba0de..5cb92bdf71e 100644 --- a/tools/conftest.py +++ b/tools/conftest.py @@ -23,6 +23,13 @@ # Local imports import test_settings as test_settings # isort: skip from _device_split import DEVICE_SPLIT_PASSES, is_device_split_file # isort: skip +from _kit_batching import ( # isort: skip + BATCH_TIMEOUT_CUTOFF, + batch_size, + batching_enabled, + group_test_files, + split_batch_status, +) logging.basicConfig(level=logging.INFO, format="%(message)s") logger = logging.getLogger(__name__) @@ -42,6 +49,21 @@ def pytest_ignore_collect(collection_path, config): on-disk cache is populated. """ +_CAMERA_MARKERS = ("enable_cameras=True", "launch_kit(cameras=True)", "pytest.mark.kit_cameras") +"""Source-text signatures of a test file that starts Kit with cameras enabled. + +Matched against the file's text rather than by importing it, because importing a test +module boots Kit. ``enable_cameras=True`` covers files that still construct +``AppLauncher`` directly; the other two cover files migrated to +:func:`~isaaclab.test.launch.launch_kit`, which no longer contain that literal. +""" + + +def _enables_cameras(test_content: str) -> bool: + """Whether the given test file's source starts Kit with cameras enabled.""" + return any(marker in test_content for marker in _CAMERA_MARKERS) + + STARTUP_DEADLINE = 120 """Seconds to wait for AppLauncher init or pytest collection before declaring a startup hang. @@ -1001,7 +1023,7 @@ def run_individual_tests(test_files, workspace_root, ci_marker, test_node_ids_by # The first camera-enabled test in a fresh container compiles shaders # (~600 s). Give it extra time so that doesn't look like a test timeout. - is_cold_cache_test = not cold_cache_applied and "enable_cameras=True" in test_content + is_cold_cache_test = not cold_cache_applied and _enables_cameras(test_content) if is_cold_cache_test: timeout += COLD_CACHE_BUFFER cold_cache_applied = True @@ -1064,6 +1086,111 @@ def run_individual_tests(test_files, workspace_root, ci_marker, test_node_ids_by return failed_tests, test_status, xml_reports +def _batching_exclusions(test_files, test_node_ids_by_file, sources): + """Files that must keep a process to themselves even when batching is on. + + Batching only changes how files are grouped, so anything whose current behaviour depends + on process isolation, on its own timeout, or on being invoked more than once is left on + the per-file path. + """ + excluded = set() + for path in test_files: + name = os.path.basename(path) + source = sources.get(path, "") + if name in PROCESS_FAILURE_RETRIES_BY_FILE: + excluded.add(path) # retried in a fresh process after stale render state + elif os.path.normpath(path) in test_node_ids_by_file: + excluded.add(path) # node-ID selection is expressed per file + elif is_device_split_file(path, source=source): + excluded.add(path) # already invoked once per device with different -k + elif test_settings.PER_TEST_TIMEOUTS.get(name, 0) >= BATCH_TIMEOUT_CUTOFF: + excluded.add(path) # a batch timeout is the sum of its members' + elif name in getattr(test_settings, "NEVER_BATCH", ()): + excluded.add(path) + return excluded + + +def run_batched_tests(batches, workspace_root, ci_marker, cold_cache_applied=False): + """Run each batch as a single pytest invocation and split the results per file. + + Args: + batches: Batches to run; each must contain more than one file. + workspace_root: Repository root, passed to pytest's ``--config-file``. + ci_marker: Optional marker expression applied to every invocation. + cold_cache_applied: Whether the cold-shader-cache buffer was already granted. + + Returns: + A 4-tuple ``(failed_tests, test_status, xml_reports, leftovers)``. ``leftovers`` are + files the batch never reached because the shared process died; the caller re-runs + them on the per-file path, which is the floor this can degrade to. + """ + failed_tests, test_status, xml_reports, leftovers = [], {}, [], [] + global_k_expr = os.environ.get("TEST_K_EXPR", "").strip() or None + + for batch in batches: + logger.info(f"\n\n🚀 Running {len(batch.files)} '{batch.profile}' files in one Kit process...\n") + for path in batch.files: + logger.info(f" {path}") + + env = os.environ.copy() + env["PYTHONFAULTHANDLER"] = "1" + + # A batch's budget is the sum of its members', so no file gets less time than it + # would have had alone. + timeout = sum( + test_settings.PER_TEST_TIMEOUTS.get(os.path.basename(p), test_settings.DEFAULT_TIMEOUT) for p in batch.files + ) + is_cold_cache = not cold_cache_applied and batch.profile == "kit_cameras" + if is_cold_cache: + timeout += COLD_CACHE_BUFFER + cold_cache_applied = True + logger.info(f"⏱️ Adding {COLD_CACHE_BUFFER}s cold-cache buffer (timeout now {timeout}s)") + startup_deadline = min(timeout, STARTUP_DEADLINE + (COLD_CACHE_BUFFER if is_cold_cache else 0)) + + ctx = _PassContext( + test_file=batch.label, + file_name=batch.label, + workspace_root=workspace_root, + ci_marker=ci_marker, + timeout=timeout, + startup_deadline=startup_deadline, + env=env, + inject_shard_select=False, + pytest_targets=list(batch.files), + ) + + report, status, _ = _run_one_pass(ctx, k_expr=global_k_expr, suffix="") + if report is not None: + xml_reports.append(report) + + if report is None: + # Nothing landed, so nothing can be attributed; hand the whole batch back. + logger.warning(f"⚠️ batch {batch.label} produced no report; re-running its files individually") + leftovers.extend(batch.files) + continue + + per_file = split_batch_status( + report, batch.files, wall_time=status.get("wall_time", 0.0), batch_result=status.get("result", "CRASHED") + ) + unreached = [] + for path, file_status in per_file.items(): + if file_status["result"] in ("CRASHED", "TIMEOUT", "STARTUP_HANG"): + unreached.append(path) + continue + test_status[path] = file_status + if file_status["result"] == "FAILED": + failed_tests.append(path) + + if unreached: + logger.warning( + f"⚠️ batch {batch.label} ended at {unreached[0]} ({status.get('result')});" + f" re-running {len(unreached)} remaining file(s) individually" + ) + leftovers.extend(unreached) + + return failed_tests, test_status, xml_reports, leftovers + + def _collect_test_files( source_dirs, filter_pattern, @@ -1352,9 +1479,46 @@ def pytest_sessionstart(session): # vars are set; falls back to "isaacsim_ci" when only ISAACSIM_CI_SHORT # is set. The pytest -m flag only accepts one expression. effective_marker = ci_marker or ("isaacsim_ci" if isaacsim_ci else "") + + # Files migrated to launch_kit() share one Kit app when they land in the same process, so + # group them and pay startup once per group instead of once per file. Off unless + # ISAACLAB_TEST_BATCH_KIT is set, and disabled under the work queue, which hands out files + # one at a time across containers and so cannot offer coherent groups. + batched_files, batch_results = [], ([], {}, []) + if batching_enabled() and not os.environ.get("ISAACLAB_TEST_QUEUE"): + sources = {} + for path in test_files: + try: + with open(path) as fh: + sources[path] = fh.read() + except OSError: + pass # left out of `sources`, which group_test_files treats as unbatchable + batches = group_test_files( + test_files, + sources, + unbatchable=_batching_exclusions(test_files, test_node_ids_by_file, sources), + max_size=batch_size(), + ) + multi = [b for b in batches if b.is_batched] + if multi: + batched_files = [f for b in multi for f in b.files] + logger.info( + f"⚡ Kit batching: {len(batched_files)} of {len(test_files)} files grouped into" + f" {len(multi)} process(es); the rest run individually" + ) + failed, status, reports, leftovers = run_batched_tests(multi, workspace_root, effective_marker) + batch_results = (failed, status, reports) + # Files a batch never reached fall back to the per-file path, so batching can + # never do worse than the behaviour it replaces. + batched_files = [f for f in batched_files if f not in leftovers] + + remaining = [f for f in test_files if f not in batched_files] failed_tests, test_status, xml_reports = run_individual_tests( - test_files, workspace_root, effective_marker, test_node_ids_by_file + remaining, workspace_root, effective_marker, test_node_ids_by_file ) + failed_tests = batch_results[0] + failed_tests + test_status = {**batch_results[1], **test_status} + xml_reports = batch_results[2] + xml_reports # In work-queue mode this container ran only the files it claimed; report on those. if os.environ.get("ISAACLAB_TEST_QUEUE"): diff --git a/tools/kit_test_files.py b/tools/kit_test_files.py new file mode 100644 index 00000000000..7c7848d7843 --- /dev/null +++ b/tools/kit_test_files.py @@ -0,0 +1,120 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""List the test files in a directory that can share one Kit app. + +The ``kit`` / ``kit_cameras`` / ``kit_solo`` markers already record which files can share a +Kit app; this turns that into the file list a runner needs, so the two never drift. Anything +that hardcodes such a list has to be updated by hand whenever a file is added, renamed, or +reclassified, and a stale list is silently wrong rather than loudly broken. + +One profile at a time. ``kit`` and ``kit_cameras`` files cannot share a process in either +direction: cameras cannot be enabled after startup, and a camera-enabled app is not a drop-in +replacement for a plain one because some tests assert that offscreen rendering is off. Each +profile is a separate batch, so the caller asks for one. + +Selection: files marked with the requested profile, minus those also marked ``kit_solo`` and +those in :data:`tools.test_settings.TESTS_TO_SKIP`. + +Markers are read from the file's source rather than by importing it, because importing a +Kit-dependent test module boots Kit. + +Usage:: + + python3 tools/kit_test_files.py source/isaaclab/test/sim --profile kit --format paths + python3 tools/kit_test_files.py source/isaaclab/test/sim --profile kit_cameras --format names +""" + +from __future__ import annotations + +import argparse +import re +import sys +from pathlib import Path + +# `kit` must not match `kit_cameras` or `kit_solo`, hence the boundary on the plain pattern. +_MARK_KIT = re.compile(r"pytest\.mark\.kit(?![\w])") +_MARK_CAMERAS = re.compile(r"pytest\.mark\.kit_cameras\b") +_MARK_SOLO = re.compile(r"pytest\.mark\.kit_solo\b") + + +def _tests_to_skip() -> frozenset[str]: + """Names from ``tools/test_settings.py``, which the per-file runner also honours.""" + sys.path.insert(0, str(Path(__file__).resolve().parent)) + try: + from test_settings import TESTS_TO_SKIP # noqa: PLC0415 + except ImportError: + return frozenset() + return frozenset(TESTS_TO_SKIP) + + +def shareable_test_files(directory: Path, profile: str = "kit") -> list[Path]: + """Return the files under ``directory`` that can share one Kit app of ``profile``. + + Args: + directory: Directory to scan, non-recursively matching ``test_*.py``. + profile: Which launch configuration to select, ``"kit"`` or ``"kit_cameras"``. + + Returns: + The selected files, sorted by name. + + Raises: + ValueError: If ``profile`` is not a known launch configuration. + """ + if profile not in ("kit", "kit_cameras"): + raise ValueError(f"unknown profile {profile!r}; expected 'kit' or 'kit_cameras'") + + skip = _tests_to_skip() + selected = [] + for path in sorted(directory.glob("test_*.py")): + if path.name in skip: + continue + source = path.read_text(encoding="utf-8", errors="replace") + if _MARK_SOLO.search(source): + continue + # `kit_cameras` implies the file also matches the plain `kit` pattern's prefix, so + # classify on the more specific marker first. + if _MARK_CAMERAS.search(source): + if profile == "kit_cameras": + selected.append(path) + elif _MARK_KIT.search(source) and profile == "kit": + selected.append(path) + return selected + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("directory", type=Path, help="directory to scan for test files") + parser.add_argument( + "--profile", + choices=("kit", "kit_cameras"), + default="kit", + help="which launch configuration to select; the two never share a process", + ) + parser.add_argument( + "--format", + choices=("paths", "names"), + default="paths", + help="'paths' for space-separated repo paths (pytest arguments); " + "'names' for comma-separated file names (the include-files input)", + ) + args = parser.parse_args(argv) + + if not args.directory.is_dir(): + parser.error(f"not a directory: {args.directory}") + + files = shareable_test_files(args.directory, args.profile) + if not files: + parser.error(f"no {args.profile} test files found in {args.directory}") + + if args.format == "paths": + print(" ".join(path.as_posix() for path in files)) + else: + print(",".join(path.name for path in files)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main())