diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 9105aea8..4b2cf081 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -100,17 +100,12 @@ jobs: steps: - name: pre-checkout setup run: | - export APT_KEY_DONT_WARN_ON_DANGEROUS_USAGE=1 DEBIAN_FRONTEND=noninteractive + export DEBIAN_FRONTEND=noninteractive apt-get update -o APT::Acquire::Retries=4 -qq - apt-get install -o APT::Acquire::Retries=4 -o Dpkg::Use-Pty=0 -qy --no-install-recommends ca-certificates gnupg sudo - apt-key adv --keyserver keyserver.ubuntu.com --recv-keys E1DD270288B4E6030699E45FA1715D88E1DF1F24 - echo 'deb http://ppa.launchpad.net/git-core/ppa/ubuntu noble main' > /etc/apt/sources.list.d/git.list - apt-get update -o APT::Acquire::Retries=4 -qq - apt-get install -o APT::Acquire::Retries=4 -o Dpkg::Use-Pty=0 -qy --no-install-recommends git + apt-get install -o APT::Acquire::Retries=4 -o Dpkg::Use-Pty=0 -qy --no-install-recommends ca-certificates sudo git # Create non-root user for running repository setup / tests useradd -m -s /bin/bash runner echo 'runner ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers - rm -rf /var/cache/apt/archives/*.deb /var/cache/apt/archives/partial/*.deb /var/cache/apt/*.bin /var/lib/apt/lists/* /var/log/apt/* shell: bash - name: checkout PR code @@ -148,7 +143,7 @@ jobs: # Switch to runner user for remaining commands sudo -H -u runner ./setup/submodule_checkout # Install system packages as root - ./setup/ubuntu/24.04/install_prereqs.sh + INSTALL_JUPYTER=0 ./setup/ubuntu/24.04/install_prereqs.sh sudo -H -u runner python3 -m pip install --user --break-system-packages poetry==2.1.4 sudo -H -u runner python3 -m poetry config virtualenvs.in-project true sudo -H -u runner python3 -m poetry install --all-extras --with dev @@ -166,9 +161,18 @@ jobs: set -euo pipefail VENV_PYTHON="$(pwd)/.venv/bin/python" sudo -H -u runner env -u DISPLAY XDG_CACHE_HOME=/tmp \ - DRAKE_ALLOW_NETWORK=lcm:meshcat "$VENV_PYTHON" -m pytest -ra + DRAKE_ALLOW_NETWORK=lcm:meshcat "$VENV_PYTHON" -m pytest -ra --durations=30 --junitxml=test-results/pytest.xml shell: bash + - name: upload test timings + if: always() + uses: actions/upload-artifact@v4 + with: + name: pytest-${{ github.job }} + path: test-results/pytest.xml + if-no-files-found: ignore + retention-days: 14 + # TODO(russt): add noble-pip-core workflow which only runs the tests in manipulation, # but uses the manipulation module from pip. noble-pip-all: @@ -191,17 +195,12 @@ jobs: steps: - name: pre-checkout setup run: | - export APT_KEY_DONT_WARN_ON_DANGEROUS_USAGE=1 DEBIAN_FRONTEND=noninteractive - apt-get update -o APT::Acquire::Retries=4 -qq - apt-get install -o APT::Acquire::Retries=4 -o Dpkg::Use-Pty=0 -qy --no-install-recommends ca-certificates gnupg sudo - apt-key adv --keyserver keyserver.ubuntu.com --recv-keys E1DD270288B4E6030699E45FA1715D88E1DF1F24 - echo 'deb http://ppa.launchpad.net/git-core/ppa/ubuntu noble main' > /etc/apt/sources.list.d/git.list + export DEBIAN_FRONTEND=noninteractive apt-get update -o APT::Acquire::Retries=4 -qq - apt-get install -o APT::Acquire::Retries=4 -o Dpkg::Use-Pty=0 -qy --no-install-recommends git + apt-get install -o APT::Acquire::Retries=4 -o Dpkg::Use-Pty=0 -qy --no-install-recommends ca-certificates sudo git # Create non-root user for running repository setup / tests useradd -m -s /bin/bash runner echo 'runner ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers - rm -rf /var/cache/apt/archives/*.deb /var/cache/apt/archives/partial/*.deb /var/cache/apt/*.bin /var/lib/apt/lists/* /var/log/apt/* shell: bash - name: checkout PR code @@ -241,7 +240,14 @@ jobs: # Install system packages as root ./setup/ubuntu/24.04/install_prereqs.sh rm -rf ./manipulation # remove local module, force pip module. - sudo -H -u runner python3 -m pip install --user --break-system-packages --upgrade manipulation[all] pytest pytest-timeout + # CPU wheels are a CI-only choice; leave published requirements unchanged. + sudo -H -u runner python3 -m pip install --user --break-system-packages \ + --index-url https://download.pytorch.org/whl/cpu 'torch>=2.3,<3.0' 'torchvision>=0.18,<1.0' + # Keep the subsequent resolver from replacing these with GPU builds. + sudo -H -u runner python3 -c 'from importlib.metadata import version; print("\n".join(f"{p}=={version(p)}" for p in ("torch", "torchvision")))' > /tmp/ci-torch-constraints.txt + sudo -H -u runner python3 -m pip install --user --break-system-packages \ + --constraint /tmp/ci-torch-constraints.txt --upgrade manipulation[all] pytest pytest-timeout + sudo -H -u runner python3 -c 'import torch, torchvision; print(torch.__version__, torchvision.__version__); assert "+cpu" in torch.__version__ and "+cpu" in torchvision.__version__; assert torch.version.cuda is None' rm -rf /var/cache/apt/archives/*.deb /var/cache/apt/archives/partial/*.deb /var/cache/apt/*.bin /var/lib/apt/lists/* /var/log/apt/* shell: bash @@ -257,9 +263,18 @@ jobs: find book/htmlbook -type f -name 'test_*.py' -delete # remove htmlbook tests (extra Python requirements) sudo -H -u runner env -u DISPLAY MANIPULATION_DATA_DIR="$(pwd)/book/data" \ XDG_CACHE_HOME=/tmp DRAKE_ALLOW_NETWORK=lcm:meshcat \ - python3 -m pytest -ra + python3 -m pytest -ra --durations=30 --junitxml=test-results/pytest.xml shell: bash + - name: upload test timings + if: always() + uses: actions/upload-artifact@v4 + with: + name: pytest-${{ github.job }} + path: test-results/pytest.xml + if-no-files-found: ignore + retention-days: 14 + sequoia: name: macos sequoia 15 runs-on: macos-15 @@ -328,9 +343,18 @@ jobs: - name: test run: | set -euo pipefail - XDG_CACHE_HOME=/tmp DRAKE_ALLOW_NETWORK=lcm:meshcat .venv/bin/python -m pytest -ra + XDG_CACHE_HOME=/tmp DRAKE_ALLOW_NETWORK=lcm:meshcat .venv/bin/python -m pytest -ra --durations=30 --junitxml=test-results/pytest.xml shell: zsh -efuo pipefail {0} + - name: upload test timings + if: always() + uses: actions/upload-artifact@v4 + with: + name: pytest-${{ github.job }} + path: test-results/pytest.xml + if-no-files-found: ignore + retention-days: 14 + lint: runs-on: ubuntu-latest needs: [check-approval] diff --git a/docs/testing.md b/docs/testing.md index a8a877b8..2848add3 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -27,3 +27,48 @@ by tests; it deliberately excludes models such as Gymnasium Robotics that notebooks only download in interactive mode. See Drake's [network policy documentation](https://drake.mit.edu/doxygen_cxx/group__allow__network.html). + +# CI timings + +Each test job prints its 30 slowest pytest phases and uploads a JUnit report as +`pytest-`, retained for 14 days and uploaded even on test failure when +available. Compare per-test times alongside Actions step timings. Test execution +remains serial. + +Download caching was evaluated in CI, including archive restoration overhead. +Linux restoration cost more than the installation time it saved, and macOS did +not show a consistent setup improvement. The workflow therefore keeps fresh +dependency installation and model prefetch without download cache actions. + +The Linux pip job installs CPU-only PyTorch and torchvision wheels from PyTorch's +CPU index. A temporary constraints file preserves those builds during the +`manipulation[all]` install, and the job verifies their build tags afterward. +This choice is confined to CI: the published dependency requirements and Poetry +lockfile are unchanged, and downstream users can continue using GPU builds. + +# Menagerie conversion coverage + +The default conversion test uses Panda (STL meshes, includes, and defaults) and +ANYmal B (textures and materials). It copies each model to a temporary directory, +converts its scene, checks mesh references, and loads the result with Drake. +[Drake's Menagerie tests](https://github.com/RobotLocomotion/drake/blob/master/multibody/parsing/test/detail_mujoco_parser_examples_test.cc) +exercise raw MJCF parsing, not this conversion code. + +When updating the Menagerie revision in `manipulation/remotes.py`, run the full +conversion sweep locally: + +```sh +TEST_ALL_MENAGERIE=1 .venv/bin/python -m pytest \ + manipulation/test/test_make_drake_compatible_model.py -k mujoco_menagerie +``` + +This checks conversion for all matching scenes. Asset references and Drake loading +are asserted for the two representatives; other upstream scenes can contain +dangling material references or use unsupported MJCF features. Generated files stay in temporary directories, outside the model cache. + +Ubuntu Poetry CI sets `INSTALL_JUPYTER=0` when installing system prerequisites because +its Python environment supplies the notebook dependencies. The prerequisite +script still installs system Jupyter by default for local users and pip CI, +whose test harness relies on those packages. CI uses +Ubuntu's Git package and retains apt lists through setup to avoid redundant +repository setup and downloads. diff --git a/manipulation/test/test_letter_generation.py b/manipulation/test/test_letter_generation.py index 3f7f67b1..b97510da 100644 --- a/manipulation/test/test_letter_generation.py +++ b/manipulation/test/test_letter_generation.py @@ -99,7 +99,9 @@ def test_create_sdf_asset_different_fonts(self): font_name=font, letter_height_meters=0.4, extrusion_depth_meters=0.15, - output_dir=self._tmp_dir, + output_dir=f"{self._tmp_dir}/{font}", + # Exercise font geometry without repeating CoACD coverage. + use_bbox_collision_geometry=True, ) self.assertIsNotNone( @@ -114,54 +116,35 @@ def test_create_sdf_asset_different_parameters(self): """Test SDF asset creation with different parameters.""" letter = "C" - # Test with different letter heights - for letter_height in [0.2, 0.4, 0.6]: - with self.subTest(letter_height=letter_height): + dimensions = [(height, 0.15) for height in (0.2, 0.4, 0.6)] + dimensions += [(0.4, depth) for depth in (0.1, 0.2)] + for height, depth in dimensions: + with self.subTest(height=height, depth=depth): sdf_path = create_sdf_asset_from_letter( text=letter, font_name="DejaVu Sans", - letter_height_meters=letter_height, - extrusion_depth_meters=0.15, - output_dir=self._tmp_dir, - ) - - self.assertIsNotNone( - sdf_path, - f"SDF path should not be None for letter_height {letter_height}", - ) - if sdf_path: - self.assertTrue( - sdf_path.exists(), - f"SDF file should exist for letter_height {letter_height}", - ) - - # Test with different extrusion depths - for depth in [0.1, 0.15, 0.2]: - with self.subTest(extrusion_depth=depth): - sdf_path = create_sdf_asset_from_letter( - text=letter, - font_name="DejaVu Sans", - letter_height_meters=0.4, + letter_height_meters=height, extrusion_depth_meters=depth, - output_dir=self._tmp_dir, - ) - - self.assertIsNotNone( - sdf_path, f"SDF path should not be None for depth {depth}" + output_dir=f"{self._tmp_dir}/dimensions_{height}_{depth}", + use_bbox_collision_geometry=True, ) - if sdf_path: - self.assertTrue( - sdf_path.exists(), f"SDF file should exist for depth {depth}" - ) - + self.assertIsNotNone(sdf_path) + self.assertTrue(sdf_path.exists()) + mesh = trimesh.load_mesh(sdf_path.with_suffix(".obj")) + self.assertAlmostEqual(mesh.extents[1], height) + self.assertAlmostEqual(mesh.extents[2], depth) + + def test_create_sdf_asset_collision_geometry(self): + """Exercise both collision methods independently of dimension tests.""" + letter = "C" for use_bbox_collision_geometry in [True, False]: - with self.subTest(extrusion_depth=depth): + with self.subTest(use_bbox_collision_geometry=use_bbox_collision_geometry): sdf_path = create_sdf_asset_from_letter( text=letter, font_name="DejaVu Sans", letter_height_meters=0.4, - extrusion_depth_meters=depth, - output_dir=self._tmp_dir, + extrusion_depth_meters=0.15, + output_dir=f"{self._tmp_dir}/collision_{use_bbox_collision_geometry}", use_bbox_collision_geometry=use_bbox_collision_geometry, ) diff --git a/manipulation/test/test_make_drake_compatible_model.py b/manipulation/test/test_make_drake_compatible_model.py index f1075de6..649fc26a 100644 --- a/manipulation/test/test_make_drake_compatible_model.py +++ b/manipulation/test/test_make_drake_compatible_model.py @@ -1,10 +1,13 @@ import os -import re +import shutil import tempfile import unittest +from pathlib import Path from lxml import etree -from pydrake.multibody.parsing import PackageMap +from pydrake.multibody.parsing import PackageMap, Parser +from pydrake.multibody.plant import AddMultibodyPlantSceneGraph +from pydrake.systems.framework import DiagramBuilder from manipulation.utils import FindResource @@ -173,48 +176,71 @@ def test_mjcf_defaults(self): os.remove(output_filename) def test_mujoco_menagerie(self): - """Test all files in the mujoco_menagerie package.""" + """Cover our conversion pipeline; Drake tests raw Menagerie parsing.""" package_map = PackageMap() AddMujocoMenagerie(package_map) - menagerie = package_map.GetPath("mujoco_menagerie") - # Find all XML files recursively under the menagerie path - results = "" - for root, dirs, files in os.walk(menagerie): - for file in files: - if file.endswith(".drake.xml"): + menagerie = Path(package_map.GetPath("mujoco_menagerie")) + representative_scenes = ( + "franka_emika_panda/scene.xml", # STL meshes, includes, defaults. + "anybotics_anymal_b/scene.xml", # File-backed textures and materials. + ) + scenes = [menagerie / scene for scene in representative_scenes] + if os.environ.get("TEST_ALL_MENAGERIE") == "1": + scenes = sorted(menagerie.rglob("*scene.xml")) + self.assertTrue(scenes) + for scene in scenes: + relative_scene = scene.relative_to(menagerie).as_posix() + with self.subTest( + scene=relative_scene + ), tempfile.TemporaryDirectory() as tmp: + # Keep generated meshes out of the shared package cache. Recompute + # outputs even if an older local run left converted assets there. + model_dir = Path(tmp) / scene.parent.name + shutil.copytree(scene.parent, model_dir) + output = model_dir / "scene.drake.xml" + MakeDrakeCompatibleModel( + str(model_dir / scene.name), str(output), overwrite=True + ) + self.assertTrue(output.is_file()) + # Preserve the full sweep's conversion-smoke-test contract. + # Some other upstream OBJ files have dangling material references. + if relative_scene not in representative_scenes: continue - if file.endswith("scene.xml"): - with self.subTest(file=file): - original_file = os.path.join(root, file) - drake_compatible_file = original_file.replace( - ".xml", ".drake.xml" - ) - try: - MakeDrakeCompatibleModel( - original_file, drake_compatible_file - ) - results += ( - f"PASS: {os.path.relpath(root, menagerie)}/{file}\n" - ) - except Exception as e: - rel_path = os.path.relpath(root, menagerie) - # Known type/message pairs that we expect to encounter - known_exceptions = [ - # No more known exceptions (yeah!)... but the format is: - # (KeyError, r".*'file'.*", "Need to parse defaults"), - ] - known_failure = False - for exc_type, msg_pattern, note in known_exceptions: - if isinstance(e, exc_type) and re.match( - msg_pattern, str(e) - ): - results += f"FAIL: {os.path.join(rel_path, file)}: {note}\n" - known_failure = True - break - if not known_failure: - results += f"FAIL: {os.path.join(rel_path, file)}: Unregistered exception\n" - raise # Re-raise if not a known exception - print(results) + root = etree.parse(output) + self.assertFalse(root.findall(".//include")) + meshdir = "" + for compiler in root.findall(".//compiler"): + meshdir = compiler.get("meshdir", compiler.get("assetdir", meshdir)) + meshes = root.findall(".//asset/mesh") + self.assertTrue(meshes) + textures = [] + for mesh in meshes: + mesh_path = model_dir / meshdir / mesh.get("file") + self.assertTrue(mesh_path.is_file()) + if mesh_path.suffix.lower() == ".obj": + for line in mesh_path.read_text().splitlines(): + if line.startswith("mtllib "): + material = ( + mesh_path.parent + / line.removeprefix("mtllib ").strip() + ) + self.assertTrue(material.is_file()) + for entry in material.read_text().splitlines(): + if entry.startswith("map_Kd "): + texture = ( + material.parent + / entry.removeprefix("map_Kd ").strip() + ) + self.assertTrue(texture.is_file()) + textures.append(texture) + if relative_scene == "anybotics_anymal_b/scene.xml": + self.assertTrue(textures) + + builder = DiagramBuilder() + plant, _ = AddMultibodyPlantSceneGraph(builder, time_step=0.001) + models = Parser(plant).AddModels(str(output)) + self.assertTrue(models) + plant.Finalize() if __name__ == "__main__": diff --git a/setup/ubuntu/24.04/install_prereqs.sh b/setup/ubuntu/24.04/install_prereqs.sh index 82cd11c2..69f94bc5 100755 --- a/setup/ubuntu/24.04/install_prereqs.sh +++ b/setup/ubuntu/24.04/install_prereqs.sh @@ -53,17 +53,20 @@ fi apt-get install -o APT::Acquire::Retries=4 -o Dpkg::Use-Pty=0 -qy \ --no-install-recommends ca-certificates gnupg -apt-get update -qq || (sleep 15; apt-get update -qq) +# CI installs notebook dependencies in its Python environment instead. +jupyter_packages=() +if [[ "${INSTALL_JUPYTER:-1}" != "0" ]]; then + jupyter_packages=(jupyter-notebook jupyter-nbconvert) +fi # Keep this up to date with Drake's # setup/ubuntu/binary_distribution/packages-noble.txt, except that we choose to # not install most of the system `python3-*` packages. The second batch are new # requirements from this repo. apt-get install -o APT::Acquire::Retries=4 -o Dpkg::Use-Pty=0 -qy \ - --no-install-recommends $(cat <