Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 49 additions & 90 deletions .agents/skills/add-unit-tests/SKILL.md
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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/<layer>/<package>/
├── src/ # production source (Python or C++)
├── test/
│ ├── test_<name>.py # ← unit test SOURCE (canonical location)
│ ├── test_<name>.py # ← unit test SOURCE (collected directly)
│ ├── test_<name>.cpp # ← C++ gtest SOURCE (optional)
│ └── fake_<name>.hpp # ← C++ test doubles (optional)
└── CMakeLists.txt # wires ament_add_gtest under BUILD_TESTING

tests/robot/<layer>/<package>/
└── test_<name>.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 <pkg>` | Real test in `package/test/` directly |
| `colcon test --packages-select <pkg>` | Real test in `package/test/` (incl. linters + C++) |

## Step-by-Step: Adding a Python Unit Test

Expand Down Expand Up @@ -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/<layer>/<package>/test_<name>.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 <package> 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/<layer>/<package>/test/test_<name>.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/<layer>/<package>/test"
_real_file = _pkg_test / "test_<name>.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_<name> 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("_<package>_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
- <your_package> # ← add here
pytest_args: "-m not linter"
```

**Counting `parents[N]` to reach the repo root:**

| Proxy location | `parents[N]` for repo root |
|---|---|
| `tests/robot/<layer>/<package>/` | `parents[4]` |
| `tests/sim/<tool>/` | `parents[3]` |
| `tests/gcs/<package>/` | `parents[3]` |

### 4. Ensure the tests/ directory structure exists

```bash
mkdir -p tests/robot/<layer>/<package>
touch tests/robot/<layer>/<package>/__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/**/<your_package>/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

Expand All @@ -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/<layer>/<package>/test_<name>.py::test_my_function_basic
<- ../robot/ros_ws/src/<layer>/<package>/test/test_<name>.py PASSED
../robot/ros_ws/src/<layer>/<package>/test/test_<name>.py::test_my_function_basic PASSED
```

### 6. CI picks it up automatically
Expand All @@ -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/`

Expand Down Expand Up @@ -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 `tests/harness/discovery.py` (`_WORKSPACE_PKG_TEST_GLOBS`): `robot` →
`robot/ros_ws/src/**/<pkg>/test`, `sim` → `simulation/**/<pkg>/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:
- <isaac_extension_name> # → simulation/**/<ext>/test collected directly
```
simulation/.../<tool>/test/test_<name>.py ← source
tests/sim/<tool>/test_<name>.py ← proxy (parents[3] = repo root)
```

**GCS modules**:
```
gcs/.../<pkg>/test/test_<name>.py ← source
tests/gcs/<pkg>/test_<name>.py ← proxy (parents[3] = repo root)
```

`pytest tests/ -m unit` discovers them through the proxy without any
pytest.ini or CI changes needed.

---

Expand All @@ -279,14 +240,14 @@ pytest.ini or CI changes needed.
| Concern | Answer |
|---|---|
| Where does test source live? | `<component>/…/<package>/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

Expand All @@ -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
1 change: 1 addition & 0 deletions .agents/skills/docker-build-profiles/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
16 changes: 9 additions & 7 deletions .agents/skills/run-system-tests/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 | `<pkg>/test/test_*.py` (proxied via `tests/robot/`) | `tests/system/` |
| Source location | `<pkg>/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:
Expand All @@ -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 |
Expand Down Expand Up @@ -80,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).
Expand Down Expand Up @@ -294,7 +295,7 @@ If your test...
- File: `tests/system/test_<short_descriptor>.py` — matches pytest's default test discovery (`test_*.py`) under the system suite
- Class: `Test<CamelCase>` with the mark applied at the class level: `@pytest.mark.<mark>`
- Add a class-level `@pytest.mark.timeout(<seconds>)` — 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`

Expand All @@ -304,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 |
|--------|---------|
Expand Down Expand Up @@ -421,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)
Expand Down
2 changes: 1 addition & 1 deletion .env
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading
Loading