CI/CD Tuning PR - pytest collection bug fix

.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/), 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.
+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 components.
 license: MIT
 metadata:
   author: AirLab CMU
@@ -15,17 +15,17 @@ Use this skill when:
 
 - Adding Python unit tests for a ROS 2 package (perception, sensors, local, global, behavior, interface)
 - Adding C++ unit tests (`gtest`) to a package already using `ament_cmake`
-- Extending unit tests to sim-side Python (`tests/sim/`) or GCS modules (`tests/gcs/`)
-- Verifying that `airstack test -m unit` and `pytest tests/` (CI) pick up your new tests
+- Extending unit tests to sim-side Python (`simulation/**/<extension>/test/`)
+- Verifying that `airstack test -m unit` picks up your new tests
 
 For system tests (full Docker stack, sim, sensors, takeoff/hover/land) see the
 `run-system-tests` skill instead.
 
 ## Architecture Overview
 
 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.
+`tests/colcon_unit_test_packages.yaml` lists which packages have unit tests, and the root
+harness collects them from there — you only edit files under the package itself.
 
 ```
 robot/ros_ws/src/<layer>/<package>/
@@ -47,10 +47,37 @@ auto-tagged `@pytest.mark.unit` by path, so `-m unit` selects it. ament lint fil
 
 | Invocation | What runs |
 |---|---|
-| `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/` (incl. linters + C++) |
+| `airstack test -m unit` | Package `test/test_*.py`, collected directly from source |
+| `cd tests && pytest -m unit` | Same path — the containerless equivalent |
+| `pytest tests/ -m unit` | Same path — what CI runs |
+| `colcon test --packages-select <pkg>` | C++ gtests and linters; Python only for `ament_python` packages (see below) |
+
+**Two runners, split by language — because C++ needs a build and Python does not.** A
+gtest is a binary: it must be compiled against the package's headers and rclcpp, so it can
+only run where the ROS toolchain is. That is `colcon test` inside the robot container,
+which CI reaches via the **`build_packages`** mark
+(`tests/system/test_build_packages.py::test_colcon_test_robot`, which builds with
+`-DBUILD_TESTING=ON` first). Python unit tests are deliberately hermetic — they stub ROS
+at the import boundary and touch no ROS runtime — so they need no build and no container,
+which is what lets the root harness run all of them in about a second.
+
+Preserve that property when adding tests: a Python test that needs a live ROS node belongs
+in `tests/integration/` or `tests/system/`, not here.
+
+Whether `colcon test` *also* picks up a package's Python tests depends on its build type:
+
+| Package | Build type | Python tests under `colcon test` |
+|---|---|---|
+| `natnet_ros2` | `ament_cmake` | **No** — `CMakeLists.txt` registers `ament_add_gtest` but no `ament_add_pytest_test` |
+| `lidar_point_cloud_filter` | `ament_python` | **Yes** — `setup.cfg` sets `testpaths = test`, so colcon's pytest runner finds them |
+
+So a Python test in an `ament_cmake` package runs *only* via the root harness — which is
+fine, since that is what CI invokes.
+
+Naming a path *below* `tests/` narrows the run and skips the injection, so
+`pytest tests/system/test_x.py` stays fast and does not drag in unit tests. The rule lives
+in `harness.discovery.collection_is_broad` and is pinned by
+`tests/meta/test_collection_contract.py`.
 
 ## Step-by-Step: Adding a Python Unit Test
 
@@ -87,13 +114,17 @@ if str(_src) not in sys.path:
 from my_module import my_function  # noqa: E402
 
 
-@pytest.mark.unit
 def test_my_function_basic():
     assert my_function(1, 2) == 3
 ```
 
 **Key points:**
-- Always decorate with `@pytest.mark.unit` — this is the filter for fast runs.
+- **Do not write `@pytest.mark.unit`.** `pytest_itemcollected` in `tests/conftest.py`
+  applies it by file location to everything under a registered package's `test/` dir.
+  Writing it by hand is redundant, and it warns (`PytestUnknownMarkWarning`) under any
+  invocation where `tests/pytest.ini` is not the configfile — e.g. `colcon test`.
+- Import `pytest` only if you need its API (`approx`, `raises`, `parametrize`,
+  `importorskip`).
 - Compute paths relative to `__file__` (`parent.parent / "src"`) — never hardcode
   absolute paths.
 - For packages with a Python module directory (`<pkg>/<pkg>/`), add the package
