diff --git a/.github/actions/run-package-tests/action.yml b/.github/actions/run-package-tests/action.yml index 6afdde7cbc73..ccd3fa55b68a 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/.github/workflows/build.yaml b/.github/workflows/build.yaml index 853698d0aece..3a29cdb3dcbe 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -892,6 +892,116 @@ jobs: omni-github-test-type: warp-cache-warm #endregion + #region kit-reuse timing probe + # TEMPORARY. Measures what sharing one Kit app across test files is worth, so the decision to + # migrate the remaining ~125 files is based on a number rather than an estimate. Both jobs run + # the same 24 `kit` files from source/isaaclab/test/sim; the only difference is how many Kit + # apps get booted. Compare the two job durations in the Actions UI, then delete this region. + # + # The four `kit_cameras` files in that directory are excluded from both sides. The first + # camera-enabled boot in a fresh container compiles shaders for ~600 s, which is an order of + # magnitude larger than the Kit startup being measured and would swamp the comparison. + # + # The file lists are spelled out rather than selected with `-m kit` because pytest's marker + # filtering deselects tests but still imports every collected module, so `-m` alone cannot keep + # a kit_cameras module from calling launch_kit(cameras=True). Files in TESTS_TO_SKIP are left + # out of both sides so the two jobs cover exactly the same tests. + test-kit-reuse-probe-per-file: + name: "kit-reuse-probe-per-file" + runs-on: [self-hosted, gpu] + timeout-minutes: 120 + continue-on-error: true + needs: [build, config] + if: needs.build.result == 'success' + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 1 + lfs: true + # Baseline: the default test-path of "tools" runs tools/conftest.py, which gives each file + # its own subprocess, so Kit boots 30 times. + - uses: ./.github/actions/run-package-tests + with: + image-tag: ${{ needs.config.outputs.ci_image_tag }} + isaacsim-base-image: ${{ needs.config.outputs.isaacsim_image_name }} + isaacsim-version: ${{ needs.config.outputs.isaacsim_image_tag }} + filter-pattern: "isaaclab/test/sim" + include-files: >- + test_articulation_fragments.py, + test_build_simulation_context_headless.py, + test_cloner.py, + test_collision_fragments.py, + test_joint_drive_fragments.py, + test_mass_fragments.py, + test_material_fragments.py, + test_mesh_collision_fragments.py, + test_mesh_converter.py, + test_schema_fragments.py, + test_schema_writer_nested_targets.py, + test_schemas.py, + test_simulation_context.py, + test_spawn_from_files.py, + test_spawn_lights.py, + test_spawn_materials.py, + test_spawn_meshes.py, + test_spawn_sensors.py, + test_spawn_shapes.py, + test_spawn_wrappers.py, + test_tendon_fragments.py, + test_utils_stage.py, + test_utils_transforms.py, + test_views_xform_prim.py + container-name: isaac-lab-kit-reuse-probe-per-file + omni-github-test-type: kit-reuse-probe-per-file + + test-kit-reuse-probe-batched: + name: "kit-reuse-probe-batched" + runs-on: [self-hosted, gpu] + timeout-minutes: 120 + continue-on-error: true + needs: [build, config] + if: needs.build.result == 'success' + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 1 + lfs: true + # Batched: pointing test-path at the files themselves bypasses tools/conftest.py, so all 24 + # run in one pytest process and launch_kit() boots Kit once. + - uses: ./.github/actions/run-package-tests + with: + image-tag: ${{ needs.config.outputs.ci_image_tag }} + isaacsim-base-image: ${{ needs.config.outputs.isaacsim_image_name }} + isaacsim-version: ${{ needs.config.outputs.isaacsim_image_tag }} + test-path: >- + source/isaaclab/test/sim/test_articulation_fragments.py + source/isaaclab/test/sim/test_build_simulation_context_headless.py + source/isaaclab/test/sim/test_cloner.py + source/isaaclab/test/sim/test_collision_fragments.py + source/isaaclab/test/sim/test_joint_drive_fragments.py + source/isaaclab/test/sim/test_mass_fragments.py + source/isaaclab/test/sim/test_material_fragments.py + source/isaaclab/test/sim/test_mesh_collision_fragments.py + source/isaaclab/test/sim/test_mesh_converter.py + source/isaaclab/test/sim/test_schema_fragments.py + source/isaaclab/test/sim/test_schema_writer_nested_targets.py + source/isaaclab/test/sim/test_schemas.py + source/isaaclab/test/sim/test_simulation_context.py + source/isaaclab/test/sim/test_spawn_from_files.py + source/isaaclab/test/sim/test_spawn_lights.py + source/isaaclab/test/sim/test_spawn_materials.py + source/isaaclab/test/sim/test_spawn_meshes.py + source/isaaclab/test/sim/test_spawn_sensors.py + source/isaaclab/test/sim/test_spawn_shapes.py + source/isaaclab/test/sim/test_spawn_wrappers.py + source/isaaclab/test/sim/test_tendon_fragments.py + source/isaaclab/test/sim/test_utils_stage.py + source/isaaclab/test/sim/test_utils_transforms.py + source/isaaclab/test/sim/test_views_xform_prim.py + container-name: isaac-lab-kit-reuse-probe-batched + omni-github-test-type: kit-reuse-probe-batched + #endregion + #region disabled quarantined tests # test-quarantined: # name: "Quarantined Tests" diff --git a/pyproject.toml b/pyproject.toml index d743d6a2d54b..b34bf53c547c 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 000000000000..1b2acabc992e --- /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 000000000000..1b9f423f6b4e --- /dev/null +++ b/source/isaaclab/isaaclab/test/launch.py @@ -0,0 +1,84 @@ +# 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) +""" + +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 a camera-enabled app is requested but Kit is already running in + this process without cameras, 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 and not _cameras: + raise RuntimeError( + "launch_kit(cameras=True) was called, but Kit is already running in this process" + " without cameras. Camera extensions cannot be enabled after startup. Mark this" + " file `pytest.mark.kit_cameras` so it is grouped with other camera tests instead" + " of with plain `pytest.mark.kit` files." + ) + 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 1925c6673a0d..8db637450a33 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 2319363122eb..68de1554d211 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 cf266f73f4fe..cc3e98ebabfb 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 2ce2345062c8..fd5fe7137ab1 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 7bf436e70234..485f0ee09ba1 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 712390bc2f56..c3c4005c4490 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 a9c5534ede37..1a6be46df6a6 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 f017d9d2d16b..f08578ac18d3 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 c09362c5efd0..c69d51c71e8b 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 ae33dbb938d2..5be29b24ea08 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 f4551b4ba829..2120df259f2f 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 e6ca68c3ddda..c8a00f31cfff 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 b1cf4331a048..1aa9146a2c32 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 337dd2b69304..92f9adcfbf83 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 34de268685d3..02c9445d04fd 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 f91947fc32b9..bda761131630 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,7 @@ from isaaclab.utils.assets import ISAACLAB_NUCLEUS_DIR from isaaclab.utils.version import get_isaac_sim_version -pytestmark = pytest.mark.integration +pytestmark = [pytest.mark.kit_cameras, 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 0a771c956f2c..4515555fb1bd 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 59c771880782..bea78e909159 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 d1cb86c87029..93ccd392f7c6 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 a9ad5158c2f3..1a2fc76f2964 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 9e50b54496bc..af0df8b714a5 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 be59ea011d01..def648d5e7e4 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 a0be9336a56f..c66d9fd7dafa 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 c7569081a164..65e485911db9 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 117aaced1608..c1703011b082 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 973e7e718565..92997d04b09f 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 926a2d0d80a4..c88f9e0d8dfe 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 39a70a076f71..3bcd26e66361 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 e7cc178b65d5..1af8ce75bea1 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 9217ca537d05..dfa5ad2372ad 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 @@ -36,7 +36,7 @@ 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] +pytestmark = [pytest.mark.kit, pytest.mark.integration, pytest.mark.isaacsim_ci] PARENT_POS = (0.0, 0.0, 1.0) 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 000000000000..20c6ba41ed5c --- /dev/null +++ b/source/isaaclab/test/test_kit_marker_contract.py @@ -0,0 +1,348 @@ +# 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 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_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/codemods/kit_launch_migration.py b/tools/codemods/kit_launch_migration.py new file mode 100644 index 000000000000..f9eec8dad2cd --- /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 b391c8ba0dee..ad4023345225 100644 --- a/tools/conftest.py +++ b/tools/conftest.py @@ -42,6 +42,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 +1016,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