From a151edd8a16c3db902c8639c7e69d387c25d736e Mon Sep 17 00:00:00 2001 From: John Date: Tue, 21 Jul 2026 12:58:30 -0400 Subject: [PATCH 01/12] feat(l4t): make robot-l4t deployment knobs overridable + document name resolution Parametrize the robot-l4t compose service so a single service covers real deployments without editing compose: - AUTONOMY_ROLE and FCU_URL are now ${VAR:-default} overridable (and FCU_URL is unquoted so the literal serial path reaches mavros). - Rosbag output path is BAG_STORAGE_PATH-overridable. Update the configure-multi-robot skill to reflect the honor-pre-set-ROBOT_NAME guard (#370): document pinning ROBOT_NAME in an override for a single real robot, the never-on-the-shared-service caveat, and the unknown_robot fallback fixes by topology. Co-Authored-By: Claude Opus 4.8 --- .agents/skills/configure-multi-robot/SKILL.md | 36 ++++++++++++++++--- robot/docker/docker-compose.yaml | 6 ++-- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/.agents/skills/configure-multi-robot/SKILL.md b/.agents/skills/configure-multi-robot/SKILL.md index d147fdcf2..c70ba8dde 100644 --- a/.agents/skills/configure-multi-robot/SKILL.md +++ b/.agents/skills/configure-multi-robot/SKILL.md @@ -42,6 +42,9 @@ docker-compose.yaml (ROBOT_NAME_SOURCE=container_name | hostname, │ ▼ robot/docker/.bashrc (runs on container shell start) + │ + ├─ ROBOT_NAME already set in env? → KEEP IT, skip resolution entirely + │ (guard: `if [ -z "${ROBOT_NAME:-}" ]`; lets an override/compose pin the name) │ ├─ ROBOT_NAME_SOURCE=container_name → resolve `hostname` back to docker container name │ (e.g. `airstack-robot-desktop-1`) @@ -67,7 +70,7 @@ The default mapping rule in [`robot/docker/robot_name_map/default_robot_name_map robot: 'robot_{1}' domain_id: '{1}' - pattern: '.*' # catch-all - robot: 'unknown-robot' + robot: 'unknown_robot' # must be a valid ROS token (no hyphen) or launch fails domain_id: '0' ``` @@ -97,7 +100,18 @@ docker exec airstack-robot-desktop-1 bash -c 'echo $ROBOT_NAME $ROS_DOMAIN_ID' # robot_1 1 ``` -If you need a non-default name (custom hostname scheme on a physical robot, or you want `drone_alpha` instead of `robot_1`), write a new mapping YAML in `robot/docker/robot_name_map/` and point `ROBOT_NAME_MAP_CONFIG_FILE` at it. Do **not** hardcode `ROBOT_NAME=...` in compose unless you know what you are doing — it bypasses the resolver and you lose `ROS_DOMAIN_ID` co-assignment. +If you need a non-default name (custom hostname scheme on a physical robot, or you want `drone_alpha` instead of `robot_1`), you have two options: + +1. **Write a mapping YAML** in `robot/docker/robot_name_map/` and point `ROBOT_NAME_MAP_CONFIG_FILE` at it. Preferred when the name should be derived from the machine (hostname/container) — keeps the resolver in charge of `ROS_DOMAIN_ID` co-assignment. +2. **Pin `ROBOT_NAME` directly** in a per-deployment override env file. `.bashrc` honors a pre-set `ROBOT_NAME` (guard: `if [ -z "${ROBOT_NAME:-}" ]`) and skips the map lookup. This is the clean shortcut for a **single real robot** whose hostname doesn't match `robot-` (see [Real robots and the `unknown_robot` fallback](#real-robots-and-the-unknown_robot-fallback) below): + + ```bash + # overrides/.env — single robot, named directly + ROBOT_NAME=robot_1 + ROS_DOMAIN_ID=1 # set alongside — pinning ROBOT_NAME skips the map's domain co-assignment + ``` + +**Only pin `ROBOT_NAME` in an *override env file*, never on the shared `robot-desktop`/`robot-l4t` *service* in compose.** The service is reused for every replica; a hardcoded `ROBOT_NAME` there collapses all robots onto one name/domain and silently breaks multi-robot. And when you pin it, set `ROS_DOMAIN_ID` too — the resolver is what normally co-assigns the domain, and skipping it leaves the domain at whatever the environment defaults to. For a one-off override (e.g. ad hoc debugging): @@ -286,7 +300,7 @@ Without `allow_substs="true"`, the substitution string is loaded literally and t If two robots share a domain, every topic collides — both `/robot_1/odometry` publishers will be visible to both subscribers, and DDS will sometimes deliver crossed data. The default `robot_name_map` derives the domain from the robot index, so this only happens if you: - Hardcode `ROS_DOMAIN_ID` in compose to the same value for two replicas -- Use a hostname that doesn't match any rule and falls through to the catch-all (both robots get `unknown-robot`, domain `0`) +- Use a hostname that doesn't match any rule and falls through to the catch-all (both robots get `unknown_robot`, domain `0`) Always verify after starting: @@ -329,9 +343,21 @@ This is a common foot-gun: Either keep the remap relative (`to="odometry"`) so it joins the namespace, or write the full path explicitly (`to="/$(env ROBOT_NAME)/odometry"`). -### 9. Hostname doesn't match any rule on real robots +### 9. Real robots and the `unknown_robot` fallback + +On VOXL/Jetson the service uses `ROBOT_NAME_SOURCE=hostname`, so the **OS hostname** is what gets mapped — not a compose replica index. The stock `default_robot_name_map.yaml` only matches `robot-`, so a device named `airlab-jetson-42` falls through to the catch-all and comes up as **`ROBOT_NAME=unknown_robot`, domain `0`** (with a map that has *no* catch-all, the resolver instead exits non-zero and `ROBOT_NAME` is left unset — same confusing "empty namespace" symptom). This is the usual "why is my real robot `unknown_robot`?" report. -On VOXL/Jetson with `ROBOT_NAME_SOURCE=hostname`, the device hostname must match a rule in the mapping YAML. If `hostname` returns `airlab-jetson-42` and your config only matches `robot-N`, the resolver exits non-zero and `ROBOT_NAME` is unset — the autonomy stack will then launch with empty namespaces and break in confusing ways. Either rename the device or extend the mapping config. +Pick whichever fix matches your topology (see [Configuring a Single Robot](#configuring-a-single-robot)): + +- **One robot, quickest:** pin `ROBOT_NAME=robot_1` + `ROS_DOMAIN_ID=1` in the deployment's override env file. The `.bashrc` guard honors it and skips the lookup — no hostname change, no map file. +- **One robot, machine-derived:** rename the device hostname to `robot-1` so the default map resolves it automatically. +- **A fleet:** name each machine `robot-` (default map handles it) **or** ship a mapping YAML that matches your hostnames and point `ROBOT_NAME_MAP_CONFIG_FILE` at it. Do **not** pin a single `ROBOT_NAME` on the shared service — every robot would collide on it. + +Verify on the device: + +```bash +docker exec bash -c 'echo "$(hostname) -> ROBOT_NAME=$ROBOT_NAME ROS_DOMAIN_ID=$ROS_DOMAIN_ID"' +``` ## Pre-Merge Checklist diff --git a/robot/docker/docker-compose.yaml b/robot/docker/docker-compose.yaml index 71b71e425..31b3e368a 100644 --- a/robot/docker/docker-compose.yaml +++ b/robot/docker/docker-compose.yaml @@ -209,15 +209,15 @@ services: - ROBOT_NAME_SOURCE=hostname - AUTOLAUNCH=${AUTOLAUNCH:-true} - LAUNCH_PACKAGE=autonomy_bringup - - AUTONOMY_ROLE=full # l4t profile: Jetson runs everything onboard + - AUTONOMY_ROLE=${AUTONOMY_ROLE:-full} - LAUNCH_NATNET=${LAUNCH_NATNET:-false} # mavros mavlink settings - - FCU_URL="/dev/ttyTHS4:115200" + - FCU_URL=${FCU_URL:-/dev/ttyTHS4:115200} - TGT_SYSTEM=1 # assumes network isolation via a physical router, so uses network_mode=host network_mode: host volumes: - - /media/airlab/Storage/airstack_collection:/bags:rw + - ${BAG_STORAGE_PATH:-/media/airlab/Storage/airstack_collection}:/bags:rw # ----------------------- # Jetson in lite mode: only local/perception/interface modules run onboard. From db50f988b3b34e9af32a11c98eb2f59a084748d7 Mon Sep 17 00:00:00 2001 From: John Date: Tue, 21 Jul 2026 12:58:43 -0400 Subject: [PATCH 02/12] feat(l4t): add site-agnostic l4t-px4-realrobot override template MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deployment override for a single real PX4 robot on a Jetson (aarch64/l4t). Surfaces the common knobs at the top with sensible defaults: ROBOT_NAME pinned directly (single-robot shortcut honored by .bashrc), FCU_URL, AUTONOMY_ROLE, BAG_STORAGE_PATH, and RECORD_BAGS. Mocap-agnostic — NatNet/external-vision settings are added by a separate optitrack override. Co-Authored-By: Claude Opus 4.8 --- overrides/l4t-px4-realrobot.env | 34 +++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 overrides/l4t-px4-realrobot.env diff --git a/overrides/l4t-px4-realrobot.env b/overrides/l4t-px4-realrobot.env new file mode 100644 index 000000000..271726801 --- /dev/null +++ b/overrides/l4t-px4-realrobot.env @@ -0,0 +1,34 @@ +# Real-robot deployment on an NVIDIA Jetson (aarch64 / l4t) with a PX4 flight +# controller (e.g. Cube Orange) over serial. + +# Build (first time / after image changes): +# airstack image-build --profile l4t robot-l4t +# Run: +# airstack up --env-file overrides/l4t-px4-realrobot.env robot-l4t + +# Only bring up the Jetson stack (robot-l4t + zed-l4t). +COMPOSE_PROFILES="l4t" + +# Launch the autonomy stack automatically on container start. +AUTOLAUNCH="true" +NUM_ROBOTS="1" + +# --- Robot identity ----------------------------------------------------------- +# Shortcut to name only a single agent. +ROBOT_NAME="robot_1" +ROS_DOMAIN_ID="1" + +# Launches entire robot autonomy stack +AUTONOMY_ROLE="full" + +# --- Flight controller (MAVROS) ---------------------------------------------- +# Default is the Jetson UART (ttyTHS4). +FCU_URL="/dev/ttyTHS4:115200" + +# --- Robot description (PX4 iris w/ sensors; override for your airframe) ------ +URDF_FILE="robot_descriptions/iris/urdf/iris_with_sensors.pegasus.robot.urdf" + +# --- Flight-data recording ---------------------------------------------------- +# Where rosbags land on the host (bind-mounted to /bags in-container). +BAG_STORAGE_PATH="/media/airlab/Storage/airstack_collection" +RECORD_BAGS="false" From aa1e9acfb0febcf6c69987f0b66a302f93c44689 Mon Sep 17 00:00:00 2001 From: John Date: Tue, 21 Jul 2026 12:58:43 -0400 Subject: [PATCH 03/12] fix(l4t): entrypoint passthrough + ZED SDK 5.2; document build gotchas Two real-hardware build fixes for the Jetson profile: - Dockerfile.l4t-stack-base: overwrite dustynv's /ros_entrypoint.sh with an `exec "$@"` passthrough. Its prebuilt source-ROS libs (fastcdr 2.2.5) were shadowing the apt Jazzy (2.2.7) that Dockerfile.robot layers on, crashing apt-built nodes like mavros with symbol-lookup errors under tmux autolaunch. - zed/Dockerfile.zed-l4t: bump ZED SDK 4.2 -> 5.2 and move the coupled ROS deps together (zed_msgs 5.2.1, point_cloud_transport(_plugins) 4.x, add backward_ros). Document both gotchas in the docker-build-profiles skill, and correct the stale unknown-robot -> unknown_robot in the robot_identity reference doc. Co-Authored-By: Claude Opus 4.8 --- .agents/skills/docker-build-profiles/SKILL.md | 2 ++ docs/robot/docker/robot_identity.md | 2 +- robot/docker/Dockerfile.l4t-stack-base | 4 ++++ robot/docker/zed/Dockerfile.zed-l4t | 10 ++++++---- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/.agents/skills/docker-build-profiles/SKILL.md b/.agents/skills/docker-build-profiles/SKILL.md index 64b579a01..c57f85f3e 100644 --- a/.agents/skills/docker-build-profiles/SKILL.md +++ b/.agents/skills/docker-build-profiles/SKILL.md @@ -49,6 +49,8 @@ Troubleshooting notes - YAML quirk: unquoted `3.10` may be parsed as float `3.1` — this changes path strings and breaks imports (e.g., `python3.1` instead of `python3.10`). - Jetson/L4T builds may require `network: host` during the build to avoid kernel iptables/raw table missing-module errors. - Jetson **`robot-l4t`** builds from **`robot-l4t-stack-base`** (`robot/docker/Dockerfile.l4t-stack-base`), not raw dustynv, so **`Dockerfile.robot` stays Ubuntu-shaped.** `airstack image-build --profile l4t robot-l4t` triggers **`robot-l4t-stack-base`** first (`airstack.sh`); bare `compose build robot-l4t` can still parallelize badly, so list stack-base explicitly if not using AirStack CLI. +- **dustynv `/ros_entrypoint.sh` shadows the apt Jazzy runtime (mavros symbol-lookup crash).** The dustynv base sources a prebuilt *source* ROS at `$ROS_ROOT/install` from PID 1, prepending its older libs (e.g. `fastcdr` 2.2.5) ahead of the apt Jazzy (2.2.7) that `Dockerfile.robot` layers on top — apt-built nodes like mavros then die with symbol-lookup errors under tmux autolaunch. `Dockerfile.l4t-stack-base` neutralizes it by overwriting `/ros_entrypoint.sh` with a `exec "$@"` passthrough; shells get ROS from `/opt/ros/jazzy/setup.bash` via `.bashrc`. If a Jetson node suddenly can't resolve symbols after a base-image bump, check whether the entrypoint passthrough is still in place. +- **ZED SDK version is pinned across `zed/Dockerfile.zed-l4t`** — the `ZED_SDK_URL` (e.g. `.../zedsdk/5.2/...`) and the ROS dep args (`ZED_MSGS_VERSION`, `POINTCLOUD_TRANSPORT*_VERSION`, `BACKWARD_ROS_VERSION`) must move together; a mismatched `zed_msgs` vs SDK breaks the driver build. Bumping the SDK is camera-firmware-coupled, so confirm the target camera runs that SDK line before merging. Examples of agent prompts - "Check `robot/docker/docker-compose.yaml` for `PYTHON_VERSION` entries and quote any unquoted numeric values; open a PR with the fixes and include a test log from a builder-stage build." diff --git a/docs/robot/docker/robot_identity.md b/docs/robot/docker/robot_identity.md index 676f20896..494b5daae 100644 --- a/docs/robot/docker/robot_identity.md +++ b/docs/robot/docker/robot_identity.md @@ -51,7 +51,7 @@ mappings: domain_id: '{1}' - pattern: '.*' - robot: 'unknown-robot' + robot: 'unknown_robot' # must be a valid ROS token (no hyphen) or launch fails domain_id: '0' ``` diff --git a/robot/docker/Dockerfile.l4t-stack-base b/robot/docker/Dockerfile.l4t-stack-base index 0ebb8cd14..b0ef6aaf7 100644 --- a/robot/docker/Dockerfile.l4t-stack-base +++ b/robot/docker/Dockerfile.l4t-stack-base @@ -48,3 +48,7 @@ ENV PYTHON_EXECUTABLE=/usr/bin/python3 # Prefer system/cuda tooling over any removed venv prefix (CUDA symlink is usually /usr/local/cuda). ENV PATH="/usr/local/cuda/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" + +# Neutralize dustynv's /ros_entrypoint.sh: it prepends a prebuilt source-ROS lib dir +# that shadows the apt Jazzy (older fastcdr) and crashes mavros. See docker-build-profiles skill. +RUN printf '#!/bin/bash\nexec "$@"\n' > /ros_entrypoint.sh && chmod +x /ros_entrypoint.sh diff --git a/robot/docker/zed/Dockerfile.zed-l4t b/robot/docker/zed/Dockerfile.zed-l4t index b39ec2bc7..a59733e31 100644 --- a/robot/docker/zed/Dockerfile.zed-l4t +++ b/robot/docker/zed/Dockerfile.zed-l4t @@ -15,7 +15,7 @@ ARG ROS2_DIST=jazzy ENV DEBIAN_FRONTEND noninteractive # ZED SDK link -ENV ZED_SDK_URL="https://download.stereolabs.com/zedsdk/4.2/l4t$L4T_MAJOR.$L4T_MINOR/jetsons" +ENV ZED_SDK_URL="https://download.stereolabs.com/zedsdk/5.2/l4t$L4T_MAJOR.$L4T_MINOR/jetsons" RUN mkdir -p /tmp && chmod 1777 /tmp @@ -53,12 +53,13 @@ ARG XACRO_VERSION=2.0.8 ARG DIAGNOSTICS_VERSION=4.0.0 ARG AMENT_LINT_VERSION=0.12.11 ARG ROBOT_LOCALIZATION_VERSION=3.5.3 -ARG ZED_MSGS_VERSION=4.2.2 +ARG ZED_MSGS_VERSION=5.2.1 ARG NMEA_MSGS_VERSION=2.0.0 ARG ANGLES_VERSION=1.15.0 ARG GEOGRAPHIC_INFO_VERSION=1.0.6 -ARG POINTCLOUD_TRANSPORT_VERSION=1.0.18 -ARG POINTCLOUD_TRANSPORT_PLUGINS_VERSION=1.0.11 +ARG POINTCLOUD_TRANSPORT_VERSION=4.0.9 +ARG POINTCLOUD_TRANSPORT_PLUGINS_VERSION=4.0.4 +ARG BACKWARD_ROS_VERSION=1.0.8 RUN wget https://github.com/ros/xacro/archive/refs/tags/${XACRO_VERSION}.tar.gz -O - | tar -xvz && mv xacro-${XACRO_VERSION} xacro && \ wget https://github.com/ros/diagnostics/archive/refs/tags/${DIAGNOSTICS_VERSION}.tar.gz -O - | tar -xvz && mv diagnostics-${DIAGNOSTICS_VERSION} diagnostics && \ @@ -69,6 +70,7 @@ RUN wget https://github.com/ros/xacro/archive/refs/tags/${XACRO_VERSION}.tar.gz wget https://github.com/ros/angles/archive/refs/tags/${ANGLES_VERSION}.tar.gz -O - | tar -xvz && mv angles-${ANGLES_VERSION} angles && \ wget https://github.com/ros-perception/point_cloud_transport/archive/refs/tags/${POINTCLOUD_TRANSPORT_VERSION}.tar.gz -O - | tar -xvz && mv point_cloud_transport-${POINTCLOUD_TRANSPORT_VERSION} point_cloud_transport && \ wget https://github.com/ros-perception/point_cloud_transport_plugins/archive/refs/tags/${POINTCLOUD_TRANSPORT_PLUGINS_VERSION}.tar.gz -O - | tar -xvz && mv point_cloud_transport_plugins-${POINTCLOUD_TRANSPORT_PLUGINS_VERSION} point_cloud_transport_plugins && \ + wget https://github.com/pal-robotics/backward_ros/archive/refs/tags/${BACKWARD_ROS_VERSION}.tar.gz -O - | tar -xvz && mv backward_ros-${BACKWARD_ROS_VERSION} backward_ros && \ wget https://github.com/ros-geographic-info/geographic_info/archive/refs/tags/${GEOGRAPHIC_INFO_VERSION}.tar.gz -O - | tar -xvz && mv geographic_info-${GEOGRAPHIC_INFO_VERSION} geographic-info && \ cp -r geographic-info/geographic_msgs/ . && \ rm -rf geographic-info From 61a94b929c401abdc95265048fd9856abfdfe388 Mon Sep 17 00:00:00 2001 From: John Date: Tue, 21 Jul 2026 12:59:25 -0400 Subject: [PATCH 04/12] chore: bump version to 0.19.0-alpha.7 Version-increment gate: bump above develop's 0.19.0-alpha.6 and record the l4t deployment changes in the changelog. Co-Authored-By: Claude Opus 4.8 --- .env | 2 +- CHANGELOG.md | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.env b/.env index bdd6f251e..85280be19 100644 --- a/.env +++ b/.env @@ -12,7 +12,7 @@ PROJECT_NAME="airstack" # If you've run ./airstack.sh setup, then this will auto-generate from the git commit hash every time a change is made # to a Dockerfile or docker-compose.yaml file. Otherwise this can also be set explicitly to make a release version. # auto-generated from git commit hash -VERSION="0.19.0-alpha.6" +VERSION="0.19.0-alpha.7" # Choose "dev" or "prebuilt". "dev" is for mounted code that must be built live. "prebuilt" is for built ros_ws baked into the image DOCKER_IMAGE_BUILD_MODE="dev" # Where to push and pull images from. Can replace with your docker hub username if using docker hub. diff --git a/CHANGELOG.md b/CHANGELOG.md index 747f670b4..2bde19e49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,11 +12,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Battery and telemetry display in GCS RQT control panel (voltage and percentage per robot when MAVROS battery topic is bridged) - `TARGET_ARCH` build arg (default `x86_64`) in `Dockerfile.robot` to arch-parametrize `LD_LIBRARY_PATH`; `docker-compose.yaml` passes `TARGET_ARCH: aarch64` to the `voxl` and `l4t` real-robot image builds - `ros-${ROS_DISTRO}-mavros-extras` in the robot image (provides the vision_pose plugin used for external-pose deployments) +- `overrides/l4t-px4-realrobot.env` — site-agnostic deployment override for a single real PX4 robot on a Jetson (aarch64/l4t) + +### Changed + +- `robot-l4t` compose service knobs are now env-overridable (`AUTONOMY_ROLE`, `FCU_URL`, and the rosbag path via `BAG_STORAGE_PATH`); `FCU_URL` unquoted so the literal serial path reaches MAVROS +- `zed-l4t` image: ZED SDK 4.2 → 5.2 with the coupled ROS deps (`zed_msgs` 5.2.1, `point_cloud_transport(_plugins)` 4.x, add `backward_ros`) ### Fixed - Robot name resolution now honors a pre-set `ROBOT_NAME` (e.g. injected via docker compose) instead of always overriding it from the container/hostname mapping (`robot/docker/.bashrc`) - Robot name-map catch-all fallback now maps to `unknown_robot` (valid ROS namespace token) instead of `unknown-robot` (`default_robot_name_map.yaml`) +- l4t robot image: replace dustynv's `/ros_entrypoint.sh` with a passthrough so its prebuilt source-ROS libs (older `fastcdr`) no longer shadow the apt Jazzy runtime and crash apt-built nodes like MAVROS ## [1.0.0] - 2024-12-19 From 77d385189ca806981704a06cd13342136724babc Mon Sep 17 00:00:00 2001 From: John Date: Wed, 22 Jul 2026 11:29:53 -0400 Subject: [PATCH 05/12] test(infra): collect co-located unit tests via the package list + integration tier Unit tests are defined by tests/colcon_unit_test_packages.yaml: conftest.py resolves each listed package to its /test dir and collects the non-linter test_*.py files under --import-mode=importlib (set in pytest.ini), marking each `unit` by path. ament lint files are skipped (they run under colcon test). Removes two now-unnecessary files under tests/robot/; the package test/ dirs are collected directly. Also add an integration test tier: tests/integration/ + `integration` mark + a shared robot_autonomy_stack fixture (robot-desktop container, no sim/GPU), slotted into _MODULE_ORDER between build_packages and the sim tiers. Co-Authored-By: Claude Opus 4.8 --- tests/README.md | 29 ++- tests/colcon_unit_test_packages.yaml | 6 +- tests/conftest.py | 172 ++++++++++++++++-- tests/integration/README.md | 36 ++++ tests/pytest.ini | 3 +- tests/robot/README.md | 40 +--- .../natnet_ros2/test_natnet_ros2.py | 32 ---- .../test_validation_core.py | 38 ---- 8 files changed, 232 insertions(+), 124 deletions(-) create mode 100644 tests/integration/README.md delete mode 100644 tests/robot/perception/natnet_ros2/test_natnet_ros2.py delete mode 100644 tests/robot/sensors/lidar_point_cloud_filter/test_validation_core.py diff --git a/tests/README.md b/tests/README.md index 6aec42e1b..f8ee839ce 100644 --- a/tests/README.md +++ b/tests/README.md @@ -1,10 +1,10 @@ # Testing (`tests/`) -AirStack's **pytest** tree under `tests/` has three roles: +AirStack's **pytest** tree under `tests/` has these roles: 1. **`tests/system/`** — Docker stack tests (sim + robot + GCS): liveliness, sensor Hz, takeoff/hover/land, image/workspace builds. -2. **`tests/robot/`** — Fast **unit** tests that mirror `robot/ros_ws/src/` (`behavior`, `global`, `interface`, `local`, `perception`, `sensors`). Mark: `unit`. -3. **`tests/sim/`** — Unit tests for simulation-side helpers (e.g. Motive / NatNet emulator). Mark: `unit`. +2. **Unit tests** — Fast hermetic tests (`unit` mark) whose **source is co-located** with each ROS 2 package at `/test/`. [`colcon_unit_test_packages.yaml`](colcon_unit_test_packages.yaml) lists which packages have unit tests, and `pytest tests/` collects them from there. +3. **`tests/integration/`** — Cross-component tests (`integration` mark) that wire the robot container to a host-side component, without a sim or GPU. Shared fixtures live in `tests/conftest.py`. Use `airstack test -m unit -v` for hermetic tests only, or the marks below for the full stack. @@ -25,15 +25,18 @@ Shared fixtures live in `tests/conftest.py`. Use `airstack test -m unit -v` for | [`system/test_takeoff_hover_land.py`](system/test_takeoff_hover_land.py) | `takeoff_hover_land` | End-to-end flight: PX4 readiness gate, takeoff to 10 m, hover stability, land — one chain per (sim, num_robots, iteration, velocity) | Docker daemon, GPU, sim license | | [`system/test_fixed_trajectory.py`](system/test_fixed_trajectory.py) | `autonomy` | Fixed-pattern trajectory evaluation: takeoff, execute a trajectory (Circle, Figure8, Racetrack, Line), record path deviation metrics, land — one chain per (sim, num_robots, iteration, trajectory_type) | Docker daemon, GPU, sim license | -### Unit tests (`tests/robot/`, `tests/sim/`) +### Unit tests (co-located) Hermetic tests use `@pytest.mark.unit` (see [`pytest.ini`](pytest.ini)). -**Co-location + proxy pattern:** test source lives alongside its ROS 2 package at +Test source lives alongside its ROS 2 package at `robot/ros_ws/src///test/test_*.py` (the ROS 2 / colcon convention). -Files in `tests/robot/` are thin proxies that re-export those tests so that -`pytest tests/` discovers them. Both `airstack test -m unit` and -`colcon test --packages-select ` run the same test source. +[`colcon_unit_test_packages.yaml`](colcon_unit_test_packages.yaml) lists which packages +have unit tests; `conftest.py` resolves each to its `test/` dir and collects the +non-linter `test_*.py` files under `--import-mode=importlib`, tagging each `unit`. To add +a package's unit tests, list it in that YAML. Both `airstack test -m unit` and +`colcon test --packages-select ` run the same source (colcon also runs the ament +linters + C++ gtests). Example: `robot/ros_ws/src/sensors/lidar_point_cloud_filter/test/test_validation_core.py` tests the numpy-only range validation rules in @@ -43,8 +46,16 @@ tests the numpy-only range validation rules in See [Unit Testing Guide](../docs/development/intermediate/testing/unit_testing.md) and the `add-unit-tests` agent skill for full details. +### Integration tests (`tests/integration/`) + +Cross-component tests (`integration` mark) that wire a few real components together — the +robot autonomy container plus a host-side component — **without** a sim or GPU. The +shared `robot_autonomy_stack` fixture (in `conftest.py`) reuses a running `robot-desktop` +container or brings one up automatically (like `build_packages`), then tears it down. +Collection order runs integration after `build_packages` and before the sim tiers. + Marks can be combined with pytest logic: -`-m unit`, `-m "build_docker or build_packages"`, `-m liveliness`, `-m sensors`, `-m takeoff_hover_land`, `-m autonomy`, or e.g. `-m "liveliness or sensors"` (see **Bring-up scope** below). +`-m unit`, `-m "build_docker or build_packages"`, `-m integration`, `-m liveliness`, `-m sensors`, `-m takeoff_hover_land`, `-m autonomy`, or e.g. `-m "liveliness or sensors"` (see **Bring-up scope** below). ### Bring-up scope (`airstack_env`) diff --git a/tests/colcon_unit_test_packages.yaml b/tests/colcon_unit_test_packages.yaml index 5e0bd0ebb..65cdd38cc 100644 --- a/tests/colcon_unit_test_packages.yaml +++ b/tests/colcon_unit_test_packages.yaml @@ -1,7 +1,9 @@ -# Packages run via `colcon test` in system.test_build_packages.test_colcon_test_robot. +# Defines where the unit tests live: each listed package's /test/ dir is +# collected by `pytest tests/` (via conftest.py) and run by `colcon test` in +# system.test_build_packages.test_colcon_test_robot. # # Add a package here when it has gtests (ament_add_gtest) and/or pytest tests under -# /test/. Keep in sync with tests/robot/ proxies for Python unit tests. +# /test/. # # See: docs/development/intermediate/testing/unit_testing.md diff --git a/tests/conftest.py b/tests/conftest.py index 2a51c569a..38d46411f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -83,10 +83,89 @@ def colcon_test_robot_command(workspace="robot"): if pytest_args: cmd += f' --pytest-args "{pytest_args}"' return cmd -# Unit tests live co-located with their ROS 2 packages in robot/ros_ws/src/. -# Thin proxy files under tests/robot/ re-export those tests so that -# `pytest tests/` and `airstack test -m unit` discover them without any -# sys.path manipulation here. Each proxy file sets up its own paths. + + +def repo_path(*parts: str) -> Path: + """Resolve a path relative to the repo root (``AIRSTACK_ROOT``). + + Single source of truth for cross-tree paths — no test or hook should hardcode + ``Path(__file__).parents[N]`` walks. + """ + return Path(AIRSTACK_ROOT).joinpath(*parts) + + +# Unit-test SOURCE lives co-located with each package (colcon convention). +# colcon_unit_test_packages.yaml lists which packages have unit tests; every listed +# package resolves to its /test dir (globbed per workspace) and the non-linter +# test_*.py files there are collected under --import-mode=importlib (set in pytest.ini). +# See unit_test_files (what gets collected), pytest_configure (adds them to the run), +# and pytest_itemcollected (auto `unit` mark). ament lint tests are excluded (they run +# under colcon test). +_WORKSPACE_PKG_TEST_GLOBS = { + "robot": "robot/ros_ws/src/**/{pkg}/test", + "sim": "simulation/**/{pkg}/test", +} + +# ament lint tests ship in every ROS package's test/ dir and import ament_* at +# module load (unavailable outside the built workspace). Skip them here — they run +# under `colcon test` instead (see colcon_test_robot_command's `-m not linter`). +_LINTER_TEST_FILENAMES = { + "test_copyright.py", "test_flake8.py", "test_pep257.py", + "test_pep8.py", "test_xmllint.py", "test_lint_cmake.py", +} + + +def unit_test_dirs(): + """Every co-located unit-test dir resolved from colcon_unit_test_packages.yaml.""" + if not COLCON_UNIT_TEST_PACKAGES_YAML.is_file(): + return [] + with COLCON_UNIT_TEST_PACKAGES_YAML.open(encoding="utf-8") as f: + data = yaml.safe_load(f) or {} + dirs = [] + for workspace, glob_tmpl in _WORKSPACE_PKG_TEST_GLOBS.items(): + cfg = data.get(workspace) or {} + for pkg in cfg.get("packages") or []: + for match in repo_path().glob(glob_tmpl.format(pkg=pkg)): + if match.is_dir(): + dirs.append(match.resolve()) + return dirs + + +_UNIT_TEST_DIRS = None + + +def _unit_test_dirs_cached(): + global _UNIT_TEST_DIRS + if _UNIT_TEST_DIRS is None: + _UNIT_TEST_DIRS = unit_test_dirs() + return _UNIT_TEST_DIRS + + +def _is_unit_item(item): + """True when a collected item's file lives under a co-located unit-test dir.""" + try: + p = Path(str(item.path)).resolve() + except Exception: + return False + return any(p.is_relative_to(d) for d in _unit_test_dirs_cached()) + + +def unit_test_files(): + """Co-located unit-test files to collect: every ``test_*.py`` under a package + ``test/`` dir, minus the ament lint files. + + We inject explicit files (not the dirs) because pytest does not apply ignore + rules to files it recurses into from an explicitly-passed directory — passing + the exact files is the only deterministic way to keep ament lint tests out. + """ + files = [] + for d in _unit_test_dirs_cached(): + for f in sorted(d.glob("test_*.py")): + if f.name not in _LINTER_TEST_FILENAMES: + files.append(f) + return files + + RUN_DIR = None ROS_DISTRO_SETUP = "/opt/ros/jazzy/setup.bash" _LAST_CMD_OUTPUT: dict[str, str] = {} @@ -134,6 +213,26 @@ def pytest_configure(config): RUN_DIR.mkdir(parents=True, exist_ok=True) config.option.xmlpath = str(RUN_DIR / "results.xml") + # Collect co-located unit tests: their files live outside tests/, so add the + # explicit non-linter test files to the collection args. Skip when an explicit + # path was given on the CLI (args_source == ARGS) so `pytest tests/system/foo.py` + # still narrows as expected. + src_name = getattr(getattr(config, "args_source", None), "name", "TESTPATHS") + if src_name != "ARGS": + for f in unit_test_files(): + entry = str(f) + if entry not in config.args: + config.args.append(entry) + + +def pytest_itemcollected(item): + """Auto-mark co-located unit tests `unit` (before -m filtering runs). + + Idempotent when the source already declares the mark. + """ + if _is_unit_item(item): + item.add_marker(pytest.mark.unit) + def pytest_runtest_setup(item): global _CURRENT_ITEM @@ -191,13 +290,15 @@ def pytest_generate_tests(metafunc): # docker image builds → colcon workspace builds → liveliness (infra) → sensors # (ROS topic streams) → autonomy flight tests. _MODULE_ORDER = [ - # Unit tests first — fast, hermetic, no Docker. Any module whose dotted - # name starts with "robot." or "sim." is a proxy for a package-level unit - # test and sorts into this leading slot via the prefix check below. + # Unit tests first — fast, hermetic, no Docker. Co-located package unit tests + # (see unit_test_files) sort into this leading slot via the path check below. "__unit__", # System tests follow in dependency order. "system.test_build_docker", "system.test_build_packages", + # Integration tests (tests/integration/) need the robot-desktop image + colcon + # build, so they run after build_packages and before the sim tiers. + "__integration__", "system.test_liveliness", "system.test_sensors", "system.test_takeoff_hover_land", @@ -235,12 +336,14 @@ def _rank(name, order): def _module_key(item): """Return the ordering key for an item. - Unit-test proxies live under ``robot/``, ``sim/``, or ``gcs/`` and are - identified by their nodeid prefix. Everything else uses the dotted module - ``__name__`` looked up against ``_MODULE_ORDER``. + Co-located unit tests (collected from package ``test/`` dirs outside ``tests/``) + are identified by path. Integration tests live under ``integration/``. + Everything else uses the dotted module ``__name__`` against ``_MODULE_ORDER``. """ - if item.nodeid.startswith(("robot/", "sim/", "gcs/")): + if _is_unit_item(item): return _rank("__unit__", _MODULE_ORDER) + if item.nodeid.startswith("integration/"): + return _rank("__integration__", _MODULE_ORDER) return _rank(getattr(item.module, "__name__", ""), _MODULE_ORDER) @@ -808,4 +911,49 @@ def airstack_env(request): airstack_cmd("down", timeout=120, log_name=log) down_duration_s = round(time.time() - t3, 2) logger.info("Teardown finished in %.2fs", down_duration_s) - m.record(tid, "airstack_down_duration_s", down_duration_s, unit="s") \ No newline at end of file + m.record(tid, "airstack_down_duration_s", down_duration_s, unit="s") + +# ── integration tier (tests/integration/) ───────────────────────────────── + +_INTEGRATION_ROBOT_PATTERN = "robot.*desktop" +# Robot-only bring-up: autonomy stack on, no sim profile, single robot. +_INTEGRATION_ENV = { + "AUTOLAUNCH": "true", + "NUM_ROBOTS": "1", + "COMPOSE_PROFILES": "desktop", +} + + +@pytest.fixture(scope="module") +def robot_autonomy_stack(request): + """Robot-desktop container for integration tests (no sim, no GPU). + + Yields ``{"container": , "brought_up": bool}``. Reuses an already + running container (fast local iteration, left running afterward); otherwise + runs ``airstack up robot-desktop`` and tears it down after the module. + Behaves like the ``build_packages`` fixture — always brings up Docker when + no container is found. + """ + existing = find_container(_INTEGRATION_ROBOT_PATTERN) + if existing and container_running(existing): + yield {"container": existing, "brought_up": False} + return + + log = "robot_autonomy_stack" + with logger_to(log): + missing = missing_images(env=_INTEGRATION_ENV) + if missing: + pytest.skip("robot-desktop image not built locally: " + ", ".join(missing)) + airstack_cmd("down", timeout=120, log_name=log) + result = airstack_cmd("up", "robot-desktop", + env_overrides=_INTEGRATION_ENV, timeout=180, log_name=log) + if result.returncode != 0: + pytest.fail(f"`airstack up robot-desktop` failed:\n{read_log_tail(log)}") + + container = wait_for_container(_INTEGRATION_ROBOT_PATTERN, timeout=120) + assert container, "robot-desktop container not Running after 120s" + try: + yield {"container": container, "brought_up": True} + finally: + with logger_to(log): + airstack_cmd("down", timeout=120, log_name=log) diff --git a/tests/integration/README.md b/tests/integration/README.md new file mode 100644 index 000000000..6521b08c6 --- /dev/null +++ b/tests/integration/README.md @@ -0,0 +1,36 @@ +# Integration tests (`tests/integration/`) + +Cross-component tests (`@pytest.mark.integration`) that wire a few **real** components +together — the robot autonomy container plus a host-side component — **without** a +simulator or GPU. They sit between the hermetic unit tests and the full sim-based system +tests: heavier than a unit test (they need the `robot-desktop` image + a running +container), lighter than a system test (no sim license, no GPU). + +## The `robot_autonomy_stack` fixture + +Defined in [`../conftest.py`](../conftest.py). Module-scoped. It: + +- reuses an already-running `robot-desktop` container when one is present (fast local + iteration — left running afterward), otherwise runs `airstack up robot-desktop` with + `AUTOLAUNCH=true NUM_ROBOTS=1 COMPOSE_PROFILES=desktop` and tears it down after the + module (same behavior as the `build_packages` fixture); +- **skips** cleanly when the `robot-desktop` image isn't built locally. + +It yields `{"container": , "brought_up": bool}`. + +## Collection order + +Integration runs after `build_docker` / `build_packages` (it needs the image + a colcon +build) and before the sim tiers — see `_MODULE_ORDER` in `conftest.py`. + +## Running + +```bash +airstack test -m integration -v +``` + +## Adding an integration test + +Create `tests/integration//test_*.py`, request the `robot_autonomy_stack` fixture, +and mark the module `pytestmark = pytest.mark.integration`. Keep it sim-free and +GPU-free — anything needing a simulator belongs in `tests/system/`. diff --git a/tests/pytest.ini b/tests/pytest.ini index 03fee8c32..dc8c939de 100644 --- a/tests/pytest.ini +++ b/tests/pytest.ini @@ -3,12 +3,13 @@ markers = unit: Fast hermetic tests (no Docker stack; numpy / pure Python) build_docker: Docker image build tests build_packages: Colcon workspace build tests + integration: Cross-component integration tests (robot container + a host-side component; no sim/GPU) liveliness: Container and process health (Docker, tmux, sentinel ROS 2 nodes) sensors: Sim and robot sensor topic rates, LiDAR validation, sim RTF takeoff_hover_land: End-to-end takeoff / hover / land action tests autonomy: Fixed-pattern trajectory path-tracker benchmark (test_fixed_trajectory.py) testpaths = . -addopts = -v --durations=0 +addopts = -v --durations=0 --import-mode=importlib cache_dir = /tmp/.pytest_cache log_cli = true log_cli_level = INFO diff --git a/tests/robot/README.md b/tests/robot/README.md index a4409c4b1..3961d90cc 100644 --- a/tests/robot/README.md +++ b/tests/robot/README.md @@ -1,37 +1,17 @@ -# Robot-side unit test proxies +# Robot-side unit tests -Layout mirrors [`robot/ros_ws/src/`](../../robot/ros_ws/src/) autonomy layers: - -| Directory | Maps to ROS workspace | -|-----------|----------------------| -| `behavior/` | `robot/ros_ws/src/behavior/` | -| `global/` | `robot/ros_ws/src/global/` | -| `interface/` | `robot/ros_ws/src/interface/` | -| `local/` | `robot/ros_ws/src/local/` | -| `perception/` | `robot/ros_ws/src/perception/` | -| `sensors/` | `robot/ros_ws/src/sensors/` | - -## Design: co-location + proxy - -**Test source** lives co-located with each ROS 2 package (the standard colcon -convention): +Unit-test **source is co-located** with each ROS 2 package (the standard colcon +convention) and is collected by `pytest tests/`: ``` robot/ros_ws/src///test/test_.py ← source of truth ``` -**This directory** contains thin proxy files that load the real test module via -`importlib` and re-export its `test_*` functions, making them discoverable by -`pytest tests/` and `airstack test -m unit` without any changes to the CI -workflow. Each proxy is ~15 lines. - -``` -tests/robot///test_.py ← proxy (re-exports above) -``` - -Both `airstack test -m unit` (pytest path via proxy) and -`colcon test --packages-select ` (direct path to source) run the same -test functions from the same file. +[`../colcon_unit_test_packages.yaml`](../colcon_unit_test_packages.yaml) lists which +packages have unit tests; `tests/conftest.py` resolves each to its `test/` dir and +collects the non-linter `test_*.py` files under `--import-mode=importlib`, tagging each +`@pytest.mark.unit`. Both `airstack test -m unit` and `colcon test --packages-select ` +run the same source. -All test functions must carry `@pytest.mark.unit`. For adding new tests see the -`add-unit-tests` agent skill. +To add a package's unit tests, list it under `robot.packages` in the YAML — see the +`add-unit-tests` agent skill. The per-layer subdirectories here hold only documentation. diff --git a/tests/robot/perception/natnet_ros2/test_natnet_ros2.py b/tests/robot/perception/natnet_ros2/test_natnet_ros2.py deleted file mode 100644 index fc9a78bde..000000000 --- a/tests/robot/perception/natnet_ros2/test_natnet_ros2.py +++ /dev/null @@ -1,32 +0,0 @@ -# Copyright (c) 2024 Carnegie Mellon University -# MIT License - see LICENSE in the repository root for full text. -"""Proxy: re-exposes natnet_ros2 unit tests from the package source tree. - -Unit test logic lives co-located with its package (ROS 2 / colcon convention): - robot/ros_ws/src/perception/natnet_ros2/test/test_natnet_ros2.py - -This file makes those tests discoverable by ``pytest tests/`` (CI) and -``airstack test -m unit`` without any changes to the CI workflow. -Run ``colcon test --packages-select natnet_ros2`` to also execute the C++ -gtests and ament linters. -""" - -import importlib.util -import sys -from pathlib import Path - -_repo_root = Path(__file__).resolve().parents[4] -_pkg_test = _repo_root / "robot/ros_ws/src/perception/natnet_ros2/test" -_real_file = _pkg_test / "test_natnet_ros2.py" - -# Load the real module under a unique name to avoid the circular-import that -# would occur if we used `from test_natnet_ros2 import *` (this file has the -# same name and pytest adds its directory to sys.path at collection time). -_spec = importlib.util.spec_from_file_location("_natnet_ros2_unit_tests", _real_file) -_real = importlib.util.module_from_spec(_spec) -_spec.loader.exec_module(_real) - -# Re-export every test_* symbol so pytest collects them from this proxy. -for _name in dir(_real): - if _name.startswith("test_"): - globals()[_name] = getattr(_real, _name) diff --git a/tests/robot/sensors/lidar_point_cloud_filter/test_validation_core.py b/tests/robot/sensors/lidar_point_cloud_filter/test_validation_core.py deleted file mode 100644 index e7babf9d4..000000000 --- a/tests/robot/sensors/lidar_point_cloud_filter/test_validation_core.py +++ /dev/null @@ -1,38 +0,0 @@ -# Copyright (c) 2024 Carnegie Mellon University -# MIT License - see LICENSE in the repository root for full text. -"""Proxy: re-exposes validation_core unit tests from the package source tree. - -Unit test logic lives co-located with its package (ROS 2 / colcon convention): - robot/ros_ws/src/sensors/lidar_point_cloud_filter/test/test_validation_core.py - -This file makes those tests discoverable by ``pytest tests/`` (CI) and -``airstack test -m unit`` without any changes to the CI workflow. -Run ``colcon test --packages-select lidar_point_cloud_filter`` to also execute -the ament linters. -""" - -import importlib.util -import sys -from pathlib import Path - -_repo_root = Path(__file__).resolve().parents[4] -_pkg_test = _repo_root / "robot/ros_ws/src/sensors/lidar_point_cloud_filter/test" -_pkg_root = _pkg_test.parent # adds lidar_point_cloud_filter/ package to sys.path -_real_file = _pkg_test / "test_validation_core.py" - -# Make the package module importable so the real test can do -# `from lidar_point_cloud_filter.validation_core import ...` -if str(_pkg_root) not in sys.path: - sys.path.insert(0, str(_pkg_root)) - -# Load the real module under a unique name to avoid the circular-import that -# would occur if we used `from test_validation_core import *` (this file has -# the same name and pytest adds its directory to sys.path at collection time). -_spec = importlib.util.spec_from_file_location("_lidar_validation_unit_tests", _real_file) -_real = importlib.util.module_from_spec(_spec) -_spec.loader.exec_module(_real) - -# Re-export every test_* symbol so pytest collects them from this proxy. -for _name in dir(_real): - if _name.startswith("test_"): - globals()[_name] = getattr(_real, _name) From 26f7d97e455f1b48b10ae550cf28f1c3bbff7266 Mon Sep 17 00:00:00 2001 From: John Date: Wed, 22 Jul 2026 11:29:53 -0400 Subject: [PATCH 06/12] docs(testing): describe unit tests as co-located and listed in the package YAML Update the add-unit-tests and run-system-tests skills, AGENTS.md, and the unit-testing docs: adding a unit test is "list the package in colcon_unit_test_packages.yaml", and the source lives in the package's own test/ dir. Document the `integration` mark/tier. Co-Authored-By: Claude Opus 4.8 --- .agents/skills/add-unit-tests/SKILL.md | 139 ++++++------------ .agents/skills/run-system-tests/SKILL.md | 7 +- AGENTS.md | 6 +- .../development/intermediate/testing/index.md | 9 +- .../intermediate/testing/unit_testing.md | 68 ++++----- 5 files changed, 87 insertions(+), 142 deletions(-) diff --git a/.agents/skills/add-unit-tests/SKILL.md b/.agents/skills/add-unit-tests/SKILL.md index 7d1d3b582..5265e7eef 100644 --- a/.agents/skills/add-unit-tests/SKILL.md +++ b/.agents/skills/add-unit-tests/SKILL.md @@ -1,6 +1,6 @@ --- name: add-unit-tests -description: Add Python or C++ unit tests to an AirStack ROS 2 package. Covers the co-location pattern (test source in package/test/), the thin proxy that makes tests discoverable by pytest tests/ and airstack test -m unit, and how to extend the pattern to sim and GCS modules. +description: Add Python or C++ unit tests to an AirStack ROS 2 package. Covers the co-location pattern (test source in package/test/), registering the package in colcon_unit_test_packages.yaml so pytest tests/ and airstack test -m unit collect it, and how to extend to sim and GCS modules. license: MIT metadata: author: AirLab CMU @@ -23,30 +23,34 @@ For system tests (full Docker stack, sim, sensors, takeoff/hover/land) see the ## Architecture Overview -Unit tests follow a **co-location + proxy** pattern: +Unit test **source lives co-located with its package** (ROS 2 / colcon convention). +`tests/colcon_unit_test_packages.yaml` lists which packages have unit tests, and +`pytest tests/` collects them from there — you only edit files under the package itself. ``` robot/ros_ws/src/// ├── src/ # production source (Python or C++) ├── test/ -│ ├── test_.py # ← unit test SOURCE (canonical location) +│ ├── test_.py # ← unit test SOURCE (collected directly) │ ├── test_.cpp # ← C++ gtest SOURCE (optional) │ └── fake_.hpp # ← C++ test doubles (optional) └── CMakeLists.txt # wires ament_add_gtest under BUILD_TESTING -tests/robot/// -└── test_.py # ← thin PROXY (re-exports tests from above) +tests/colcon_unit_test_packages.yaml # ← list the package here (single source of truth) ``` -The **proxy** is a one-file shim that loads the real test module with `importlib` -and re-exports every `test_*` function. This means: +`tests/conftest.py` reads the YAML, resolves each listed package to its `test/` dir, +and injects the non-linter `test_*.py` files into collection under +`--import-mode=importlib` (set in `tests/pytest.ini`). Each collected item is +auto-tagged `@pytest.mark.unit` by path, so `-m unit` selects it. ament lint files +(`test_copyright.py`, etc.) are excluded — they run under `colcon test`. This means: | Invocation | What runs | |---|---| -| `pytest tests/ -m unit` | Proxy in `tests/robot/` → loads real test from package | +| `pytest tests/ -m unit` | Package `test/test_*.py`, collected directly from source | | `airstack test -m unit` | Same path | | CI `system-tests.yml` (PR open / approved) | Same path via `pytest tests/` | -| `colcon test --packages-select ` | Real test in `package/test/` directly | +| `colcon test --packages-select ` | Real test in `package/test/` (incl. linters + C++) | ## Step-by-Step: Adding a Python Unit Test @@ -112,62 +116,25 @@ For `rclpy.node.Node` subclasses use a real dummy base class instead of a `MagicMock()` to ensure `__init_subclass__` fires and method bodies are defined (see `test_natnet_ros2.py` for the full pattern). -### 3. Write the thin proxy in tests/robot/ +### 3. Register the package in colcon_unit_test_packages.yaml -Create `tests/robot///test_.py`: +If the package isn't already listed, add it under the `robot` workspace in +[`tests/colcon_unit_test_packages.yaml`](../../../tests/colcon_unit_test_packages.yaml): -```python -# Copyright (c) 2024 Carnegie Mellon University -# MIT License - see LICENSE in the repository root for full text. -"""Proxy: re-exposes unit tests from the package source tree. - -Unit test logic lives co-located with the package source (ROS 2 / colcon convention): - robot/ros_ws/src///test/test_.py - -This file makes those tests discoverable by ``pytest tests/`` (CI) and -``airstack test -m unit`` without any changes to the CI workflow. -""" -import importlib.util -import sys -from pathlib import Path - -_repo_root = Path(__file__).resolve().parents[N] # adjust N so this resolves to repo root -_pkg_test = _repo_root / "robot/ros_ws/src///test" -_real_file = _pkg_test / "test_.py" - -# If the test imports from a package module, ensure the package root is on sys.path. -# Example: _pkg_root = _pkg_test.parent; sys.path.insert(0, str(_pkg_root)) - -# Load the real module under a unique name to avoid the circular import that -# would occur if we used `from test_ import *` (this file has the same -# name, and pytest adds its directory to sys.path at collection time). -_spec = importlib.util.spec_from_file_location("__unit_tests", _real_file) -_real = importlib.util.module_from_spec(_spec) -_spec.loader.exec_module(_real) - -# Re-export every test_* symbol so pytest collects them from this proxy. -for _name in dir(_real): - if _name.startswith("test_"): - globals()[_name] = getattr(_real, _name) +```yaml +robot: + packages: + - natnet_ros2 + - lidar_point_cloud_filter + - # ← add here + pytest_args: "-m not linter" ``` -**Counting `parents[N]` to reach the repo root:** - -| Proxy location | `parents[N]` for repo root | -|---|---| -| `tests/robot///` | `parents[4]` | -| `tests/sim//` | `parents[3]` | -| `tests/gcs//` | `parents[3]` | - -### 4. Ensure the tests/ directory structure exists - -```bash -mkdir -p tests/robot// -touch tests/robot///__init__.py # only if needed for conftest path discovery -``` - -The READMEs in `tests/robot/behavior/`, `tests/robot/global/`, etc. describe the -purpose of each layer mirror. Update the layer README when you add a new package. +That's the whole registration. `conftest.py` globs +`robot/ros_ws/src/**//test`, collects its non-linter `test_*.py`, and marks +them `unit`. The test file must be self-contained: if it imports package code, set up +`sys.path` at the top of the test file (see `test_validation_core.py`, which inserts its +package root). Same YAML, different workspace key (`sim:`), for Isaac-extension unit tests. ### 5. Run locally to verify @@ -179,10 +146,10 @@ pytest -m unit -v airstack test -m unit -v ``` -All 14+ existing tests plus your new ones should pass. The proxy output shows: +All 14+ existing tests plus your new ones should pass. Collected items point straight +at the co-located source: ``` -robot///test_.py::test_my_function_basic - <- ../robot/ros_ws/src///test/test_.py PASSED +../robot/ros_ws/src///test/test_.py::test_my_function_basic PASSED ``` ### 6. CI picks it up automatically @@ -194,8 +161,7 @@ Unit tests are discovered by `pytest tests/` and run as part of `system-tests.ym ## Step-by-Step: Adding a C++ gtest -C++ tests don't use the proxy pattern — they live entirely within the package and -run exclusively via `colcon test`. +C++ tests live entirely within the package and run exclusively via `colcon test`. ### 1. Write the test in `package/test/` @@ -255,22 +221,17 @@ there are listed in [`tests/colcon_unit_test_packages.yaml`](../../../tests/colc ## Extending to sim and GCS -The same proxy pattern applies verbatim: +The same mechanism applies — add the package under a workspace key in the YAML. The +workspace→source glob is defined in `conftest.py` (`_WORKSPACE_PKG_TEST_GLOBS`): `robot` → +`robot/ros_ws/src/**//test`, `sim` → `simulation/**//test`. Add a new workspace +key there (e.g. `gcs`) if you extend to a new tree. -**Sim-side Python** (e.g. motive emulator protocol logic): +```yaml +# tests/colcon_unit_test_packages.yaml +sim: + packages: + - # → simulation/**//test collected directly ``` -simulation/...//test/test_.py ← source -tests/sim//test_.py ← proxy (parents[3] = repo root) -``` - -**GCS modules**: -``` -gcs/...//test/test_.py ← source -tests/gcs//test_.py ← proxy (parents[3] = repo root) -``` - -`pytest tests/ -m unit` discovers them through the proxy without any -pytest.ini or CI changes needed. --- @@ -279,14 +240,14 @@ pytest.ini or CI changes needed. | Concern | Answer | |---|---| | Where does test source live? | `/…//test/` (co-located with the package) | -| Where does pytest discover tests? | `tests/robot/` (or `tests/sim/`, `tests/gcs/`) via thin proxy | -| How does the proxy avoid circular import? | `importlib.util.spec_from_file_location` with a unique module name | -| What mark do all unit tests use? | `@pytest.mark.unit` | +| Where does pytest discover tests? | From the package `test/` dir listed in `colcon_unit_test_packages.yaml` | +| How are duplicate basenames handled? | `--import-mode=importlib` (set in `pytest.ini`) | +| What mark do all unit tests use? | `@pytest.mark.unit` (auto-applied by path in `conftest.py`) | | What CI workflow runs them? | `system-tests.yml` — runs `pytest tests/` which includes unit tests | | When does that workflow trigger? | PR opened, `/pytest` comment, `workflow_dispatch` | | Do system tests (`liveliness`, etc.) run too? | No — `-m unit` filters to hermetic tests only | | Does `colcon test` also run these? | Yes — Python tests in `package/test/` are discovered by colcon's pytest runner | -| Can I add pure C++ gtests? | Yes — `ament_add_gtest` in CMakeLists.txt, no proxy needed | +| Can I add pure C++ gtests? | Yes — `ament_add_gtest` in CMakeLists.txt | ## Reference Implementations @@ -296,14 +257,12 @@ pytest.ini or CI changes needed. | `natnet_ros2` (C++) | `robot/ros_ws/src/perception/natnet_ros2/test/test_natnet_logic.cpp` | `build_covariance_6x6`, `negotiate()`, `INatNetClient` seam | | `lidar_point_cloud_filter` | `robot/ros_ws/src/sensors/lidar_point_cloud_filter/test/test_validation_core.py` | Pure-numpy range validation rules | -Corresponding proxies: `tests/robot/perception/natnet_ros2/test_natnet_ros2.py`, -`tests/robot/sensors/lidar_point_cloud_filter/test_validation_core.py`. +Both are collected from their package `test/` dir. ## Files to Know - `.github/workflows/system-tests.yml` — CI workflow (runs `pytest tests/` including unit tests) -- `tests/pytest.ini` — mark registration (`unit`, `build_docker`, etc.) -- `tests/robot/` — proxy layer mirroring `robot/ros_ws/src/` -- `tests/sim/` — proxy layer for sim-side code (future) -- `tests/gcs/` — proxy layer for GCS code (future) +- `tests/pytest.ini` — mark registration + `--import-mode=importlib` +- `tests/colcon_unit_test_packages.yaml` — the package list driving unit-test collection +- `tests/conftest.py` — `unit_test_files()` / `pytest_configure` inject package tests; `pytest_itemcollected` auto-marks `unit` - `tests/README.md` — full test harness reference diff --git a/.agents/skills/run-system-tests/SKILL.md b/.agents/skills/run-system-tests/SKILL.md index f9b41b727..197971def 100644 --- a/.agents/skills/run-system-tests/SKILL.md +++ b/.agents/skills/run-system-tests/SKILL.md @@ -25,7 +25,8 @@ This skill is about the **test harness itself** — pytest marks, fixtures, the The suite lives at `tests/` (repo root) and is fully pytest-based. Configuration is in `tests/pytest.ini` and shared infrastructure in `tests/conftest.py`. - **`tests/system/`** — Docker stack integration tests. Marks: `build_docker`, `build_packages`, `liveliness`, `sensors`, `takeoff_hover_land`, `autonomy`. -- **`tests/robot/`** and **`tests/sim/`** — Hermetic **unit** tests (`@pytest.mark.unit`). These are **thin proxy files** that re-export tests from each ROS 2 package's own `test/` directory (co-located with the source, the ROS 2 / colcon convention). The proxy pattern keeps test source next to the code it tests while making tests discoverable by `pytest tests/`. +- **`tests/integration/`** — Cross-component tests (`integration` mark): robot container + a host-side component, no sim/GPU. +- **Unit tests** (`@pytest.mark.unit`) — Hermetic. Source is **co-located** with each ROS 2 package in its own `test/` dir (ROS 2 / colcon convention). `tests/colcon_unit_test_packages.yaml` lists which packages have unit tests; `conftest.py` resolves each to its `test/` dir and collects the non-linter `test_*.py` under `--import-mode=importlib`. ### Unit tests vs system tests @@ -34,7 +35,7 @@ The suite lives at `tests/` (repo root) and is fully pytest-based. Configuration | Hardware required | None — pure Python | Docker daemon, NVIDIA GPU, sim license | | CI workflow | `system-tests.yml` (included in `pytest tests/`) | `system-tests.yml` (GPU OpenStack VM) | | Trigger | Every push + PR (automatic) | PR opened, `/pytest` comment, `workflow_dispatch` | -| Source location | `/test/test_*.py` (proxied via `tests/robot/`) | `tests/system/` | +| Source location | `/test/test_*.py` (collected directly, listed in `colcon_unit_test_packages.yaml`) | `tests/system/` | | How to add | See `add-unit-tests` skill | See *Adding a New System Test* below | Run unit tests without any Docker stack: @@ -45,7 +46,7 @@ airstack test -m unit -v pytest tests/ -m unit -v # AIRSTACK_ROOT=$(pwd) for direct pytest ``` -For details on the proxy pattern and adding new unit tests, see the +For details on the co-located layout and adding new unit tests, see the `add-unit-tests` skill. | File | Mark | What it tests | Hardware required | diff --git a/AGENTS.md b/AGENTS.md index 0a6b86015..ca6ffebfd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -93,7 +93,7 @@ For detailed step-by-step instructions, refer to the **`.agents/skills/`** direc | [debug-module](.agents/skills/debug-module) | Autonomous debugging of ROS 2 modules | | [update-documentation](.agents/skills/update-documentation) | Documenting new modules and updating mkdocs | | [test-in-simulation](.agents/skills/test-in-simulation) | End-to-end simulation testing of a module | -| [add-unit-tests](.agents/skills/add-unit-tests) | Adding Python or C++ unit tests to a ROS 2 package (co-location + proxy pattern, CI workflow, extending to sim/GCS) | +| [add-unit-tests](.agents/skills/add-unit-tests) | Adding Python or C++ unit tests to a ROS 2 package (co-located test/ dir listed in colcon_unit_test_packages.yaml, CI workflow, extending to sim/GCS) | | [run-system-tests](.agents/skills/run-system-tests) | Running the pytest system test harness (marks, MetricsRecorder, /pytest PR trigger) | | [add-behavior-tree-node](.agents/skills/add-behavior-tree-node) | Creating behavior tree nodes | | [use-airstack-cli](.agents/skills/use-airstack-cli) | Using the `airstack` CLI and the non-interactive `docker exec` pattern | @@ -196,13 +196,13 @@ docker exec airstack-robot-desktop-1 bash -c "ros2 topic echo --onc - Verify module behavior in isolation - Test with synthetic data - Located in module's `test/` directory - - **Run in the robot container** with `colcon test` (after `bws`) for the full ROS 2 build + test. The same co-located test source is re-exported to the root [`tests/`](tests/) suite via thin proxies (see Unit tests below), so `airstack test -m unit` runs it too. Marks are declared in [`tests/pytest.ini`](tests/pytest.ini) (`unit`, `build_docker`, `build_packages`, `liveliness`, `sensors`, `takeoff_hover_land`, `autonomy`). + - **Run in the robot container** with `colcon test` (after `bws`) for the full ROS 2 build + test. The same co-located test source is collected by the root [`tests/`](tests/) suite (the packages with unit tests are listed in [`tests/colcon_unit_test_packages.yaml`](tests/colcon_unit_test_packages.yaml)), so `airstack test -m unit` runs it too. Marks are declared in [`tests/pytest.ini`](tests/pytest.ini) (`unit`, `build_docker`, `build_packages`, `integration`, `liveliness`, `sensors`, `takeoff_hover_land`, `autonomy`). ```bash docker exec airstack-robot-desktop-1 bash -c "sws && colcon test --packages-select natnet_ros2 --event-handlers console_direct+" ``` -2. **Unit tests (`pytest`, `unit` mark):** Fast, hermetic checks. Test **source** lives co-located with each ROS 2 package in `/test/` (standard colcon convention). Thin **proxy** files in [`tests/robot/`](tests/robot/) and [`tests/sim/`](tests/sim/) re-export those tests so `pytest tests/` discovers them. Unit tests run as part of the `system-tests.yml` suite. Example: `airstack test -m unit -v`. See `add-unit-tests` skill. +2. **Unit tests (`pytest`, `unit` mark):** Fast, hermetic checks. Test **source** lives co-located with each ROS 2 package in `/test/` (standard colcon convention). [`tests/colcon_unit_test_packages.yaml`](tests/colcon_unit_test_packages.yaml) lists which packages have unit tests, and `tests/conftest.py` collects them from there under `--import-mode=importlib`. To add a package's unit tests, list it in that YAML. Unit tests run as part of the `system-tests.yml` suite. Example: `airstack test -m unit -v`. See `add-unit-tests` skill. 3. **System Level (`tests/system/`):** Full simulation tests (Isaac Sim or Microsoft AirSim legacy) - End-to-end autonomy stack testing diff --git a/docs/development/intermediate/testing/index.md b/docs/development/intermediate/testing/index.md index e6ce09c0b..c23195477 100644 --- a/docs/development/intermediate/testing/index.md +++ b/docs/development/intermediate/testing/index.md @@ -12,8 +12,9 @@ hardware requirement: ## Unit tests (`pytest -m unit`) Fast, hermetic Python tests that run in seconds with no Docker or GPU. Test source -lives **co-located with its ROS 2 package** (`/test/`) and is re-exported -through thin proxy files in `tests/robot/` for centralized discovery. +lives **co-located with its ROS 2 package** (`/test/`); the packages with unit +tests are listed in `tests/colcon_unit_test_packages.yaml`, and `pytest tests/` collects +them from there. ```bash airstack test -m unit -v @@ -24,7 +25,7 @@ pytest tests/ -m unit -v Unit tests run as part of `system-tests.yml` via `pytest tests/` and can also be run locally with no Docker or GPU needed. -→ **[Unit Testing Guide](unit_testing.md)** — patterns, proxy layout, CI workflow, +→ **[Unit Testing Guide](unit_testing.md)** — patterns, CI workflow, how to add tests for new packages (Python and C++ gtest). ## System tests (`tests/system/`) @@ -83,7 +84,7 @@ airstack test -m "build_packages or autonomy" \ ## Other testing docs -- [Unit Testing](unit_testing.md) — `@pytest.mark.unit`, proxy pattern, CI workflow +- [Unit Testing](unit_testing.md) — `@pytest.mark.unit`, co-located tests, CI workflow - [Testing frameworks](testing_frameworks.md) — `colcon test`, rostest patterns - [Integration testing](integration_testing.md) - [CI/CD](ci_cd.md) — pipeline overview diff --git a/docs/development/intermediate/testing/unit_testing.md b/docs/development/intermediate/testing/unit_testing.md index f69056008..1b7919c14 100644 --- a/docs/development/intermediate/testing/unit_testing.md +++ b/docs/development/intermediate/testing/unit_testing.md @@ -5,8 +5,8 @@ AirStack unit tests are **fast, hermetic, and purely Python** — no Docker stac ## Design principles - **Co-located with source.** Test files live in `/test/` alongside the code they test. This is the standard ROS 2 / colcon convention and ensures tests are discovered by both `colcon test` and `pytest`. -- **Proxy for centralized discovery.** A thin shim in `tests/robot///` re-exports the test functions so `pytest tests/` (the CI command) and `airstack test -m unit` discover them without any changes to the CI workflow. -- **`@pytest.mark.unit` on every test.** The `unit` mark is the filter that keeps unit tests isolated from system tests that need Docker, GPUs, and sim licenses. +- **Listed in one place.** `tests/colcon_unit_test_packages.yaml` lists which packages have unit tests. `tests/conftest.py` resolves each to its `test/` dir and collects the non-linter `test_*.py` files under `--import-mode=importlib`. To add a package's unit tests, list it in that YAML. +- **`@pytest.mark.unit` on every test.** Auto-applied by path in `conftest.py` (source files may also declare it). The `unit` mark keeps unit tests isolated from system tests that need Docker, GPUs, and sim licenses. ## Repository layout @@ -15,24 +15,18 @@ robot/ros_ws/src/ └── // ├── src/ # production source └── test/ - ├── test_.py # unit test source ← canonical location + ├── test_.py # unit test source ← canonical location (collected directly) ├── test_.cpp # C++ gtest source (optional) └── fake_.hpp # C++ test doubles (optional) tests/ -├── robot/ -│ └── // -│ └── test_.py # thin proxy → package test/ -├── sim/ # future: sim-side unit tests -└── gcs/ # future: GCS unit tests +└── colcon_unit_test_packages.yaml # lists the packages whose test/ dirs are collected ``` -When pytest collects `tests/robot/…/test_.py`, the `<-` annotation in the -output shows the actual source location: +Collected items point straight at the co-located source: ``` -robot/perception/natnet_ros2/test_natnet_ros2.py::test_canonical_quaternion_identity - <- ../robot/ros_ws/src/perception/natnet_ros2/test/test_natnet_ros2.py PASSED +../robot/ros_ws/src/perception/natnet_ros2/test/test_natnet_ros2.py::test_canonical_quaternion_identity PASSED ``` ## Running unit tests @@ -117,29 +111,19 @@ sys.modules["rclpy.node"] = _rclpy_node_mod # ... then import your module ``` -**2. Write the thin proxy in `tests/robot/`:** +**2. Register the package in `tests/colcon_unit_test_packages.yaml`:** -```python -# tests/robot///test_my_module.py -"""Proxy: re-exposes unit tests from the package source tree.""" -import importlib.util -from pathlib import Path - -_repo_root = Path(__file__).resolve().parents[4] # adjust depth if needed -_real_file = _repo_root / "robot/ros_ws/src///test/test_my_module.py" - -_spec = importlib.util.spec_from_file_location("__unit_tests", _real_file) -_real = importlib.util.module_from_spec(_spec) -_spec.loader.exec_module(_real) - -for _name in dir(_real): - if _name.startswith("test_"): - globals()[_name] = getattr(_real, _name) +```yaml +robot: + packages: + - # ← add here; conftest.py collects /test/test_*.py + pytest_args: "-m not linter" ``` -The unique module name (e.g. `"__unit_tests"`) prevents a circular import: -pytest adds the proxy's directory to `sys.path` at collection time, which would -cause `from test_my_module import *` to import the proxy itself. +That's the whole registration. If the test imports package code, set up `sys.path` at the +top of the test file — see `test_validation_core.py`, which inserts its package root. +`--import-mode=importlib` (set in `pytest.ini`) means duplicate `test_*.py` basenames +across packages don't collide. **3. Verify:** @@ -149,7 +133,7 @@ pytest tests/ -m unit -v ### C++ (gtest) -C++ tests live entirely in the package and run via `colcon test` — no proxy needed. +C++ tests live entirely in the package and run via `colcon test`. **`CMakeLists.txt`:** @@ -183,16 +167,16 @@ The `build_packages` CI job (`tests/system/test_build_packages.py`) also runs ## Extending to sim and GCS -The proxy pattern extends to other components. As sim-side Python logic (e.g. the -[Motive emulator](../../../../tests/sim/motive_emulator/README.md)) and GCS modules -acquire unit-testable code, follow the same layout: - -``` -simulation/...//test/test_.py ← source -tests/sim//test_.py ← proxy (parents[3] to reach repo root) +The mechanism extends to other components via the same YAML. `conftest.py` +(`_WORKSPACE_PKG_TEST_GLOBS`) maps each workspace key to a source glob — `robot` → +`robot/ros_ws/src/**//test`, `sim` → `simulation/**//test`. Add a `sim:` (or a +new `gcs:`) workspace to the YAML, adding the glob for a new tree in `conftest.py`: -gcs/...//test/test_.py ← source -tests/gcs//test_.py ← proxy +```yaml +# tests/colcon_unit_test_packages.yaml +sim: + packages: + - # → simulation/**//test collected directly ``` `pytest tests/ -m unit` discovers them automatically — no changes to `pytest.ini` From 21e28f7380570c88a9c8416b6a2f44127e984244 Mon Sep 17 00:00:00 2001 From: John Date: Wed, 22 Jul 2026 11:29:53 -0400 Subject: [PATCH 07/12] chore: bump version to 0.19.0-alpha.8 Version-increment gate: bump above develop (0.19.0-alpha.6); alpha.7 is taken by the l4t-deployment-fix PR. Record the test-infra changes in the changelog. Co-Authored-By: Claude Opus 4.8 --- .env | 2 +- CHANGELOG.md | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.env b/.env index 85280be19..00cd4c639 100644 --- a/.env +++ b/.env @@ -12,7 +12,7 @@ PROJECT_NAME="airstack" # If you've run ./airstack.sh setup, then this will auto-generate from the git commit hash every time a change is made # to a Dockerfile or docker-compose.yaml file. Otherwise this can also be set explicitly to make a release version. # auto-generated from git commit hash -VERSION="0.19.0-alpha.7" +VERSION="0.19.0-alpha.8" # Choose "dev" or "prebuilt". "dev" is for mounted code that must be built live. "prebuilt" is for built ros_ws baked into the image DOCKER_IMAGE_BUILD_MODE="dev" # Where to push and pull images from. Can replace with your docker hub username if using docker hub. diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bde19e49..a5ea32f38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,11 +13,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `TARGET_ARCH` build arg (default `x86_64`) in `Dockerfile.robot` to arch-parametrize `LD_LIBRARY_PATH`; `docker-compose.yaml` passes `TARGET_ARCH: aarch64` to the `voxl` and `l4t` real-robot image builds - `ros-${ROS_DISTRO}-mavros-extras` in the robot image (provides the vision_pose plugin used for external-pose deployments) - `overrides/l4t-px4-realrobot.env` — site-agnostic deployment override for a single real PX4 robot on a Jetson (aarch64/l4t) +- `integration` test tier (`tests/integration/`, `integration` mark) with a shared `robot_autonomy_stack` fixture (robot container, no sim/GPU) ### Changed - `robot-l4t` compose service knobs are now env-overridable (`AUTONOMY_ROLE`, `FCU_URL`, and the rosbag path via `BAG_STORAGE_PATH`); `FCU_URL` unquoted so the literal serial path reaches MAVROS - `zed-l4t` image: ZED SDK 4.2 → 5.2 with the coupled ROS deps (`zed_msgs` 5.2.1, `point_cloud_transport(_plugins)` 4.x, add `backward_ros`) +- Unit tests are defined by `tests/colcon_unit_test_packages.yaml`: `conftest.py` collects each listed package's co-located `test/` dir under `--import-mode=importlib` and marks it `unit` (ament lint files are skipped and run under `colcon test`) ### Fixed From ef3492fced09f898dbf85eebed844d74dd6045cc Mon Sep 17 00:00:00 2001 From: John Date: Wed, 22 Jul 2026 11:55:23 -0400 Subject: [PATCH 08/12] refactor(tests): split unit-test discovery + session state into tests/harness/ Begin modularizing conftest.py (959 lines) by concern. Extract two self-contained pieces into a new tests/harness/ package: - harness/session.py: session-scoped mutable state (results dir, current pytest item, last subprocess output, logger) with setter/getter accessors. Hooks write it; helpers read it, so helper modules no longer reach into conftest globals. - harness/discovery.py: unit-test discovery driven by colcon_unit_test_packages.yaml (repo_path, load_colcon_unit_test_config, colcon_test_robot_command, unit_test_dirs, unit_test_files, _is_unit_item). conftest.py imports from harness and its hooks delegate to the session accessors; it re-exports AIRSTACK_ROOT / colcon_test_robot_command / load_colcon_unit_test_config / logger so existing `from conftest import ...` in the system tests keeps working unchanged. Behavior-preserving (host-validated): `-m unit` still 14 passed / 152 deselected, 166 collected, same order. Follow-on: the commands/containers/metrics/sim helpers and collection ordering move out the same way. Co-Authored-By: Claude Opus 4.8 --- tests/conftest.py | 195 +++++++------------------------------ tests/harness/__init__.py | 26 +++++ tests/harness/discovery.py | 125 ++++++++++++++++++++++++ tests/harness/session.py | 59 +++++++++++ 4 files changed, 246 insertions(+), 159 deletions(-) create mode 100644 tests/harness/__init__.py create mode 100644 tests/harness/discovery.py create mode 100644 tests/harness/session.py diff --git a/tests/conftest.py b/tests/conftest.py index 38d46411f..564947680 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,4 @@ import json -import logging import os import re import shlex @@ -9,11 +8,27 @@ import time from concurrent.futures import ThreadPoolExecutor, as_completed from contextlib import contextmanager -from datetime import datetime from pathlib import Path import pytest -import yaml + +# Make tests/ importable so `from harness import ...` (and `from run_summary import ...`) +# resolve regardless of pytest's import mode. +_TESTS_DIR = str(Path(__file__).resolve().parent) +if _TESTS_DIR not in sys.path: + sys.path.insert(0, _TESTS_DIR) + +from harness import session +# AIRSTACK_ROOT, colcon_test_robot_command, load_colcon_unit_test_config and logger are +# imported here so existing `from conftest import ` in the system tests keeps working. +from harness.session import logger +from harness.discovery import ( + AIRSTACK_ROOT, + colcon_test_robot_command, + load_colcon_unit_test_config, + unit_test_files, + _is_unit_item, +) SIM_CONFIG = { "msairsim": { @@ -45,142 +60,10 @@ }, } -AIRSTACK_ROOT = os.environ.get("AIRSTACK_ROOT", str(Path(__file__).parent.parent)) -COLCON_UNIT_TEST_PACKAGES_YAML = ( - Path(AIRSTACK_ROOT) / "tests" / "colcon_unit_test_packages.yaml" -) - - -def load_colcon_unit_test_config(workspace="robot"): - """Load colcon test package list and pytest args from tests/colcon_unit_test_packages.yaml.""" - if not COLCON_UNIT_TEST_PACKAGES_YAML.is_file(): - raise FileNotFoundError( - f"Missing {COLCON_UNIT_TEST_PACKAGES_YAML} — add packages to gate in colcon test." - ) - with COLCON_UNIT_TEST_PACKAGES_YAML.open(encoding="utf-8") as f: - data = yaml.safe_load(f) or {} - if workspace not in data: - raise KeyError( - f"No '{workspace}' entry in {COLCON_UNIT_TEST_PACKAGES_YAML.name}" - ) - cfg = data[workspace] or {} - packages = cfg.get("packages") or [] - if not packages: - raise ValueError( - f"'{workspace}.packages' is empty in {COLCON_UNIT_TEST_PACKAGES_YAML.name}" - ) - return packages, cfg.get("pytest_args", "") - - -def colcon_test_robot_command(workspace="robot"): - """Shell command for colcon test over unit-test packages (robot workspace).""" - packages, pytest_args = load_colcon_unit_test_config(workspace) - pkg_list = " ".join(packages) - cmd = ( - f"colcon test --packages-select {pkg_list} " - "--event-handlers console_direct+ --return-code-on-test-failure" - ) - if pytest_args: - cmd += f' --pytest-args "{pytest_args}"' - return cmd - - -def repo_path(*parts: str) -> Path: - """Resolve a path relative to the repo root (``AIRSTACK_ROOT``). - - Single source of truth for cross-tree paths — no test or hook should hardcode - ``Path(__file__).parents[N]`` walks. - """ - return Path(AIRSTACK_ROOT).joinpath(*parts) - - -# Unit-test SOURCE lives co-located with each package (colcon convention). -# colcon_unit_test_packages.yaml lists which packages have unit tests; every listed -# package resolves to its /test dir (globbed per workspace) and the non-linter -# test_*.py files there are collected under --import-mode=importlib (set in pytest.ini). -# See unit_test_files (what gets collected), pytest_configure (adds them to the run), -# and pytest_itemcollected (auto `unit` mark). ament lint tests are excluded (they run -# under colcon test). -_WORKSPACE_PKG_TEST_GLOBS = { - "robot": "robot/ros_ws/src/**/{pkg}/test", - "sim": "simulation/**/{pkg}/test", -} - -# ament lint tests ship in every ROS package's test/ dir and import ament_* at -# module load (unavailable outside the built workspace). Skip them here — they run -# under `colcon test` instead (see colcon_test_robot_command's `-m not linter`). -_LINTER_TEST_FILENAMES = { - "test_copyright.py", "test_flake8.py", "test_pep257.py", - "test_pep8.py", "test_xmllint.py", "test_lint_cmake.py", -} - - -def unit_test_dirs(): - """Every co-located unit-test dir resolved from colcon_unit_test_packages.yaml.""" - if not COLCON_UNIT_TEST_PACKAGES_YAML.is_file(): - return [] - with COLCON_UNIT_TEST_PACKAGES_YAML.open(encoding="utf-8") as f: - data = yaml.safe_load(f) or {} - dirs = [] - for workspace, glob_tmpl in _WORKSPACE_PKG_TEST_GLOBS.items(): - cfg = data.get(workspace) or {} - for pkg in cfg.get("packages") or []: - for match in repo_path().glob(glob_tmpl.format(pkg=pkg)): - if match.is_dir(): - dirs.append(match.resolve()) - return dirs - - -_UNIT_TEST_DIRS = None - - -def _unit_test_dirs_cached(): - global _UNIT_TEST_DIRS - if _UNIT_TEST_DIRS is None: - _UNIT_TEST_DIRS = unit_test_dirs() - return _UNIT_TEST_DIRS - - -def _is_unit_item(item): - """True when a collected item's file lives under a co-located unit-test dir.""" - try: - p = Path(str(item.path)).resolve() - except Exception: - return False - return any(p.is_relative_to(d) for d in _unit_test_dirs_cached()) - - -def unit_test_files(): - """Co-located unit-test files to collect: every ``test_*.py`` under a package - ``test/`` dir, minus the ament lint files. - - We inject explicit files (not the dirs) because pytest does not apply ignore - rules to files it recurses into from an explicitly-passed directory — passing - the exact files is the only deterministic way to keep ament lint tests out. - """ - files = [] - for d in _unit_test_dirs_cached(): - for f in sorted(d.glob("test_*.py")): - if f.name not in _LINTER_TEST_FILENAMES: - files.append(f) - return files - - -RUN_DIR = None ROS_DISTRO_SETUP = "/opt/ros/jazzy/setup.bash" -_LAST_CMD_OUTPUT: dict[str, str] = {} -_DEFAULT_LOG_KEY = "_last" - -# Track the currently-running pytest item so current_log() and current_test_id() -# can pick up the parametrize id without tests having to pass `request` around. -_CURRENT_ITEM = None METRICS = None -logger = logging.getLogger("airstack") -logger.setLevel(logging.INFO) - - # ── pytest config / hooks ────────────────────────────────────────────────── def pytest_addoption(parser): @@ -206,12 +89,8 @@ def pytest_addoption(parser): def pytest_configure(config): - global RUN_DIR - timestamp = datetime.now().strftime("%Y-%m-%d_%H-%M-%S") - results_root = Path(AIRSTACK_ROOT) / "tests" / "results" - RUN_DIR = results_root / timestamp - RUN_DIR.mkdir(parents=True, exist_ok=True) - config.option.xmlpath = str(RUN_DIR / "results.xml") + run_dir = session.init_run_dir(AIRSTACK_ROOT) + config.option.xmlpath = str(run_dir / "results.xml") # Collect co-located unit tests: their files live outside tests/, so add the # explicit non-linter test files to the collection args. Skip when an explicit @@ -235,22 +114,21 @@ def pytest_itemcollected(item): def pytest_runtest_setup(item): - global _CURRENT_ITEM - _CURRENT_ITEM = item + session.set_current_item(item) def pytest_runtest_teardown(item): - global _CURRENT_ITEM - _CURRENT_ITEM = None + session.set_current_item(None) -def pytest_sessionfinish(session, exitstatus): +def pytest_sessionfinish(exitstatus): """Write summary.txt with key metrics so users don't need to dig through logs.""" - if RUN_DIR is None: + run_dir = session.run_dir() + if run_dir is None: return try: from run_summary import write_summary - summary_path = write_summary(RUN_DIR) + summary_path = write_summary(run_dir) logger.info("Wrote run summary to %s", summary_path) except Exception as exc: logger.warning("Failed to write run summary: %s", exc) @@ -418,15 +296,15 @@ def current_log(): Subprocess helpers default to this so every call fired from a test auto-logs to the right file without plumbing log_name through every layer.""" - if _CURRENT_ITEM is None: + item = session.current_item() + if item is None: return None - return _nodeid_dotted(_CURRENT_ITEM.nodeid, with_path_sep=True) + return _nodeid_dotted(item.nodeid, with_path_sep=True) def read_log_tail(log_name=None, lines=50): """Return the tail of the most recent subprocess output for this context.""" - key = log_name or _DEFAULT_LOG_KEY - text = _LAST_CMD_OUTPUT.get(key) or _LAST_CMD_OUTPUT.get(_DEFAULT_LOG_KEY, "") + text = session.last_cmd_output(log_name) if not text: return "" return "\n".join(text.splitlines()[-lines:]) @@ -440,9 +318,7 @@ def _run_teed(cmd_list, timeout, log_name=None, env=None, cwd=None): cmd_list, capture_output=True, text=True, timeout=timeout, env=env, cwd=cwd, ) combined = (result.stdout or "") + (result.stderr or "") - key = log_name or _DEFAULT_LOG_KEY - _LAST_CMD_OUTPUT[key] = combined - _LAST_CMD_OUTPUT[_DEFAULT_LOG_KEY] = combined + session.record_cmd_output(combined, log_name) return result @@ -673,16 +549,17 @@ def record_list(self, test_name, key, values): def get_metrics(): global METRICS if METRICS is None: - METRICS = MetricsRecorder(RUN_DIR / "metrics.json") + METRICS = MetricsRecorder(session.run_dir() / "metrics.json") return METRICS def current_test_id(): """Test id used as the metrics.json key. Matches JUnit XML's classname.name format so parse_metrics.py can merge results.xml and metrics.json entries.""" - if _CURRENT_ITEM is None: + item = session.current_item() + if item is None: return "unknown" - return _nodeid_dotted(_CURRENT_ITEM.nodeid) + return _nodeid_dotted(item.nodeid) # ── shared sim test infrastructure (liveliness, sensors, comms, takeoff reuse) ── @@ -851,7 +728,7 @@ def airstack_env(request): # test id (see pytest_collection_modifyitems), so airstack up/down output # lands next to the triggering test's own log instead of under pytest's # stale callspec.id. - log = f"airstack_env.{_nodeid_dotted(_CURRENT_ITEM.nodeid, with_path_sep=True)}" + log = f"airstack_env.{_nodeid_dotted(session.current_item().nodeid, with_path_sep=True)}" headless = not request.config.getoption("--gui") env_overrides = { diff --git a/tests/harness/__init__.py b/tests/harness/__init__.py new file mode 100644 index 000000000..c08975c72 --- /dev/null +++ b/tests/harness/__init__.py @@ -0,0 +1,26 @@ +"""AirStack test harness — helpers split out of conftest.py by concern. + +conftest.py holds the pytest hooks and fixtures and imports what it needs from here. +Public helpers are re-exported so tests and conftest can ``from harness import ``. +""" +from harness.discovery import ( + AIRSTACK_ROOT, + COLCON_UNIT_TEST_PACKAGES_YAML, + colcon_test_robot_command, + load_colcon_unit_test_config, + repo_path, + unit_test_dirs, + unit_test_files, +) +from harness.session import logger + +__all__ = [ + "AIRSTACK_ROOT", + "COLCON_UNIT_TEST_PACKAGES_YAML", + "colcon_test_robot_command", + "load_colcon_unit_test_config", + "repo_path", + "unit_test_dirs", + "unit_test_files", + "logger", +] diff --git a/tests/harness/discovery.py b/tests/harness/discovery.py new file mode 100644 index 000000000..c0178a92d --- /dev/null +++ b/tests/harness/discovery.py @@ -0,0 +1,125 @@ +"""Unit-test discovery: which packages have unit tests and where their files live. + +Driven by ``tests/colcon_unit_test_packages.yaml``. ``conftest.pytest_configure`` adds +``unit_test_files()`` to the pytest run, and ``pytest_itemcollected`` marks each of those +items ``unit`` via ``_is_unit_item``. ament lint tests are excluded here — they run under +``colcon test`` (see ``colcon_test_robot_command``'s ``-m not linter``). +""" +import os +from pathlib import Path + +import yaml + +AIRSTACK_ROOT = os.environ.get("AIRSTACK_ROOT", str(Path(__file__).resolve().parents[2])) +COLCON_UNIT_TEST_PACKAGES_YAML = ( + Path(AIRSTACK_ROOT) / "tests" / "colcon_unit_test_packages.yaml" +) + + +def repo_path(*parts: str) -> Path: + """Resolve a path relative to the repo root (``AIRSTACK_ROOT``). + + Single source of truth for cross-tree paths — no test or hook should hardcode + ``Path(__file__).parents[N]`` walks. + """ + return Path(AIRSTACK_ROOT).joinpath(*parts) + + +def load_colcon_unit_test_config(workspace="robot"): + """Load colcon test package list and pytest args from tests/colcon_unit_test_packages.yaml.""" + if not COLCON_UNIT_TEST_PACKAGES_YAML.is_file(): + raise FileNotFoundError( + f"Missing {COLCON_UNIT_TEST_PACKAGES_YAML} — add packages to gate in colcon test." + ) + with COLCON_UNIT_TEST_PACKAGES_YAML.open(encoding="utf-8") as f: + data = yaml.safe_load(f) or {} + if workspace not in data: + raise KeyError( + f"No '{workspace}' entry in {COLCON_UNIT_TEST_PACKAGES_YAML.name}" + ) + cfg = data[workspace] or {} + packages = cfg.get("packages") or [] + if not packages: + raise ValueError( + f"'{workspace}.packages' is empty in {COLCON_UNIT_TEST_PACKAGES_YAML.name}" + ) + return packages, cfg.get("pytest_args", "") + + +def colcon_test_robot_command(workspace="robot"): + """Shell command for colcon test over unit-test packages (robot workspace).""" + packages, pytest_args = load_colcon_unit_test_config(workspace) + pkg_list = " ".join(packages) + cmd = ( + f"colcon test --packages-select {pkg_list} " + "--event-handlers console_direct+ --return-code-on-test-failure" + ) + if pytest_args: + cmd += f' --pytest-args "{pytest_args}"' + return cmd + + +# Each listed package resolves to its /test dir via these per-workspace globs. +_WORKSPACE_PKG_TEST_GLOBS = { + "robot": "robot/ros_ws/src/**/{pkg}/test", + "sim": "simulation/**/{pkg}/test", +} + +# ament lint tests ship in every ROS package's test/ dir and import ament_* at +# module load (unavailable outside the built workspace). Skip them here — they run +# under `colcon test` instead (see colcon_test_robot_command's `-m not linter`). +_LINTER_TEST_FILENAMES = { + "test_copyright.py", "test_flake8.py", "test_pep257.py", + "test_pep8.py", "test_xmllint.py", "test_lint_cmake.py", +} + + +def unit_test_dirs(): + """Every co-located unit-test dir resolved from colcon_unit_test_packages.yaml.""" + if not COLCON_UNIT_TEST_PACKAGES_YAML.is_file(): + return [] + with COLCON_UNIT_TEST_PACKAGES_YAML.open(encoding="utf-8") as f: + data = yaml.safe_load(f) or {} + dirs = [] + for workspace, glob_tmpl in _WORKSPACE_PKG_TEST_GLOBS.items(): + cfg = data.get(workspace) or {} + for pkg in cfg.get("packages") or []: + for match in repo_path().glob(glob_tmpl.format(pkg=pkg)): + if match.is_dir(): + dirs.append(match.resolve()) + return dirs + + +_UNIT_TEST_DIRS = None + + +def _unit_test_dirs_cached(): + global _UNIT_TEST_DIRS + if _UNIT_TEST_DIRS is None: + _UNIT_TEST_DIRS = unit_test_dirs() + return _UNIT_TEST_DIRS + + +def _is_unit_item(item): + """True when a collected item's file lives under a co-located unit-test dir.""" + try: + p = Path(str(item.path)).resolve() + except Exception: + return False + return any(p.is_relative_to(d) for d in _unit_test_dirs_cached()) + + +def unit_test_files(): + """Co-located unit-test files to collect: every ``test_*.py`` under a package + ``test/`` dir, minus the ament lint files. + + We collect explicit files (not the dirs) because pytest does not apply ignore + rules to files it recurses into from an explicitly-passed directory — passing + the exact files is the only deterministic way to keep ament lint tests out. + """ + files = [] + for d in _unit_test_dirs_cached(): + for f in sorted(d.glob("test_*.py")): + if f.name not in _LINTER_TEST_FILENAMES: + files.append(f) + return files diff --git a/tests/harness/session.py b/tests/harness/session.py new file mode 100644 index 000000000..54277dacc --- /dev/null +++ b/tests/harness/session.py @@ -0,0 +1,59 @@ +"""Session-scoped mutable state shared between conftest hooks and harness helpers. + +pytest hooks (in conftest.py) *write* this state — ``init_run_dir`` in +``pytest_configure``, ``set_current_item`` in ``pytest_runtest_setup``/``teardown``, +``record_cmd_output`` from the subprocess helpers. Helpers *read* it — ``run_dir``, +``current_item``, ``last_cmd_output``. Keeping it here means helper modules never reach +back into conftest globals. +""" +import logging +from datetime import datetime +from pathlib import Path + +# Shared logger used across the harness and the tests. +logger = logging.getLogger("airstack") +logger.setLevel(logging.INFO) + +_DEFAULT_LOG_KEY = "_last" + +_run_dir = None +_current_item = None +_last_cmd_output: dict[str, str] = {} + + +def init_run_dir(airstack_root) -> Path: + """Create and record this session's timestamped results dir; return it.""" + global _run_dir + timestamp = datetime.now().strftime("%Y-%m-%d_%H-%M-%S") + _run_dir = Path(airstack_root) / "tests" / "results" / timestamp + _run_dir.mkdir(parents=True, exist_ok=True) + return _run_dir + + +def run_dir(): + """This session's results dir (``None`` before ``pytest_configure`` runs).""" + return _run_dir + + +def set_current_item(item): + """Record the currently-running pytest item (``None`` between tests).""" + global _current_item + _current_item = item + + +def current_item(): + """The currently-running pytest item, or ``None`` outside a test.""" + return _current_item + + +def record_cmd_output(text, log_name=None): + """Store the latest subprocess output, keyed by ``log_name`` and as the default.""" + key = log_name or _DEFAULT_LOG_KEY + _last_cmd_output[key] = text + _last_cmd_output[_DEFAULT_LOG_KEY] = text + + +def last_cmd_output(log_name=None) -> str: + """The most recent subprocess output for ``log_name`` (or the default).""" + key = log_name or _DEFAULT_LOG_KEY + return _last_cmd_output.get(key) or _last_cmd_output.get(_DEFAULT_LOG_KEY, "") From 24919d5db6a532034d023950ffdb9a2dd3666b63 Mon Sep 17 00:00:00 2001 From: John Date: Wed, 22 Jul 2026 12:41:22 -0400 Subject: [PATCH 09/12] refactor(tests): extract commands/containers/metrics/sim helpers into tests/harness/ Continue modularizing conftest.py. Move the subprocess/ros2 command helpers (harness/commands.py), docker container + compute-usage + image helpers (harness/containers.py), MetricsRecorder + get_metrics/current_test_id (harness/metrics.py), and the sim target configs + ros2 topic sampling (harness/sim.py) out of conftest.py. conftest.py drops from 836 to 360 lines and re-exports the harness helper API (`from harness import *`) so `from conftest import ` in the system tests + sensor_probes keeps working unchanged. Behavior-preserving: -m unit still 14 passed / 152 deselected, 166 collected, same order. Remaining in conftest: pytest hooks, collection ordering, and the airstack_env / robot_autonomy_stack fixtures. Co-Authored-By: Claude Opus 4.8 --- tests/conftest.py | 486 +----------------------------------- tests/harness/__init__.py | 51 +++- tests/harness/commands.py | 92 +++++++ tests/harness/containers.py | 164 ++++++++++++ tests/harness/metrics.py | 55 ++++ tests/harness/sim.py | 189 ++++++++++++++ 6 files changed, 549 insertions(+), 488 deletions(-) create mode 100644 tests/harness/commands.py create mode 100644 tests/harness/containers.py create mode 100644 tests/harness/metrics.py create mode 100644 tests/harness/sim.py diff --git a/tests/conftest.py b/tests/conftest.py index 564947680..d67f70433 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,12 +1,5 @@ -import json -import os -import re -import shlex -import subprocess import sys -import threading import time -from concurrent.futures import ThreadPoolExecutor, as_completed from contextlib import contextmanager from pathlib import Path @@ -19,50 +12,11 @@ sys.path.insert(0, _TESTS_DIR) from harness import session -# AIRSTACK_ROOT, colcon_test_robot_command, load_colcon_unit_test_config and logger are -# imported here so existing `from conftest import ` in the system tests keeps working. -from harness.session import logger -from harness.discovery import ( - AIRSTACK_ROOT, - colcon_test_robot_command, - load_colcon_unit_test_config, - unit_test_files, - _is_unit_item, -) - -SIM_CONFIG = { - "msairsim": { - "profile": "ms-airsim", - "sim_container": "ms-airsim", - "sim_setup_bash": "/root/ros_ws/install/setup.bash", - "robot_setup_bash": "/root/AirStack/robot/ros_ws/install/setup.bash", - "extra_env": { - "URDF_FILE": "robot_descriptions/iris/urdf/iris_stereo.ms-airsim.urdf", - # Clear any user-set paths in .env so entrypoint auto-fetches Blocks. - # Shell env wins over --env-file in docker compose substitution. - "MS_AIRSIM_ENV_DIR": "", - "MS_AIRSIM_BINARY_PATH": "", - }, - }, - "isaacsim": { - "profile": "isaac-sim", - "sim_container": "isaac-sim", - "sim_setup_bash": "/opt/ros/jazzy/setup.bash", - "robot_setup_bash": "/root/AirStack/robot/ros_ws/install/setup.bash", - "extra_env": { - "ISAAC_SIM_USE_STANDALONE": "true", - "ISAAC_SIM_SCRIPT_NAME": "example_multi_px4_pegasus_launch_script.py", - "PLAY_SIM_ON_START": "true", - # Multi script gates RTX LiDAR on this flag; example_one always spawns it. - # `sensors` tests expect ouster topics + lidar_point_cloud_filter path. - "ENABLE_LIDAR": "true", - }, - }, -} - -ROS_DISTRO_SETUP = "/opt/ros/jazzy/setup.bash" -METRICS = None - +# Re-export the harness helper API so existing `from conftest import ` in the +# system tests + sensor_probes keeps working unchanged. +from harness import * # noqa: F401,F403 +from harness.commands import _nodeid_dotted +from harness.discovery import _is_unit_item # ── pytest config / hooks ────────────────────────────────────────────────── @@ -282,436 +236,6 @@ def _sort_key(item, _mod=mod_name): item._nodeid = item._nodeid.replace(f"[{cs.id}]", f"[{new_id}]") -# ── logging / subprocess helpers ─────────────────────────────────────────── - -def _nodeid_dotted(nodeid, with_path_sep=False): - """pytest nodeid → `module.Class.test_name[params]` form. When - `with_path_sep=True`, also flattens `/` in path prefixes (for log filenames).""" - out = nodeid.replace(".py::", ".").replace("::", ".") - return out.replace("/", ".") if with_path_sep else out - - -def current_log(): - """Log name for the currently-running pytest item, or None outside a test. - - Subprocess helpers default to this so every call fired from a test auto-logs - to the right file without plumbing log_name through every layer.""" - item = session.current_item() - if item is None: - return None - return _nodeid_dotted(item.nodeid, with_path_sep=True) - - -def read_log_tail(log_name=None, lines=50): - """Return the tail of the most recent subprocess output for this context.""" - text = session.last_cmd_output(log_name) - if not text: - return "" - return "\n".join(text.splitlines()[-lines:]) - - -def _run_teed(cmd_list, timeout, log_name=None, env=None, cwd=None): - """Run a subprocess and capture stdout+stderr for parsing and failure messages.""" - quoted = " ".join(shlex.quote(a) for a in cmd_list) - logger.info("$ %s", quoted) - result = subprocess.run( - cmd_list, capture_output=True, text=True, timeout=timeout, env=env, cwd=cwd, - ) - combined = (result.stdout or "") + (result.stderr or "") - session.record_cmd_output(combined, log_name) - return result - - -def docker_exec(container, cmd, timeout=60, log_name=None): - full_cmd = ["docker", "exec", container, "bash", "-c", cmd] - return _run_teed(full_cmd, timeout=timeout, log_name=log_name) - - -def airstack_cmd(*args, env_overrides=None, timeout=1800, log_name=None): - env = os.environ.copy() - if env_overrides: - env.update(env_overrides) - cmd = [str(Path(AIRSTACK_ROOT) / "airstack.sh")] + list(args) - return _run_teed(cmd, timeout=timeout, log_name=log_name, - env=env, cwd=AIRSTACK_ROOT) - - -def ros2_env(setup_bash, domain_id): - """Shell prefix that makes `ros2` available on the requested domain.""" - return ( - f"source {ROS_DISTRO_SETUP} && source {setup_bash} " - f"&& export ROS_DOMAIN_ID={domain_id}" - ) - - -def ros2_exec(container, ros2_cmd, domain_id=0, setup_bash=None, timeout=15, log_name=None): - """Run `ros2 ...` inside a container with the right workspace sourced.""" - setup = setup_bash or "/root/AirStack/robot/ros_ws/install/setup.bash" - inner = f"{ros2_env(setup, domain_id)} && {ros2_cmd}" - return docker_exec(container, inner, timeout=timeout, log_name=log_name) - - -_HZ_RE = re.compile(r"average rate:\s+([\d.]+)") - - -def _parse_hz(text): - m = _HZ_RE.search(text or "") - return float(m.group(1)) if m else None - - -# ── container helpers ────────────────────────────────────────────────────── - -def find_all_containers(name_pattern): - result = _run_teed( - ["docker", "ps", "--filter", f"name={name_pattern}", "--format", "{{.Names}}"], - timeout=10, - ) - return [n for n in result.stdout.strip().splitlines() if n] - - -def find_container(name_pattern): - names = find_all_containers(name_pattern) - return names[0] if names else None - - -def get_robot_containers(pattern="robot.*desktop"): - """Return running robot containers sorted by their replica index""" - def _index(name): - tail = name.rsplit("-", 1)[-1] - return int(tail) if tail.isdigit() else 0 - return sorted(find_all_containers(pattern), key=_index) - - -def container_running(name): - """True if the named container is currently Running.""" - result = _run_teed( - ["docker", "inspect", "-f", "{{.State.Running}}", name], - timeout=10, - ) - return "true" in result.stdout - - -def wait_for_container(name_pattern, timeout=120): - deadline = time.time() + timeout - while time.time() < deadline: - name = find_container(name_pattern) - if name and container_running(name): - return name - time.sleep(5) - raise TimeoutError(f"Container matching '{name_pattern}' not running after {timeout}s") - - -# ── compute-usage sampling ───────────────────────────────────────────────── - -_BYTES_RE = re.compile(r"([\d.]+)\s*([kKMGT]?i?B)$") -_BYTES_TO_MB = { - "B": 1 / (1024 * 1024), - "KiB": 1 / 1024, "KB": 1 / 1000, "kB": 1 / 1000, - "MiB": 1, "MB": 1, - "GiB": 1024, "GB": 1000, - "TiB": 1024 * 1024, "TB": 1_000_000, -} - - -def _parse_docker_bytes(s): - """Parse a docker-stats byte string (e.g. '123.4MiB', '0B') to MB.""" - m = _BYTES_RE.match((s or "").strip()) - if not m: - return 0.0 - return float(m.group(1)) * _BYTES_TO_MB.get(m.group(2), 1) - - -def sample_compute_usage(sim_container): - """Snapshot of compute resources: per-container CPU/mem/disk-IO/net-IO plus - global host CPU/mem and GPU util/VRAM/temp/power. Returns {key: value}, - keys shaped `{entity}.{metric}` where entity is the full container name or - 'host'. Per-robot replicas (e.g. airstack-robot-desktop-1/2/3) are kept - distinct so raw metrics.json preserves per-robot data; parse_metrics - pools them at report time. Silently omits metrics that fail to sample.""" - import psutil - - out = {} - - stats = _run_teed( - ["docker", "stats", "--no-stream", "--format", "{{json .}}"], - timeout=20, - ) - for line in stats.stdout.strip().splitlines(): - try: - d = json.loads(line) - except json.JSONDecodeError: - continue - name = d.get("Name", "") - if not name or name.startswith("docker-test-run"): - continue - out[f"{name}.cpu_pct"] = float(d.get("CPUPerc", "0%").rstrip("%") or 0) - mem_raw = d.get("MemUsage", "").split("/")[0].strip() - out[f"{name}.mem_mb"] = _parse_docker_bytes(mem_raw) - for io_field, metric in (("BlockIO", "disk_io_mb"), ("NetIO", "net_io_mb")): - parts = (d.get(io_field, "") or "").split("/") - total = sum(_parse_docker_bytes(p.strip()) for p in parts) - out[f"{name}.{metric}"] = total - - out["host.cpu_pct"] = psutil.cpu_percent(interval=0.5) - out["host.mem_mb"] = psutil.virtual_memory().used / (1024 * 1024) - - gpu = _run_teed( - ["docker", "exec", sim_container, "nvidia-smi", - "--query-gpu=utilization.gpu,memory.used,temperature.gpu,power.draw", - "--format=csv,noheader,nounits"], - timeout=10, - ) - if gpu.returncode == 0 and gpu.stdout.strip(): - fields = [f.strip() for f in gpu.stdout.strip().splitlines()[0].split(",")] - if len(fields) >= 4: - try: - out["host.gpu_pct"] = float(fields[0]) - out["host.vram_mb"] = float(fields[1]) - out["host.gpu_temp_c"] = float(fields[2]) - out["host.gpu_power_w"] = float(fields[3]) - except ValueError: - pass - - return out - - -def _compose_images(env=None): - """Resolved image refs that `docker compose up` would use under `env`.""" - compose_env = os.environ.copy() - if env: - compose_env.update(env) - result = _run_teed( - ["docker", "compose", "-f", str(Path(AIRSTACK_ROOT) / "docker-compose.yaml"), - "config", "--images"], - timeout=30, env=compose_env, cwd=AIRSTACK_ROOT, - ) - return [l.strip() for l in result.stdout.strip().splitlines() if l.strip()] - - -def missing_images(env=None): - """Images required by the current compose config but not present locally. - Used by airstack_env to fail fast instead of letting `airstack up` hang - pulling/building when images haven't been prebuilt.""" - missing = [] - for image in _compose_images(env=env): - result = _run_teed( - ["docker", "image", "inspect", image, "--format", "{{.Id}}"], - timeout=10, - ) - if result.returncode != 0: - missing.append(image) - return missing - - -def docker_image_size_mb(service, env=None): - image = next((i for i in _compose_images(env=env) if service in i), None) - if not image: - return None - result = _run_teed( - ["docker", "image", "inspect", image, "--format", "{{.Size}}"], - timeout=10, - ) - if result.returncode == 0 and result.stdout.strip(): - return round(int(result.stdout.strip()) / 1_000_000, 1) - return None - - -# ── metrics ──────────────────────────────────────────────────────────────── - -class MetricsRecorder: - def __init__(self, path): - self._path = path - self._data = json.loads(path.read_text()) if path.exists() else {} - self._lock = threading.Lock() - - def _flush(self): - tmp = self._path.with_suffix(self._path.suffix + ".tmp") - tmp.write_text(json.dumps(self._data, indent=2)) - os.replace(tmp, self._path) - - def record(self, test_name, key, value, unit="", direction="lower_is_better", **extra): - with self._lock: - if test_name not in self._data: - self._data[test_name] = {} - entry = {"value": value, "unit": unit, "direction": direction} - entry.update(extra) - self._data[test_name][key] = entry - self._flush() - - def record_list(self, test_name, key, values): - """Store a raw list (time series) — not scored by parse_metrics.""" - with self._lock: - if test_name not in self._data: - self._data[test_name] = {} - self._data[test_name][key] = {"samples": values} - self._flush() - -def get_metrics(): - global METRICS - if METRICS is None: - METRICS = MetricsRecorder(session.run_dir() / "metrics.json") - return METRICS - - -def current_test_id(): - """Test id used as the metrics.json key. Matches JUnit XML's classname.name - format so parse_metrics.py can merge results.xml and metrics.json entries.""" - item = session.current_item() - if item is None: - return "unknown" - return _nodeid_dotted(item.nodeid) - - -# ── shared sim test infrastructure (liveliness, sensors, comms, takeoff reuse) ── - -def wait_for_first_message(container, topic, domain_id, setup_bash, timeout=60): - """Wait up to `timeout` seconds for one message on `topic`. Returns seconds - elapsed on success, None on timeout. Each attempt sources the workspace - and runs `ros2 topic echo --once`; if the workspace isn't built yet or the - topic has no publisher, the attempt fails fast and we retry. - """ - start = time.time() - deadline = start + timeout - logger.info("Probing %s on domain %d in %s (timeout=%ds)", - topic, domain_id, container, timeout) - attempt = 0 - while time.time() < deadline: - attempt += 1 - per_attempt = min(max(1, int(deadline - time.time())), 10) - try: - result = ros2_exec( - container, - f"timeout {per_attempt} ros2 topic echo --once {topic}", - domain_id=domain_id, setup_bash=setup_bash, timeout=per_attempt + 5, - ) - except subprocess.TimeoutExpired: - logger.warning("Attempt %d subprocess timeout for %s, retrying", attempt, topic) - time.sleep(2) - continue - # ros2 prints "---" on its own line after a real message. - if result.stdout.rstrip().endswith("---"): - elapsed = round(time.time() - start, 2) - logger.info("Got first message on %s after %.2fs (attempt %d)", - topic, elapsed, attempt) - return elapsed - logger.warning("Attempt %d failed for %s, retrying", attempt, topic) - time.sleep(2) - logger.error("Timed out waiting for first message on %s after %ds", - topic, timeout) - return None - - -def sample_hz(container, topic, domain_id, setup_bash, duration=5, window=10): - """Sample publish rate on `topic` for `duration` seconds. Returns float or None.""" - result = ros2_exec( - container, - f"timeout {duration} ros2 topic hz --window {window} {topic} 2>&1", - domain_id=domain_id, setup_bash=setup_bash, timeout=duration + 15, - ) - return _parse_hz(result.stdout + result.stderr) - - -def parallel_sample_hz(container, topic_domain_pairs, setup_bash, duration=5, window=10): - """Sample Hz for multiple topics concurrently; return {topic: hz_or_None}. - - One `docker exec` that backgrounds each `ros2 topic hz` probe, waits for all, - then cats each probe's temp file. - """ - probes = [] - temp_files = {} - for i, (topic, domain) in enumerate(topic_domain_pairs): - fname = f"/tmp/hz_{i}.out" - temp_files[topic] = fname - probes.append( - f"(ROS_DOMAIN_ID={domain} timeout {duration} " - f"ros2 topic hz --window {window} {topic} > {fname} 2>&1) &" - ) - # Newlines, not `&& ... &`: bash precedence makes `A && B && C & D &` only - # apply the && chain to C, so later backgrounded probes would miss the - # sourced PATH. One statement per line sidesteps this entirely. - lines = [f"source {ROS_DISTRO_SETUP}", f"source {setup_bash}"] + probes + ["wait"] - for fname in temp_files.values(): - lines.append(f"echo '===FILE {fname}==='") - lines.append(f"cat {fname} 2>/dev/null || true") - script = "\n".join(lines) - result = _run_teed( - ["docker", "exec", container, "bash", "-c", script], - timeout=duration + 30, - ) - rates = {} - if result.returncode == 0 or result.stdout: - chunks = result.stdout.split("===FILE ") - for chunk in chunks[1:]: - header, _, content = chunk.partition("===") - fname = header.strip() - topic = next((t for t, f in temp_files.items() if f == fname), None) - if topic: - rates[topic] = _parse_hz(content) - for topic, _ in topic_domain_pairs: - rates.setdefault(topic, None) - return rates - - -def _echo_once_received_message(result): - """True if ``ros2 topic echo --once`` printed a full message (trailing ``---``).""" - out = (result.stdout or "").rstrip() - return out.endswith("---") - - -def parallel_echo_once_robot_topics( - probes, setup_bash, per_topic_timeout, -): - """Liveliness for heavy topics (e.g. PointCloud2): ``echo --once`` per probe in parallel. - - ``ros2 topic hz`` often never reports a rate on large point clouds (decode backlog). - - Parameters - ---------- - probes : list[tuple[str, str, int]] - ``(container_name, topic, ros_domain_id)`` — use the **robot container** that - hosts that domain's graph (replica ``n`` for ``robot_n``). - setup_bash : str - Workspace ``setup.bash`` path inside the container. - per_topic_timeout : int - Wall seconds per ``timeout … ros2 topic echo --once``. - - Returns - ------- - dict[str, float | None] - ``{topic: 1.0}`` if a message arrived, else ``{topic: None}`` (metrics use 1.0 - as a nonzero "alive" placeholder, not a measured Hz). - """ - rates = {} - - def _one(container, topic, domain_id): - cmd = f"timeout {per_topic_timeout} ros2 topic echo --once {topic}" - return topic, ros2_exec( - container, - cmd, - domain_id=domain_id, - setup_bash=setup_bash, - timeout=per_topic_timeout + 15, - ) - - with ThreadPoolExecutor(max_workers=max(1, len(probes))) as pool: - futures = { - pool.submit(_one, container, topic, domain_id): topic - for container, topic, domain_id in probes - } - for fut in as_completed(futures): - topic = futures[fut] - try: - _, result = fut.result() - except Exception as e: - logger.warning("echo-once probe failed for %s: %s", topic, e) - rates[topic] = None - continue - rates[topic] = 1.0 if _echo_once_received_message(result) else None - for _, topic, _ in probes: - rates.setdefault(topic, None) - return rates - - @pytest.fixture def airstack_env(request): """Parametrized fixture: runs `airstack up`, yields env dict, tears down. diff --git a/tests/harness/__init__.py b/tests/harness/__init__.py index c08975c72..05efe2144 100644 --- a/tests/harness/__init__.py +++ b/tests/harness/__init__.py @@ -3,6 +3,25 @@ conftest.py holds the pytest hooks and fixtures and imports what it needs from here. Public helpers are re-exported so tests and conftest can ``from harness import ``. """ +from harness.commands import ( + ROS_DISTRO_SETUP, + airstack_cmd, + current_log, + docker_exec, + read_log_tail, + ros2_env, + ros2_exec, +) +from harness.containers import ( + container_running, + docker_image_size_mb, + find_all_containers, + find_container, + get_robot_containers, + missing_images, + sample_compute_usage, + wait_for_container, +) from harness.discovery import ( AIRSTACK_ROOT, COLCON_UNIT_TEST_PACKAGES_YAML, @@ -12,15 +31,33 @@ unit_test_dirs, unit_test_files, ) +from harness.metrics import MetricsRecorder, current_test_id, get_metrics from harness.session import logger +from harness.sim import ( + SIM_CONFIG, + parallel_echo_once_robot_topics, + parallel_sample_hz, + sample_hz, + wait_for_first_message, +) __all__ = [ - "AIRSTACK_ROOT", - "COLCON_UNIT_TEST_PACKAGES_YAML", - "colcon_test_robot_command", - "load_colcon_unit_test_config", - "repo_path", - "unit_test_dirs", - "unit_test_files", + # discovery + "AIRSTACK_ROOT", "COLCON_UNIT_TEST_PACKAGES_YAML", "repo_path", + "colcon_test_robot_command", "load_colcon_unit_test_config", + "unit_test_dirs", "unit_test_files", + # session "logger", + # commands + "ROS_DISTRO_SETUP", "airstack_cmd", "current_log", "docker_exec", + "read_log_tail", "ros2_env", "ros2_exec", + # containers + "find_all_containers", "find_container", "get_robot_containers", + "container_running", "wait_for_container", "missing_images", + "sample_compute_usage", "docker_image_size_mb", + # metrics + "MetricsRecorder", "get_metrics", "current_test_id", + # sim + "SIM_CONFIG", "wait_for_first_message", "sample_hz", "parallel_sample_hz", + "parallel_echo_once_robot_topics", ] diff --git a/tests/harness/commands.py b/tests/harness/commands.py new file mode 100644 index 000000000..9b6bc2fb9 --- /dev/null +++ b/tests/harness/commands.py @@ -0,0 +1,92 @@ +"""Subprocess / docker-exec / ros2 command helpers with per-test output capture. + +Every subprocess runs through ``_run_teed``, which records combined stdout+stderr in the +session so ``read_log_tail`` can surface it in failure messages. ``current_log`` ties +output to the running test's id so callers don't have to plumb a log name through every +layer. +""" +import os +import re +import shlex +import subprocess +from pathlib import Path + +from harness.discovery import AIRSTACK_ROOT +from harness.session import current_item, last_cmd_output, logger, record_cmd_output + +ROS_DISTRO_SETUP = "/opt/ros/jazzy/setup.bash" + + +def _nodeid_dotted(nodeid, with_path_sep=False): + """pytest nodeid → `module.Class.test_name[params]` form. When + `with_path_sep=True`, also flattens `/` in path prefixes (for log filenames).""" + out = nodeid.replace(".py::", ".").replace("::", ".") + return out.replace("/", ".") if with_path_sep else out + + +def current_log(): + """Log name for the currently-running pytest item, or None outside a test. + + Subprocess helpers default to this so every call fired from a test auto-logs + to the right file without plumbing log_name through every layer.""" + item = current_item() + if item is None: + return None + return _nodeid_dotted(item.nodeid, with_path_sep=True) + + +def read_log_tail(log_name=None, lines=50): + """Return the tail of the most recent subprocess output for this context.""" + text = last_cmd_output(log_name) + if not text: + return "" + return "\n".join(text.splitlines()[-lines:]) + + +def _run_teed(cmd_list, timeout, log_name=None, env=None, cwd=None): + """Run a subprocess and capture stdout+stderr for parsing and failure messages.""" + quoted = " ".join(shlex.quote(a) for a in cmd_list) + logger.info("$ %s", quoted) + result = subprocess.run( + cmd_list, capture_output=True, text=True, timeout=timeout, env=env, cwd=cwd, + ) + combined = (result.stdout or "") + (result.stderr or "") + record_cmd_output(combined, log_name) + return result + + +def docker_exec(container, cmd, timeout=60, log_name=None): + full_cmd = ["docker", "exec", container, "bash", "-c", cmd] + return _run_teed(full_cmd, timeout=timeout, log_name=log_name) + + +def airstack_cmd(*args, env_overrides=None, timeout=1800, log_name=None): + env = os.environ.copy() + if env_overrides: + env.update(env_overrides) + cmd = [str(Path(AIRSTACK_ROOT) / "airstack.sh")] + list(args) + return _run_teed(cmd, timeout=timeout, log_name=log_name, + env=env, cwd=AIRSTACK_ROOT) + + +def ros2_env(setup_bash, domain_id): + """Shell prefix that makes `ros2` available on the requested domain.""" + return ( + f"source {ROS_DISTRO_SETUP} && source {setup_bash} " + f"&& export ROS_DOMAIN_ID={domain_id}" + ) + + +def ros2_exec(container, ros2_cmd, domain_id=0, setup_bash=None, timeout=15, log_name=None): + """Run `ros2 ...` inside a container with the right workspace sourced.""" + setup = setup_bash or "/root/AirStack/robot/ros_ws/install/setup.bash" + inner = f"{ros2_env(setup, domain_id)} && {ros2_cmd}" + return docker_exec(container, inner, timeout=timeout, log_name=log_name) + + +_HZ_RE = re.compile(r"average rate:\s+([\d.]+)") + + +def _parse_hz(text): + m = _HZ_RE.search(text or "") + return float(m.group(1)) if m else None diff --git a/tests/harness/containers.py b/tests/harness/containers.py new file mode 100644 index 000000000..71a64d480 --- /dev/null +++ b/tests/harness/containers.py @@ -0,0 +1,164 @@ +"""Docker container discovery, compute-usage sampling, and image helpers.""" +import json +import os +import re +import time +from pathlib import Path + +from harness.commands import _run_teed +from harness.discovery import AIRSTACK_ROOT + + +def find_all_containers(name_pattern): + result = _run_teed( + ["docker", "ps", "--filter", f"name={name_pattern}", "--format", "{{.Names}}"], + timeout=10, + ) + return [n for n in result.stdout.strip().splitlines() if n] + + +def find_container(name_pattern): + names = find_all_containers(name_pattern) + return names[0] if names else None + + +def get_robot_containers(pattern="robot.*desktop"): + """Return running robot containers sorted by their replica index""" + def _index(name): + tail = name.rsplit("-", 1)[-1] + return int(tail) if tail.isdigit() else 0 + return sorted(find_all_containers(pattern), key=_index) + + +def container_running(name): + """True if the named container is currently Running.""" + result = _run_teed( + ["docker", "inspect", "-f", "{{.State.Running}}", name], + timeout=10, + ) + return "true" in result.stdout + + +def wait_for_container(name_pattern, timeout=120): + deadline = time.time() + timeout + while time.time() < deadline: + name = find_container(name_pattern) + if name and container_running(name): + return name + time.sleep(5) + raise TimeoutError(f"Container matching '{name_pattern}' not running after {timeout}s") + + +# ── compute-usage sampling ───────────────────────────────────────────────── + +_BYTES_RE = re.compile(r"([\d.]+)\s*([kKMGT]?i?B)$") +_BYTES_TO_MB = { + "B": 1 / (1024 * 1024), + "KiB": 1 / 1024, "KB": 1 / 1000, "kB": 1 / 1000, + "MiB": 1, "MB": 1, + "GiB": 1024, "GB": 1000, + "TiB": 1024 * 1024, "TB": 1_000_000, +} + + +def _parse_docker_bytes(s): + """Parse a docker-stats byte string (e.g. '123.4MiB', '0B') to MB.""" + m = _BYTES_RE.match((s or "").strip()) + if not m: + return 0.0 + return float(m.group(1)) * _BYTES_TO_MB.get(m.group(2), 1) + + +def sample_compute_usage(sim_container): + """Snapshot of compute resources: per-container CPU/mem/disk-IO/net-IO plus + global host CPU/mem and GPU util/VRAM/temp/power. Returns {key: value}, + keys shaped `{entity}.{metric}` where entity is the full container name or + 'host'. Per-robot replicas (e.g. airstack-robot-desktop-1/2/3) are kept + distinct so raw metrics.json preserves per-robot data; parse_metrics + pools them at report time. Silently omits metrics that fail to sample.""" + import psutil + + out = {} + + stats = _run_teed( + ["docker", "stats", "--no-stream", "--format", "{{json .}}"], + timeout=20, + ) + for line in stats.stdout.strip().splitlines(): + try: + d = json.loads(line) + except json.JSONDecodeError: + continue + name = d.get("Name", "") + if not name or name.startswith("docker-test-run"): + continue + out[f"{name}.cpu_pct"] = float(d.get("CPUPerc", "0%").rstrip("%") or 0) + mem_raw = d.get("MemUsage", "").split("/")[0].strip() + out[f"{name}.mem_mb"] = _parse_docker_bytes(mem_raw) + for io_field, metric in (("BlockIO", "disk_io_mb"), ("NetIO", "net_io_mb")): + parts = (d.get(io_field, "") or "").split("/") + total = sum(_parse_docker_bytes(p.strip()) for p in parts) + out[f"{name}.{metric}"] = total + + out["host.cpu_pct"] = psutil.cpu_percent(interval=0.5) + out["host.mem_mb"] = psutil.virtual_memory().used / (1024 * 1024) + + gpu = _run_teed( + ["docker", "exec", sim_container, "nvidia-smi", + "--query-gpu=utilization.gpu,memory.used,temperature.gpu,power.draw", + "--format=csv,noheader,nounits"], + timeout=10, + ) + if gpu.returncode == 0 and gpu.stdout.strip(): + fields = [f.strip() for f in gpu.stdout.strip().splitlines()[0].split(",")] + if len(fields) >= 4: + try: + out["host.gpu_pct"] = float(fields[0]) + out["host.vram_mb"] = float(fields[1]) + out["host.gpu_temp_c"] = float(fields[2]) + out["host.gpu_power_w"] = float(fields[3]) + except ValueError: + pass + + return out + + +def _compose_images(env=None): + """Resolved image refs that `docker compose up` would use under `env`.""" + compose_env = os.environ.copy() + if env: + compose_env.update(env) + result = _run_teed( + ["docker", "compose", "-f", str(Path(AIRSTACK_ROOT) / "docker-compose.yaml"), + "config", "--images"], + timeout=30, env=compose_env, cwd=AIRSTACK_ROOT, + ) + return [l.strip() for l in result.stdout.strip().splitlines() if l.strip()] + + +def missing_images(env=None): + """Images required by the current compose config but not present locally. + Used by airstack_env to fail fast instead of letting `airstack up` hang + pulling/building when images haven't been prebuilt.""" + missing = [] + for image in _compose_images(env=env): + result = _run_teed( + ["docker", "image", "inspect", image, "--format", "{{.Id}}"], + timeout=10, + ) + if result.returncode != 0: + missing.append(image) + return missing + + +def docker_image_size_mb(service, env=None): + image = next((i for i in _compose_images(env=env) if service in i), None) + if not image: + return None + result = _run_teed( + ["docker", "image", "inspect", image, "--format", "{{.Size}}"], + timeout=10, + ) + if result.returncode == 0 and result.stdout.strip(): + return round(int(result.stdout.strip()) / 1_000_000, 1) + return None diff --git a/tests/harness/metrics.py b/tests/harness/metrics.py new file mode 100644 index 000000000..3d6e5e518 --- /dev/null +++ b/tests/harness/metrics.py @@ -0,0 +1,55 @@ +"""Per-run metrics recording (``metrics.json``).""" +import json +import os +import threading + +from harness.commands import _nodeid_dotted +from harness.session import current_item, run_dir + + +class MetricsRecorder: + def __init__(self, path): + self._path = path + self._data = json.loads(path.read_text()) if path.exists() else {} + self._lock = threading.Lock() + + def _flush(self): + tmp = self._path.with_suffix(self._path.suffix + ".tmp") + tmp.write_text(json.dumps(self._data, indent=2)) + os.replace(tmp, self._path) + + def record(self, test_name, key, value, unit="", direction="lower_is_better", **extra): + with self._lock: + if test_name not in self._data: + self._data[test_name] = {} + entry = {"value": value, "unit": unit, "direction": direction} + entry.update(extra) + self._data[test_name][key] = entry + self._flush() + + def record_list(self, test_name, key, values): + """Store a raw list (time series) — not scored by parse_metrics.""" + with self._lock: + if test_name not in self._data: + self._data[test_name] = {} + self._data[test_name][key] = {"samples": values} + self._flush() + + +_METRICS = None + + +def get_metrics(): + global _METRICS + if _METRICS is None: + _METRICS = MetricsRecorder(run_dir() / "metrics.json") + return _METRICS + + +def current_test_id(): + """Test id used as the metrics.json key. Matches JUnit XML's classname.name + format so parse_metrics.py can merge results.xml and metrics.json entries.""" + item = current_item() + if item is None: + return "unknown" + return _nodeid_dotted(item.nodeid) diff --git a/tests/harness/sim.py b/tests/harness/sim.py new file mode 100644 index 000000000..ec4c8a159 --- /dev/null +++ b/tests/harness/sim.py @@ -0,0 +1,189 @@ +"""Shared sim-topic test infrastructure: sim target configs + ros2 topic sampling. + +Used by the liveliness / sensors / takeoff system tests to probe topic liveness and +publish rates on the robot/sim containers. +""" +import subprocess +import time +from concurrent.futures import ThreadPoolExecutor, as_completed + +from harness.commands import ROS_DISTRO_SETUP, _parse_hz, _run_teed, ros2_exec +from harness.session import logger + +SIM_CONFIG = { + "msairsim": { + "profile": "ms-airsim", + "sim_container": "ms-airsim", + "sim_setup_bash": "/root/ros_ws/install/setup.bash", + "robot_setup_bash": "/root/AirStack/robot/ros_ws/install/setup.bash", + "extra_env": { + "URDF_FILE": "robot_descriptions/iris/urdf/iris_stereo.ms-airsim.urdf", + # Clear any user-set paths in .env so entrypoint auto-fetches Blocks. + # Shell env wins over --env-file in docker compose substitution. + "MS_AIRSIM_ENV_DIR": "", + "MS_AIRSIM_BINARY_PATH": "", + }, + }, + "isaacsim": { + "profile": "isaac-sim", + "sim_container": "isaac-sim", + "sim_setup_bash": "/opt/ros/jazzy/setup.bash", + "robot_setup_bash": "/root/AirStack/robot/ros_ws/install/setup.bash", + "extra_env": { + "ISAAC_SIM_USE_STANDALONE": "true", + "ISAAC_SIM_SCRIPT_NAME": "example_multi_px4_pegasus_launch_script.py", + "PLAY_SIM_ON_START": "true", + # Multi script gates RTX LiDAR on this flag; example_one always spawns it. + # `sensors` tests expect ouster topics + lidar_point_cloud_filter path. + "ENABLE_LIDAR": "true", + }, + }, +} + + +def wait_for_first_message(container, topic, domain_id, setup_bash, timeout=60): + """Wait up to `timeout` seconds for one message on `topic`. Returns seconds + elapsed on success, None on timeout. Each attempt sources the workspace + and runs `ros2 topic echo --once`; if the workspace isn't built yet or the + topic has no publisher, the attempt fails fast and we retry. + """ + start = time.time() + deadline = start + timeout + logger.info("Probing %s on domain %d in %s (timeout=%ds)", + topic, domain_id, container, timeout) + attempt = 0 + while time.time() < deadline: + attempt += 1 + per_attempt = min(max(1, int(deadline - time.time())), 10) + try: + result = ros2_exec( + container, + f"timeout {per_attempt} ros2 topic echo --once {topic}", + domain_id=domain_id, setup_bash=setup_bash, timeout=per_attempt + 5, + ) + except subprocess.TimeoutExpired: + logger.warning("Attempt %d subprocess timeout for %s, retrying", attempt, topic) + time.sleep(2) + continue + # ros2 prints "---" on its own line after a real message. + if result.stdout.rstrip().endswith("---"): + elapsed = round(time.time() - start, 2) + logger.info("Got first message on %s after %.2fs (attempt %d)", + topic, elapsed, attempt) + return elapsed + logger.warning("Attempt %d failed for %s, retrying", attempt, topic) + time.sleep(2) + logger.error("Timed out waiting for first message on %s after %ds", + topic, timeout) + return None + + +def sample_hz(container, topic, domain_id, setup_bash, duration=5, window=10): + """Sample publish rate on `topic` for `duration` seconds. Returns float or None.""" + result = ros2_exec( + container, + f"timeout {duration} ros2 topic hz --window {window} {topic} 2>&1", + domain_id=domain_id, setup_bash=setup_bash, timeout=duration + 15, + ) + return _parse_hz(result.stdout + result.stderr) + + +def parallel_sample_hz(container, topic_domain_pairs, setup_bash, duration=5, window=10): + """Sample Hz for multiple topics concurrently; return {topic: hz_or_None}. + + One `docker exec` that backgrounds each `ros2 topic hz` probe, waits for all, + then cats each probe's temp file. + """ + probes = [] + temp_files = {} + for i, (topic, domain) in enumerate(topic_domain_pairs): + fname = f"/tmp/hz_{i}.out" + temp_files[topic] = fname + probes.append( + f"(ROS_DOMAIN_ID={domain} timeout {duration} " + f"ros2 topic hz --window {window} {topic} > {fname} 2>&1) &" + ) + # Newlines, not `&& ... &`: bash precedence makes `A && B && C & D &` only + # apply the && chain to C, so later backgrounded probes would miss the + # sourced PATH. One statement per line sidesteps this entirely. + lines = [f"source {ROS_DISTRO_SETUP}", f"source {setup_bash}"] + probes + ["wait"] + for fname in temp_files.values(): + lines.append(f"echo '===FILE {fname}==='") + lines.append(f"cat {fname} 2>/dev/null || true") + script = "\n".join(lines) + result = _run_teed( + ["docker", "exec", container, "bash", "-c", script], + timeout=duration + 30, + ) + rates = {} + if result.returncode == 0 or result.stdout: + chunks = result.stdout.split("===FILE ") + for chunk in chunks[1:]: + header, _, content = chunk.partition("===") + fname = header.strip() + topic = next((t for t, f in temp_files.items() if f == fname), None) + if topic: + rates[topic] = _parse_hz(content) + for topic, _ in topic_domain_pairs: + rates.setdefault(topic, None) + return rates + + +def _echo_once_received_message(result): + """True if ``ros2 topic echo --once`` printed a full message (trailing ``---``).""" + out = (result.stdout or "").rstrip() + return out.endswith("---") + + +def parallel_echo_once_robot_topics( + probes, setup_bash, per_topic_timeout, +): + """Liveliness for heavy topics (e.g. PointCloud2): ``echo --once`` per probe in parallel. + + ``ros2 topic hz`` often never reports a rate on large point clouds (decode backlog). + + Parameters + ---------- + probes : list[tuple[str, str, int]] + ``(container_name, topic, ros_domain_id)`` — use the **robot container** that + hosts that domain's graph (replica ``n`` for ``robot_n``). + setup_bash : str + Workspace ``setup.bash`` path inside the container. + per_topic_timeout : int + Wall seconds per ``timeout … ros2 topic echo --once``. + + Returns + ------- + dict[str, float | None] + ``{topic: 1.0}`` if a message arrived, else ``{topic: None}`` (metrics use 1.0 + as a nonzero "alive" placeholder, not a measured Hz). + """ + rates = {} + + def _one(container, topic, domain_id): + cmd = f"timeout {per_topic_timeout} ros2 topic echo --once {topic}" + return topic, ros2_exec( + container, + cmd, + domain_id=domain_id, + setup_bash=setup_bash, + timeout=per_topic_timeout + 15, + ) + + with ThreadPoolExecutor(max_workers=max(1, len(probes))) as pool: + futures = { + pool.submit(_one, container, topic, domain_id): topic + for container, topic, domain_id in probes + } + for fut in as_completed(futures): + topic = futures[fut] + try: + _, result = fut.result() + except Exception as e: + logger.warning("echo-once probe failed for %s: %s", topic, e) + rates[topic] = None + continue + rates[topic] = 1.0 if _echo_once_received_message(result) else None + for _, topic, _ in probes: + rates.setdefault(topic, None) + return rates From 35be702d07f9986b1da80ebd2ed41e88245941b5 Mon Sep 17 00:00:00 2001 From: John Date: Wed, 22 Jul 2026 12:43:08 -0400 Subject: [PATCH 10/12] refactor(tests): extract collection ordering into tests/harness/collection.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Final step of the conftest.py modularization: move test ordering — _MODULE_ORDER, the per-module phase chains, _module_key, and the parametrize-id rewrite — into harness/collection.py. conftest's pytest_collection_modifyitems hook now delegates to collection.modify_items(items). conftest.py is now 246 lines (from 959): pytest hooks + the airstack_env / robot_autonomy_stack fixtures. All helpers live in tests/harness/ by concern (session, discovery, commands, containers, metrics, sim, collection). Behavior-preserving: -m unit still 14 passed / 152 deselected, 166 collected, unit → build → integration → sim order unchanged. Co-Authored-By: Claude Opus 4.8 Also sync docs/skills to the tests/harness/ layout (AGENTS.md, tests/README.md, tests/integration/README.md, run-system-tests + add-unit-tests skills, unit_testing + end_to_end_testing docs): helpers, MetricsRecorder, the workspace globs, and _MODULE_ORDER now point at tests/harness/ instead of conftest.py (still re-exported via conftest). Co-authored-by: Cursor --- .agents/skills/add-unit-tests/SKILL.md | 2 +- .agents/skills/run-system-tests/SKILL.md | 9 +- AGENTS.md | 2 +- .../testing/end_to_end_testing.md | 2 +- .../intermediate/testing/unit_testing.md | 4 +- tests/README.md | 14 +- tests/conftest.py | 118 +---------------- tests/harness/collection.py | 125 ++++++++++++++++++ tests/integration/README.md | 2 +- 9 files changed, 150 insertions(+), 128 deletions(-) create mode 100644 tests/harness/collection.py diff --git a/.agents/skills/add-unit-tests/SKILL.md b/.agents/skills/add-unit-tests/SKILL.md index 5265e7eef..d49a36d73 100644 --- a/.agents/skills/add-unit-tests/SKILL.md +++ b/.agents/skills/add-unit-tests/SKILL.md @@ -222,7 +222,7 @@ there are listed in [`tests/colcon_unit_test_packages.yaml`](../../../tests/colc ## Extending to sim and GCS The same mechanism applies — add the package under a workspace key in the YAML. The -workspace→source glob is defined in `conftest.py` (`_WORKSPACE_PKG_TEST_GLOBS`): `robot` → +workspace→source glob is defined in `tests/harness/discovery.py` (`_WORKSPACE_PKG_TEST_GLOBS`): `robot` → `robot/ros_ws/src/**//test`, `sim` → `simulation/**//test`. Add a new workspace key there (e.g. `gcs`) if you extend to a new tree. diff --git a/.agents/skills/run-system-tests/SKILL.md b/.agents/skills/run-system-tests/SKILL.md index 197971def..3923b6842 100644 --- a/.agents/skills/run-system-tests/SKILL.md +++ b/.agents/skills/run-system-tests/SKILL.md @@ -81,7 +81,7 @@ rates if too many `ros2 topic hz` processes run concurrently. - **Robot-side (Isaac):** two passes — both stereo images, then both depths. **ms-airsim** keeps a single four-topic parallel batch on the robot container. - **Filtered LiDAR** (`PointCloud2`): uses `ros2 topic echo --once` per robot - (see `parallel_echo_once_robot_topics` in `conftest.py`), not `topic hz`. + (see `parallel_echo_once_robot_topics` in `tests/harness/sim.py`), not `topic hz`. - **Multi-drone Pegasus script:** pytest sets `ENABLE_LIDAR=true` in `conftest.py` `SIM_CONFIG["isaacsim"]["extra_env"]` so LiDAR matches the single-drone example (which always enables RTX LiDAR). @@ -295,7 +295,7 @@ If your test... - File: `tests/system/test_.py` — matches pytest's default test discovery (`test_*.py`) under the system suite - Class: `Test` with the mark applied at the class level: `@pytest.mark.` - Add a class-level `@pytest.mark.timeout()` — long-running sim tests need it -- Imports: pull helpers from `conftest` directly (`from conftest import ...`); `tests/` is on `sys.path` because `testpaths = .` in pytest.ini +- Imports: pull helpers from `conftest` directly (`from conftest import ...`); they physically live in the `tests/harness/` package but are re-exported through `conftest`, so either `from conftest import ...` or `from harness import ...` works. `tests/` is on `sys.path` because `testpaths = .` in pytest.ini ### 3. Decide if you need `airstack_env` @@ -305,7 +305,7 @@ If your test... ### 4. Use the existing helpers -`conftest.py` exports a deliberate API. Prefer these over rolling your own: +The `tests/harness/` package exports a deliberate API (re-exported through `conftest`). Prefer these over rolling your own: | Helper | Purpose | |--------|---------| @@ -422,7 +422,8 @@ python tests/parse_metrics.py \ ### Files to know -- `tests/conftest.py` — fixtures, helpers, `MetricsRecorder`, ordering hooks +- `tests/conftest.py` — pytest hooks + the `airstack_env` / `robot_autonomy_stack` fixtures (re-exports the harness API) +- `tests/harness/` — helpers split by concern: `session`, `discovery`, `commands`, `containers`, `metrics` (`MetricsRecorder`), `sim`, `collection` (ordering) - `tests/pytest.ini` — mark registration, log format - `tests/parse_metrics.py` — markdown reporter, regression diff - `tests/README.md` — user-facing docs (CLI options, output layout, CI/CD orchestrator) diff --git a/AGENTS.md b/AGENTS.md index ca6ffebfd..884fc1e0a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -223,7 +223,7 @@ Pytest-based system tests live under [`tests/system/`](tests/system/). They brin | [`tests/system/test_takeoff_hover_land.py`](tests/system/test_takeoff_hover_land.py) | `takeoff_hover_land` | 4-phase flight chain (PX4 ready → takeoff → hover → land) per (sim, num_robots, iter, velocity) | Docker, GPU, sim license | | [`tests/system/test_fixed_trajectory.py`](tests/system/test_fixed_trajectory.py) | `autonomy` | 4-phase flight chain (PX4 ready → takeoff → execute Circle/Figure8/Racetrack/Line trajectory → land) per (sim, num_robots, iter, trajectory_type); records cross-track error and path RMSE | Docker, GPU, sim license | -Shared fixtures, the `airstack_env` parametrized fixture, and `MetricsRecorder` live in [`tests/conftest.py`](tests/conftest.py). Each run produces a timestamped directory under `tests/results//` with `summary.txt`, `results.xml`, and `metrics.json` (no per-test log files — live output streams to the terminal via `log_cli`). [`tests/parse_metrics.py`](tests/parse_metrics.py) generates a markdown report (single-run or diff-vs-baseline; exits 1 on regression). +The pytest hooks and the `airstack_env` / `robot_autonomy_stack` fixtures live in [`tests/conftest.py`](tests/conftest.py); the shared helpers are split by concern into the [`tests/harness/`](tests/harness/) package (`session`, `discovery`, `commands`, `containers`, `metrics` (with `MetricsRecorder`), `sim`, `collection`) and re-exported through `conftest`, so `from conftest import ` still resolves. Each run produces a timestamped directory under `tests/results//` with `summary.txt`, `results.xml`, and `metrics.json` (no per-test log files — live output streams to the terminal via `log_cli`). [`tests/parse_metrics.py`](tests/parse_metrics.py) generates a markdown report (single-run or diff-vs-baseline; exits 1 on regression). **Run via the CLI** (containerized runner — no local Python needed): diff --git a/docs/development/intermediate/testing/end_to_end_testing.md b/docs/development/intermediate/testing/end_to_end_testing.md index 4863d7964..6cddf945b 100644 --- a/docs/development/intermediate/testing/end_to_end_testing.md +++ b/docs/development/intermediate/testing/end_to_end_testing.md @@ -219,7 +219,7 @@ tests/results// | -------- | -------- | --- | | `summary.txt` | `tests/run_summary.py` (auto at session end via `conftest.py`) | Quick pass/fail + key numbers per trajectory type | | `results.xml` | pytest `--junitxml` | CI, phase wall times | -| `metrics.json` | `MetricsRecorder` in `conftest.py` | Regression diffs | +| `metrics.json` | `MetricsRecorder` in `tests/harness/metrics.py` | Regression diffs | ### Regenerate or inspect diff --git a/docs/development/intermediate/testing/unit_testing.md b/docs/development/intermediate/testing/unit_testing.md index 1b7919c14..dd2f72cf3 100644 --- a/docs/development/intermediate/testing/unit_testing.md +++ b/docs/development/intermediate/testing/unit_testing.md @@ -167,10 +167,10 @@ The `build_packages` CI job (`tests/system/test_build_packages.py`) also runs ## Extending to sim and GCS -The mechanism extends to other components via the same YAML. `conftest.py` +The mechanism extends to other components via the same YAML. `tests/harness/discovery.py` (`_WORKSPACE_PKG_TEST_GLOBS`) maps each workspace key to a source glob — `robot` → `robot/ros_ws/src/**//test`, `sim` → `simulation/**//test`. Add a `sim:` (or a -new `gcs:`) workspace to the YAML, adding the glob for a new tree in `conftest.py`: +new `gcs:`) workspace to the YAML, adding the glob for a new tree in `tests/harness/discovery.py`: ```yaml # tests/colcon_unit_test_packages.yaml diff --git a/tests/README.md b/tests/README.md index f8ee839ce..93a07dda8 100644 --- a/tests/README.md +++ b/tests/README.md @@ -6,7 +6,7 @@ AirStack's **pytest** tree under `tests/` has these roles: 2. **Unit tests** — Fast hermetic tests (`unit` mark) whose **source is co-located** with each ROS 2 package at `/test/`. [`colcon_unit_test_packages.yaml`](colcon_unit_test_packages.yaml) lists which packages have unit tests, and `pytest tests/` collects them from there. 3. **`tests/integration/`** — Cross-component tests (`integration` mark) that wire the robot container to a host-side component, without a sim or GPU. -Shared fixtures live in `tests/conftest.py`. Use `airstack test -m unit -v` for hermetic tests only, or the marks below for the full stack. +Pytest hooks and the shared fixtures live in `tests/conftest.py`; reusable helpers are split by concern into the [`tests/harness/`](harness/) package (re-exported through `conftest`). Use `airstack test -m unit -v` for hermetic tests only, or the marks below for the full stack. @@ -65,7 +65,17 @@ Marks can be combined with pytest logic: ## Test Infrastructure -All shared fixtures, helpers, and configuration live in [`conftest.py`](conftest.py). +[`conftest.py`](conftest.py) holds the pytest hooks and the `airstack_env` / `robot_autonomy_stack` fixtures. The reusable helpers are split by concern into the [`tests/harness/`](harness/) package and re-exported through `conftest`, so `from conftest import ` (and `from harness import `) both work: + +| Module | Contents | +|--------|----------| +| [`harness/session.py`](harness/session.py) | Session-scoped state (results dir, current item, last cmd output) + shared `logger` | +| [`harness/discovery.py`](harness/discovery.py) | Unit-test discovery driven by `colcon_unit_test_packages.yaml` (`AIRSTACK_ROOT`, `repo_path`, `unit_test_files`, …) | +| [`harness/commands.py`](harness/commands.py) | Subprocess / `docker exec` / `ros2` helpers with per-test output capture (`airstack_cmd`, `docker_exec`, `ros2_exec`, `read_log_tail`) | +| [`harness/containers.py`](harness/containers.py) | Container discovery, compute-usage sampling, image checks (`find_container`, `wait_for_container`, `sample_compute_usage`, `missing_images`) | +| [`harness/metrics.py`](harness/metrics.py) | `MetricsRecorder`, `get_metrics`, `current_test_id` (writes `metrics.json`) | +| [`harness/sim.py`](harness/sim.py) | `SIM_CONFIG` sim targets + ros2 topic sampling (`sample_hz`, `parallel_sample_hz`, `wait_for_first_message`) | +| [`harness/collection.py`](harness/collection.py) | Cross-module test ordering + parametrize-id rewrite (`modify_items`) | ### `airstack_env` fixture diff --git a/tests/conftest.py b/tests/conftest.py index d67f70433..47a668e65 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,7 +11,7 @@ if _TESTS_DIR not in sys.path: sys.path.insert(0, _TESTS_DIR) -from harness import session +from harness import collection, session # Re-export the harness helper API so existing `from conftest import ` in the # system tests + sensor_probes keeps working unchanged. from harness import * # noqa: F401,F403 @@ -118,122 +118,8 @@ def pytest_generate_tests(metafunc): metafunc.parametrize("airstack_env", params, ids=ids, indirect=True, scope="class") -# Run cheap/fast-fail tests first so real problems surface early: -# docker image builds → colcon workspace builds → liveliness (infra) → sensors -# (ROS topic streams) → autonomy flight tests. -_MODULE_ORDER = [ - # Unit tests first — fast, hermetic, no Docker. Co-located package unit tests - # (see unit_test_files) sort into this leading slot via the path check below. - "__unit__", - # System tests follow in dependency order. - "system.test_build_docker", - "system.test_build_packages", - # Integration tests (tests/integration/) need the robot-desktop image + colcon - # build, so they run after build_packages and before the sim tiers. - "__integration__", - "system.test_liveliness", - "system.test_sensors", - "system.test_takeoff_hover_land", - "system.test_fixed_trajectory", -] - -# Within test_takeoff_hover_land, each (env, velocity) runs phases in this chain order. -_AUTONOMY_PHASE_ORDER = [ - "test_px4_ready", - "test_takeoff", - "test_hover", - "test_landing", -] - -# Within test_fixed_trajectory, each (env, trajectory_type) runs phases in this order. -_FIXED_TRAJ_PHASE_ORDER = [ - "test_px4_ready", - "test_takeoff", - "test_fixed_trajectory", - "test_landing", -] - -# Maps module name → phase order list for per-module chain sorting. -_MODULE_PHASE_ORDERS = { - "system.test_takeoff_hover_land": _AUTONOMY_PHASE_ORDER, - "system.test_fixed_trajectory": _FIXED_TRAJ_PHASE_ORDER, -} - - -def _rank(name, order): - """Index of `name` in `order`; `len(order)` if unknown (i.e., sort last).""" - return order.index(name) if name in order else len(order) - - -def _module_key(item): - """Return the ordering key for an item. - - Co-located unit tests (collected from package ``test/`` dirs outside ``tests/``) - are identified by path. Integration tests live under ``integration/``. - Everything else uses the dotted module ``__name__`` against ``_MODULE_ORDER``. - """ - if _is_unit_item(item): - return _rank("__unit__", _MODULE_ORDER) - if item.nodeid.startswith("integration/"): - return _rank("__integration__", _MODULE_ORDER) - return _rank(getattr(item.module, "__name__", ""), _MODULE_ORDER) - - def pytest_collection_modifyitems(items): - # 1. Cross-module: enforce `_MODULE_ORDER`. Stable sort keeps within-module - # order intact, so pytest's default file/class order survives. - items.sort(key=_module_key) - - # 2. Within each parametrized autonomy-style module, sort by - # (airstack_env, secondary_param, phase) so each env brings up the stack - # once and the drone goes ground→air→ground per secondary parameter. - for mod_name, phase_order in _MODULE_PHASE_ORDERS.items(): - def _phase(item, _order=phase_order, _mod=mod_name): - if getattr(item.module, "__name__", "") != _mod: - return None - name = item.originalname or item.name.split("[", 1)[0] - return _rank(name, _order) - - def _sort_key(item, _mod=mod_name): - cs = getattr(item, "callspec", None) - env = cs.params.get("airstack_env", ()) if cs else () - # test_takeoff_hover_land sweeps velocity; test_fixed_trajectory sweeps type - secondary = ( - float(cs.params["velocity"]) if cs and "velocity" in cs.params - else (cs.params.get("trajectory_type", "") if cs else "") - ) - return (env, secondary, _phase(item)) - - slots = [(i, it) for i, it in enumerate(items) if _phase(it) is not None] - if slots: - sorted_items = sorted((it for _, it in slots), key=_sort_key) - for (i, _), new_item in zip(slots, sorted_items): - items[i] = new_item - - # 3. Rewrite bracketed test IDs into a consistent hierarchy: - # sim > robots > secondary param > iteration. - for item in items: - cs = getattr(item, "callspec", None) - if cs is None: - continue - env = cs.params.get("airstack_env") - parts = [] - if env: - sim, n, i = env - parts.append(f"{sim}-rob#{n}") - if "velocity" in cs.params: - parts.append(f"v{cs.params['velocity']}") - if "trajectory_type" in cs.params: - parts.append(f"traj{cs.params['trajectory_type']}") - if env: - parts.append(f"iter{i}") - if not parts: - continue - new_id = "-".join(parts) - if cs.id == new_id: - continue - item.name = item.name.replace(f"[{cs.id}]", f"[{new_id}]") - item._nodeid = item._nodeid.replace(f"[{cs.id}]", f"[{new_id}]") + collection.modify_items(items) @pytest.fixture diff --git a/tests/harness/collection.py b/tests/harness/collection.py new file mode 100644 index 000000000..aa20c9f1e --- /dev/null +++ b/tests/harness/collection.py @@ -0,0 +1,125 @@ +"""Test collection ordering. + +Cross-module order (unit → docker/package builds → integration → sim tiers), the +per-module phase chains for the autonomy flight tests, and a readable rewrite of the +parametrize ids. conftest's ``pytest_collection_modifyitems`` hook delegates to +``modify_items``. +""" +from harness.discovery import _is_unit_item + +# Run cheap/fast-fail tests first so real problems surface early: +# docker image builds → colcon workspace builds → liveliness (infra) → sensors +# (ROS topic streams) → autonomy flight tests. +_MODULE_ORDER = [ + # Unit tests first — fast, hermetic, no Docker. Co-located package unit tests + # (see unit_test_files) sort into this leading slot via the path check below. + "__unit__", + # System tests follow in dependency order. + "system.test_build_docker", + "system.test_build_packages", + # Integration tests (tests/integration/) need the robot-desktop image + colcon + # build, so they run after build_packages and before the sim tiers. + "__integration__", + "system.test_liveliness", + "system.test_sensors", + "system.test_takeoff_hover_land", + "system.test_fixed_trajectory", +] + +# Within test_takeoff_hover_land, each (env, velocity) runs phases in this chain order. +_AUTONOMY_PHASE_ORDER = [ + "test_px4_ready", + "test_takeoff", + "test_hover", + "test_landing", +] + +# Within test_fixed_trajectory, each (env, trajectory_type) runs phases in this order. +_FIXED_TRAJ_PHASE_ORDER = [ + "test_px4_ready", + "test_takeoff", + "test_fixed_trajectory", + "test_landing", +] + +# Maps module name → phase order list for per-module chain sorting. +_MODULE_PHASE_ORDERS = { + "system.test_takeoff_hover_land": _AUTONOMY_PHASE_ORDER, + "system.test_fixed_trajectory": _FIXED_TRAJ_PHASE_ORDER, +} + + +def _rank(name, order): + """Index of `name` in `order`; `len(order)` if unknown (i.e., sort last).""" + return order.index(name) if name in order else len(order) + + +def _module_key(item): + """Return the ordering key for an item. + + Co-located unit tests (collected from package ``test/`` dirs outside ``tests/``) + are identified by path. Integration tests live under ``integration/``. + Everything else uses the dotted module ``__name__`` against ``_MODULE_ORDER``. + """ + if _is_unit_item(item): + return _rank("__unit__", _MODULE_ORDER) + if item.nodeid.startswith("integration/"): + return _rank("__integration__", _MODULE_ORDER) + return _rank(getattr(item.module, "__name__", ""), _MODULE_ORDER) + + +def modify_items(items): + # 1. Cross-module: enforce `_MODULE_ORDER`. Stable sort keeps within-module + # order intact, so pytest's default file/class order survives. + items.sort(key=_module_key) + + # 2. Within each parametrized autonomy-style module, sort by + # (airstack_env, secondary_param, phase) so each env brings up the stack + # once and the drone goes ground→air→ground per secondary parameter. + for mod_name, phase_order in _MODULE_PHASE_ORDERS.items(): + def _phase(item, _order=phase_order, _mod=mod_name): + if getattr(item.module, "__name__", "") != _mod: + return None + name = item.originalname or item.name.split("[", 1)[0] + return _rank(name, _order) + + def _sort_key(item, _mod=mod_name): + cs = getattr(item, "callspec", None) + env = cs.params.get("airstack_env", ()) if cs else () + # test_takeoff_hover_land sweeps velocity; test_fixed_trajectory sweeps type + secondary = ( + float(cs.params["velocity"]) if cs and "velocity" in cs.params + else (cs.params.get("trajectory_type", "") if cs else "") + ) + return (env, secondary, _phase(item)) + + slots = [(i, it) for i, it in enumerate(items) if _phase(it) is not None] + if slots: + sorted_items = sorted((it for _, it in slots), key=_sort_key) + for (i, _), new_item in zip(slots, sorted_items): + items[i] = new_item + + # 3. Rewrite bracketed test IDs into a consistent hierarchy: + # sim > robots > secondary param > iteration. + for item in items: + cs = getattr(item, "callspec", None) + if cs is None: + continue + env = cs.params.get("airstack_env") + parts = [] + if env: + sim, n, i = env + parts.append(f"{sim}-rob#{n}") + if "velocity" in cs.params: + parts.append(f"v{cs.params['velocity']}") + if "trajectory_type" in cs.params: + parts.append(f"traj{cs.params['trajectory_type']}") + if env: + parts.append(f"iter{i}") + if not parts: + continue + new_id = "-".join(parts) + if cs.id == new_id: + continue + item.name = item.name.replace(f"[{cs.id}]", f"[{new_id}]") + item._nodeid = item._nodeid.replace(f"[{cs.id}]", f"[{new_id}]") diff --git a/tests/integration/README.md b/tests/integration/README.md index 6521b08c6..05da4c689 100644 --- a/tests/integration/README.md +++ b/tests/integration/README.md @@ -21,7 +21,7 @@ It yields `{"container": , "brought_up": bool}`. ## Collection order Integration runs after `build_docker` / `build_packages` (it needs the image + a colcon -build) and before the sim tiers — see `_MODULE_ORDER` in `conftest.py`. +build) and before the sim tiers — see `_MODULE_ORDER` in `tests/harness/collection.py`. ## Running From 0b131d62feeba78e3534ff89e5fec497b02ad8d3 Mon Sep 17 00:00:00 2001 From: John Date: Wed, 22 Jul 2026 18:30:55 -0400 Subject: [PATCH 11/12] fix(robot): pin pytest to 7.4.* so apt launch_pytest stays compatible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The builder-stage pip block pulled pytest >=8 transitively into /usr/local (copied into the runtime image), shadowing Jazzy's apt python3-pytest 7.4. pytest 8 removed the `path` argument from pytest_pycollect_makemodule, which apt's launch_pytest plugin still declares — so every pytest invocation in the robot container aborted at plugin registration. This broke `colcon test` for ament_python packages (e.g. lidar_point_cloud_filter in test_colcon_test_robot), while ament_cmake gtest packages were unaffected. Pin pytest to Jazzy's version so the container is internally consistent and launch_testing / launch_pytest remain usable for future launch-based tests. The test runner (tests/docker) is a separate interpreter and keeps its newer pytest. Co-Authored-By: Claude Opus 4.8 --- .agents/skills/docker-build-profiles/SKILL.md | 1 + robot/docker/Dockerfile.robot | 1 + 2 files changed, 2 insertions(+) diff --git a/.agents/skills/docker-build-profiles/SKILL.md b/.agents/skills/docker-build-profiles/SKILL.md index c57f85f3e..3cff9e38c 100644 --- a/.agents/skills/docker-build-profiles/SKILL.md +++ b/.agents/skills/docker-build-profiles/SKILL.md @@ -51,6 +51,7 @@ Troubleshooting notes - Jetson **`robot-l4t`** builds from **`robot-l4t-stack-base`** (`robot/docker/Dockerfile.l4t-stack-base`), not raw dustynv, so **`Dockerfile.robot` stays Ubuntu-shaped.** `airstack image-build --profile l4t robot-l4t` triggers **`robot-l4t-stack-base`** first (`airstack.sh`); bare `compose build robot-l4t` can still parallelize badly, so list stack-base explicitly if not using AirStack CLI. - **dustynv `/ros_entrypoint.sh` shadows the apt Jazzy runtime (mavros symbol-lookup crash).** The dustynv base sources a prebuilt *source* ROS at `$ROS_ROOT/install` from PID 1, prepending its older libs (e.g. `fastcdr` 2.2.5) ahead of the apt Jazzy (2.2.7) that `Dockerfile.robot` layers on top — apt-built nodes like mavros then die with symbol-lookup errors under tmux autolaunch. `Dockerfile.l4t-stack-base` neutralizes it by overwriting `/ros_entrypoint.sh` with a `exec "$@"` passthrough; shells get ROS from `/opt/ros/jazzy/setup.bash` via `.bashrc`. If a Jetson node suddenly can't resolve symbols after a base-image bump, check whether the entrypoint passthrough is still in place. - **ZED SDK version is pinned across `zed/Dockerfile.zed-l4t`** — the `ZED_SDK_URL` (e.g. `.../zedsdk/5.2/...`) and the ROS dep args (`ZED_MSGS_VERSION`, `POINTCLOUD_TRANSPORT*_VERSION`, `BACKWARD_ROS_VERSION`) must move together; a mismatched `zed_msgs` vs SDK breaks the driver build. Bumping the SDK is camera-firmware-coupled, so confirm the target camera runs that SDK line before merging. +- **`pytest` is pinned to `7.4.*` in `Dockerfile.robot` — do not remove or bump it.** The builder-stage `pip3 install` pulls `pytest` transitively into `/usr/local` (copied into the runtime image), which shadows Jazzy's apt `python3-pytest` 7.4. `pytest` 8 removed the `path` argument from the `pytest_pycollect_makemodule` hook, which apt's `launch_pytest` plugin still declares — so an unpinned (>=8) pytest aborts **every** pytest run in the container at plugin registration. That breaks `colcon test` for `ament_python` packages (e.g. `lidar_point_cloud_filter` in `test_colcon_test_robot`), while `ament_cmake` gtest packages are unaffected. Keeping the pin at Jazzy's version keeps `launch_testing` / `launch_pytest` usable for launch-based tests. The `tests/docker` runner is a separate interpreter and is free to use a newer pytest. Examples of agent prompts - "Check `robot/docker/docker-compose.yaml` for `PYTHON_VERSION` entries and quote any unquoted numeric values; open a PR with the fixes and include a test log from a builder-stage build." diff --git a/robot/docker/Dockerfile.robot b/robot/docker/Dockerfile.robot index 92fd116fe..3ff968750 100644 --- a/robot/docker/Dockerfile.robot +++ b/robot/docker/Dockerfile.robot @@ -131,6 +131,7 @@ RUN if echo "$BASE_IMAGE" | grep -qE "(nvidia|l4t)" && [ "${SKIP_TENSORRT}" != " # Note: numpy>=1.26 required for Python 3.12 compatibility # Using --ignore-installed to avoid conflicts with system packages RUN pip3 install --break-system-packages --ignore-installed \ + "pytest==7.4.*" \ empy==3.3.4 \ future \ lxml \ From cc833a300b78fbb8b6cc78549312a1a29335cfbc Mon Sep 17 00:00:00 2001 From: John Date: Thu, 23 Jul 2026 01:22:01 -0400 Subject: [PATCH 12/12] fix(isaac-sim): clear LD_LIBRARY_PATH for PX4 ubuntu.sh so ca-certificates configures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The global ENV LD_LIBRARY_PATH puts isaac-sim's bundled libs (.../isaacsim.ros2.bridge/jazzy/lib) on the linker path. Its older libcrypto.so.3 shadows the system one, so when the updated ca-certificates (20240203 → 20260601~24.04.1) runs its postinst `openssl`, it fails with `version 'OPENSSL_3.0.9' not found`, aborting the apt transaction and failing the isaac-sim image build (PX4 Tools/setup/ubuntu.sh, exit 100). Clear LD_LIBRARY_PATH for that RUN only so apt/openssl use the system libcrypto; the global ENV still applies to every other layer. Environmental break (new ca-certificates × isaac-sim's stale bundled openssl) — not a code regression. Co-Authored-By: Claude Opus 4.8 --- simulation/isaac-sim/docker/Dockerfile.isaac-ros | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/simulation/isaac-sim/docker/Dockerfile.isaac-ros b/simulation/isaac-sim/docker/Dockerfile.isaac-ros index 0dca11fb7..92ac63fdf 100644 --- a/simulation/isaac-sim/docker/Dockerfile.isaac-ros +++ b/simulation/isaac-sim/docker/Dockerfile.isaac-ros @@ -155,8 +155,10 @@ RUN sed -i \ /isaac-sim/PX4-Autopilot/ROMFS/px4fmu_common/init.d-posix/px4-rc.simulator # install px4 dependencies and build +# LD_LIBRARY_PATH= so apt/openssl use the system libcrypto, not isaac-sim's older +# bundled one (which breaks the ca-certificates postinst). Cleared for this step only. RUN cd PX4-Autopilot && \ - DEBIAN_FRONTEND=noninteractive ./Tools/setup/ubuntu.sh + LD_LIBRARY_PATH= DEBIAN_FRONTEND=noninteractive ./Tools/setup/ubuntu.sh # build px4 RUN cd PX4-Autopilot && \ make px4_sitl