@@ -127,35 +158,40 @@ robot:
     - natnet_ros2
     - lidar_point_cloud_filter
     - <your_package>          # ← add here
-  pytest_args: "-m not linter"
+  pytest_args: []
 ```
 
+Leave `pytest_args` empty. It is forwarded to `colcon test` via `PYTEST_ADDOPTS`, and
+ament's pytest runner ignores `-m` there — a marker expression in this field silently
+does nothing.
+
 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
+### 4. Run locally to verify
 
 ```bash
-# From repo root — no container needed
-cd tests
-pytest -m unit -v
-# or
 airstack test -m unit -v
+# or, containerless:
+AIRSTACK_ROOT=$(pwd) pytest tests/ -m unit -v
 ```
 
-All 14+ existing tests plus your new ones should pass. Collected items point straight
+All 155 existing tests plus your new ones should pass. Collected items point straight
 at the co-located source:
 ```
 ../robot/ros_ws/src/<layer>/<package>/test/test_<name>.py::test_my_function_basic PASSED
 ```
 
-### 6. CI picks it up automatically
+### 5. Running in CI
 
-Unit tests are discovered by `pytest tests/` and run as part of `system-tests.yml`
-(triggered on PR open) — no changes to CI needed.
+`unit-tests.yml` invokes `pytest tests/ -m unit` on GitHub-hosted `ubuntu-latest`
+whenever a PR targeting `main` or `develop` is opened, synchronized, or reopened.
+It does not consume an OSMO GPU.
+C++ gtests still run through the OSMO `build_packages` mark because they require the
+ROS workspace and toolchain inside the robot container.
 
 ---
 
@@ -242,11 +278,11 @@ sim:
 | Where does test source live? | `<component>/…/<package>/test/` (co-located with the package) |
 | 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` |
+| What mark do all unit tests use? | `@pytest.mark.unit` — auto-applied by path in `conftest.py`; do not write it yourself |
+| How do I run them? | `airstack test -m unit`, `cd tests && pytest -m unit`, or `pytest tests/ -m unit` |
+| What CI workflow runs them? | Python: `unit-tests.yml`; C++: the `build_packages` path in `system-tests.yml` — see §5 |
 | 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 |
+| Does `colcon test` also run these? | Only if the package registers them. `ament_add_gtest` covers C++; a Python test needs `ament_add_pytest_test`, which `natnet_ros2` does **not** have — its Python tests run only under the root harness |
 | Can I add pure C++ gtests? | Yes — `ament_add_gtest` in CMakeLists.txt |
 
 ## Reference Implementations
@@ -261,8 +297,8 @@ 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 + `--import-mode=importlib`
+- `.airstack/modules/dev.sh` — what `airstack test` runs (bare `pytest` with `working_dir` `tests/`)
+- `tests/pytest.ini` — mark registration + `--import-mode=importlib` + `testpaths`
 - `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

.agents/skills/bump-version-and-release/SKILL.md

@@ -260,5 +260,5 @@ For a true release (dropping the pre-release suffix):
 
 ## Related Skills
 
-- [`run-system-tests`](../run-system-tests) — what fires on every PR alongside the version check
+- [`run-system-tests`](../run-system-tests) — automatic unit/package gates and how to request simulation campaigns
 - [`update-documentation`](../update-documentation) — for docs-only PRs that may still need a VERSION bump to clear the gate

.agents/skills/run-system-tests/SKILL.md

@@ -1,6 +1,6 @@
 ---
 name: run-system-tests
-description: Run, interpret, and extend AirStack's pytest system test suite (build_packages, build_docker, liveliness, sensors, takeoff_hover_land, autonomy), trigger runs via /pytest PR comments, and read metrics.json regression reports. Use for invoking tests, debugging failures from results.xml/metrics.json, or adding a new system test.
+description: Run, interpret, and extend AirStack's pytest system test suite (build_packages, build_docker, liveliness, sensors, takeoff_hover_land, autonomy), trigger runs via /pytest PR comments, and read run_meta.json/metrics.json reports. Use for invoking tests, distinguishing infrastructure failures from policy regressions, or adding a new system test.
 license: Apache-2.0
 metadata:
   author: AirLab CMU
@@ -14,7 +14,7 @@ metadata:
 Use this skill when you need to:
 
 - Invoke the pytest system tests locally (via `airstack test`) or on CI (via `/pytest` PR comment or `workflow_dispatch`)
-- Diagnose a failing system test — interpret `summary.txt`, `results.xml`, and `metrics.json` from `tests/results/<timestamp>/`
+- Diagnose a failing system test — interpret `summary.txt`, `results.xml`, `run_meta.json`, and `metrics.json` from `tests/results/<timestamp>/`
 - Compare metrics against a baseline run (`parse_metrics.py --baseline`) to confirm a regression or improvement
 - Add a new system test to `tests/`: pick the right mark, wire up `airstack_env` parametrization, and record metrics with `MetricsRecorder`
 
@@ -33,17 +33,17 @@ The suite lives at `tests/` (repo root) and is fully pytest-based. Configuration
 | Concern | Unit (`-m unit`) | System (`-m liveliness` etc.) |
 |---|---|---|
 | 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` |
