From 3ca5502b65b111a2b1481823273a5bf112a05e2e Mon Sep 17 00:00:00 2001 From: "-T.K.-" Date: Sat, 22 Aug 2026 21:32:38 -0700 Subject: [PATCH 1/2] Drop a rename note from a test docstring The docstring states why each spelling is rejected, which is what the assertions check. That the names once differed is not part of it. --- tests/test_ros2_artifacts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_ros2_artifacts.py b/tests/test_ros2_artifacts.py index b1d39c2..a7bb27d 100644 --- a/tests/test_ros2_artifacts.py +++ b/tests/test_ros2_artifacts.py @@ -163,7 +163,7 @@ def test_backend_args_follow_ros2_control_naming(robot_dir): """The backend switches use the current ros2_control / Universal Robots names. `use_fake_hardware` is the pre-Iron spelling, and a bare `use_sim` shadows the - unrelated `use_sim_time` node parameter. Both were renamed. + unrelated `use_sim_time` node parameter. """ cfg = json.loads((robot_dir / "cad" / "ros2_control.json").read_text()) args = cfg["args"] From 33b3dbe8c2917e6bc1c68c92995b5453e9f79ec8 Mon Sep 17 00:00:00 2001 From: "-T.K.-" Date: Mon, 24 Aug 2026 14:26:04 -0700 Subject: [PATCH 2/2] Expand xacro tests over every backend, not just mock The expansion test and the CI loop both ran with default args, which select mock hardware. The real backend is the one that emits a block per bus -- three for lite_biped against mock's one -- so every hardware bring-up depended on a macro that nothing exercised. Parametrise over all four (use_mock_hardware, sim_mujoco) selections and add a test asserting the real backend emits one block per bus plus one per sensor, with distinct component names. Drop the mesh_root override from both: xacro ignores it, because the assembly never declares it as an arg. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EotyWG9YmYvg5xE2v3Zkpt --- .github/workflows/test.yml | 21 +++++++++++++------- tests/test_ros2_artifacts.py | 38 +++++++++++++++++++++++++++++++----- 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9a456d2..25120a2 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -62,12 +62,19 @@ jobs: # `$(find lite_description)/...` xacro includes resolve. source "$GITHUB_WORKSPACE/ws/install/setup.bash" set -euo pipefail + # Each assembly must expand on every backend it supports, not just the + # default. Only the real backend emits the per-bus + # blocks that the hardware bring-ups depend on. for assembly in robots/*/xacro/*.urdf.xacro; do - robot_dir=$(dirname "$(dirname "$assembly")") - meshes="$robot_dir/meshes/visual" - out="/tmp/$(basename "$assembly" .urdf.xacro).urdf" - echo "::group::$assembly" - xacro "$assembly" "mesh_root:=$meshes" > "$out" - check_urdf "$out" - echo "::endgroup::" + for backend in \ + "" \ + "use_mock_hardware:=false" \ + "sim_mujoco:=true" \ + "use_mock_hardware:=false sim_mujoco:=true"; do + out="/tmp/$(basename "$assembly" .urdf.xacro).urdf" + echo "::group::$assembly [${backend:-default}]" + xacro "$assembly" $backend > "$out" + check_urdf "$out" + echo "::endgroup::" + done done diff --git a/tests/test_ros2_artifacts.py b/tests/test_ros2_artifacts.py index a7bb27d..fa2e07a 100644 --- a/tests/test_ros2_artifacts.py +++ b/tests/test_ros2_artifacts.py @@ -38,6 +38,16 @@ def _robots_with_ros2_control() -> list[Path]: RC_DIRS = _robots_with_ros2_control() RC_IDS = [p.name for p in RC_DIRS] +# The four backend selections the description supports. Only the real one emits +# per-bus blocks; the rest collapse to a single combined component. +BACKENDS = [ + [], + ["use_mock_hardware:=false"], + ["sim_mujoco:=true"], + ["use_mock_hardware:=false", "sim_mujoco:=true"], +] +BACKEND_IDS = ["mock", "real", "sim", "sim_over_real"] + @pytest.mark.skipif(not ROBOT_DIRS, reason="no generated xacro robots") @pytest.mark.parametrize("robot_dir", ROBOT_DIRS, ids=ROBOT_IDS) @@ -236,17 +246,35 @@ def test_description_has_base_link_and_mesh_root(robot_dir): @pytest.mark.skipif(shutil.which("xacro") is None or shutil.which("check_urdf") is None, reason="xacro/check_urdf not installed (ROS / RoboStack-pixi only)") @pytest.mark.parametrize("robot_dir", ROBOT_DIRS, ids=ROBOT_IDS) -def test_xacro_expands_and_check_urdf(robot_dir, tmp_path): +@pytest.mark.parametrize("backend", BACKENDS, ids=BACKEND_IDS) +def test_xacro_expands_and_check_urdf(robot_dir, backend, tmp_path): """Expand the top assembly and validate with check_urdf (the ros2_control_demos test).""" robot = robot_dir.name assembly = robot_dir / "xacro" / f"{robot}.urdf.xacro" out = tmp_path / f"{robot}.urdf" - # mesh_root override so package:// doesn't need an installed ament workspace expanded = subprocess.run( - ["xacro", str(assembly), f"mesh_root:={robot_dir / 'meshes' / 'visual'}"], - capture_output=True, text=True, - ) + ["xacro", str(assembly), *backend], capture_output=True, text=True) assert expanded.returncode == 0, expanded.stderr out.write_text(expanded.stdout) checked = subprocess.run(["check_urdf", str(out)], capture_output=True, text=True) assert checked.returncode == 0, checked.stderr + + +@pytest.mark.skipif(shutil.which("xacro") is None, reason="xacro not installed") +@pytest.mark.parametrize("robot_dir", RC_DIRS, ids=RC_IDS) +def test_real_backend_emits_one_block_per_bus(robot_dir): + """The real backend must emit a block per bus, plus one per sensor. + + The other three backends collapse to a single combined block, so only this one + exercises the per-bus macro that every hardware bring-up depends on. + """ + robot = robot_dir.name + cfg = json.loads((robot_dir / "cad" / "ros2_control.json").read_text()) + expected = len(cfg["groups"]) + (1 if cfg.get("imu") else 0) + expanded = subprocess.run( + ["xacro", str(robot_dir / "xacro" / f"{robot}.urdf.xacro"), "use_mock_hardware:=false"], + capture_output=True, text=True) + assert expanded.returncode == 0, expanded.stderr + names = [e.get("name") for e in ET.fromstring(expanded.stdout).iter("ros2_control")] + assert len(names) == expected, f"{robot}: expected {expected} blocks, got {names}" + assert len(set(names)) == len(names), f"{robot}: duplicate component names {names}"