From d3a9c152628fd82347cc91e37e7cc01372fa7690 Mon Sep 17 00:00:00 2001 From: Matthew Taylor Date: Sun, 2 Aug 2026 16:10:27 -0400 Subject: [PATCH 1/9] Add Kit test markers and a shared launch_kit() helper Kit-dependence is currently a property of importing a test file: 156 test modules construct AppLauncher at module scope, so Isaac Sim boots during pytest collection. Because nothing declares that dependency, tools/conftest.py has to run every test file in its own subprocess, paying Kit startup once per file. Introduce the two pieces needed to change that: launch_kit() is an idempotent module-scope replacement for AppLauncher. The first test module in a process boots Kit; later modules receive the running app, so a pytest run covering several files pays startup once. It raises rather than silently returning a mismatched app when a file asks for cameras after a camera-less boot. The kit / kit_cameras / kitless markers let a file declare which launch configuration it needs, so files that can share a process can be grouped without importing them. kit_solo opts a file out of any such grouping. test_kit_marker_contract.py keeps the markers from drifting: it checks by AST that a file's declaration matches what it does at module scope. The checks are AST-based rather than text-based because several kit-free files mention AppLauncher only in a docstring saying they do not use it. Files are not yet required to carry a marker; _ENFORCED_ROOTS is empty and grows per package as files are migrated. No test file changes behaviour: nothing is marked kit or kitless yet, and no file calls launch_kit() yet. The guard found one pre-existing bug on its first run. test_operational_space assigned pytestmark twice, and the second assignment discarded arm_ci, so the file had been excluded from the ARM CI lane. Merged into a single list. --- pyproject.toml | 5 + .../changelog.d/mataylor-kit-test-markers.rst | 15 + source/isaaclab/isaaclab/test/launch.py | 84 +++++ .../controllers/test_operational_space.py | 4 +- .../isaaclab/test/test_kit_marker_contract.py | 348 ++++++++++++++++++ 5 files changed, 453 insertions(+), 3 deletions(-) create mode 100644 source/isaaclab/changelog.d/mataylor-kit-test-markers.rst create mode 100644 source/isaaclab/isaaclab/test/launch.py create mode 100644 source/isaaclab/test/test_kit_marker_contract.py 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/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) From 02010869e90056302b5c9e2c15b548f1ab7c6907 Mon Sep 17 00:00:00 2001 From: Matthew Taylor Date: Sun, 2 Aug 2026 18:18:54 -0400 Subject: [PATCH 2/9] Migrate source/isaaclab/test/sim to launch_kit() Replace the module-scope AppLauncher construction in the Kit-dependent files under source/isaaclab/test/sim with launch_kit(), and declare the matching kit or kit_cameras marker on each file. Because launch_kit() is idempotent, a pytest process covering several of these files now boots Kit once instead of once per file. Nothing forces them into one process yet -- tools/conftest.py still runs a subprocess per file -- so this changes how the files launch Kit, not how CI schedules them. 24 files map to `kit` and 4 to `kit_cameras`. The two groups must not share a process in that order: a camera-enabled app can serve tests that do not need cameras, but cameras cannot be enabled after startup, so launch_kit() raises rather than handing back an app that would silently fail to render. The transform is applied by tools/codemods/kit_launch_migration.py, added here because ~125 files in other packages remain to migrate. It edits line ranges in place rather than round-tripping through ast.unparse, which would discard comments and isort directives, and it preserves each launch call's position so the Kit-dependent imports below it still run after Kit starts. The codemod refuses anything it cannot rewrite without changing behaviour, and reports it. In particular it rejects a conditional launch such as `AppLauncher(...).app if _USE_KIT else None`, which test_mjcf_converter.py and test_urdf_converter.py use so they can run kitlessly when the standalone importer wheel is installed; collapsing that ternary would have made the boot unconditional. It also refuses a file that references AppLauncher for anything other than the launch call, since the import is removed. --- .../test/sim/test_articulation_fragments.py | 11 +- .../test_build_simulation_context_headless.py | 11 +- ...st_build_simulation_context_nonheadless.py | 11 +- source/isaaclab/test/sim/test_cloner.py | 11 +- .../test/sim/test_collision_fragments.py | 11 +- .../test/sim/test_joint_drive_fragments.py | 11 +- .../isaaclab/test/sim/test_mass_fragments.py | 11 +- .../test/sim/test_material_fragments.py | 11 +- .../test/sim/test_mesh_collision_fragments.py | 11 +- .../isaaclab/test/sim/test_mesh_converter.py | 11 +- .../test/sim/test_schema_fragments.py | 11 +- .../sim/test_schema_writer_nested_targets.py | 11 +- source/isaaclab/test/sim/test_schemas.py | 11 +- .../test/sim/test_simulation_context.py | 13 +- .../sim/test_simulation_stage_in_memory.py | 12 +- .../test/sim/test_spawn_from_files.py | 11 +- source/isaaclab/test/sim/test_spawn_lights.py | 12 +- .../isaaclab/test/sim/test_spawn_materials.py | 12 +- source/isaaclab/test/sim/test_spawn_meshes.py | 12 +- .../isaaclab/test/sim/test_spawn_sensors.py | 12 +- source/isaaclab/test/sim/test_spawn_shapes.py | 11 +- .../isaaclab/test/sim/test_spawn_wrappers.py | 12 +- .../test/sim/test_tendon_fragments.py | 11 +- source/isaaclab/test/sim/test_utils_prims.py | 11 +- .../isaaclab/test/sim/test_utils_queries.py | 11 +- .../isaaclab/test/sim/test_utils_semantics.py | 11 +- source/isaaclab/test/sim/test_utils_stage.py | 11 +- .../test/sim/test_utils_transforms.py | 11 +- .../test/sim/test_views_xform_prim.py | 8 +- tools/codemods/kit_launch_migration.py | 325 ++++++++++++++++++ 30 files changed, 415 insertions(+), 234 deletions(-) create mode 100644 tools/codemods/kit_launch_migration.py 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/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()) From ded9dfd1ce59e694b862bbf3eedd8945bfd1b61e Mon Sep 17 00:00:00 2001 From: Matthew Taylor Date: Sun, 2 Aug 2026 18:18:55 -0400 Subject: [PATCH 3/9] Add a CI probe measuring what sharing one Kit app saves Whether migrating the remaining ~125 test files off module-scope AppLauncher is worth doing depends on how much Kit startup actually costs, which is not something the current pipeline reports directly. Add two temporary jobs that run the same 30 files from source/isaaclab/test/sim and differ only in how many Kit apps they boot. kit-reuse-probe-per-file keeps the default test-path of "tools", so tools/conftest.py gives each file its own subprocess and Kit boots 30 times. kit-reuse-probe-batched points pytest at the files directly, so they share one process and launch_kit() boots Kit once. The difference between the two job durations is what reuse is worth per 30 files. Both jobs list their files explicitly instead of selecting with `-m kit`, because pytest's marker filtering deselects tests but still imports every collected module, and importing a kit_cameras module calls launch_kit(cameras=True) regardless of whether its tests will run. The batched job lists the four kit_cameras files first: a camera-enabled app can serve tests that do not need cameras, but cameras cannot be enabled after startup, so the opposite order makes launch_kit() raise. Files in TESTS_TO_SKIP are excluded from both sides so the jobs cover the same tests. To let a job bypass the per-file orchestrator, run-package-tests gains a test-path input. It defaults to "tools", the value that was previously hard-coded, so every existing caller is unaffected. Both jobs are continue-on-error and are meant to be deleted once the measurement is recorded. --- .github/actions/run-package-tests/action.yml | 9 +- .github/workflows/build.yaml | 116 +++++++++++++++++++ 2 files changed, 124 insertions(+), 1 deletion(-) 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..8b2699f5b9b1 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -892,6 +892,122 @@ 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 30 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 file lists are spelled out rather than selected with `-m kit` because pytest's marker + # filtering deselects tests but still imports every collected module, and importing a + # kit_cameras module calls 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_simulation_stage_in_memory.py, + test_utils_prims.py, + test_utils_queries.py, + test_utils_semantics.py, + 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 30 + # run in one pytest process and launch_kit() boots Kit once. The four kit_cameras files are + # listed first on purpose: a camera-enabled app can serve tests that do not need cameras, but + # cameras cannot be turned on after startup, so the reverse order makes launch_kit() raise. + - 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_simulation_stage_in_memory.py + source/isaaclab/test/sim/test_utils_prims.py + source/isaaclab/test/sim/test_utils_queries.py + source/isaaclab/test/sim/test_utils_semantics.py + 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" From 0c714ba82d9a556443c55e38607cff37f3f0f015 Mon Sep 17 00:00:00 2001 From: Matthew Taylor Date: Sun, 2 Aug 2026 19:48:42 -0400 Subject: [PATCH 4/9] Keep the cold-cache buffer working for migrated camera tests The per-file runner grants the first camera-enabled test file an extra 700 s of timeout, because that file compiles RTX shaders (~600 s) on a cold cache. It identified such files by searching their source for the literal string "enable_cameras=True". Migrating a file to launch_kit(cameras=True) removes that literal, so the buffer stopped being applied and the file was killed at the 120 s startup deadline instead. That is what happened to test_simulation_stage_in_memory.py in the kit-reuse-probe-per-file job: it was reported as a startup hang at 120.94 s having run no tests. Match the marker and the launch_kit call as well as the old literal, so the buffer applies both before and after a file is migrated. Also narrow the probe to the 24 `kit` files and drop the four `kit_cameras` ones from both sides. The cold shader compile is roughly thirty times the Kit startup the probe is trying to measure, so including those files tells us about shader caching rather than about app reuse. --- .github/workflows/build.yaml | 28 +++++++++++----------------- tools/conftest.py | 17 ++++++++++++++++- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 8b2699f5b9b1..3a29cdb3dcbe 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -895,13 +895,17 @@ jobs: #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 30 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 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, and importing a - # kit_cameras module calls launch_kit(cameras=True). Files in TESTS_TO_SKIP are left out of - # both sides so the two jobs cover exactly the same tests. + # 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] @@ -923,10 +927,6 @@ jobs: isaacsim-version: ${{ needs.config.outputs.isaacsim_image_tag }} filter-pattern: "isaaclab/test/sim" include-files: >- - test_simulation_stage_in_memory.py, - test_utils_prims.py, - test_utils_queries.py, - test_utils_semantics.py, test_articulation_fragments.py, test_build_simulation_context_headless.py, test_cloner.py, @@ -966,20 +966,14 @@ jobs: with: fetch-depth: 1 lfs: true - # Batched: pointing test-path at the files themselves bypasses tools/conftest.py, so all 30 - # run in one pytest process and launch_kit() boots Kit once. The four kit_cameras files are - # listed first on purpose: a camera-enabled app can serve tests that do not need cameras, but - # cameras cannot be turned on after startup, so the reverse order makes launch_kit() raise. + # 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_simulation_stage_in_memory.py - source/isaaclab/test/sim/test_utils_prims.py - source/isaaclab/test/sim/test_utils_queries.py - source/isaaclab/test/sim/test_utils_semantics.py 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 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 From 505971275a80dbc4e6308bc752bba064007a349d Mon Sep 17 00:00:00 2001 From: Matthew Taylor Date: Mon, 3 Aug 2026 15:13:27 -0400 Subject: [PATCH 5/9] Record the two files that do not tolerate a shared Kit app The kit-reuse-probe-batched job surfaced two ways a test file can misbehave once it no longer has a Kit process to itself. test_simulation_stage_in_memory.py aborted the interpreter immediately after collection, with no Python traceback, while the same test passes in its own process. Creating the stage in memory is sensitive to what else has already touched the stage or the extension set. The cause is not understood yet, so mark the file kit_solo to keep it out of any future batching rather than leave a landmine for whoever wires that up. test_views_xform_prim.py calls enable_extension() at module scope, so in a shared process it mutates the running app's extension set during collection, before any test runs. That is harmless today and the file is not being changed, but it is the kind of import-time side effect that batching turns into a cross-file interaction, so say so at the call site. Neither change affects how these tests run today; both files still get their own process from tools/conftest.py. --- .../isaaclab/test/sim/test_simulation_stage_in_memory.py | 7 ++++++- source/isaaclab/test/sim/test_views_xform_prim.py | 3 +++ 2 files changed, 9 insertions(+), 1 deletion(-) 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 bda761131630..450d893d4ed3 100644 --- a/source/isaaclab/test/sim/test_simulation_stage_in_memory.py +++ b/source/isaaclab/test/sim/test_simulation_stage_in_memory.py @@ -22,7 +22,12 @@ from isaaclab.utils.assets import ISAACLAB_NUCLEUS_DIR from isaaclab.utils.version import get_isaac_sim_version -pytestmark = [pytest.mark.kit_cameras, 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_views_xform_prim.py b/source/isaaclab/test/sim/test_views_xform_prim.py index dfa5ad2372ad..c3b5a91ae9ad 100644 --- a/source/isaaclab/test/sim/test_views_xform_prim.py +++ b/source/isaaclab/test/sim/test_views_xform_prim.py @@ -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): From 6f10aec250193119af1cca92ad063ccdcd96360d Mon Sep 17 00:00:00 2001 From: Matthew Taylor Date: Mon, 3 Aug 2026 15:26:10 -0400 Subject: [PATCH 6/9] Batch every sim test file that can share a Kit app The batched probe was passing over files rather than classifying them: the four kit_cameras files had been dropped from both sides to keep the measurement clean, which meant three files that can share an app were being run one-app-each for no reason. Mark the files that genuinely cannot share, and batch everything else. test_views_xform_prim.py is the one this run identified. Its test_compare_get_world_poses_with_isaacsim reaches Isaac Sim's SimulationManager, a process-global singleton that caches the PhysxScene wrapping /physicsScene. In a shared process that prim belongs to a stage an earlier file has already torn down, so the cached wrapper is dangling and the test fails with "Accessed invalid expired 'PhysicsScene' prim". The other 62 tests in the file are fine; the marker is per file, so the file goes solo until SimulationManager can be reset between files. That leaves 26 of the directory's files sharing one app, up from 24, with two marked kit_solo and one already in TESTS_TO_SKIP. The three kit_cameras files are listed first in the batched job because a camera-enabled app can serve tests that do not need cameras while the reverse makes launch_kit() raise. Both jobs run the identical 26 so the durations stay comparable. --- .github/workflows/build.yaml | 32 ++++++++++++------- .../test/sim/test_views_xform_prim.py | 8 ++++- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 3a29cdb3dcbe..a152d31cec7b 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -895,17 +895,23 @@ jobs: #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 same 26 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 26 are every file in that directory that can share a Kit app. Excluded are the two marked + # kit_solo, which demonstrably cannot -- see the comments on their pytestmark for what each one + # does to a shared process -- and anything in TESTS_TO_SKIP. Both jobs use the identical set so + # the durations stay comparable. # - # The file lists are spelled out rather than selected with `-m kit` because pytest's marker + # The three `kit_cameras` files come first in the batched list. A camera-enabled app can serve + # tests that do not need cameras, but cameras cannot be turned on after startup, so the reverse + # order would make launch_kit() raise. Both sides pay the one-off ~600 s cold shader compile + # that the first camera-enabled boot in a fresh container incurs, so it does not bias the + # comparison. + # + # The file lists are spelled out rather than selected with `-m` 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. + # a kit_cameras module from calling launch_kit(cameras=True). test-kit-reuse-probe-per-file: name: "kit-reuse-probe-per-file" runs-on: [self-hosted, gpu] @@ -927,6 +933,9 @@ jobs: isaacsim-version: ${{ needs.config.outputs.isaacsim_image_tag }} filter-pattern: "isaaclab/test/sim" include-files: >- + test_utils_prims.py, + test_utils_queries.py, + test_utils_semantics.py, test_articulation_fragments.py, test_build_simulation_context_headless.py, test_cloner.py, @@ -949,8 +958,7 @@ jobs: test_spawn_wrappers.py, test_tendon_fragments.py, test_utils_stage.py, - test_utils_transforms.py, - test_views_xform_prim.py + test_utils_transforms.py container-name: isaac-lab-kit-reuse-probe-per-file omni-github-test-type: kit-reuse-probe-per-file @@ -974,6 +982,9 @@ jobs: isaacsim-base-image: ${{ needs.config.outputs.isaacsim_image_name }} isaacsim-version: ${{ needs.config.outputs.isaacsim_image_tag }} test-path: >- + source/isaaclab/test/sim/test_utils_prims.py + source/isaaclab/test/sim/test_utils_queries.py + source/isaaclab/test/sim/test_utils_semantics.py 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 @@ -997,7 +1008,6 @@ jobs: 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 diff --git a/source/isaaclab/test/sim/test_views_xform_prim.py b/source/isaaclab/test/sim/test_views_xform_prim.py index c3b5a91ae9ad..8b56f5f39f61 100644 --- a/source/isaaclab/test/sim/test_views_xform_prim.py +++ b/source/isaaclab/test/sim/test_views_xform_prim.py @@ -39,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.kit, 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) From 207b26c0a19bba9d5c8ac1ffccc3c95a338d8b89 Mon Sep 17 00:00:00 2001 From: Matthew Taylor Date: Mon, 3 Aug 2026 15:31:37 -0400 Subject: [PATCH 7/9] Derive the batched file list from the markers instead of listing it Both probe jobs carried a hand-written list of the files that can share a Kit app, duplicated between them in two different formats. That has to be edited by hand whenever a file is added, renamed, or reclassified, and the two copies have to be kept identical or the timing comparison silently stops comparing like with like. A stale list is wrong quietly rather than loudly. The markers already record which files can share an app, so make them the only source. tools/kit_test_files.py selects the files marked kit or kit_cameras, drops those marked kit_solo and those in TESTS_TO_SKIP, and puts the kit_cameras files first because a camera-enabled app can serve tests that do not need cameras while the reverse makes launch_kit() raise. Each job calls it in a step and passes the result through, so the two jobs cannot disagree with each other or with the markers. A marker expression still cannot replace this: pytest's -m deselects tests but imports every collected module regardless, so it cannot stop a kit_cameras module from calling launch_kit(cameras=True) in a run that booted without cameras. The list has to be settled before pytest starts. Markers are read from the source text rather than by importing the modules, since importing a Kit-dependent test module boots Kit. test_kit_marker_contract.py now also checks the two invariants a caller depends on: the derived list matches the markers, and cameras sort first. Verified the ordering check fails when the script's ordering is reversed. --- .github/workflows/build.yaml | 101 +++++------------ .../isaaclab/test/test_kit_marker_contract.py | 42 +++++++ tools/kit_test_files.py | 103 ++++++++++++++++++ 3 files changed, 175 insertions(+), 71 deletions(-) create mode 100644 tools/kit_test_files.py diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index a152d31cec7b..8abd1cba9669 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -895,23 +895,18 @@ jobs: #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 26 files from source/isaaclab/test/sim; the only difference is how many Kit apps get + # the same 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 26 are every file in that directory that can share a Kit app. Excluded are the two marked - # kit_solo, which demonstrably cannot -- see the comments on their pytestmark for what each one - # does to a shared process -- and anything in TESTS_TO_SKIP. Both jobs use the identical set so - # the durations stay comparable. + # Neither job hardcodes a file list. tools/kit_test_files.py derives it from the kit / + # kit_cameras / kit_solo markers, so the two jobs cannot drift apart from each other or from + # the markers as files are added, renamed, or reclassified. It also fixes the order: the + # kit_cameras files come first, because a camera-enabled app can serve tests that do not need + # cameras while the reverse makes launch_kit() raise. # - # The three `kit_cameras` files come first in the batched list. A camera-enabled app can serve - # tests that do not need cameras, but cameras cannot be turned on after startup, so the reverse - # order would make launch_kit() raise. Both sides pay the one-off ~600 s cold shader compile - # that the first camera-enabled boot in a fresh container incurs, so it does not bias the - # comparison. - # - # The file lists are spelled out rather than selected with `-m` 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). + # A marker expression cannot replace the explicit list here: pytest's -m deselects tests but + # still imports every collected module, so it cannot stop a kit_cameras module from calling + # launch_kit(cameras=True) in a run that booted without cameras. test-kit-reuse-probe-per-file: name: "kit-reuse-probe-per-file" runs-on: [self-hosted, gpu] @@ -924,41 +919,23 @@ jobs: with: fetch-depth: 1 lfs: true + - name: Resolve shareable test files + id: files + shell: bash + run: | + set -euo pipefail + names=$(python3 tools/kit_test_files.py source/isaaclab/test/sim --format names) + echo "names=$names" >> "$GITHUB_OUTPUT" + echo "Resolved $(echo "$names" | tr ',' ' ' | wc -w) shareable test files" # Baseline: the default test-path of "tools" runs tools/conftest.py, which gives each file - # its own subprocess, so Kit boots 30 times. + # its own subprocess, so Kit boots once per file. - 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_utils_prims.py, - test_utils_queries.py, - test_utils_semantics.py, - 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 + include-files: ${{ steps.files.outputs.names }} container-name: isaac-lab-kit-reuse-probe-per-file omni-github-test-type: kit-reuse-probe-per-file @@ -974,40 +951,22 @@ jobs: 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. + - name: Resolve shareable test files + id: files + shell: bash + run: | + set -euo pipefail + paths=$(python3 tools/kit_test_files.py source/isaaclab/test/sim --format paths) + echo "paths=$paths" >> "$GITHUB_OUTPUT" + echo "Resolved $(echo "$paths" | wc -w) shareable test files" + # Batched: pointing test-path at the files themselves bypasses tools/conftest.py, so they + # all 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_utils_prims.py - source/isaaclab/test/sim/test_utils_queries.py - source/isaaclab/test/sim/test_utils_semantics.py - 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 + test-path: ${{ steps.files.outputs.paths }} container-name: isaac-lab-kit-reuse-probe-batched omni-github-test-type: kit-reuse-probe-batched #endregion diff --git a/source/isaaclab/test/test_kit_marker_contract.py b/source/isaaclab/test/test_kit_marker_contract.py index 20c6ba41ed5c..0b9ec15da9c0 100644 --- a/source/isaaclab/test/test_kit_marker_contract.py +++ b/source/isaaclab/test/test_kit_marker_contract.py @@ -303,6 +303,48 @@ def test_migrated_packages_declare_a_marker(facts: list[_FileFacts]): ) +def test_shareable_file_list_is_derived_from_the_markers(): + """``tools/kit_test_files.py`` must agree with the markers, and order cameras first. + + 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" + selected = shareable_test_files(directory) + names = [path.name for path in selected] + assert names, f"no shareable files found in {directory}" + assert len(names) == len(set(names)), f"duplicate entries: {names}" + + sources = {path.name: path.read_text(encoding="utf-8") for path in directory.glob("test_*.py")} + + def marks(name: str, marker: str) -> bool: + return f"pytest.mark.{marker}" in sources[name] + + expected = { + name + for name, source in sources.items() + if name not in TESTS_TO_SKIP and "pytest.mark.kit" in source and "pytest.mark.kit_solo" not in source + } + assert set(names) == expected, ( + "the derived list disagrees with the markers:" + f"\n only in list: {sorted(set(names) - expected)}" + f"\n only in markers: {sorted(expected - set(names))}" + ) + + # A camera-enabled app can serve tests that do not need cameras, but cameras cannot be + # enabled after startup, so every kit_cameras file must precede every plain kit file. + is_camera = [marks(name, "kit_cameras") for name in names] + assert is_camera == sorted(is_camera, reverse=True), ( + "kit_cameras files must come first, otherwise a plain `kit` file boots the app without" + f" cameras and the later launch_kit(cameras=True) raises. Got: {names}" + ) + + def test_kitless_files_import_without_kit(facts: list[_FileFacts]): """Importing every `kitless` module must not pull in Kit through a helper module. diff --git a/tools/kit_test_files.py b/tools/kit_test_files.py new file mode 100644 index 000000000000..370fe02a0205 --- /dev/null +++ b/tools/kit_test_files.py @@ -0,0 +1,103 @@ +# 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, in a safe order. + +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. + +Selection: every file marked ``kit`` or ``kit_cameras``, minus those marked ``kit_solo`` and +those in :data:`tools.test_settings.TESTS_TO_SKIP`. + +Order: ``kit_cameras`` files first. A camera-enabled app can serve tests that do not need +cameras, but cameras cannot be enabled after startup, so a plain ``kit`` file booting first +would make a later ``launch_kit(cameras=True)`` raise. + +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 --format paths + python3 tools/kit_test_files.py source/isaaclab/test/sim --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) -> list[Path]: + """Return the files under ``directory`` that can share a Kit app, cameras first. + + Args: + directory: Directory to scan, non-recursively matching ``test_*.py``. + + Returns: + The selected files: ``kit_cameras`` ones first, each group sorted by name. + """ + skip = _tests_to_skip() + cameras, plain = [], [] + 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 + if _MARK_CAMERAS.search(source): + cameras.append(path) + elif _MARK_KIT.search(source): + plain.append(path) + return cameras + plain + + +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( + "--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) + if not files: + parser.error(f"no Kit-marked 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()) From 12b8bb69bded0619190ad47747fd28b66d15960d Mon Sep 17 00:00:00 2001 From: Matthew Taylor Date: Tue, 4 Aug 2026 15:40:46 -0400 Subject: [PATCH 8/9] Keep kit and kit_cameras files in separate processes The probe batched the kit_cameras files together with the plain kit ones, on the assumption that a camera-enabled app is a superset: it can serve tests that do not need cameras, so booting cameras first would satisfy everyone. That is wrong, and CI showed exactly where. test_simulation_context.py's test_headless_mode asserts not sim.has_gui and not sim.has_offscreen_render so it fails in an app that was booted with cameras. The evidence is clean: in the batch that contained no camera files that test passed 43/43, and in the batch that booted cameras first it was the single failure out of 462 tests. So the relationship is not superset but mutual exclusion. Cameras cannot be enabled after startup, which rules out one order, and some tests require them to be off, which rules out the other. Treat the two as separate batches. launch_kit() now raises on any mismatch rather than only when cameras are requested after a plain boot, so a file can never silently receive an app configured differently from what its marker declares. kit_test_files.py takes a --profile and returns one group, which also removes the cameras-first ordering it previously had to arrange. Both probe jobs ask for the kit group: it is much the larger, and a camera batch would mostly measure the one-off ~600 s cold shader compile rather than Kit startup. The contract test now checks each profile against the markers separately and asserts the two groups do not overlap, replacing the ordering check that this change makes meaningless. --- .github/workflows/build.yaml | 21 ++++--- source/isaaclab/isaaclab/test/launch.py | 28 +++++++--- .../isaaclab/test/test_kit_marker_contract.py | 56 ++++++++++--------- tools/kit_test_files.py | 53 ++++++++++++------ 4 files changed, 96 insertions(+), 62 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 8abd1cba9669..f18a72398487 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -900,13 +900,18 @@ jobs: # # Neither job hardcodes a file list. tools/kit_test_files.py derives it from the kit / # kit_cameras / kit_solo markers, so the two jobs cannot drift apart from each other or from - # the markers as files are added, renamed, or reclassified. It also fixes the order: the - # kit_cameras files come first, because a camera-enabled app can serve tests that do not need - # cameras while the reverse makes launch_kit() raise. + # the markers as files are added, renamed, or reclassified. # - # A marker expression cannot replace the explicit list here: pytest's -m deselects tests but - # still imports every collected module, so it cannot stop a kit_cameras module from calling - # launch_kit(cameras=True) in a run that booted without cameras. + # Both select --profile kit. The kit and kit_cameras groups are separate batches and never + # share a process: cameras cannot be enabled after startup, and a camera-enabled app is not a + # drop-in for a plain one either, since test_simulation_context.py::test_headless_mode asserts + # that offscreen rendering is off. The kit group is the one worth measuring -- it is much the + # larger, and a camera batch is dominated by the one-off ~600 s cold shader compile rather + # than by Kit startup. + # + # A marker expression cannot replace the resolved list: pytest's -m deselects tests but still + # imports every collected module, so it cannot stop a module from calling launch_kit() with + # the wrong profile in a run that booted the other one. test-kit-reuse-probe-per-file: name: "kit-reuse-probe-per-file" runs-on: [self-hosted, gpu] @@ -924,7 +929,7 @@ jobs: shell: bash run: | set -euo pipefail - names=$(python3 tools/kit_test_files.py source/isaaclab/test/sim --format names) + names=$(python3 tools/kit_test_files.py source/isaaclab/test/sim --profile kit --format names) echo "names=$names" >> "$GITHUB_OUTPUT" echo "Resolved $(echo "$names" | tr ',' ' ' | wc -w) shareable test files" # Baseline: the default test-path of "tools" runs tools/conftest.py, which gives each file @@ -956,7 +961,7 @@ jobs: shell: bash run: | set -euo pipefail - paths=$(python3 tools/kit_test_files.py source/isaaclab/test/sim --format paths) + paths=$(python3 tools/kit_test_files.py source/isaaclab/test/sim --profile kit --format paths) echo "paths=$paths" >> "$GITHUB_OUTPUT" echo "Resolved $(echo "$paths" | wc -w) shareable test files" # Batched: pointing test-path at the files themselves bypasses tools/conftest.py, so they diff --git a/source/isaaclab/isaaclab/test/launch.py b/source/isaaclab/isaaclab/test/launch.py index 1b9f423f6b4e..1234ef57501a 100644 --- a/source/isaaclab/isaaclab/test/launch.py +++ b/source/isaaclab/isaaclab/test/launch.py @@ -25,6 +25,12 @@ 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 @@ -49,20 +55,24 @@ def launch_kit(*, cameras: bool = False) -> Any: 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. + 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 and not _cameras: + if cameras != _cameras: + wanted = "with" if cameras else "without" + running = "with" if _cameras else "without" 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." + 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 diff --git a/source/isaaclab/test/test_kit_marker_contract.py b/source/isaaclab/test/test_kit_marker_contract.py index 0b9ec15da9c0..b434090c1b88 100644 --- a/source/isaaclab/test/test_kit_marker_contract.py +++ b/source/isaaclab/test/test_kit_marker_contract.py @@ -36,6 +36,7 @@ import ast import json +import re import subprocess import sys import textwrap @@ -304,7 +305,7 @@ def test_migrated_packages_declare_a_marker(facts: list[_FileFacts]): def test_shareable_file_list_is_derived_from_the_markers(): - """``tools/kit_test_files.py`` must agree with the markers, and order cameras first. + """``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 @@ -315,34 +316,35 @@ def test_shareable_file_list_is_derived_from_the_markers(): from test_settings import TESTS_TO_SKIP # noqa: PLC0415 directory = _REPO_ROOT / "source" / "isaaclab" / "test" / "sim" - selected = shareable_test_files(directory) - names = [path.name for path in selected] - assert names, f"no shareable files found in {directory}" - assert len(names) == len(set(names)), f"duplicate entries: {names}" - sources = {path.name: path.read_text(encoding="utf-8") for path in directory.glob("test_*.py")} - def marks(name: str, marker: str) -> bool: - return f"pytest.mark.{marker}" in sources[name] - - expected = { - name - for name, source in sources.items() - if name not in TESTS_TO_SKIP and "pytest.mark.kit" in source and "pytest.mark.kit_solo" not in source - } - assert set(names) == expected, ( - "the derived list disagrees with the markers:" - f"\n only in list: {sorted(set(names) - expected)}" - f"\n only in markers: {sorted(expected - set(names))}" - ) - - # A camera-enabled app can serve tests that do not need cameras, but cameras cannot be - # enabled after startup, so every kit_cameras file must precede every plain kit file. - is_camera = [marks(name, "kit_cameras") for name in names] - assert is_camera == sorted(is_camera, reverse=True), ( - "kit_cameras files must come first, otherwise a plain `kit` file boots the app without" - f" cameras and the later launch_kit(cameras=True) raises. Got: {names}" - ) + 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]): diff --git a/tools/kit_test_files.py b/tools/kit_test_files.py index 370fe02a0205..7c7848d7843d 100644 --- a/tools/kit_test_files.py +++ b/tools/kit_test_files.py @@ -3,27 +3,28 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""List the test files in a directory that can share one Kit app, in a safe order. +"""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. -Selection: every file marked ``kit`` or ``kit_cameras``, minus those marked ``kit_solo`` and -those in :data:`tools.test_settings.TESTS_TO_SKIP`. +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. -Order: ``kit_cameras`` files first. A camera-enabled app can serve tests that do not need -cameras, but cameras cannot be enabled after startup, so a plain ``kit`` file booting first -would make a later ``launch_kit(cameras=True)`` raise. +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 --format paths - python3 tools/kit_test_files.py source/isaaclab/test/sim --format names + 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 @@ -49,33 +50,49 @@ def _tests_to_skip() -> frozenset[str]: return frozenset(TESTS_TO_SKIP) -def shareable_test_files(directory: Path) -> list[Path]: - """Return the files under ``directory`` that can share a Kit app, cameras first. +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: ``kit_cameras`` ones first, each group sorted by name. + 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() - cameras, plain = [], [] + 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): - cameras.append(path) - elif _MARK_KIT.search(source): - plain.append(path) - return cameras + plain + 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"), @@ -88,9 +105,9 @@ def main(argv: list[str] | None = None) -> int: if not args.directory.is_dir(): parser.error(f"not a directory: {args.directory}") - files = shareable_test_files(args.directory) + files = shareable_test_files(args.directory, args.profile) if not files: - parser.error(f"no Kit-marked test files found in {args.directory}") + 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)) From 930e770e30ec1eb90d0504b3149a7b6bf0f69ba4 Mon Sep 17 00:00:00 2001 From: Matthew Taylor Date: Tue, 4 Aug 2026 17:13:15 -0400 Subject: [PATCH 9/9] Batch same-profile test files into one Kit process Files migrated to launch_kit() share the app when they land in the same process, but the runner still gives every file its own subprocess, so the sharing never happens and Kit startup is paid once per file. The temporary probe measured what that costs: 23 files took 9m47s per-file against 5m29s batched, a 44% reduction, with 17-18s of the per-file wall time being Kit startup rather than tests. Group files by launch profile and hand each group to pytest as one invocation. tools/_kit_batching.py decides the grouping and takes the resulting JUnit report back apart per file, so the summary table, the failed-file list, and the uploaded artifact stay keyed by file exactly as before. Both are pure functions over paths and strings, which is why they can be tested on any platform while the process machinery around them cannot. Off unless ISAACLAB_TEST_BATCH_KIT is set. The per-file path is untouched and remains the default. Kept out of batches: unmarked files, kit_solo, device_split files (already invoked once per device with different -k), files with node-ID selection, the visualizer files that are retried in a fresh process, and anything whose own timeout reaches 2000s. A batch's timeout is the sum of its members', so one hang would consume the whole budget -- and those long files are exactly where Kit startup is a rounding error, so excluding them drops most of the risk and almost none of the gain. Batching is also disabled under the work queue, which hands out files one at a time and cannot offer coherent groups. When a batch dies early the files it never reached are re-run individually, so batching degrades to the behaviour it replaces rather than losing results. Batches carry an index because the label becomes a JUnit report filename: without it, two same-profile batches of equal size collided on one path and the second silently overwrote the first. There is a regression test for that. The probe jobs are removed; they were scaffolding for the measurement above and were re-running the same files a second and third time on every PR. --- .github/workflows/build.yaml | 84 ------- source/isaaclab/test/test_kit_batching.py | 212 +++++++++++++++++ tools/_kit_batching.py | 272 ++++++++++++++++++++++ tools/conftest.py | 151 +++++++++++- 4 files changed, 634 insertions(+), 85 deletions(-) create mode 100644 source/isaaclab/test/test_kit_batching.py create mode 100644 tools/_kit_batching.py diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index fdc71627ae9c..2524575de36f 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -892,90 +892,6 @@ 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 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. - # - # Neither job hardcodes a file list. tools/kit_test_files.py derives it from the kit / - # kit_cameras / kit_solo markers, so the two jobs cannot drift apart from each other or from - # the markers as files are added, renamed, or reclassified. - # - # Both select --profile kit. The kit and kit_cameras groups are separate batches and never - # share a process: cameras cannot be enabled after startup, and a camera-enabled app is not a - # drop-in for a plain one either, since test_simulation_context.py::test_headless_mode asserts - # that offscreen rendering is off. The kit group is the one worth measuring -- it is much the - # larger, and a camera batch is dominated by the one-off ~600 s cold shader compile rather - # than by Kit startup. - # - # A marker expression cannot replace the resolved list: pytest's -m deselects tests but still - # imports every collected module, so it cannot stop a module from calling launch_kit() with - # the wrong profile in a run that booted the other one. - 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 - - name: Resolve shareable test files - id: files - shell: bash - run: | - set -euo pipefail - names=$(python3 tools/kit_test_files.py source/isaaclab/test/sim --profile kit --format names) - echo "names=$names" >> "$GITHUB_OUTPUT" - echo "Resolved $(echo "$names" | tr ',' ' ' | wc -w) shareable test files" - # Baseline: the default test-path of "tools" runs tools/conftest.py, which gives each file - # its own subprocess, so Kit boots once per file. - - 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: ${{ steps.files.outputs.names }} - 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 - - name: Resolve shareable test files - id: files - shell: bash - run: | - set -euo pipefail - paths=$(python3 tools/kit_test_files.py source/isaaclab/test/sim --profile kit --format paths) - echo "paths=$paths" >> "$GITHUB_OUTPUT" - echo "Resolved $(echo "$paths" | wc -w) shareable test files" - # Batched: pointing test-path at the files themselves bypasses tools/conftest.py, so they - # all 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: ${{ steps.files.outputs.paths }} - 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/source/isaaclab/test/test_kit_batching.py b/source/isaaclab/test/test_kit_batching.py new file mode 100644 index 000000000000..93c698db885b --- /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/tools/_kit_batching.py b/tools/_kit_batching.py new file mode 100644 index 000000000000..41229bf73b01 --- /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/conftest.py b/tools/conftest.py index ad4023345225..5cb92bdf71e4 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__) @@ -1079,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, @@ -1367,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"):