+| CI workflow | `unit-tests.yml` (`ubuntu-latest`) | `system-tests.yml` (ephemeral OSMO GPU pod) |
+| Trigger | PR opened, synchronized, or reopened | Automatic `build_packages` on PR open/update/reopen; simulation via `/pytest` or `workflow_dispatch` |
 | 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:
 
 ```bash
 airstack test -m unit -v
-# or
-pytest tests/ -m unit -v   # AIRSTACK_ROOT=$(pwd) for direct pytest
+# or directly:
+AIRSTACK_ROOT=$(pwd) pytest tests/ -m unit -v
 ```
 
 For details on the co-located layout and adding new unit tests, see the
@@ -180,7 +180,10 @@ Total parametrize cardinality for sim tests = `len(sims) × len(num_robots) × s
 
 The `system-tests.yml` workflow accepts three trigger paths:
 
-1. **PR opened** (same-repo only) — auto-runs pytest with conftest defaults. Fork PRs are skipped to keep arbitrary code off the self-hosted runner.
+1. **PR opened, synchronized, or reopened** (same-repo only) — auto-runs the
+   `build_packages` mark. Fork PRs are skipped to keep arbitrary code off the
+   privileged self-hosted runner. Python unit tests run separately in
+   `unit-tests.yml`, including for fork PRs.
 2. **`/pytest` issue comment** on a PR — only honored from users with `OWNER`, `MEMBER`, or `COLLABORATOR` author association. Fork PRs are explicitly rejected by the `Resolve PR head` step (the PR's head repo must equal `${context.repo.owner}/${context.repo.repo}`).
 3. **`workflow_dispatch`** — manual run from the Actions tab with form inputs (`marks`, `sim`, `num_robots`, `stress_iterations`, `stable_duration`, `baseline_run_id`).
 
@@ -205,14 +208,14 @@ notes: testing the new altitude controller
 The workflow:
 1. Posts an acknowledgment PR comment showing the resolved `pytest tests/ <args>` command and a link to the run
 2. Opens an in-progress GitHub Check Run on the PR's head SHA so the run shows up in the **Checks** tab (issue_comment events otherwise associate runs with the default branch)
-3. Runs pytest on a freshly-spawned ephemeral OpenStack runner (`runs-on: [self-hosted, airstack-ephemeral]`)
+3. Runs pytest on a freshly-spawned ephemeral OSMO GPU pod (`runs-on: [self-hosted, airstack-ephemeral]`)
 4. Uploads `tests/results/` as artifact `test-results-<sha>-<run_id>` (90-day retention)
-5. The downstream `report` job runs `parse_metrics.py` against the latest baseline artifact from the PR's base branch and posts a markdown table back as a PR comment + job summary
+5. The downstream `report` job runs `parse_metrics.py`, compares only a matching complete simulation baseline, posts the result, and finalizes the PR-head Check Run
 6. Closes the Check Run with the final conclusion
 
 ### Why fork PRs are blocked
 
-The runner is GPU-equipped, has Docker root access, and is reused (briefly) across the lifetime of one job. Running arbitrary fork code on it would let a contributor exfiltrate registry creds, mine crypto, or pivot into the OpenStack tenant. The same-repo guard is the only line of defense and **must not be removed**. If you need to test a fork PR, mirror the branch into the upstream repo first.
+The runner is GPU-equipped, has Docker root access, and is reused (briefly) across the lifetime of one job. Running arbitrary fork code on it would let a contributor exfiltrate registry creds, mine crypto, or abuse the privileged OSMO CI pool. The same-repo guard is the only line of defense and **must not be removed**. If you need to test a fork PR, mirror the branch into the upstream repo first.
 
 ## Interpreting Results and Metrics
 
@@ -273,7 +276,7 @@ The report has three sections per test module:
 - **Sim publishing rates** — pivoted Hz aggregates per topic (`mean`, `start_mean`, `end_mean`, `min`, `max`) from the `sensors` mark (sim + robot streams)
 - **Compute usage** — pivoted CPU/mem/GPU per container
 
-Regressions exceeding `--threshold` (default 20%) are flagged `:red_circle:`; improvements beyond threshold get `:green_circle:`. CI fails the job on any regression.
+Regressions exceeding `--threshold` (default 20%) are flagged `:red_circle:`; improvements beyond threshold get `:green_circle:`. CI fails only when both artifacts are complete and have the same simulation campaign fingerprint.
 
 When local-debugging a CI regression, download both artifacts (`test-results-<sha>-<run_id>` from the PR run and from the base branch's most recent run), unzip them under `tests/results/`, and run `parse_metrics.py` locally to see the same table the bot posted.
 
@@ -360,7 +363,7 @@ If multiple tests need the same setup, add a fixture in `conftest.py` (not in yo
 - **Letting parametrize cardinality explode**. Default `--num-robots 1,3` (and `--sim msairsim` if you opt in) multiplies stack bring-ups for each selected mark (`liveliness`, `sensors`, `takeoff_hover_land`, …) — expensive. Override locally to a single tuple while iterating. `--sim` defaults to `isaacsim` only.
 - **Hardcoded container names**. Always use `find_container`, `get_robot_containers`, or `wait_for_container` — replica suffixes (`-1`, `-2`, `-3`) and compose project prefixes change.
 - **Asserting on stdout instead of using `read_log_tail`**. The conftest captures each subprocess's combined stdout/stderr in memory; assertions should reference it via `read_log_tail()` (`f"airstack up failed:\n{read_log_tail()}"`) so failures attach the relevant context to the JUnit XML.
-- **Trying to SSH into a CI runner mid-job**. Workers are ephemeral OpenStack VMs destroyed within ~30s of job completion. Re-running the job creates a fresh VM. For genuine debugging on the runner, see `.github/orchestrator/README.md` (also exposed at `tests/ci-cd-orchestrator.md`) — but in 99% of cases, reproduce locally with `airstack test`.
+- **Trying to SSH into a CI runner mid-job**. Workers are ephemeral OSMO pods destroyed after job completion. Re-running creates a fresh pod. For genuine runner debugging, see `.github/orchestrator/README.md` (also exposed at `tests/ci-cd-orchestrator.md`) — but in most cases, reproduce locally with `airstack test`.
 - **Forgetting to register a new mark**. Adding `@pytest.mark.my_new_mark` without updating `tests/pytest.ini` produces "PytestUnknownMarkWarning" and makes `-m my_new_mark` fail to filter as expected.
 
 ## Quick Reference
@@ -424,12 +427,12 @@ python tests/parse_metrics.py \
 ### Files to know
 
 - `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/harness/` — helpers split by concern: `session`, `discovery`, `commands`, `containers`, `metrics` (`MetricsRecorder`), `run_meta`, `test_ids`, `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)
 - `.github/workflows/system-tests.yml` — CI workflow with `/pytest` comment trigger
-- `.github/orchestrator/README.md` — ephemeral OpenStack runner setup and SSH-debug procedure
+- `.github/orchestrator/README.md` — ephemeral OSMO runner setup and worker-debug procedure
 
 ## References
 

.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.16"
+VERSION="0.19.0-alpha.17"
 # 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.