From 7d2b4dae9d71c4e7e6817bff9e924320d39d34dc Mon Sep 17 00:00:00 2001 From: Pranav Kumara Date: Fri, 21 Aug 2026 17:23:32 -0400 Subject: [PATCH 1/6] Make system-test outcomes trustworthy and actionable Separate infrastructure failures, test assertions, and advisory metric changes so CI blocks only on real test or integrity failures while preserving comparable performance evidence. --- .agents/skills/run-system-tests/SKILL.md | 16 +- .github/workflows/system-tests.yml | 132 +++++++++---- .github/workflows/unit-tests.yml | 2 + airstack.sh | 28 ++- .../development/intermediate/testing/ci_cd.md | 25 +-- tests/README.md | 41 +++-- tests/conftest.py | 26 ++- tests/harness/__init__.py | 6 +- tests/harness/baseline.py | 44 +++++ tests/harness/commands.py | 2 +- tests/harness/diagnostics.py | 126 +++++++++++++ tests/harness/image_prep.py | 66 +++++++ tests/harness/run_meta.py | 144 +++++++++++++-- tests/harness/session.py | 14 +- tests/harness/sim.py | 25 ++- tests/harness/test_ids.py | 30 ++- .../meta/test_campaign_reporting_contract.py | 173 ++++++++++++++++++ tests/meta/test_collection_contract.py | 2 +- tests/meta/test_diagnostics_contract.py | 66 +++++++ tests/meta/test_launch_intent_contract.py | 27 +-- tests/meta/test_workflow_contract.py | 63 +++++++ tests/parse_metrics.py | 34 ++-- tests/report-requirements.txt | 3 + tests/run_summary.py | 22 ++- tests/system/test_liveliness.py | 53 +++++- tests/system/test_optitrack_e2e.py | 48 ++++- tests/system/test_sensors.py | 38 +++- 27 files changed, 1104 insertions(+), 152 deletions(-) create mode 100644 tests/harness/baseline.py create mode 100644 tests/harness/diagnostics.py create mode 100644 tests/harness/image_prep.py create mode 100644 tests/meta/test_campaign_reporting_contract.py create mode 100644 tests/meta/test_diagnostics_contract.py create mode 100644 tests/meta/test_workflow_contract.py create mode 100644 tests/report-requirements.txt diff --git a/.agents/skills/run-system-tests/SKILL.md b/.agents/skills/run-system-tests/SKILL.md index c2260d29e..caad3f7ec 100644 --- a/.agents/skills/run-system-tests/SKILL.md +++ b/.agents/skills/run-system-tests/SKILL.md @@ -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`, `run_meta.json`, and `metrics.json` from `tests/results//` +- Diagnose a failing system test — interpret `summary.txt`, `results.xml`, schema-v2 `run_meta.json`, `metrics.json`, and bounded `diagnostics/` from `tests/results//` - 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` @@ -210,7 +210,7 @@ The workflow: 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 OSMO GPU pod (`runs-on: [self-hosted, airstack-ephemeral]`) 4. Uploads `tests/results/` as artifact `test-results--` (90-day retention) -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 +5. The downstream `report` job selects the newest matching complete simulation baseline, runs `parse_metrics.py`, posts the advisory comparison, and finalizes the PR-head Check Run 6. Closes the Check Run with the final conclusion ### Why fork PRs are blocked @@ -262,7 +262,7 @@ Keys follow `test_node_id → metric_key → {value, unit, direction, ...}`. Tim # Single-run report — markdown table, exits 0 always python tests/parse_metrics.py --current tests/results/2025-04-21_14-30-00/ -# Diff mode — side-by-side, exits 1 on regression +# Comparison mode — side-by-side; numeric deltas are advisory python tests/parse_metrics.py \ --current tests/results/2025-04-21_14-30-00/ \ --baseline tests/results/2025-04-20_09-00-00/ \ @@ -276,7 +276,11 @@ 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 only when both artifacts are complete and have the same simulation campaign fingerprint. +Changes exceeding `--threshold` (default 20%) are flagged `:red_circle:` or +`:green_circle:` for review. Numeric deltas never fail CI. Pytest assertions, +infrastructure/prerequisite failures, missing artifacts, and report-parser +errors remain blocking. The fingerprint includes normalized tests and all +behavior-changing campaign options. When local-debugging a CI regression, download both artifacts (`test-results--` 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. @@ -427,9 +431,9 @@ 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`), `run_meta`, `test_ids`, `sim`, `collection` (ordering) +- `tests/harness/` — helpers split by concern: `session`, `discovery`, `commands`, `containers`, `metrics` (`MetricsRecorder`), `run_meta`, `baseline`, `diagnostics`, `test_ids`, `sim`, `collection` (ordering) - `tests/pytest.ini` — mark registration, log format -- `tests/parse_metrics.py` — markdown reporter, regression diff +- `tests/parse_metrics.py` — markdown reporter and advisory comparison - `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 OSMO runner setup and worker-debug procedure diff --git a/.github/workflows/system-tests.yml b/.github/workflows/system-tests.yml index 48ab4f639..c6cce0d4f 100644 --- a/.github/workflows/system-tests.yml +++ b/.github/workflows/system-tests.yml @@ -58,8 +58,8 @@ jobs: startsWith(github.event.comment.body, '/pytest') && contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association)) concurrency: - group: system-tests-${{ github.event_name == 'pull_request' && github.event.pull_request.number || github.run_id }} - cancel-in-progress: ${{ github.event_name == 'pull_request' }} + group: system-tests-${{ github.event.pull_request.number || github.event.issue.number || github.run_id }} + cancel-in-progress: true timeout-minutes: 120 # Adding any `permissions:` entry disables GITHUB_TOKEN's defaults, so # every scope used here has to be re-granted explicitly: @@ -111,7 +111,6 @@ jobs: core.setOutput('base_ref', pr.data.base.ref); - name: Resolve tested revision identity - if: always() id: identity env: EVENT_NAME: ${{ github.event_name }} @@ -121,7 +120,11 @@ jobs: EVENT_PR_NUMBER: ${{ github.event.pull_request.number }} run: | if [[ "$EVENT_NAME" == "issue_comment" ]]; then - echo "tested_sha=${COMMENT_HEAD_SHA:-$EVENT_SHA}" >> "$GITHUB_OUTPUT" + if [[ -z "$COMMENT_HEAD_SHA" ]]; then + echo "::error::Refusing /pytest run: PR head SHA was not resolved." + exit 1 + fi + echo "tested_sha=$COMMENT_HEAD_SHA" >> "$GITHUB_OUTPUT" echo "pr_number=$COMMENT_PR_NUMBER" >> "$GITHUB_OUTPUT" else echo "tested_sha=$EVENT_SHA" >> "$GITHUB_OUTPUT" @@ -369,17 +372,38 @@ jobs: SIM_INPUT: ${{ steps.parse.outputs.sim }} NO_IMAGE_BUILD: ${{ steps.parse.outputs.no_image_build }} run: | + mkdir -p tests/results + image_result=tests/results/image-preparation.json + image_outcome=already-present + pulled=() + retagged=() + built=() + missing=() profiles=desktop [[ ",$SIM_INPUT," == *,msairsim,* ]] && profiles="$profiles,ms-airsim" [[ ",$SIM_INPUT," == *,isaacsim,* ]] && profiles="$profiles,isaac-sim" export COMPOSE_PROFILES="$profiles" echo "Pulling images for COMPOSE_PROFILES=$COMPOSE_PROFILES (no_image_build=$NO_IMAGE_BUILD)" + declare -A present_before=() + while IFS= read -r img; do + [[ -z "$img" ]] && continue + if docker image inspect "$img" --format '{{.Id}}' >/dev/null 2>&1; then + present_before["$img"]=1 + fi + done < <(docker compose -f docker-compose.yaml config --images) # Pull from registry; tolerate per-image failures so we can detect # what's still missing afterwards instead of aborting on the first # gap. `--progress=quiet` suppresses per-layer progress; errors # still surface on stderr. ./airstack.sh --progress=quiet image-pull --ignore-pull-failures || true + while IFS= read -r img; do + [[ -z "$img" || -n "${present_before[$img]:-}" ]] && continue + if docker image inspect "$img" --format '{{.Id}}' >/dev/null 2>&1; then + pulled+=("$img") + image_outcome=pulled-versioned + fi + done < <(docker compose -f docker-compose.yaml config --images) # VERSION tags miss on every PR. Seed from floating cache_* tags. cache_tag="$(grep -E '^CACHE_TAG=' .env 2>/dev/null | cut -d= -f2 | tr -d '"' || true)" @@ -395,6 +419,8 @@ jobs: if docker pull --quiet "$cache_img"; then docker tag "$cache_img" "$img" echo "Retagged $cache_img -> $img" + retagged+=("$img") + image_outcome=cache-retagged else echo "Cache tag pull failed for $cache_img" fi @@ -412,18 +438,38 @@ jobs: echo "Images still missing after pull/retag:" printf ' - %s\n' "${missing[@]}" if [[ "$NO_IMAGE_BUILD" == "true" ]]; then + IMAGE_OUTCOME=missing IMAGE_RESULT="$image_result" \ + MISSING_IMAGES="$(printf '%s\n' "${missing[@]}")" \ + PYTHONPATH=tests python3 -m harness.image_prep "$image_result" echo "::error::Pull-only mode (--no-image-build or -m build_packages) will not image-build. Run /pytest -m build_docker once, or omit --no-image-build." exit 1 fi echo "Falling back to image-build" ./airstack.sh --progress=quiet image-build + built=("${missing[@]}") + image_outcome=locally-built else echo "All required images present after pull/retag — skipping build." fi + IMAGE_OUTCOME="$image_outcome" IMAGE_RESULT="$image_result" \ + PULLED_IMAGES="$(printf '%s\n' "${pulled[@]}")" \ + RETAGGED_IMAGES="$(printf '%s\n' "${retagged[@]}")" \ + BUILT_IMAGES="$(printf '%s\n' "${built[@]}")" \ + PYTHONPATH=tests python3 -m harness.image_prep "$image_result" + echo "### Image preparation: $image_outcome" >> "$GITHUB_STEP_SUMMARY" + + - name: Record test-owned image preparation + if: ${{ steps.parse.outputs.skip_image_prep == 'true' }} + run: | + IMAGE_OUTCOME=delegated-to-build-docker PYTHONPATH=tests \ + python3 -m harness.image_prep tests/results/image-preparation.json + echo "### Image preparation: delegated to build_docker tests" >> "$GITHUB_STEP_SUMMARY" - name: Run tests env: AIRSTACK_ROOT: ${{ github.workspace }} + AIRSTACK_TESTED_SHA: ${{ steps.identity.outputs.tested_sha }} + AIRSTACK_PR_NUMBER: ${{ steps.identity.outputs.pr_number }} DISPLAY: "" PYTEST_ARGS: ${{ steps.parse.outputs.pytest_args }} run: | @@ -486,6 +532,7 @@ jobs: if: > always() && needs.run-tests.result != 'skipped' && + needs.run-tests.outputs.tested_sha != '' && (needs.run-tests.result != 'cancelled' || github.event_name != 'pull_request') permissions: actions: read @@ -505,7 +552,7 @@ jobs: python-version: "3.12" - name: Install report dependencies - run: pip install tabulate + run: pip install -r tests/report-requirements.txt - name: Resolve PR base branch if: github.event_name == 'issue_comment' || github.event_name == 'pull_request' @@ -531,15 +578,19 @@ jobs: # the PR's base branch (e.g. develop or main). - name: Download baseline results (PR) if: github.event_name == 'issue_comment' || github.event_name == 'pull_request' - uses: dawidd6/action-download-artifact@v6 - continue-on-error: true - with: - workflow: system-tests.yml - branch: ${{ steps.pr_ctx.outputs.base_ref }} - name_is_regexp: true - name: "test-results-.*" - path: baseline-results/ - if_no_artifact_found: warn + env: + GH_TOKEN: ${{ github.token }} + BASE_REF: ${{ steps.pr_ctx.outputs.base_ref }} + run: | + mkdir -p baseline-results + gh api --method GET \ + "repos/${{ github.repository }}/actions/workflows/system-tests.yml/runs" \ + -f branch="$BASE_REF" -f status=success -f per_page=20 \ + --jq '.workflow_runs[].id' | + while read -r run_id; do + gh run download "$run_id" --repo "${{ github.repository }}" \ + --pattern "test-results-*" --dir "baseline-results/$run_id" || true + done # Manual dispatch with explicit baseline run ID - name: Download baseline results (manual, explicit run ID) @@ -560,29 +611,38 @@ jobs: if: > github.event_name == 'workflow_dispatch' && inputs.baseline_run_id == '' - uses: dawidd6/action-download-artifact@v6 - continue-on-error: true - with: - workflow: system-tests.yml - branch: main - name_is_regexp: true - name: "test-results-.*" - path: baseline-results/ - if_no_artifact_found: warn + env: + GH_TOKEN: ${{ github.token }} + run: | + mkdir -p baseline-results + gh api --method GET \ + "repos/${{ github.repository }}/actions/workflows/system-tests.yml/runs" \ + -f branch=main -f status=success -f per_page=20 \ + --jq '.workflow_runs[].id' | + while read -r run_id; do + gh run download "$run_id" --repo "${{ github.repository }}" \ + --pattern "test-results-*" --dir "baseline-results/$run_id" || true + done - name: Locate result directories id: dirs - # Find the dir holding results.xml. Nesting depth differs by downloader: - # actions/download-artifact@v4 (single name) extracts straight into the - # path, while dawidd6/action-download-artifact@v6 with name_is_regexp - # wraps each artifact in a subdir named after it. `find` handles both. + # Find the current dir, then choose a baseline from the recent-run + # candidate tree by completed campaign fingerprint. run: | CURRENT_XML=$(find current-results/ -name results.xml 2>/dev/null | sort -r | head -1) [ -n "$CURRENT_XML" ] && echo "current=$(dirname "$CURRENT_XML")" >> "$GITHUB_OUTPUT" - BASELINE_XML=$(find baseline-results/ -name results.xml 2>/dev/null | sort -r | head -1) - if [ -n "$BASELINE_XML" ]; then - echo "baseline=$(dirname "$BASELINE_XML")" >> "$GITHUB_OUTPUT" + if [ -n "$CURRENT_XML" ] && [ -d baseline-results ]; then + CURRENT_DIR="$(dirname "$CURRENT_XML")" + BASELINE=$(PYTHONPATH=tests python3 - "$CURRENT_DIR" <<'PYEOF' + import sys + from pathlib import Path + from harness.baseline import select_baseline_path + selected = select_baseline_path(Path(sys.argv[1]), Path("baseline-results")) + print(selected or "") + PYEOF + ) + echo "baseline=$BASELINE" >> "$GITHUB_OUTPUT" else echo "baseline=" >> "$GITHUB_OUTPUT" fi @@ -602,8 +662,8 @@ jobs: Pass-rate and regression tables are suppressed because no completed test campaign is available. EOF - echo "parser_exit=0" >> "$GITHUB_OUTPUT" - exit 0 + echo "parser_exit=2" >> "$GITHUB_OUTPUT" + exit 2 fi set +e @@ -653,14 +713,10 @@ jobs: echo "_No metrics report generated._" >> "$GITHUB_STEP_SUMMARY" fi - - name: Fail on regression + - name: Fail on report integrity error if: steps.report.outcome == 'failure' run: | - if [ "${{ steps.report.outputs.parser_exit }}" = "1" ]; then - echo "::error::Metric regression detected — see the report above for details." - else - echo "::error::Metrics report generation failed — see the report step log." - fi + echo "::error::Metrics report generation failed — see the report step log." exit 1 - name: Finalize check on PR head diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index a2bf33022..fd4a0b002 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -35,6 +35,8 @@ jobs: - name: Run unit tests env: AIRSTACK_ROOT: ${{ github.workspace }} + AIRSTACK_TESTED_SHA: ${{ github.sha }} + AIRSTACK_PR_NUMBER: ${{ github.event.pull_request.number }} run: pytest tests/ -m unit - name: Upload unit-test results diff --git a/airstack.sh b/airstack.sh index eba64b112..2624c73af 100755 --- a/airstack.sh +++ b/airstack.sh @@ -927,6 +927,7 @@ function parse_launch_intent { AIRSTACK_INTENT_PLAY="" AIRSTACK_INTENT_AUTOLAUNCH="" AIRSTACK_DRY_RUN="" + AIRSTACK_CONFIG_ONLY="" AIRSTACK_UP_WAIT="" local args=("$@") i=0 a @@ -945,6 +946,7 @@ function parse_launch_intent { # NOTE: shadows compose's own `up --dry-run`; ours validates the # derived launch config and exits without starting services. --dry-run) AIRSTACK_DRY_RUN="1";; + --config-only) AIRSTACK_CONFIG_ONLY="1"; AIRSTACK_DRY_RUN="1";; *) _rest_out+=("$a");; esac i=$((i+1)) @@ -1114,14 +1116,23 @@ function preflight_up { fi # 4. Files the isaac-sim service hard-requires - if [ ! -f "$PROJECT_ROOT/simulation/isaac-sim/docker/omni_pass.env" ]; then - _pf_error "simulation/isaac-sim/docker/omni_pass.env is missing (Nucleus credentials). Run 'airstack setup' to create it." - fi - if [ ! -e "$PROJECT_ROOT/simulation/isaac-sim/extensions/PegasusSimulator/extensions/pegasus.simulator" ]; then - _pf_error "PegasusSimulator submodule is empty — the Isaac launch script will fail to import pegasus. Run: git submodule update --init --recursive" + if [[ "$AIRSTACK_CONFIG_ONLY" != "1" ]]; then + if [ ! -f "$PROJECT_ROOT/simulation/isaac-sim/docker/omni_pass.env" ]; then + _pf_error "simulation/isaac-sim/docker/omni_pass.env is missing (Nucleus credentials). Run 'airstack setup' to create it." + fi + if [ ! -e "$PROJECT_ROOT/simulation/isaac-sim/extensions/PegasusSimulator/extensions/pegasus.simulator" ]; then + _pf_error "PegasusSimulator submodule is empty — the Isaac launch script will fail to import pegasus. Run: git submodule update --init --recursive" + fi fi fi + # Configuration contracts intentionally stop before Docker, credentials, + # images, GPU, and checked-out submodule prerequisites. + if [[ "$AIRSTACK_CONFIG_ONLY" == "1" ]]; then + unset -f _pf_error + return $errors + fi + # 5. Missing images: compose 'up' silently starts a very long build local imgs img missing=() imgs=$(run_docker_compose -f "$PROJECT_ROOT/docker-compose.yaml" "${_pf_global[@]}" config --images 2>/dev/null | sort -u) @@ -1146,12 +1157,13 @@ function preflight_up { } function cmd_up { - check_docker - # Airstack launch-intent flags (consumed before compose sees the args) local rest_args=() parse_launch_intent rest_args "$@" || exit 1 apply_launch_intent "${rest_args[@]}" || exit 1 + if [[ "$AIRSTACK_CONFIG_ONLY" != "1" ]]; then + check_docker + fi local global_args=() local subcmd_args=() @@ -1688,7 +1700,7 @@ function register_builtin_commands { COMMAND_HELP["image-pull"]="Pull Docker Compose service images from a registry" COMMAND_HELP["images"]="List Docker images filtered by PROJECT_NAME from .env" COMMAND_HELP["image-delete"]="Delete all Docker images matching PROJECT_NAME (prompts unless -y)" - COMMAND_HELP["up"]="Start services [--sim isaac|airsim] [--robots N] [--headless] [--play|--no-play] [--no-autolaunch] [--wait] [--dry-run]" + COMMAND_HELP["up"]="Start services [--sim isaac|airsim] [--robots N] [--headless] [--play|--no-play] [--no-autolaunch] [--wait] [--dry-run] [--config-only]" COMMAND_HELP["down"]="down services" COMMAND_HELP["clean"]="Remove all ROS 2 build artifacts (build/, install/, log/)" COMMAND_HELP["connect"]="Connect to a running container (supports partial name matching)" diff --git a/docs/development/intermediate/testing/ci_cd.md b/docs/development/intermediate/testing/ci_cd.md index 3727642bd..b5e45a8dd 100644 --- a/docs/development/intermediate/testing/ci_cd.md +++ b/docs/development/intermediate/testing/ci_cd.md @@ -28,8 +28,8 @@ to fit CI into your day-to-day development loop. | Where do CI jobs run? | Python unit tests: `ubuntu-latest`. Build and simulation tests: a fresh OSMO GPU pod, destroyed afterward. | | What triggers a run? | PR open/update/reopen runs unit + package-build gates; maintainers select simulations with `/pytest`; `workflow_dispatch` is also available. | | What gets tested? | Automatically: Python units/contracts and ROS package builds/tests. Selectably: Docker builds, liveliness, sensors, flight policies, and OptiTrack. | -| How do I see results? | Checks plus a report comment and `test-results-*` artifact (`summary.txt`, `results.xml`, `run_meta.json`, `metrics.json`). | -| What fails the build? | Any failed test, or a comparable simulation metric regressing more than 20%. Invalid/incomplete campaigns are labeled, not scored as policy failures. | +| How do I see results? | Checks plus a report comment and `test-results-*` artifact (`summary.txt`, `results.xml`, `run_meta.json`, `metrics.json`, and bounded failure diagnostics when needed). | +| What fails the build? | Test assertions, infrastructure/prerequisite failures, or report integrity failures. Comparable numeric metric deltas are advisory. | | Who holds the secrets? | Only the orchestrator host. Workers get a single-use JIT token valid for one registration. | --- @@ -238,8 +238,8 @@ flowchart TD k --> m["pytest tests/ with resolved args"] l --> m m --> n["Upload tests/results/ artifact, 90-day retention"] - n --> o["Finalize Check Run with the job conclusion"] - o --> p["report job on ubuntu-latest"] + n --> p["report job on ubuntu-latest"] + p --> o["Post report, then finalize Check Run"] ``` The image-prep step is what makes runs on a cold pod tolerable: it pulls the @@ -385,8 +385,9 @@ Run one mark at a time unless you genuinely need both. After `run-tests` finishes — pass or fail — a `report` job on `ubuntu-latest` downloads the current artifact plus a **baseline** artifact and runs [`parse_metrics.py`](https://github.com/castacks/AirStack/blob/main/tests/parse_metrics.py) -in diff mode only when both artifacts have the same complete simulation -campaign fingerprint (selected tests and parameters). +in diff mode only after selecting the newest completed artifact with the same +simulation campaign fingerprint (normalized selected tests and all relevant +CLI/configuration parameters). | Run type | Baseline used | |---|---| @@ -399,7 +400,8 @@ For a complete simulation campaign, the comment has pass rates plus a flat the `sensors` mark), and a **Compute usage** pivot (CPU / memory / GPU per container). Regressions are marked with a red circle, improvements with a green one, and the job **fails** if any comparable metric moves more than the -20% threshold in the wrong direction. +20% display threshold in the wrong direction. These numeric deltas are advisory: +they inform review but do not fail the PR. `run_meta.json` separates those policy results from CI failures. A collection error, zero-test selection, internal pytest error, cancellation, or timeout is @@ -416,8 +418,9 @@ simulation result and keeps its recorded error metrics. tests/results/2026-08-06_14-30-00/ ├── summary.txt # human-readable per-chain summary — open this first ├── results.xml # JUnit XML: durations, pass/fail per test -├── run_meta.json # completion state, pytest exit, selected/executed sim counts -└── metrics.json # every recorded metric, including time series +├── run_meta.json # schema-v2 completion/failure class + exact campaign config +├── metrics.json # every recorded metric, including time series +└── diagnostics/ # on failure: bounded config, panes, logs, ROS/GPU/command ring ``` There are no per-test log files. Live output streams to the Actions log via @@ -541,7 +544,7 @@ down the list. | `No space left on device` | Pod | Bump `storage` in `config.yaml`; Isaac assets plus all images are large | | Runner registered, then pytest failed | Tests | A real test failure — the GitHub Actions log and `summary.txt` are canonical | | Report says “simulation metrics are not comparable” | Collection/infrastructure | Read the run outcome and pytest exit status in `run_meta.json`; no policy regression was scored | -| Metrics report job failed with no test failures | Report | A like-for-like metric regressed past the 20% threshold, or report generation itself failed; read the report step log | +| Metrics report job failed with no test failures | Report | Report generation or artifact integrity failed; numeric metric deltas are advisory and do not cause this conclusion | To map a GitHub job to its pod: @@ -573,7 +576,7 @@ Full runbook, including credential rotation and worker-side diagnostics: | [`.github/orchestrator/config.example.yaml`](https://github.com/castacks/AirStack/blob/main/.github/orchestrator/config.example.yaml) | Every tunable: pool, platform, resources, limits, poll intervals | | [`.github/orchestrator/setup.sh`](https://github.com/castacks/AirStack/blob/main/.github/orchestrator/setup.sh) | One-time orchestrator host install | | [`tests/conftest.py`](https://github.com/castacks/AirStack/blob/main/tests/conftest.py) | `airstack_env` fixture, collection order, `MetricsRecorder` | -| [`tests/parse_metrics.py`](https://github.com/castacks/AirStack/blob/main/tests/parse_metrics.py) | Report generation and the regression gate | +| [`tests/parse_metrics.py`](https://github.com/castacks/AirStack/blob/main/tests/parse_metrics.py) | Comparable advisory report generation and report-integrity gate | | [`tests/run_summary.py`](https://github.com/castacks/AirStack/blob/main/tests/run_summary.py) | `summary.txt` generation | ## See also diff --git a/tests/README.md b/tests/README.md index 306567a1a..7a458e91a 100644 --- a/tests/README.md +++ b/tests/README.md @@ -116,23 +116,23 @@ Writes custom metrics to `tests/results//metrics.json` after each `re ### Output files -Every test run produces a timestamped directory containing only `summary.txt`, -`results.xml`, `run_meta.json`, and `metrics.json` — there is **no** `logs/` subdirectory and no -per-test log files are written under the run directory. +Every test run produces a timestamped directory with the finalized results. +Simulator/startup failures additionally create a bounded `diagnostics/` JSON +bundle; full unbounded logs are never copied into the artifact. ``` tests/results/ └── 2025-04-21_14-30-00/ ├── summary.txt # Human-readable key metrics — open this first ├── results.xml # JUnit XML — test durations and pass/fail status - ├── run_meta.json # Completion/outcome and campaign fingerprint - └── metrics.json # Custom metrics (image sizes, Hz, compute, timing) + ├── run_meta.json # Schema-v2 completion/failure class + exact campaign + ├── metrics.json # Custom metrics (image sizes, Hz, compute, timing) + └── diagnostics/ # On failure: config, panes, log tails, ROS/GPU/commands ``` -Live test output goes to the terminal (pytest `log_cli`). On failure, assertion -messages include the tail of the last subprocess output (the in-memory -`read_log_tail` of the relevant `docker` / `ros2` subprocess) — no per-test log -files are written under the run directory. +Live test output goes to the terminal (pytest `log_cli`). Diagnostics are +bounded (container log tails and a 30-command ring) and exclude secret-bearing +environment variables. --- @@ -506,17 +506,19 @@ python tests/parse_metrics.py \ Prints a markdown table of all recorded metrics. Always exits 0. -### Diff / regression check +### Advisory comparison ```bash python tests/parse_metrics.py \ --current tests/results/2025-04-21_14-30-00/ \ --baseline tests/results/2025-04-20_09-00-00/ \ - --threshold 20 # optional: regression if change% exceeds this (default 20) + --threshold 20 # optional: highlight if change% exceeds this (default 20) --output report.md # optional: also write to file ``` -Prints a side-by-side comparison. Exits **1** if any metric regresses beyond the threshold; exits 0 otherwise. +Prints a side-by-side comparison. Numeric deltas are advisory and always exit +0. Report parsing/integrity failures exit 2 and block CI; pytest assertions and +infrastructure failures are enforced by the test job. For a completed test campaign, the report has three sections per test module: @@ -528,7 +530,10 @@ Regressions are flagged with :red_circle:, improvements with :green_circle:. Collection errors, command/internal errors, zero-test runs, and jobs that stop before pytest finalizes are labeled **not comparable**. Their pass-rate and regression tables are suppressed so an infrastructure failure cannot appear as 0% policy performance. -`run_meta.json` records the pytest exit status and simulation tests selected/completed. +`run_meta.json` records normalized selected IDs, behavior-changing CLI options, +completion state, and failure class. Its fingerprint includes both tests and +configuration, preventing unlike robot counts, trajectories, tolerances, or +stress settings from being compared. Per-robot metric keys remain visible. --- @@ -570,14 +575,10 @@ opened, updated, or reopened against `main` or `develop`. **`report`** runs on `ubuntu-latest` after `run-tests` (even if it failed). It: 1. Downloads the current artifact -2. Downloads a baseline artifact (from the base branch for PRs, from `main` for manual runs, or from the specified `baseline_run_id`) -3. Runs `parse_metrics.py` in diff mode only when both artifacts have the same complete simulation campaign fingerprint; otherwise reports the current run without comparison +2. Downloads baseline candidates (from the base branch for PRs, from `main` for manual runs, or from the specified `baseline_run_id`) +3. Selects the newest completed candidate with the exact same test/configuration fingerprint; otherwise reports the current run without comparison 4. Posts the markdown report as a PR comment (PR runs) or to the job summary (all runs) -5. Fails with `::error::` only for a comparable metric regression; invalid/incomplete campaigns are reported as infrastructure outcomes - -#### Required third-party action - -The workflow uses [`dawidd6/action-download-artifact@v6`](https://github.com/dawidd6/action-download-artifact) to download artifacts from other workflow runs by branch name. This is a community action and must be trusted in your repository's Actions settings if you use a restricted allowed-actions policy. +5. Fails only if report generation/integrity fails. Comparable metric deltas are advisory; assertions and infrastructure failures remain blocking in `run-tests` --- diff --git a/tests/conftest.py b/tests/conftest.py index 38aec5c5f..43002caf1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -137,12 +137,26 @@ def pytest_sessionfinish(session, exitstatus): for entries in getattr(terminal, "stats", {}).values() for report in entries ] + campaign_config = {} + for key in ( + "sim", "num_robots", "stress_iterations", "stable_duration", + "stable_interval", "gui", "takeoff_velocities", + "trajectory_types", "waypoints", "waypoint_tolerance", + "goal_tolerance", "waypoint_timeout", + ): + try: + campaign_config[key] = session.config.getoption( + f"--{key.replace('_', '-')}" + ) + except (ValueError, AttributeError): + continue meta_path = write_run_meta( run_dir, session.items, exitstatus, session.config.option.markexpr, reports, + campaign_config, ) logger.info("Wrote run metadata to %s", meta_path) except Exception as exc: @@ -239,8 +253,16 @@ def airstack_env(request): up_cmd_duration_s = round(time.time() - t0, 2) logger.info("airstack up returned %d in %.2fs", up_result.returncode, up_cmd_duration_s) - assert up_result.returncode == 0, \ - f"airstack up failed:\n{read_log_tail(log)}" + if up_result.returncode != 0: + diagnostics = collect_failure_diagnostics( + env_overrides, + f"airstack up failed with status {up_result.returncode}", + harness_session.current_item().nodeid, + ) + pytest.fail( + f"airstack up failed:\n{read_log_tail(log)}\n" + f"diagnostics: {diagnostics}" + ) env = { "sim": sim, diff --git a/tests/harness/__init__.py b/tests/harness/__init__.py index 7b1210a7c..c9a209a62 100644 --- a/tests/harness/__init__.py +++ b/tests/harness/__init__.py @@ -34,10 +34,12 @@ unit_test_dirs, unit_test_files, ) +from harness.diagnostics import collect_failure_diagnostics from harness.metrics import MetricsRecorder, current_test_id, get_metrics from harness.session import logger from harness.sim import ( SIM_CONFIG, + SimulatorHealthError, parallel_echo_once_robot_topics, parallel_sample_hz, sample_hz, @@ -50,7 +52,7 @@ "colcon_test_robot_command", "collection_is_broad", "format_pytest_addopts", "load_colcon_unit_test_config", "unit_test_dirs", "unit_test_files", # session - "logger", + "logger", "collect_failure_diagnostics", # commands "ROS_DISTRO_SETUP", "airstack_cmd", "current_log", "docker_exec", "read_log_tail", "ros2_env", "ros2_exec", @@ -61,6 +63,6 @@ # metrics "MetricsRecorder", "get_metrics", "current_test_id", # sim - "SIM_CONFIG", "wait_for_first_message", "sample_hz", "parallel_sample_hz", + "SIM_CONFIG", "SimulatorHealthError", "wait_for_first_message", "sample_hz", "parallel_sample_hz", "parallel_echo_once_robot_topics", ] diff --git a/tests/harness/baseline.py b/tests/harness/baseline.py new file mode 100644 index 000000000..39e106061 --- /dev/null +++ b/tests/harness/baseline.py @@ -0,0 +1,44 @@ +"""Select a completed, configuration-identical simulation baseline.""" + +from __future__ import annotations + +from pathlib import Path +from typing import Iterable + +from harness.run_meta import classify_run, comparability_reason + + +def select_baseline( + candidate_dirs: Iterable[Path], + current_meta: dict, +) -> tuple[Path | None, list[str]]: + """Return newest comparable candidate and human-readable rejection reasons.""" + matches: list[Path] = [] + rejected: list[str] = [] + for candidate in {Path(path) for path in candidate_dirs}: + meta = classify_run(candidate) + reason = comparability_reason(current_meta, meta) + if reason: + rejected.append(f"{candidate}: {reason}") + else: + matches.append(candidate) + if not matches: + return None, sorted(rejected) + matches.sort( + key=lambda path: (path / "run_meta.json").stat().st_mtime + if (path / "run_meta.json").exists() + else path.stat().st_mtime, + reverse=True, + ) + return matches[0], sorted(rejected) + + +def select_baseline_path(current_dir: Path, baseline_root: Path) -> Path | None: + """Convenience API used by CI after downloading several artifacts.""" + current_meta = classify_run(Path(current_dir)) + candidates = [ + path.parent + for path in Path(baseline_root).rglob("run_meta.json") + ] + selected, _ = select_baseline(candidates, current_meta) + return selected diff --git a/tests/harness/commands.py b/tests/harness/commands.py index 7c593e4de..a8d687c8b 100644 --- a/tests/harness/commands.py +++ b/tests/harness/commands.py @@ -51,7 +51,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 "") - record_cmd_output(combined, log_name) + record_cmd_output(combined, log_name, quoted) return result diff --git a/tests/harness/diagnostics.py b/tests/harness/diagnostics.py new file mode 100644 index 000000000..08abb9baa --- /dev/null +++ b/tests/harness/diagnostics.py @@ -0,0 +1,126 @@ +"""Bounded, best-effort simulator failure diagnostics.""" + +from __future__ import annotations + +import json +import os +import re +import subprocess +from pathlib import Path + +from harness import session + +MAX_OUTPUT_CHARS = 16_000 +SAFE_ENV_KEYS = ( + "COMPOSE_PROFILES", + "NUM_ROBOTS", + "URDF_FILE", + "AUTOLAUNCH", + "PLAY_SIM_ON_START", + "ISAAC_SIM_SCRIPT_NAME", + "ISAAC_SIM_HEADLESS", + "MS_AIRSIM_HEADLESS", + "MS_AIRSIM_ENV_DIR", + "MS_AIRSIM_BINARY_PATH", + "LAUNCH_NATNET", + "PX4_PARAM_SET", +) + + +def _bounded_run(args, timeout=10) -> dict: + try: + result = subprocess.run( + args, + capture_output=True, + text=True, + timeout=timeout, + ) + output = (result.stdout or "") + (result.stderr or "") + return { + "returncode": result.returncode, + "output": output[-MAX_OUTPUT_CHARS:], + } + except Exception as exc: + return {"error": f"{type(exc).__name__}: {exc}"} + + +def _safe_name(value: str) -> str: + return re.sub(r"[^A-Za-z0-9_.-]+", "_", value)[:180] + + +def collect_failure_diagnostics( + env: dict | None = None, + reason: str = "", + test_id: str = "session", +) -> Path | None: + """Persist a bounded JSON bundle; diagnostic failures never mask the test.""" + run_dir = session.run_dir() + if run_dir is None: + return None + env = env or {} + containers_result = _bounded_run( + ["docker", "ps", "--format", "{{.Names}}"], timeout=10 + ) + containers = containers_result.get("output", "").splitlines()[:20] + container_data = {} + pane_cmd = ( + "tmux list-panes -a -F " + "'#{session_name}:#{window_name}|#{pane_pid}|#{pane_title}|#{pane_dead}'" + ) + for container in containers: + container_data[container] = { + "logs": _bounded_run( + ["docker", "logs", "--tail", "200", container], timeout=15 + ), + "tmux": _bounded_run( + ["docker", "exec", container, "bash", "-c", pane_cmd], timeout=10 + ), + } + robot_graph = {} + for index, container in enumerate( + [name for name in containers if "robot" in name and "desktop" in name], + start=1, + ): + robot_graph[container] = _bounded_run( + [ + "docker", "exec", "-e", f"ROS_DOMAIN_ID={index}", container, + "bash", "-lc", + "source /opt/ros/jazzy/setup.bash 2>/dev/null; " + "source /root/AirStack/robot/ros_ws/install/setup.bash 2>/dev/null; " + "echo NODES; ros2 node list 2>&1; " + "echo TOPICS; ros2 topic list 2>&1", + ], + timeout=15, + ) + payload = { + "schema_version": 1, + "reason": reason[:4000], + "effective_config": { + key: str(env.get(key, os.environ.get(key, "")))[:2000] + for key in SAFE_ENV_KEYS + if env.get(key, os.environ.get(key)) is not None + }, + "containers": container_data, + "ros_graph": robot_graph, + "gpu": _bounded_run( + [ + "nvidia-smi", + "--query-gpu=name,driver_version,utilization.gpu,memory.used,memory.total", + "--format=csv,noheader", + ], + timeout=10, + ), + "command_ring": [ + { + "command": str(entry.get("command", ""))[:1000], + "log_name": str(entry.get("log_name", ""))[:300], + "output": str(entry.get("output", ""))[-12000:], + } + for entry in session.recent_cmd_outputs()[-30:] + ], + } + diagnostics_dir = Path(run_dir) / "diagnostics" + diagnostics_dir.mkdir(parents=True, exist_ok=True) + path = diagnostics_dir / f"{_safe_name(test_id)}.json" + path.write_text(json.dumps(payload, indent=2, sort_keys=True) + "\n") + return path diff --git a/tests/harness/image_prep.py b/tests/harness/image_prep.py new file mode 100644 index 000000000..b41e78d0f --- /dev/null +++ b/tests/harness/image_prep.py @@ -0,0 +1,66 @@ +"""Structured Docker image-preparation outcomes for CI artifacts.""" + +from __future__ import annotations + +import argparse +import json +import os +from pathlib import Path + +OUTCOMES = { + "already-present", + "pulled-versioned", + "cache-retagged", + "locally-built", + "missing", + "delegated-to-build-docker", +} + + +def build_image_preparation( + outcome: str, + *, + pulled=(), + retagged=(), + built=(), + missing=(), +) -> dict: + if outcome not in OUTCOMES: + raise ValueError(f"unknown image preparation outcome: {outcome}") + return { + "schema_version": 1, + "outcome": outcome, + "versioned_pulled": sorted(filter(None, pulled)), + "cache_retagged": sorted(filter(None, retagged)), + "locally_built": sorted(filter(None, built)), + "missing": sorted(filter(None, missing)), + } + + +def write_from_environment(path: Path) -> Path: + def lines(name): + return os.environ.get(name, "").splitlines() + + payload = build_image_preparation( + os.environ["IMAGE_OUTCOME"], + pulled=lines("PULLED_IMAGES"), + retagged=lines("RETAGGED_IMAGES"), + built=lines("BUILT_IMAGES"), + missing=lines("MISSING_IMAGES"), + ) + path = Path(path) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(payload, indent=2, sort_keys=True) + "\n") + return path + + +def main() -> int: + parser = argparse.ArgumentParser() + parser.add_argument("output", type=Path) + args = parser.parse_args() + write_from_environment(args.output) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/harness/run_meta.py b/tests/harness/run_meta.py index 8cd0821a9..9afb85f60 100644 --- a/tests/harness/run_meta.py +++ b/tests/harness/run_meta.py @@ -4,10 +4,11 @@ import hashlib import json +import os import xml.etree.ElementTree as ET from pathlib import Path -from harness.test_ids import canonical_test_id +from harness.test_ids import canonical_test_id, normalize_csv RUN_META_FILENAME = "run_meta.json" @@ -28,15 +29,54 @@ def is_simulation_test_id(test_id: str) -> bool: return canonical.startswith(SIMULATION_MODULES) -def campaign_fingerprint(test_ids) -> str: - """Stable identity for the exact selected simulation campaign.""" +CAMPAIGN_OPTION_KEYS = ( + "sim", + "num_robots", + "stress_iterations", + "stable_duration", + "stable_interval", + "gui", + "takeoff_velocities", + "trajectory_types", + "waypoints", + "waypoint_tolerance", + "goal_tolerance", + "waypoint_timeout", +) + + +def normalize_campaign_config(raw: dict | None) -> dict: + """Return stable, JSON-safe behavior-changing campaign configuration.""" + raw = raw or {} + result = {} + for key in CAMPAIGN_OPTION_KEYS: + value = raw.get(key) + if key in ("sim", "num_robots", "takeoff_velocities", "trajectory_types"): + cast = int if key == "num_robots" else str + result[key] = normalize_csv(value, cast=cast) + elif isinstance(value, Path): + result[key] = str(value) + elif value is not None: + result[key] = value + return result + + +def campaign_fingerprint(test_ids, campaign_config: dict | None = None) -> str: + """Stable identity for exact tests plus behavior-changing configuration.""" canonical_ids = sorted( canonical_test_id(str(test_id).replace("::", ".")).replace(".py.", ".") for test_id in test_ids ) if not canonical_ids: return "" - payload = "\n".join(canonical_ids).encode() + payload = json.dumps( + { + "test_ids": canonical_ids, + "config": normalize_campaign_config(campaign_config), + }, + sort_keys=True, + separators=(",", ":"), + ).encode() return hashlib.sha256(payload).hexdigest() @@ -86,7 +126,8 @@ def _report_details(reports) -> tuple[dict[str, str], set[str], set[str]]: def build_run_meta(items, exitstatus: int, mark_expression: str = "", - reports=None) -> dict: + reports=None, campaign_config: dict | None = None, + tested_identity: dict | None = None) -> dict: """Build serializable run metadata from a completed pytest session.""" report_outcomes, call_nodeids, infrastructure_error_nodeids = _report_details( reports @@ -116,45 +157,72 @@ def build_run_meta(items, exitstatus: int, mark_expression: str = "", ) } completed = list(completed_by_id.values()) + selected_test_ids = sorted(canonical_test_id(item.nodeid) for item in items) simulation_items = [ item for item in items if is_simulation_test_id(str(item.nodeid)) ] simulation_completed = [ item for item in simulation_items if str(item.nodeid) in call_nodeids ] + simulation_finalized = [ + item for item in simulation_items + if str(item.nodeid) in completed_by_id + ] simulation_infrastructure_errors = [ item for item in simulation_items if str(item.nodeid) in infrastructure_error_nodeids ] + call_failures = [ + nodeid for nodeid, status in completed_by_id.items() + if status == "failed" and nodeid in call_nodeids + ] if exitstatus == 2: # Pytest uses exit 2 for both collection aborts and user/runner # interruption. Reports prove that execution had already begun. outcome = "incomplete" if completed else "collection_error" + failure_class = "interrupted" if completed else "collection" elif exitstatus in (3, 4): outcome = "internal_error" + failure_class = "ci_integrity" elif exitstatus == 5 or not items: outcome = "no_tests" + failure_class = "no_tests" elif not call_nodeids: outcome = ( "simulation_not_executed" if simulation_items else "tests_not_executed" ) + failure_class = "infrastructure" elif simulation_items and not simulation_completed: outcome = "simulation_not_executed" + failure_class = "infrastructure" elif simulation_infrastructure_errors: outcome = "incomplete" - elif len(simulation_completed) != len(simulation_items): + failure_class = "infrastructure" + elif len(simulation_finalized) != len(simulation_items): outcome = "incomplete" + failure_class = "infrastructure" elif simulation_items: outcome = "simulation" + failure_class = "assertion" if call_failures else "none" else: outcome = "non_simulation" + failure_class = "assertion" if call_failures else "none" + normalized_config = normalize_campaign_config(campaign_config) + simulation_ids = [ + canonical_test_id(item.nodeid) + for item in simulation_items + ] + fingerprint = campaign_fingerprint(simulation_ids, normalized_config) + complete = outcome in ("simulation", "non_simulation") return { - "schema_version": 1, - "complete": outcome != "incomplete", + "schema_version": 2, + "complete": complete, + "completion_state": "completed" if complete else outcome, + "failure_class": failure_class, "outcome": outcome, "pytest_exitstatus": int(exitstatus), "mark_expression": mark_expression, @@ -165,18 +233,38 @@ def build_run_meta(items, exitstatus: int, mark_expression: str = "", "skipped": completed.count("skipped"), "simulation_selected": len(simulation_items), "simulation_completed": len(simulation_completed), - "campaign_fingerprint": campaign_fingerprint( - item.nodeid for item in simulation_items - ), + "simulation_finalized": len(simulation_finalized), + "selected_test_ids": selected_test_ids, + "campaign_config": normalized_config, + "campaign_fingerprint": fingerprint, + "campaign": { + "schema_version": 1, + "selected_test_ids": sorted(simulation_ids), + "config": normalized_config, + "fingerprint": fingerprint, + }, + "tested_identity": tested_identity or { + "sha": os.environ.get("AIRSTACK_TESTED_SHA", ""), + "pr_number": os.environ.get("AIRSTACK_PR_NUMBER", ""), + }, } def write_run_meta(run_dir: Path, items, exitstatus: int, - mark_expression: str = "", reports=None) -> Path: + mark_expression: str = "", reports=None, + campaign_config: dict | None = None, + tested_identity: dict | None = None) -> Path: """Write ``run_meta.json`` for a normally completed pytest session.""" path = Path(run_dir) / RUN_META_FILENAME path.write_text(json.dumps( - build_run_meta(items, exitstatus, mark_expression, reports), + build_run_meta( + items, + exitstatus, + mark_expression, + reports, + campaign_config, + tested_identity, + ), indent=2, sort_keys=True, ) + "\n") @@ -215,7 +303,16 @@ def _classify_junit(results_xml: Path) -> dict: return { "schema_version": 1, - "complete": outcome != "incomplete", + "complete": outcome in ("simulation", "non_simulation"), + "completion_state": ( + "completed" if outcome in ("simulation", "non_simulation") else outcome + ), + "failure_class": ( + "infrastructure" if outcome == "incomplete" + else "collection" if outcome == "collection_error" + else "no_tests" if outcome == "no_tests" + else "assertion" if failures else "none" + ), "outcome": outcome, "pytest_exitstatus": None, "mark_expression": "", @@ -308,3 +405,22 @@ def simulation_metrics_comparable(meta: dict, baseline: dict | None = None) -> b and baseline.get("outcome") == "simulation" and baseline.get("campaign_fingerprint") == meta["campaign_fingerprint"] ) + + +def comparability_reason(meta: dict, baseline: dict | None = None) -> str: + """Human explanation shared by summaries, reports, and baseline selection.""" + if not meta: + return "run metadata is missing" + if not meta.get("complete"): + return f"campaign is not complete ({meta.get('completion_state', meta.get('outcome'))})" + if meta.get("outcome") != "simulation": + return f"run outcome is {meta.get('outcome', 'unknown')}, not a simulation campaign" + if not meta.get("campaign_fingerprint"): + return "campaign fingerprint is missing" + if baseline is None: + return "no baseline campaign was supplied" + if not baseline.get("complete") or baseline.get("outcome") != "simulation": + return "baseline is not a completed simulation campaign" + if baseline.get("campaign_fingerprint") != meta.get("campaign_fingerprint"): + return "baseline campaign configuration does not match" + return "" diff --git a/tests/harness/session.py b/tests/harness/session.py index 54277dacc..bc8c10a4e 100644 --- a/tests/harness/session.py +++ b/tests/harness/session.py @@ -7,6 +7,7 @@ back into conftest globals. """ import logging +from collections import deque from datetime import datetime from pathlib import Path @@ -19,6 +20,7 @@ _run_dir = None _current_item = None _last_cmd_output: dict[str, str] = {} +_command_ring = deque(maxlen=30) def init_run_dir(airstack_root) -> Path: @@ -46,14 +48,24 @@ def current_item(): return _current_item -def record_cmd_output(text, log_name=None): +def record_cmd_output(text, log_name=None, command=""): """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 + _command_ring.append({ + "command": str(command)[:1000], + "log_name": key, + "output": str(text)[-12000:], + }) 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, "") + + +def recent_cmd_outputs() -> list[dict[str, str]]: + """Bounded command/output history for failure diagnostics.""" + return list(_command_ring) diff --git a/tests/harness/sim.py b/tests/harness/sim.py index ec4c8a159..ffb9d1e1c 100644 --- a/tests/harness/sim.py +++ b/tests/harness/sim.py @@ -41,7 +41,19 @@ } -def wait_for_first_message(container, topic, domain_id, setup_bash, timeout=60): +class SimulatorHealthError(RuntimeError): + """A readiness wait stopped because its simulator process became unhealthy.""" + + +def wait_for_first_message( + container, + topic, + domain_id, + setup_bash, + timeout=60, + health_check=None, + health_grace=15, +): """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 @@ -54,6 +66,17 @@ def wait_for_first_message(container, topic, domain_id, setup_bash, timeout=60): attempt = 0 while time.time() < deadline: attempt += 1 + if health_check is not None and time.time() - start >= health_grace: + health = health_check() + if isinstance(health, tuple): + healthy, detail = health + else: + healthy, detail = bool(health), "simulator health probe failed" + if not healthy: + raise SimulatorHealthError( + f"infrastructure simulator process failure while waiting " + f"for {topic}: {detail}" + ) per_attempt = min(max(1, int(deadline - time.time())), 10) try: result = ros2_exec( diff --git a/tests/harness/test_ids.py b/tests/harness/test_ids.py index 5b0fd825c..df3da11de 100644 --- a/tests/harness/test_ids.py +++ b/tests/harness/test_ids.py @@ -1,4 +1,6 @@ -"""Canonical test identifiers shared by metrics and summary reporting.""" +"""Canonical test identifiers shared by collection, metadata, and reporting.""" + +import re def canonical_test_id(name: str) -> str: @@ -8,8 +10,24 @@ def canonical_test_id(name: str) -> str: ``system/test_liveliness.Class.test`` while JUnit uses ``system.test_liveliness.Class.test``. """ - head, dot, rest = name.partition(".") - if "/" in head: - head = head.replace("/", ".") - return head + dot + rest if dot else head - return name + value = str(name).replace("\\", "/") + value = value.replace(".py::", ".").replace("::", ".") + value = value.replace(".py.", ".") + return value.replace("/", ".").lstrip(".") + + +def normalize_csv(value, cast=str) -> list: + """Normalize a comma-separated pytest option into a stable sorted list.""" + if value is None: + return [] + if isinstance(value, (list, tuple, set)): + parts = value + else: + parts = str(value).split(",") + normalized = [cast(str(part).strip()) for part in parts if str(part).strip()] + return sorted(normalized) + + +def base_iteration_test_id(name: str) -> str: + """Canonical test ID with only the generated stress-iteration suffix removed.""" + return re.sub(r"-iter\d+(?=\])", "", canonical_test_id(name)) diff --git a/tests/meta/test_campaign_reporting_contract.py b/tests/meta/test_campaign_reporting_contract.py new file mode 100644 index 000000000..356770ed1 --- /dev/null +++ b/tests/meta/test_campaign_reporting_contract.py @@ -0,0 +1,173 @@ +"""Campaign classification, fingerprint, baseline, and advisory contracts.""" + +import json +import sys +from types import SimpleNamespace + +import pytest + +from harness.baseline import select_baseline +from harness.image_prep import build_image_preparation +from harness.run_meta import build_run_meta, campaign_fingerprint +import parse_metrics +from parse_metrics import _score + +pytestmark = pytest.mark.unit + +NODE = "system/test_liveliness.py::TestLiveliness::test_sim_ready_time[isaacsim-1-iter0]" + + +def _report(when, outcome): + return SimpleNamespace( + nodeid=NODE, + when=when, + failed=outcome == "failed", + skipped=outcome == "skipped", + passed=outcome == "passed", + ) + + +def _item(): + return SimpleNamespace(nodeid=NODE) + + +def test_schema_v2_distinguishes_assertion_from_infrastructure(): + assertion = build_run_meta( + [_item()], + 1, + reports=[_report("setup", "passed"), _report("call", "failed")], + campaign_config={"sim": "isaacsim", "num_robots": "1"}, + ) + infrastructure = build_run_meta( + [_item()], + 1, + reports=[_report("setup", "failed")], + campaign_config={"sim": "isaacsim", "num_robots": "1"}, + ) + assert assertion["schema_version"] == 2 + assert assertion["complete"] is True + assert assertion["failure_class"] == "assertion" + assert infrastructure["complete"] is False + assert infrastructure["failure_class"] == "infrastructure" + + +def test_behavior_options_participate_in_campaign_fingerprint(): + first = campaign_fingerprint([NODE], {"sim": "isaacsim", "num_robots": "1"}) + second = campaign_fingerprint([NODE], {"sim": "isaacsim", "num_robots": "3"}) + assert first != second + + +def test_assertion_with_downstream_dependency_skip_is_finalized_campaign(): + downstream = NODE.replace("test_sim_ready_time", "test_stable") + skipped = SimpleNamespace( + nodeid=downstream, + when="setup", + failed=False, + skipped=True, + passed=False, + ) + meta = build_run_meta( + [_item(), SimpleNamespace(nodeid=downstream)], + 1, + reports=[_report("call", "failed"), skipped], + campaign_config={"sim": "isaacsim", "num_robots": "1"}, + ) + assert meta["outcome"] == "simulation" + assert meta["failure_class"] == "assertion" + assert meta["simulation_completed"] == 1 + assert meta["simulation_finalized"] == 2 + + +def _write_run(path, fingerprint, complete=True): + path.mkdir() + (path / "results.xml").write_text("") + (path / "run_meta.json").write_text(json.dumps({ + "schema_version": 2, + "complete": complete, + "completion_state": "completed" if complete else "interrupted", + "outcome": "simulation" if complete else "incomplete", + "campaign_fingerprint": fingerprint, + })) + + +def test_baseline_selector_ignores_newer_mismatch_and_partial(tmp_path): + matching = tmp_path / "matching" + mismatch = tmp_path / "mismatch" + partial = tmp_path / "partial" + _write_run(matching, "wanted") + _write_run(mismatch, "other") + _write_run(partial, "wanted", complete=False) + selected, rejected = select_baseline( + [mismatch, partial, matching], + {"complete": True, "outcome": "simulation", "campaign_fingerprint": "wanted"}, + ) + assert selected == matching + assert len(rejected) == 2 + + +def test_timeout_or_missing_data_is_never_numeric_regression(): + numeric = {"value": 1.0, "direction": "lower_is_better"} + assert _score({"value": "timeout"}, numeric, 20)[1] == "" + assert _score(None, numeric, 20)[1] == "" + + +@pytest.mark.parametrize( + "outcome,field", + [ + ("already-present", None), + ("pulled-versioned", "versioned_pulled"), + ("cache-retagged", "cache_retagged"), + ("locally-built", "locally_built"), + ("missing", "missing"), + ], +) +def test_image_preparation_paths_have_explicit_outcomes(outcome, field): + kwargs = {} + if field: + argument = { + "versioned_pulled": "pulled", + "cache_retagged": "retagged", + "locally_built": "built", + "missing": "missing", + }[field] + kwargs[argument] = ["registry/image:tag"] + payload = build_image_preparation(outcome, **kwargs) + assert payload["outcome"] == outcome + if field: + assert payload[field] == ["registry/image:tag"] + + +def test_metric_delta_cli_is_advisory(monkeypatch, tmp_path): + output = tmp_path / "report.md" + monkeypatch.setattr( + parse_metrics, + "generate_report", + lambda *args, **kwargs: ("advisory", True), + ) + monkeypatch.setattr( + sys, + "argv", + ["parse_metrics.py", "--current", str(tmp_path), "--output", str(output)], + ) + with pytest.raises(SystemExit) as exc: + parse_metrics.main() + assert exc.value.code == 0 + assert output.read_text() == "advisory" + + +def test_report_parser_crash_remains_blocking(monkeypatch, tmp_path): + output = tmp_path / "report.md" + + def crash(*args, **kwargs): + raise RuntimeError("broken parser") + + monkeypatch.setattr(parse_metrics, "generate_report", crash) + monkeypatch.setattr( + sys, + "argv", + ["parse_metrics.py", "--current", str(tmp_path), "--output", str(output)], + ) + with pytest.raises(SystemExit) as exc: + parse_metrics.main() + assert exc.value.code == 2 + assert "Report generation failed" in output.read_text() diff --git a/tests/meta/test_collection_contract.py b/tests/meta/test_collection_contract.py index 998de89a4..9402c0985 100644 --- a/tests/meta/test_collection_contract.py +++ b/tests/meta/test_collection_contract.py @@ -118,7 +118,7 @@ def test_report_uses_the_revision_that_was_actually_tested(): def test_pr_head_check_is_finalized_after_metrics(): workflow = repo_path(".github", "workflows", "system-tests.yml").read_text() assert workflow.index("- name: Finalize check on PR head") > workflow.index( - "- name: Fail on regression" + "- name: Fail on report integrity error" ) assert "ref: ${{ needs.run-tests.outputs.tested_sha }}" in workflow assert "conclusion: '${{ job.status }}'" not in workflow diff --git a/tests/meta/test_diagnostics_contract.py b/tests/meta/test_diagnostics_contract.py new file mode 100644 index 000000000..0abcb477b --- /dev/null +++ b/tests/meta/test_diagnostics_contract.py @@ -0,0 +1,66 @@ +"""Hermetic contracts for bounded diagnostics and fail-fast readiness.""" + +import json +from types import SimpleNamespace + +import pytest + +from harness import diagnostics +from harness.sim import SimulatorHealthError, wait_for_first_message +from system import test_optitrack_e2e + +pytestmark = pytest.mark.unit + + +def test_diagnostic_bundle_is_bounded_and_secret_free(tmp_path, monkeypatch): + monkeypatch.setattr(diagnostics.session, "run_dir", lambda: tmp_path) + monkeypatch.setattr( + diagnostics.session, + "recent_cmd_outputs", + lambda: [{"command": "probe", "output": "x" * 50_000}], + ) + + def fake_run(args, **kwargs): + output = "container-a\n" if args[:2] == ["docker", "ps"] else "y" * 50_000 + return SimpleNamespace(returncode=0, stdout=output, stderr="") + + monkeypatch.setattr(diagnostics.subprocess, "run", fake_run) + path = diagnostics.collect_failure_diagnostics( + { + "COMPOSE_PROFILES": "desktop,isaac-sim", + "DOCKER_REGISTRY_PASSWORD": "must-not-leak", + }, + "pane died", + "system/test", + ) + payload = json.loads(path.read_text()) + assert payload["schema_version"] == 1 + assert "DOCKER_REGISTRY_PASSWORD" not in payload["effective_config"] + assert path.stat().st_size < 150_000 + + +def test_message_wait_aborts_immediately_on_dead_process(): + with pytest.raises(SimulatorHealthError, match="pane exited"): + wait_for_first_message( + "sim", + "/clock", + 1, + "/setup.bash", + timeout=600, + health_check=lambda: (False, "pane exited"), + health_grace=0, + ) + + +def test_optitrack_missing_sdk_is_explicit_infrastructure(monkeypatch): + missing = SimpleNamespace(returncode=1, stdout="", stderr="") + healthy = SimpleNamespace(returncode=0, stdout="", stderr="") + monkeypatch.setattr(test_optitrack_e2e, "ros2_exec", lambda *a, **k: missing) + monkeypatch.setattr(test_optitrack_e2e, "docker_exec", lambda *a, **k: healthy) + monkeypatch.setattr( + test_optitrack_e2e, + "collect_failure_diagnostics", + lambda *a, **k: "diagnostics.json", + ) + with pytest.raises(pytest.fail.Exception, match="licensed NatNet SDK"): + test_optitrack_e2e._check_optitrack_prerequisites("robot") diff --git a/tests/meta/test_launch_intent_contract.py b/tests/meta/test_launch_intent_contract.py index 2d541e138..2a12ce4d9 100644 --- a/tests/meta/test_launch_intent_contract.py +++ b/tests/meta/test_launch_intent_contract.py @@ -2,15 +2,14 @@ # MIT License - see LICENSE in the repository root for full text. """Contract tests for `airstack up` launch-intent flags (--sim/--robots/...). -`airstack up --dry-run` derives the launch configuration (compose profiles, -URDF, Isaac script selection, robot count), runs the preflight checks, prints +`airstack up --config-only` derives the launch configuration (compose profiles, +URDF, Isaac script selection, robot count), runs logical preflight checks, prints the effective config between marker lines, and exits without starting services. These tests pin that contract: the derivations the flags promise, the preflight guards, and the exit codes. -They shell the real ./airstack.sh (no mocking) but never start containers — ---dry-run stops before compose up. Docker itself is required (the preflight -image check runs `docker compose config`), which CI's ubuntu-latest provides. +They shell the real ./airstack.sh (no mocking) but never contact Docker or +require simulator credentials, images, GPUs, or populated submodules. """ import os import subprocess @@ -33,7 +32,7 @@ def run_up_dry(*flags, env=None, check=True): """Run `airstack up --dry-run `; return (exit_code, stdout+stderr, config_dict).""" full_env = {**os.environ, **(env or {})} result = subprocess.run( - [AIRSTACK, "up", "--dry-run", *flags], + [AIRSTACK, "up", "--config-only", *flags], capture_output=True, text=True, cwd=str(REPO), env=full_env, timeout=120, ) out = result.stdout + result.stderr @@ -165,12 +164,18 @@ def test_effective_config_dump_written(): if not os.access(REPO, os.W_OK): pytest.skip("checkout mounted read-only (tests container) — dump is best-effort") runs_dir = REPO / ".airstack" / "runs" - before = set(runs_dir.glob("*/effective_config.env")) if runs_dir.exists() else set() + before = { + path: path.stat().st_mtime_ns + for path in runs_dir.glob("*/effective_config.env") + } if runs_dir.exists() else {} run_up_dry("--sim", "isaac") - after = set(runs_dir.glob("*/effective_config.env")) - new = after - before - assert new, "dry-run did not write an effective_config.env under .airstack/runs/" - content = max(new, key=lambda p: p.stat().st_mtime).read_text() + after = list(runs_dir.glob("*/effective_config.env")) + changed = [ + path for path in after + if path not in before or path.stat().st_mtime_ns != before[path] + ] + assert changed, "config-only did not write effective_config.env under .airstack/runs/" + content = max(changed, key=lambda p: p.stat().st_mtime_ns).read_text() assert "COMPOSE_PROFILES=" in content diff --git a/tests/meta/test_workflow_contract.py b/tests/meta/test_workflow_contract.py new file mode 100644 index 000000000..680d6a9b7 --- /dev/null +++ b/tests/meta/test_workflow_contract.py @@ -0,0 +1,63 @@ +"""Contracts for trustworthy GitHub Actions result identity and policy.""" + +import pytest + +from harness.discovery import repo_path + +pytestmark = pytest.mark.unit + + +def _workflow() -> str: + return repo_path(".github", "workflows", "system-tests.yml").read_text() + + +def test_comment_head_resolution_never_falls_back_to_default_branch(): + workflow = _workflow() + assert "${COMMENT_HEAD_SHA:-$EVENT_SHA}" not in workflow + assert "PR head SHA was not resolved" in workflow + assert 'echo "tested_sha=$COMMENT_HEAD_SHA"' in workflow + + +def test_comment_runs_cancel_older_run_for_same_pr(): + workflow = _workflow() + assert "github.event.issue.number || github.run_id" in workflow + assert "cancel-in-progress: true" in workflow + + +def test_report_job_installs_declared_dependencies(): + report = _workflow().split("\n report:", 1)[1] + assert "pip install -r tests/report-requirements.txt" in report + assert "pip install tabulate" not in report + requirements = repo_path("tests", "report-requirements.txt").read_text().lower() + assert "pyyaml" in requirements + assert "tabulate" in requirements + + +def test_image_preparation_is_structured_and_uploaded(): + workflow = _workflow() + assert "image-preparation.json" in workflow + assert "python3 -m harness.image_prep" in workflow + assert "path: tests/results/" in workflow + + +def test_metric_deltas_are_advisory_but_parser_errors_block(): + workflow = _workflow() + assert "- name: Fail on report integrity error" in workflow + assert "Metric regression detected" not in workflow + assert "parser_exit=2" in workflow + + +def test_tested_identity_is_written_into_campaign_metadata(): + system = _workflow() + unit = repo_path(".github", "workflows", "unit-tests.yml").read_text() + assert "AIRSTACK_TESTED_SHA: ${{ steps.identity.outputs.tested_sha }}" in system + assert "AIRSTACK_PR_NUMBER: ${{ steps.identity.outputs.pr_number }}" in system + assert "AIRSTACK_TESTED_SHA: ${{ github.sha }}" in unit + + +def test_baseline_search_downloads_candidates_then_selects_by_fingerprint(): + workflow = _workflow() + assert "-f per_page=20" in workflow + assert 'gh run download "$run_id"' in workflow + assert "select_baseline_path" in workflow + assert "dawidd6/action-download-artifact" not in workflow diff --git a/tests/parse_metrics.py b/tests/parse_metrics.py index 9d8d77210..5ccbba549 100644 --- a/tests/parse_metrics.py +++ b/tests/parse_metrics.py @@ -3,7 +3,8 @@ between two runs when --baseline is supplied. Reads results.xml (JUnit XML) for test durations and metrics.json for custom -metrics. In diff mode, exits 1 on regression; in single mode, always exits 0. +metrics. Numeric deltas are advisory and never change the process exit status; +report-generation errors exit 2. Usage: python parse_metrics.py --current tests/results// @@ -21,7 +22,11 @@ from tabulate import tabulate -from harness.run_meta import classify_run, simulation_metrics_comparable +from harness.run_meta import ( + classify_run, + comparability_reason, + simulation_metrics_comparable, +) from harness.test_ids import canonical_test_id FLAG_SUFFIX = {"regression": " :red_circle:", "improved": " :green_circle:"} @@ -234,7 +239,9 @@ def merge_metrics(run_dir): if test_name not in merged: merged[test_name] = {} merged[test_name].update(test_metrics) - _collapse_robots(merged) + # Keep robot/container identities visible. Exact campaign fingerprints + # already require matching robot counts, so pooling replicas would hide + # asymmetric failures without improving comparability. _expand_time_series(merged) return _collapse_iterations(merged) @@ -366,14 +373,11 @@ def _score(c, b, threshold): """Compute change% and regression flag for a metric pair. Returns (change_str, flag). flag ∈ {"", "regression", "improved"}. When either entry is missing/sentinel/time-series, returns a stub with an empty flag - (except: `timeout` current after numeric baseline → regression).""" + Missing/sentinel data is never converted into a numeric regression.""" if not c or not b: return ("new" if c and not b else "removed"), "" if not _is_scored(c) or not _is_scored(b): - cv = c.get("value") if isinstance(c, dict) else None - bv = b.get("value") if isinstance(b, dict) else None - flag = "regression" if (cv == "timeout" and isinstance(bv, (int, float))) else "" - return "—", flag + return "—", "" cv, bv = c["value"], b["value"] direction = c.get("direction", "lower_is_better") change_pct = ((cv - bv) / bv) * 100 if bv != 0 else 0 @@ -608,7 +612,10 @@ def render_passrates(mod): has_regression = regressions[0] if diff_mode and has_regression: - sections.append("**Regression detected** — some metrics exceeded the threshold.") + sections.append( + "**Advisory metric changes:** some comparable metrics exceeded the " + "display threshold. These deltas do not fail CI." + ) return "\n\n".join(sections), has_regression @@ -637,6 +644,8 @@ def _non_comparable_report(meta): ) fields = [ ("Outcome", outcome), + ("Failure class", meta.get("failure_class", "unavailable")), + ("Completion state", meta.get("completion_state", "unavailable")), ("Pytest exit status", meta.get("pytest_exitstatus", "unavailable")), ("Selected tests", meta.get("selected_tests", "unavailable")), ("Completed tests", meta.get("completed_tests", "unavailable")), @@ -690,9 +699,10 @@ def generate_report(current_dir, baseline_dir=None, threshold=20): "does not apply." ) elif baseline_dir and not diff_mode: + reason = comparability_reason(current_meta, baseline_meta) notices.append( "> The baseline is not the same complete simulation campaign. " - "Showing current results without a regression comparison." + f"Showing current results without a comparison: {reason}." ) if not md: md = "_No per-test metrics were recorded._" @@ -729,7 +739,9 @@ def main(): if args.output: Path(args.output).write_text(md) - sys.exit(1 if has_regression else 0) + # Assertions and infrastructure failures are enforced by pytest/run-tests. + # Comparable numeric deltas are intentionally advisory. + sys.exit(0) if __name__ == "__main__": diff --git a/tests/report-requirements.txt b/tests/report-requirements.txt new file mode 100644 index 000000000..58220af7d --- /dev/null +++ b/tests/report-requirements.txt @@ -0,0 +1,3 @@ +# Dependencies imported by the standalone metrics/report job. +PyYAML +tabulate diff --git a/tests/run_summary.py b/tests/run_summary.py index 3e60e7112..a8d121a21 100644 --- a/tests/run_summary.py +++ b/tests/run_summary.py @@ -14,7 +14,7 @@ import xml.etree.ElementTree as ET from pathlib import Path -from harness.run_meta import classify_run +from harness.run_meta import classify_run, comparability_reason from harness.test_ids import canonical_test_id PARAM_RE = re.compile(r"\[(.+)\]$") @@ -298,9 +298,11 @@ def build_summary_lines(run_dir: Path) -> list[str]: "", ] if run_meta.get("outcome") not in ("simulation", "non_simulation"): - reason = run_meta.get("reason", run_meta.get("outcome", "unknown")) + reason = run_meta.get("reason") or comparability_reason(run_meta) lines.extend([ f"Run status: {run_meta.get('outcome', 'unknown')}", + f"Failure class: {run_meta.get('failure_class', 'unavailable')}", + f"Completion state: {run_meta.get('completion_state', 'unavailable')}", f"Simulation metrics are not comparable: {reason}.", "", ]) @@ -345,6 +347,22 @@ def build_summary_lines(run_dir: Path) -> list[str]: if not emitted: lines.append("(no key metrics recorded)") + per_robot: dict[str, list[str]] = {} + for name in test_names: + for key, entry in _metrics_blob(metrics, name).items(): + match = ROBOT_METRIC_RE.match(key) + if not match or match.group(1) not in {item[0] for item in schema}: + continue + robot = key.split(".", 1)[0] + per_robot.setdefault(robot, []).append( + f"{match.group(1)}={_format_value(match.group(1), entry)}" + ) + if len(per_robot) > 1: + lines.append("") + lines.append("Per-robot metrics:") + for robot, values in sorted(per_robot.items()): + lines.append(f" {robot}: {', '.join(values)}") + if n_iter > 1: lines.append("") lines.append(f"Aggregated over {n_iter} stress iterations (mean ± stddev).") diff --git a/tests/system/test_liveliness.py b/tests/system/test_liveliness.py index 342e5f49a..65ee51fef 100644 --- a/tests/system/test_liveliness.py +++ b/tests/system/test_liveliness.py @@ -12,6 +12,8 @@ import pytest from conftest import ( + SimulatorHealthError, + collect_failure_diagnostics, container_running, current_test_id, docker_exec, @@ -94,6 +96,32 @@ def _check_tmux_panes(env): return True, f"all tmux panes active ({summary})" +def _check_sim_startup_process(env): + """Fast simulator-specific process/prerequisite health probe.""" + if not container_running(env["sim_container"]): + return False, f"{env['sim_container']} stopped" + ok, message = _check_tmux_panes(env) + if not ok: + return ok, message + if env["sim"] != "msairsim": + return True, message + result = docker_exec( + env["sim_container"], + "binary=${MS_AIRSIM_BINARY_PATH:-" + "/ms-airsim-env/Blocks/LinuxNoEditor/Blocks.sh}; " + "test -x \"$binary\" && " + "nvidia-smi -L >/dev/null && " + "pgrep -fa 'Blocks|AirSim|UE4' >/dev/null", + timeout=10, + ) + if result.returncode != 0: + return False, ( + "Microsoft AirSim infrastructure prerequisite failed: scene binary " + "or GPU is unavailable, or the UE4 process exited" + ) + return True, "Microsoft AirSim scene and UE4 process are healthy" + + def _check_sentinel_nodes(env): """Return (ok, msg). Expected sentinels per robot domain.""" cfg = env["cfg"] @@ -203,24 +231,37 @@ def ready(): @pytest.mark.dependency(name="sim_ready", depends=["sim_container"]) def test_sim_ready_time(self, airstack_env): - """Wait for first /clock message from the sim container (600s hard timeout).""" + """Wait for /clock while failing fast if the simulator process dies.""" cfg = airstack_env["cfg"] m = get_metrics() tid = current_test_id() start = airstack_env["up_started_at"] - if ( - wait_for_first_message( + try: + ready = wait_for_first_message( airstack_env["sim_container"], "/clock", domain_id=1, setup_bash=cfg["sim_setup_bash"], timeout=600, + health_check=lambda: _check_sim_startup_process(airstack_env), + health_grace=20, + ) + except SimulatorHealthError as exc: + path = collect_failure_diagnostics( + airstack_env, str(exc), current_test_id() ) - is None - ): + pytest.fail(f"{exc}; diagnostics: {path}") + if ready is None: m.record(tid, "sim_ready_duration_s", "timeout", unit="s") - pytest.fail("sim never published /clock within 600s") + path = collect_failure_diagnostics( + airstack_env, + "sim never published /clock within 600s", + current_test_id(), + ) + pytest.fail( + f"sim never published /clock within 600s; diagnostics: {path}" + ) m.record(tid, "sim_ready_duration_s", round(time.time() - start, 2), unit="s") @pytest.mark.dependency(name="tmux", depends=["containers"]) diff --git a/tests/system/test_optitrack_e2e.py b/tests/system/test_optitrack_e2e.py index 18530c7da..1382ca049 100644 --- a/tests/system/test_optitrack_e2e.py +++ b/tests/system/test_optitrack_e2e.py @@ -7,8 +7,8 @@ This brings the NatNet stack up **once** and asserts only one NatNet-specific test. The cheap, GPU-free half of this (host emulator → ``natnet_ros2`` Hz) lives in ``tests/integration/natnet/``. -Mark: ``optitrack``. Needs Docker + GPU + Isaac Sim license; skips cleanly when the -isaac-sim image isn't built locally. +Mark: ``optitrack``. Needs Docker + GPU + Isaac Sim license; missing images/SDK +are classified as infrastructure prerequisite failures before topic waits. """ import os import re @@ -18,7 +18,9 @@ from conftest import ( # noqa: E402 — pytest adds tests/ to sys.path airstack_cmd, + collect_failure_diagnostics, container_running, + docker_exec, find_container, get_metrics, get_robot_containers, @@ -111,6 +113,39 @@ _TRAJ_CFG = {"robot_setup_bash": _ROBOT_SETUP_BASH} +def _check_optitrack_prerequisites(robot_container: str) -> None: + """Fail before topic waits when licensed/build/runtime inputs are absent.""" + node = ros2_exec( + robot_container, + "prefix=$(ros2 pkg prefix natnet_ros2 2>/dev/null) && " + "test -x \"$prefix/lib/natnet_ros2/natnet_ros2_node\"", + domain_id=_ROBOT_DOMAIN, + setup_bash=_ROBOT_SETUP_BASH, + timeout=20, + ) + emulator = docker_exec( + "isaac-sim", + "test -d /isaac-sim/AirStack/simulation/isaac-sim/extensions/" + "optitrack.natnet.emulator && " + "pgrep -fa 'example_one_px4_pegasus_natnet|isaac-sim' >/dev/null", + timeout=20, + ) + missing = [] + if node.returncode != 0: + missing.append( + "natnet_ros2_node is not installed (the licensed NatNet SDK was " + "not provisioned when the robot image was built)" + ) + if emulator.returncode != 0: + missing.append("Isaac NatNet emulator extension/process is unavailable") + if missing: + reason = "OptiTrack infrastructure prerequisite failed: " + "; ".join(missing) + diagnostics = collect_failure_diagnostics( + _E2E_ENV, reason, "optitrack-prerequisites" + ) + pytest.fail(f"{reason}; diagnostics: {diagnostics}") + + def _arm_with_retries(container: str) -> None: """Arm the vehicle, retrying while PX4's preflight is still rejecting it. @@ -149,16 +184,20 @@ def optitrack_sim_stack(request): """Bring the NatNet Isaac stack up once for the module; tear it down after. Reuses an already-running robot-desktop container (fast local iteration); - otherwise brings the stack up. Skips when the isaac-sim image isn't built. + otherwise brings the stack up. Missing images fail as infrastructure. """ existing = find_container(_ROBOT_PATTERN) if existing and container_running(existing): + _check_optitrack_prerequisites(existing) yield {"container": existing, "brought_up": False} return missing = missing_images(env=_E2E_ENV) if missing: - pytest.skip("isaac-sim / robot image not built locally: " + ", ".join(missing)) + pytest.fail( + "OptiTrack infrastructure prerequisite failed: required images are " + "missing: " + ", ".join(missing) + ) airstack_cmd("down", timeout=120, log_name="optitrack_e2e") result = airstack_cmd("up", env_overrides=_E2E_ENV, timeout=300, log_name="optitrack_e2e") @@ -167,6 +206,7 @@ def optitrack_sim_stack(request): container = wait_for_container(_ROBOT_PATTERN, timeout=180) assert container, "robot-desktop container not Running after 180s" + _check_optitrack_prerequisites(container) try: yield {"container": container, "brought_up": True} finally: diff --git a/tests/system/test_sensors.py b/tests/system/test_sensors.py index 8170286d6..0e15ed9a8 100644 --- a/tests/system/test_sensors.py +++ b/tests/system/test_sensors.py @@ -10,7 +10,14 @@ import pytest -from conftest import current_test_id, get_metrics, logger, wait_for_first_message +from conftest import ( + SimulatorHealthError, + collect_failure_diagnostics, + current_test_id, + get_metrics, + logger, + wait_for_first_message, +) from sensor_probes import ( STABLE_HZ_DURATION_S, STABLE_HZ_WINDOW, @@ -20,7 +27,11 @@ check_robot_stereo_hz, check_sim_publishing, ) -from system.test_liveliness import _check_sentinel_nodes, _poll_until +from system.test_liveliness import ( + _check_sentinel_nodes, + _check_sim_startup_process, + _poll_until, +) @pytest.mark.sensors @@ -34,18 +45,31 @@ def test_sim_clock_available(self, airstack_env): m = get_metrics() tid = current_test_id() start = airstack_env["up_started_at"] - if ( - wait_for_first_message( + try: + ready = wait_for_first_message( airstack_env["sim_container"], "/clock", domain_id=1, setup_bash=cfg["sim_setup_bash"], timeout=600, + health_check=lambda: _check_sim_startup_process(airstack_env), + health_grace=20, + ) + except SimulatorHealthError as exc: + path = collect_failure_diagnostics( + airstack_env, str(exc), current_test_id() ) - is None - ): + pytest.fail(f"{exc}; diagnostics: {path}") + if ready is None: m.record(tid, "sensors_sim_ready_duration_s", "timeout", unit="s") - pytest.fail("sim never published /clock within 600s") + path = collect_failure_diagnostics( + airstack_env, + "sim never published /clock within 600s", + current_test_id(), + ) + pytest.fail( + f"sim never published /clock within 600s; diagnostics: {path}" + ) m.record(tid, "sensors_sim_ready_duration_s", round(time.time() - start, 2), unit="s") @pytest.mark.dependency(name="sensors_nodes", depends=["sensors_sim_ready"]) From be9ed0d6a2f2b688e24945e59c90cd2c41c3296f Mon Sep 17 00:00:00 2001 From: Pranav Kumara Date: Fri, 21 Aug 2026 17:42:05 -0400 Subject: [PATCH 2/6] Classify readiness failures as infrastructure Preserve explicit infrastructure intent through pytest call reports so simulator startup crashes cannot be misreported as algorithm assertions. --- tests/conftest.py | 13 ++++++++++++- tests/harness/run_meta.py | 5 ++++- tests/meta/test_campaign_reporting_contract.py | 14 ++++++++++++++ tests/pytest.ini | 1 + tests/system/test_liveliness.py | 1 + tests/system/test_sensors.py | 1 + 6 files changed, 33 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 43002caf1..0b13665b2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -171,9 +171,20 @@ def pytest_sessionfinish(session, exitstatus): @pytest.hookimpl(hookwrapper=True) def pytest_runtest_makereport(item, call): - """Attach phase reports to the item so fixtures can inspect pass/fail.""" + """Attach phase reports and preserve the assertion/infrastructure boundary.""" outcome = yield rep = outcome.get_result() + if rep.failed: + text = str(rep.longrepr).lower() + is_infrastructure = bool( + item.get_closest_marker("infrastructure") + or rep.when in ("setup", "teardown") + or "infrastructure prerequisite" in text + or "infrastructure simulator process failure" in text + ) + rep.airstack_failure_class = ( + "infrastructure" if is_infrastructure else "assertion" + ) setattr(item, f"_rep_{rep.when}", rep) diff --git a/tests/harness/run_meta.py b/tests/harness/run_meta.py index 9afb85f60..5d8a8ab67 100644 --- a/tests/harness/run_meta.py +++ b/tests/harness/run_meta.py @@ -110,7 +110,10 @@ def _report_details(reports) -> tuple[dict[str, str], set[str], set[str]]: continue if report.failed: outcome = "failed" - if when != "call": + if ( + when != "call" + or getattr(report, "airstack_failure_class", "") == "infrastructure" + ): infrastructure_error_nodeids.add(nodeid) elif report.skipped: outcome = "skipped" diff --git a/tests/meta/test_campaign_reporting_contract.py b/tests/meta/test_campaign_reporting_contract.py index 356770ed1..1522b5071 100644 --- a/tests/meta/test_campaign_reporting_contract.py +++ b/tests/meta/test_campaign_reporting_contract.py @@ -78,6 +78,20 @@ def test_assertion_with_downstream_dependency_skip_is_finalized_campaign(): assert meta["simulation_finalized"] == 2 +def test_call_phase_infrastructure_failure_is_not_an_algorithm_assertion(): + report = _report("call", "failed") + report.airstack_failure_class = "infrastructure" + meta = build_run_meta( + [_item()], + 1, + reports=[report], + campaign_config={"sim": "msairsim", "num_robots": "1"}, + ) + assert meta["outcome"] == "incomplete" + assert meta["failure_class"] == "infrastructure" + assert meta["complete"] is False + + def _write_run(path, fingerprint, complete=True): path.mkdir() (path / "results.xml").write_text("") diff --git a/tests/pytest.ini b/tests/pytest.ini index 69538af34..686ab22d9 100644 --- a/tests/pytest.ini +++ b/tests/pytest.ini @@ -10,6 +10,7 @@ markers = autonomy: Fixed-pattern trajectory path-tracker benchmark (test_fixed_trajectory.py) waypoint_flight: Ordered-waypoint navigation judged on the odometry track (test_waypoint_flight.py) optitrack: OptiTrack NatNet end-to-end (sim emulator → natnet_ros2 → PX4 EV fusion) + infrastructure: Readiness/prerequisite checks whose failures are CI environment faults testpaths = . addopts = -v --durations=0 --import-mode=importlib cache_dir = /tmp/.pytest_cache diff --git a/tests/system/test_liveliness.py b/tests/system/test_liveliness.py index 65ee51fef..ac0675e96 100644 --- a/tests/system/test_liveliness.py +++ b/tests/system/test_liveliness.py @@ -183,6 +183,7 @@ def _poll_until(predicate, timeout, interval, fail_msg): @pytest.mark.liveliness +@pytest.mark.infrastructure @pytest.mark.timeout(1800) class TestLiveliness: diff --git a/tests/system/test_sensors.py b/tests/system/test_sensors.py index 0e15ed9a8..35e212e5d 100644 --- a/tests/system/test_sensors.py +++ b/tests/system/test_sensors.py @@ -39,6 +39,7 @@ class TestSensors: @pytest.mark.dependency(name="sensors_sim_ready") + @pytest.mark.infrastructure def test_sim_clock_available(self, airstack_env): """Wait for ``/clock`` on the sim container (same readiness gate as liveliness).""" cfg = airstack_env["cfg"] From 4640b56aa00de5d37eb801c6f9815be97a81782d Mon Sep 17 00:00:00 2001 From: Pranav Kumara Date: Fri, 21 Aug 2026 18:12:06 -0400 Subject: [PATCH 3/6] Allow focused manual flight campaigns Expose trajectory and takeoff sweeps in workflow dispatch so CI validation can run minimal algorithm samples before expanding to expensive matrices. --- .github/workflows/system-tests.yml | 14 ++++++++++++++ docs/development/intermediate/testing/ci_cd.md | 2 +- tests/README.md | 2 ++ tests/meta/test_workflow_contract.py | 8 ++++++++ 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/.github/workflows/system-tests.yml b/.github/workflows/system-tests.yml index c6cce0d4f..67b493df7 100644 --- a/.github/workflows/system-tests.yml +++ b/.github/workflows/system-tests.yml @@ -30,6 +30,14 @@ on: description: "Seconds for test_stable polling window" default: "120" required: false + trajectory_types: + description: "Fixed trajectories, comma-separated (e.g. Circle or Circle,Figure8)" + default: "Circle,Figure8,Racetrack,Line" + required: false + takeoff_velocities: + description: "Takeoff velocities, comma-separated (e.g. 0.5 or 0.5,1)" + default: "0.5" + required: false baseline_run_id: description: "Run ID to use as baseline for metric comparison (blank = latest successful run on main)" default: "" @@ -144,6 +152,8 @@ jobs: INPUT_NUM_ROBOTS: ${{ inputs.num_robots }} INPUT_ITERATIONS: ${{ inputs.stress_iterations }} INPUT_STABLE: ${{ inputs.stable_duration }} + INPUT_TRAJECTORIES: ${{ inputs.trajectory_types }} + INPUT_TAKEOFF_VELOCITIES: ${{ inputs.takeoff_velocities }} run: | python3 <<'PYEOF' import os, shlex, sys @@ -161,6 +171,10 @@ jobs: args.extend(['--stress-iterations', it]) if (st := os.environ.get('INPUT_STABLE', '').strip()): args.extend(['--stable-duration', st]) + if (trajectories := os.environ.get('INPUT_TRAJECTORIES', '').strip()): + args.extend(['--trajectory-types', trajectories]) + if (velocities := os.environ.get('INPUT_TAKEOFF_VELOCITIES', '').strip()): + args.extend(['--takeoff-velocities', velocities]) elif event == 'pull_request': # Automatic PR validation is deliberately build-scoped. Fast # Python unit tests run in unit-tests.yml; GPU simulation remains diff --git a/docs/development/intermediate/testing/ci_cd.md b/docs/development/intermediate/testing/ci_cd.md index b5e45a8dd..18c411b0c 100644 --- a/docs/development/intermediate/testing/ci_cd.md +++ b/docs/development/intermediate/testing/ci_cd.md @@ -185,7 +185,7 @@ it to Harbor. | `unit-tests.yml` pull request | PR to `main`/`develop` opened, synchronized, or reopened (including forks) | `pytest tests/ -m unit` on `ubuntu-latest` | | `system-tests.yml` pull request | PR opened, synchronized, or reopened, same-repo branches only | `-m build_packages` on an OSMO worker | | `/pytest` PR comment | Any time, from a user with `OWNER`/`MEMBER`/`COLLABORATOR` association | Whatever args you put on the first line of the comment | -| `workflow_dispatch` | Manual, from the Actions tab | The form inputs: `marks`, `sim`, `num_robots`, `stress_iterations`, `stable_duration`, `baseline_run_id` | +| `workflow_dispatch` | Manual, from the Actions tab | The form inputs: `marks`, `sim`, `num_robots`, `stress_iterations`, `stable_duration`, `trajectory_types`, `takeoff_velocities`, `baseline_run_id` | PR pushes re-run the fast unit gate and the pull-only `build_packages` gate. GPU-intensive simulations do **not** run automatically; select the campaign diff --git a/tests/README.md b/tests/README.md index 7a458e91a..372e47c56 100644 --- a/tests/README.md +++ b/tests/README.md @@ -566,6 +566,8 @@ opened, updated, or reopened against `main` or `develop`. | `num_robots` | `1` | Robot counts | | `stress_iterations` | `1` | Iterations per config | | `stable_duration` | `120` | Stability polling seconds | +| `trajectory_types` | `Circle,Figure8,Racetrack,Line` | Fixed-trajectory sweep; set `Circle` for a minimal campaign | +| `takeoff_velocities` | `0.5` | Takeoff velocity sweep | | `baseline_run_id` | _(blank)_ | Run ID for comparison; blank = latest `main` run | #### Jobs diff --git a/tests/meta/test_workflow_contract.py b/tests/meta/test_workflow_contract.py index 680d6a9b7..08ccde40d 100644 --- a/tests/meta/test_workflow_contract.py +++ b/tests/meta/test_workflow_contract.py @@ -61,3 +61,11 @@ def test_baseline_search_downloads_candidates_then_selects_by_fingerprint(): assert 'gh run download "$run_id"' in workflow assert "select_baseline_path" in workflow assert "dawidd6/action-download-artifact" not in workflow + + +def test_manual_campaign_can_select_minimal_algorithm_sweeps(): + workflow = _workflow() + assert "trajectory_types:" in workflow + assert "takeoff_velocities:" in workflow + assert "args.extend(['--trajectory-types', trajectories])" in workflow + assert "args.extend(['--takeoff-velocities', velocities])" in workflow From 2222efdd357ef827b968919e80e301c15702ab67 Mon Sep 17 00:00:00 2001 From: Pranav Kumara Date: Sat, 29 Aug 2026 00:45:29 -0400 Subject: [PATCH 4/6] Finish leftover merge markers from the develop rebase. Keep develop's images CLI and Using CI layout while preserving this PR's advisory metrics, diagnostics artifacts, and infrastructure mark. --- .github/workflows/system-tests.yml | 16 +- airstack.sh | 11 +- .../development/intermediate/testing/ci_cd.md | 170 +----------------- .../intermediate/testing/using_ci.md | 11 +- tests/README.md | 19 +- tests/pytest.ini | 1 + 6 files changed, 22 insertions(+), 206 deletions(-) diff --git a/.github/workflows/system-tests.yml b/.github/workflows/system-tests.yml index 05df2ef3f..e73fa1a3a 100644 --- a/.github/workflows/system-tests.yml +++ b/.github/workflows/system-tests.yml @@ -410,7 +410,7 @@ jobs: # what's still missing afterwards instead of aborting on the first # gap. `--progress=quiet` suppresses per-layer progress; errors # still surface on stderr. - ./airstack.sh --progress=quiet image-pull --ignore-pull-failures || true + ./airstack.sh --progress=quiet images pull --ignore-pull-failures || true while IFS= read -r img; do [[ -z "$img" || -n "${present_before[$img]:-}" ]] && continue if docker image inspect "$img" --format '{{.Id}}' >/dev/null 2>&1; then @@ -452,20 +452,16 @@ jobs: echo "Images still missing after pull/retag:" printf ' - %s\n' "${missing[@]}" if [[ "$NO_IMAGE_BUILD" == "true" ]]; then - echo "::error::Pull-only mode (--no-image-build or -m build_packages) will not image-build. Run /pytest -m build_docker once, or omit --no-image-build." - exit 1 - fi - echo "Falling back to image-build" - ./airstack.sh --progress=quiet image-build - built=("${missing[@]}") - image_outcome=locally-built -======= + IMAGE_OUTCOME=missing IMAGE_RESULT="$image_result" \ + MISSING_IMAGES="$(printf '%s\n' "${missing[@]}")" \ + PYTHONPATH=tests python3 -m harness.image_prep "$image_result" echo "::error::Pull-only mode (--no-image-build or -m build_packages) will not run 'images build'. Run /pytest -m build_docker once, or omit --no-image-build." exit 1 fi echo "Falling back to images build" ./airstack.sh --progress=quiet images build ->>>>>>> origin/develop + built=("${missing[@]}") + image_outcome=locally-built else echo "All required images present after pull/retag — skipping build." fi diff --git a/airstack.sh b/airstack.sh index 2737b71a8..0e0fe1d86 100755 --- a/airstack.sh +++ b/airstack.sh @@ -2328,17 +2328,8 @@ function register_builtin_commands { # Register help text for built-in commands COMMAND_HELP["install"]="Install dependencies (Docker Engine, NVIDIA Container Toolkit)" COMMAND_HELP["setup"]="Configure AirStack settings and add to shell profile" -<<<<<<< HEAD - COMMAND_HELP["image-build"]="Build or rebuild Docker Compose service images" - COMMAND_HELP["image-push"]="Push Docker Compose service images to a registry" - COMMAND_HELP["image-pull"]="Pull Docker Compose service images from a registry" - COMMAND_HELP["images"]="List Docker images filtered by PROJECT_NAME from .env" - COMMAND_HELP["image-delete"]="Delete all Docker images matching PROJECT_NAME (prompts unless -y)" - COMMAND_HELP["up"]="Start services [--sim isaac|airsim] [--robots N] [--headless] [--play|--no-play] [--no-autolaunch] [--wait] [--dry-run] [--config-only]" -======= COMMAND_HELP["images"]="Manage Docker Compose service images: list (default)|build|push|pull|delete|rm (see 'airstack help images')" - COMMAND_HELP["up"]="Start services [--sim isaac|airsim|simple] [--robots N] [--stack NAME] [--fleet NAME] [--headless] [--play|--no-play] [--no-autolaunch] [--wait] [--dry-run]" ->>>>>>> origin/develop + COMMAND_HELP["up"]="Start services [--sim isaac|airsim|simple] [--robots N] [--stack NAME] [--fleet NAME] [--headless] [--play|--no-play] [--no-autolaunch] [--wait] [--dry-run] [--config-only]" COMMAND_HELP["down"]="down services" COMMAND_HELP["clean"]="Remove all ROS 2 build artifacts (build/, install/, log/)" COMMAND_HELP["connect"]="Connect to a running container (supports partial name matching)" diff --git a/docs/development/intermediate/testing/ci_cd.md b/docs/development/intermediate/testing/ci_cd.md index 24a803720..24fa487bf 100644 --- a/docs/development/intermediate/testing/ci_cd.md +++ b/docs/development/intermediate/testing/ci_cd.md @@ -32,8 +32,8 @@ security model. It is aimed at maintainers of the pipeline itself. | Where do CI jobs run? | Python unit tests: `ubuntu-latest`. Build and simulation tests: a fresh OSMO GPU pod, destroyed afterward. | | What triggers a run? | PR open/update/reopen runs unit + package-build gates; maintainers select simulations with `/pytest`; `workflow_dispatch` is also available. | | What gets tested? | Automatically: Python units/contracts and ROS package builds/tests. Selectably: Docker builds, liveliness, sensors, and flight policies. (OptiTrack system tests live in the asm_optitrack module's CI.) | -| How do I see results? | Checks plus a report comment and `test-results-*` artifact (`summary.txt`, `results.xml`, `run_meta.json`, `metrics.json`). | -| What fails the build? | Any failed test, or a comparable simulation metric regressing more than 20%. Invalid/incomplete campaigns are labeled, not scored as policy failures. | +| How do I see results? | Checks plus a report comment and `test-results-*` artifact (`summary.txt`, `results.xml`, `run_meta.json`, `metrics.json`, and bounded failure diagnostics when needed). | +| What fails the build? | Test assertions, infrastructure/prerequisite failures, or report integrity failures. Comparable numeric metric deltas are advisory. | | Who holds the secrets? | Only the orchestrator host. Workers get a single-use JIT token valid for one registration. | --- @@ -281,170 +281,6 @@ Manual dispatch accepts `force_rebuild=true` to rebuild and relabel everything --- -<<<<<<< HEAD -## What the pipeline tests, and what that catches - -Tests are selected with pytest marks. Collection order is fixed in -`tests/conftest.py` so cheap and prerequisite suites always run first — a -`colcon` break fails in minutes instead of after a sim bring-up. - -```mermaid -flowchart LR - u["unit
seconds, no Docker"] --> bd["build_docker
image builds"] - bd --> bp["build_packages
colcon build in containers"] - bp --> lv["liveliness
stack comes up"] - lv --> sn["sensors
streams flow at rate"] - sn --> th["takeoff_hover_land
flight chain"] - th --> au["autonomy
trajectory tracking"] -``` - -| Mark | Module | What it verifies | Bugs it is good at catching | -|---|---|---|---| -| `unit` | `/test/` (co-located) | Hermetic Python/numpy logic co-located with each ROS 2 package | Off-by-one and boundary errors in filters, converters, validators; regressions in pure algorithm code | -| `build_docker` | `system/test_build_docker.py` | Every image builds; records image sizes | Broken Dockerfiles, deleted apt packages, upstream base-image drift, accidental image bloat | -| `build_packages` | `system/test_build_packages.py` | `colcon build` inside robot, GCS, and ms-airsim workspaces | Missing `package.xml` dependencies, uninstalled launch/config files, C++ breakage on a clean tree | -| `liveliness` | `system/test_liveliness.py` | Containers reach Running, `/clock` publishes, tmux panes alive, sentinel ROS 2 nodes present, compute snapshot, stability poll | Launch files that crash on start, nodes that die after 30 s, `ROBOT_NAME`/domain-ID misconfiguration, runaway CPU or memory | -| `sensors` | `system/test_sensors.py` | Stereo and depth publish rates on both sim and robot side, filtered LiDAR liveness plus geometry sanity, sim real-time factor, time-series stability | Broken sim-to-ROS bridges, sensor Hz that silently halves, RTF collapse from a heavy new node, LiDAR filter range regressions | -| `takeoff_hover_land` | `system/test_takeoff_hover_land.py` | Four-phase chain per (sim, robots, iteration, velocity): PX4 ready → takeoff to 10 m → hover → land | Controller tuning regressions, altitude overshoot, hover drift, state-estimation bias against ground truth, PX4/MAVROS handshake breakage | -| `autonomy` | `system/test_fixed_trajectory.py` | Same chain with a Circle / Figure8 / Racetrack / Line pattern in the middle; records cross-track error and path RMSE | Path-tracker regressions, trajectory-library math errors, velocity/acceleration limit violations that show up as corner-cutting | - -### The flight chain - -Both flight suites run as an ordered chain per parametrization, so the drone -always ends on the ground before the next configuration starts: - -```mermaid -flowchart LR - r["test_px4_ready
MAVROS + EKF"] --> t["test_takeoff
within 10% of 10 m"] - t --> x["test_hover or test_fixed_trajectory"] - x --> l["test_landing
final altitude < 0.5 m"] - r -. "failure" .-> s["remaining phases skipped"] - t -. "failure" .-> s - x -. "failure still lands" .-> l -``` - -A failure in the middle phase (`test_hover` or `test_fixed_trajectory`) does -**not** skip landing — a bad tracker must not leave a drone stuck in the air -blocking the rest of the sweep. A failure in `test_px4_ready` or `test_takeoff` -does skip the remaining phases for that configuration. - -### Bring-up scope, and why mark selection costs money - -`airstack_env` is **class-scoped** and parametrized over -`(sim, num_robots, iteration)`. Each test class does its own `airstack up` and -`airstack down`. Selecting two suites with `or` therefore performs **two full -stack cycles per tuple**: - -```text --m liveliness → 1 bring-up per (sim, robots, iter) --m "liveliness or sensors" → 2 bring-ups per (sim, robots, iter) ---sim msairsim → opt in; both sims doubles all of the above ---num-robots 1,3 → doubles it again -``` - -Run one mark at a time unless you genuinely need both. - ---- - -## Reading the results - -### The PR comment - -After `run-tests` finishes — pass or fail — a `report` job on `ubuntu-latest` -downloads the current artifact plus a **baseline** artifact and runs -[`parse_metrics.py`](https://github.com/castacks/AirStack/blob/main/tests/parse_metrics.py) -in diff mode only after selecting the newest completed artifact with the same -simulation campaign fingerprint (normalized selected tests and all relevant -CLI/configuration parameters). - -| Run type | Baseline used | -|---|---| -| PR opened or `/pytest` | Latest `system-tests.yml` artifact on the PR's base branch | -| `workflow_dispatch` with `baseline_run_id` | That specific run | -| `workflow_dispatch` without it | Latest artifact on `main` | - -For a complete simulation campaign, the comment has pass rates plus a flat -**Metrics** table, a **Sim publishing rates** pivot (topic Hz aggregates from -the `sensors` mark), and a **Compute usage** pivot (CPU / memory / GPU per -container). Regressions are marked with a red circle, improvements with a -green one, and the job **fails** if any comparable metric moves more than the -20% display threshold in the wrong direction. These numeric deltas are advisory: -they inform review but do not fail the PR. - -`run_meta.json` separates those policy results from CI failures. A collection -error, zero-test selection, internal pytest error, cancellation, or timeout is -reported as **simulation metrics are not comparable**. Pass-rate and regression -tables are suppressed in that case; the infrastructure problem cannot appear as -a false 0% policy score. A policy assertion that runs and fails remains a real -simulation result and keeps its recorded error metrics. - -### The artifact - -`test-results--`, retained 90 days: - -```text -tests/results/2026-08-06_14-30-00/ -├── summary.txt # human-readable per-chain summary — open this first -├── results.xml # JUnit XML: durations, pass/fail per test -├── run_meta.json # schema-v2 completion/failure class + exact campaign config -├── metrics.json # every recorded metric, including time series -└── diagnostics/ # on failure: bounded config, panes, logs, ROS/GPU/command ring -``` - -There are no per-test log files. Live output streams to the Actions log via -pytest's `log_cli`, and failed assertions embed the tail of the relevant -`docker` or `ros2` subprocess output directly in the failure message. - -Regenerate a report locally from a downloaded artifact: - -```bash -python tests/parse_metrics.py \ - --current path/to/current-run/ \ - --baseline path/to/baseline-run/ \ - --threshold 20 -``` - ---- - -## Using CI well while developing - -The pipeline is expensive at the far end and nearly free at the near end. Push -each class of failure as far left as it will go. - -```mermaid -flowchart TD - q{"What did you change?"} - q -- "Pure Python / numpy logic" --> u["airstack test -m unit
seconds, no GPU"] - q -- "Dockerfile / dependency" --> b["airstack test -m build_docker or build_packages
minutes, no GPU"] - q -- "Launch file / new node" --> l["airstack test -m liveliness --sim msairsim --num-robots 1"] - q -- "Sensor or bridge" --> s["airstack test -m sensors --sim isaacsim --num-robots 1"] - q -- "Controller / planner" --> a["airstack test -m autonomy --sim msairsim --trajectory-types Circle"] - u --> pr["Push branch, open PR"] - b --> pr - l --> pr - s --> pr - a --> pr - pr --> fast["unit-tests.yml on ubuntu-latest"] - pr --> ci["build_packages on an ephemeral OSMO pod"] - fast --> rep["Read automatic check results"] - ci --> rep - rep --> iter["/pytest with the relevant simulation mark"] - iter --> metrics["Read like-for-like policy metrics"] -``` - -Practical rules that follow from how the system is built: - -- **Reproduce CI locally with the same command.** `airstack test` and CI both call `pytest tests/` with the same flags. If a run fails in CI, copy the resolved command from the acknowledgment comment and run it on any GPU box — including an [interactive OSMO dev pod](../../../tutorials/airstack_on_osmo.md) if you do not have a local GPU. -- **Narrow before you re-run.** A `/pytest` with no args re-runs everything. `/pytest -m autonomy --sim msairsim --trajectory-types Circle` re-runs the one chain you are fixing, in a fraction of the time. -- **Never trust a green launch test against a stale build.** This is why `build_packages` is auto-prepended; keep it that way when writing your own `/pytest` line. -- **Read `summary.txt` before the raw log.** It groups each flight chain with per-phase wall times and status, so the failing phase is obvious without scrolling a 40-minute log. -- **Treat a like-for-like metrics diff as a review artifact.** The reporter compares only identical selected simulation campaigns; a PR that turns a metric red needs an explanation even when every assertion passed. -- **Bump `VERSION` in `.env` when image content changes.** [`check-version-increment.yml`](https://github.com/castacks/AirStack/blob/main/.github/workflows/check-version-increment.yml) gates the PR on a strictly-greater semver, and merging that bump is what triggers the release build below. - ---- - -======= ->>>>>>> origin/develop ## The release path `system-tests.yml` is not the only workflow on the ephemeral runners. @@ -548,7 +384,7 @@ Full runbook, including credential rotation and worker-side diagnostics: | [`.github/orchestrator/config.example.yaml`](../../../../.github/orchestrator/config.example.yaml) | Every tunable: pool, platform, resources, limits, poll intervals | | [`.github/orchestrator/setup.sh`](../../../../.github/orchestrator/setup.sh) | One-time orchestrator host install | | [`tests/conftest.py`](../../../../tests/conftest.py) | `airstack_env` fixture, collection order, `MetricsRecorder` | -| [`tests/parse_metrics.py`](../../../../tests/parse_metrics.py) | Report generation and the regression gate | +| [`tests/parse_metrics.py`](../../../../tests/parse_metrics.py) | Comparable advisory report generation and report-integrity gate | | [`tests/run_summary.py`](../../../../tests/run_summary.py) | `summary.txt` generation | ## See also diff --git a/docs/development/intermediate/testing/using_ci.md b/docs/development/intermediate/testing/using_ci.md index 4d6c54ff8..20a99cffd 100644 --- a/docs/development/intermediate/testing/using_ci.md +++ b/docs/development/intermediate/testing/using_ci.md @@ -142,8 +142,8 @@ For a complete simulation campaign, the comment has pass rates plus a flat **Metrics** table, a **Sim publishing rates** pivot (topic Hz aggregates from the `sensors` mark), and a **Compute usage** pivot (CPU / memory / GPU per container). Regressions are marked with a red circle, improvements with a -green one, and the job **fails** if any comparable metric moves more than the -20% threshold in the wrong direction. +green one. These numeric deltas are advisory: they inform review but do not +fail the PR. `run_meta.json` separates those policy results from CI failures. A collection error, zero-test selection, internal pytest error, cancellation, or timeout is @@ -160,8 +160,9 @@ simulation result and keeps its recorded error metrics. tests/results/2026-08-06_14-30-00/ ├── summary.txt # human-readable per-chain summary — open this first ├── results.xml # JUnit XML: durations, pass/fail per test -├── run_meta.json # completion state, pytest exit, selected/executed sim counts -└── metrics.json # every recorded metric, including time series +├── run_meta.json # schema-v2 completion/failure class + exact campaign config +├── metrics.json # every recorded metric, including time series +└── diagnostics/ # on failure: bounded config, panes, logs, ROS/GPU/command ring ``` There are no per-test log files. Live output streams to the Actions log via @@ -226,7 +227,7 @@ The failures a CI *user* can act on from the GitHub side: | `system-tests.yml` never ran on a fork PR | Trigger guard | Expected: the `pull_request` path only runs for same-repo branches; only the `ubuntu-latest` unit gate runs on forks | | Runner registered, then pytest failed | Tests | A real test failure — the GitHub Actions log and `summary.txt` are canonical | | Report says “simulation metrics are not comparable” | Collection/infrastructure | Read the run outcome and pytest exit status in `run_meta.json`; no policy regression was scored | -| Metrics report job failed with no test failures | Report | A like-for-like metric regressed past the 20% threshold, or report generation itself failed; read the report step log | +| Metrics report job failed with no test failures | Report | Report generation or artifact integrity failed; numeric metric deltas are advisory and do not cause this conclusion | | Job sits `queued` forever, no runner appears | Orchestrator / pod | Not fixable from the PR — an admin needs to inspect the orchestrator and pod; see the [pipeline troubleshooting table](ci_cd.md#troubleshooting) | Orchestrator-, OSMO-, and pod-level failures (auth errors, `dockerd did not diff --git a/tests/README.md b/tests/README.md index efcc8d619..71caf65b9 100644 --- a/tests/README.md +++ b/tests/README.md @@ -142,31 +142,22 @@ Writes custom metrics to `tests/results//metrics.json` after each `re ### Output files -<<<<<<< HEAD -Every test run produces a timestamped directory with the finalized results. -Simulator/startup failures additionally create a bounded `diagnostics/` JSON -bundle; full unbounded logs are never copied into the artifact. -======= Every test run produces a timestamped directory containing `summary.txt`, `results.xml`, `run_meta.json`, and `metrics.json` (plus a `wiring/` subdirectory -when the `wiring` mark runs) — there is **no** `logs/` subdirectory and no -per-test log files are written under the run directory. ->>>>>>> origin/develop +when the `wiring` mark runs, and a bounded `diagnostics/` JSON bundle on +simulator/startup failures). There is **no** `logs/` subdirectory and no +per-test log files are written under the run directory. Full unbounded logs +are never copied into the artifact. ``` tests/results/ └── 2025-04-21_14-30-00/ ├── summary.txt # Human-readable key metrics — open this first ├── results.xml # JUnit XML — test durations and pass/fail status -<<<<<<< HEAD ├── run_meta.json # Schema-v2 completion/failure class + exact campaign ├── metrics.json # Custom metrics (image sizes, Hz, compute, timing) - └── diagnostics/ # On failure: config, panes, log tails, ROS/GPU/commands -======= - ├── run_meta.json # Completion/outcome and campaign fingerprint - ├── metrics.json # Custom metrics (image sizes, Hz, compute, timing) + ├── diagnostics/ # On failure: config, panes, log tails, ROS/GPU/commands └── wiring/ # (wiring mark only) observed_.md graph snapshots ->>>>>>> origin/develop ``` Live test output goes to the terminal (pytest `log_cli`). Diagnostics are diff --git a/tests/pytest.ini b/tests/pytest.ini index 615e6915c..2a6adce8d 100644 --- a/tests/pytest.ini +++ b/tests/pytest.ini @@ -12,6 +12,7 @@ markers = waypoint_flight: Ordered-waypoint navigation judged on the odometry track (test_waypoint_flight.py) simple_sim: Simple-sim smoke test — containers, /clock, SIM_TYPE=simple sentinel nodes; run with --sim simplesim (test_simple_sim.py) optitrack: OptiTrack NatNet end-to-end — registered for asm_optitrack module CI (tests live in the module repo) + infrastructure: Readiness/prerequisite checks whose failures are CI environment faults testpaths = . addopts = -v --durations=0 --import-mode=importlib cache_dir = /tmp/.pytest_cache From c4d2ca21c139cf285c9183bd2b530386ba6db52a Mon Sep 17 00:00:00 2001 From: Pranav Kumara Date: Sat, 29 Aug 2026 00:53:28 -0400 Subject: [PATCH 5/6] Fix unit-test merge fallout and bump VERSION. Keep --config-only hermetic for fleet contracts, still fail AUTONOMY_ROLE as a config check, and document the new help tokens so CI can pass the increment gate. --- .env | 2 +- airstack.sh | 43 +++++++++++++----------- tests/meta/test_docs_catalog_contract.py | 9 +++++ tests/meta/test_fleet_contract.py | 14 ++++---- 4 files changed, 41 insertions(+), 27 deletions(-) diff --git a/.env b/.env index 64a1b3f6b..8e3da25f1 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.20.0-alpha.17" +VERSION="0.20.0-alpha.18" # Image-tag discriminator ONLY (appears in the image tag suffix, e.g. ..._robot-x86-64_dev). # No Dockerfile consumes it: "prebuilt" does NOT bake the built ros_ws into the image today — # a real prebuilt (workspace-baked) stage is future work. Keep "dev" (mounted code, built live). diff --git a/airstack.sh b/airstack.sh index 0e0fe1d86..05f294ebd 100755 --- a/airstack.sh +++ b/airstack.sh @@ -194,6 +194,8 @@ function print_command_help { echo " --no-autolaunch Start containers idle (AUTOLAUNCH=false; launch manually)." echo " --wait After starting, block until flight-ready (airstack ready)." echo " --dry-run Validate + print the derived launch config; start nothing." + echo " --config-only Like --dry-run, but only logical launch-config checks" + echo " (no Docker, credentials, images, or submodule prereqs)." echo "" echo "Anything else (e.g. --build, service names) is passed through to" echo "'docker compose up'." @@ -386,6 +388,7 @@ function print_command_help { echo " simple_sim Simple-sim smoke test (containers, /clock, sentinel nodes;" echo " run with --sim simplesim --num-robots 1)" echo " optitrack OptiTrack NatNet end-to-end (tests live in the asm_optitrack module)" + echo " infrastructure Readiness/prerequisite checks (CI environment faults)" echo "" echo "AirStack-specific options (defaults from tests/conftest.py):" echo " --sim=TARGETS Comma-separated sim targets: isaacsim, msairsim," @@ -1642,6 +1645,24 @@ function preflight_up { fi fi + # 5. Deprecation shim (remove in 0.21.0): LAUNCH_NATNET no longer does + # anything — OptiTrack was extracted to the asm_optitrack module. + local _pf_natnet + _pf_natnet=$(resolve_launch_var LAUNCH_NATNET "${_pf_global[@]}") + if [[ -n "$_pf_natnet" ]]; then + log_warn "LAUNCH_NATNET is gone — OptiTrack moved to the asm_optitrack module (airstack module add https://github.com/castacks/asm_optitrack --version ); see docs/development/modules.md" + fi + + # 6. Deprecation shim (remove in 0.21.0): AUTONOMY_ROLE was REMOVED + # (stacks — RFC #379 — are the only launch dispatch). A set value counts + # only via env / --env-file / .env; nothing in the compose files defaults + # it anymore. This is a configuration contract, so it runs for --config-only. + local _pf_role + _pf_role=$(resolve_launch_var AUTONOMY_ROLE "${_pf_global[@]}") + if [[ -n "$_pf_role" ]]; then + _pf_error "AUTONOMY_ROLE was removed — select a stack: airstack up --stack (see docs/development/stacks.md). Migration: full → full_default (the no-stack default), onboard → lite_default, onboard/offboard split → lite_offload_global:onboard / :offboard." + fi + # Configuration contracts intentionally stop before Docker, credentials, # images, GPU, and checked-out submodule prerequisites. if [[ "$AIRSTACK_CONFIG_ONLY" == "1" ]]; then @@ -1649,7 +1670,7 @@ function preflight_up { return $errors fi - # 5. Missing images: compose 'up' silently starts a very long build + # 7. Missing images: compose 'up' silently starts a very long build local imgs img missing=() imgs=$(run_docker_compose "${_pf_global[@]}" config --images 2>/dev/null | sort -u) for img in $imgs; do @@ -1661,31 +1682,13 @@ function preflight_up { log_warn "To use prebuilt images instead: airstack images pull (set AIRSTACK_NO_IMAGE_BUILD=1 to forbid implicit builds)" fi - # 6. ROBOT_NAME resolution needs Docker >= 29 (see robot/docker/.bashrc) + # 8. ROBOT_NAME resolution needs Docker >= 29 (see robot/docker/.bashrc) local docker_major docker_major=$(docker version --format '{{.Server.Version}}' 2>/dev/null | cut -d. -f1) if [[ "$docker_major" =~ ^[0-9]+$ ]] && (( docker_major < 29 )); then log_warn "Docker $docker_major < 29: container-name DNS resolution fails, robots will resolve as 'unknown_robot' on domain 0 (MAVROS will not connect). Upgrade Docker or set ROBOT_NAME_SOURCE=hostname." fi - # 7. Deprecation shim (remove in 0.21.0): LAUNCH_NATNET no longer does - # anything — OptiTrack was extracted to the asm_optitrack module. - local _pf_natnet - _pf_natnet=$(resolve_launch_var LAUNCH_NATNET "${_pf_global[@]}") - if [[ -n "$_pf_natnet" ]]; then - log_warn "LAUNCH_NATNET is gone — OptiTrack moved to the asm_optitrack module (airstack module add https://github.com/castacks/asm_optitrack --version ); see docs/development/modules.md" - fi - - # 8. Deprecation shim (remove in 0.21.0): AUTONOMY_ROLE was REMOVED - # (stacks — RFC #379 — are the only launch dispatch). A set value counts - # only via env / --env-file / .env; nothing in the compose files defaults - # it anymore. - local _pf_role - _pf_role=$(resolve_launch_var AUTONOMY_ROLE "${_pf_global[@]}") - if [[ -n "$_pf_role" ]]; then - _pf_error "AUTONOMY_ROLE was removed — select a stack: airstack up --stack (see docs/development/stacks.md). Migration: full → full_default (the no-stack default), onboard → lite_default, onboard/offboard split → lite_offload_global:onboard / :offboard." - fi - unset -f _pf_error return $errors } diff --git a/tests/meta/test_docs_catalog_contract.py b/tests/meta/test_docs_catalog_contract.py index e008acae5..91dc24143 100644 --- a/tests/meta/test_docs_catalog_contract.py +++ b/tests/meta/test_docs_catalog_contract.py @@ -18,6 +18,7 @@ fetch step with per-clone failure isolation (an unreachable module repo must never fail a docs deploy). """ +import re import subprocess import sys from pathlib import Path @@ -203,11 +204,19 @@ def test_every_nav_entry_points_at_an_existing_file(): Guards the 404 class: a nav entry naming a moved/renamed page ships a dead link on the published site without failing the build (we cannot run ``mkdocs --strict`` while pre-existing warnings stand). + + Submodule paths are skipped: unit CI does not checkout submodules, and + those READMEs are owned by the submodule repo. """ + gitmodules = REPO / ".gitmodules" + submodule_roots = tuple( + re.findall(r"^\s*path\s*=\s*(\S+)", gitmodules.read_text(), re.M) + ) if gitmodules.is_file() else () missing = [ path for path in _flatten_nav(_load_mkdocs()["nav"]) if not path.startswith(("http://", "https://")) + and not any(path == root or path.startswith(root + "/") for root in submodule_roots) and not (REPO / path).is_file() ] assert not missing, f"mkdocs.yml nav entries with no file on disk: {missing}" diff --git a/tests/meta/test_fleet_contract.py b/tests/meta/test_fleet_contract.py index c48a72a06..b8850aa69 100644 --- a/tests/meta/test_fleet_contract.py +++ b/tests/meta/test_fleet_contract.py @@ -15,7 +15,7 @@ - **The trajectory hard-gate holds through fleet placement** — every split stack the generated compose places passes ``gen_dds_router.py --check`` (doctor hard gate #2: command authority stays onboard). -- **Launch intent** — ``airstack up --dry-run --fleet`` exports +- **Launch intent** — ``airstack up --config-only --fleet`` exports FLEET_CONFIG_FILE + derived NUM_ROBOTS + the fleet spawner; explicit env NUM_ROBOTS beats the fleet (banner); no fleet ⇒ no new effective-config keys (byte-identical legacy contract). @@ -76,9 +76,11 @@ def run_tool(script, *args): def run_up_dry(*flags, env=None, check=True): - full_env = {**os.environ, **(env or {})} + # Scrub AUTONOMY_ROLE: preflight hard-errors on a leftover value, and + # --config-only still enforces that configuration contract. + full_env = {**os.environ, "AUTONOMY_ROLE": "", **(env or {})} result = subprocess.run( - [AIRSTACK, "up", "--dry-run", *flags], + [AIRSTACK, "up", "--config-only", *flags], capture_output=True, text=True, cwd=str(REPO), env=full_env, timeout=120, ) out = result.stdout + result.stderr @@ -96,7 +98,7 @@ def run_up_dry(*flags, env=None, check=True): key, _, value = line.partition("=") cfg[key] = value if check: - assert result.returncode == 0, f"dry-run failed unexpectedly:\n{out}" + assert result.returncode == 0, f"config-only failed unexpectedly:\n{out}" assert cfg, f"no effective-config block in output:\n{out}" return result.returncode, out, cfg @@ -225,7 +227,7 @@ def test_split_stacks_placed_by_fleet_pass_bridge_hard_gate(gen, rf): assert code == 0, f"bridge gate failed for {stack_rel}:\n{out}\n{err}" -# ── launch intent: --fleet dry-run exports + precedence ───────────────────── +# ── launch intent: --fleet config-only exports + precedence ───────────────── def test_dry_run_fleet_exports(): code, out, cfg = run_up_dry("--fleet", "sim_one_default", "--sim", "isaac") @@ -237,7 +239,7 @@ def test_dry_run_fleet_exports(): def test_dry_run_heterogeneous_fleet_swaps_profile_and_would_generate(): - """--dry-run derives the fleet config but WRITES NOTHING: the generator + """--config-only derives the fleet config but WRITES NOTHING: the generator prints what it would generate (compose services + split-stack routers).""" code, out, cfg = run_up_dry("--fleet", "sim_three_mixed", "--sim", "isaac") assert code == 0, out From 7d5a566f6c76c0c059d70ab0bcc541060bf34ac5 Mon Sep 17 00:00:00 2001 From: Andrew Jong Date: Sat, 29 Aug 2026 02:35:26 -0400 Subject: [PATCH 6/6] Strip post-extraction residue and metadata plumbing; refresh CI reference docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scope cut agreed with Andrew before merge: - Drop tests/system/test_optitrack_e2e.py: it sets LAUNCH_NATNET (a removed no-op shim) and targets robot/ros_ws/src/perception/natnet_ros2, which was extracted from trunk (70423c4c). It could never pass on develop and PR CI never executes the optitrack mark, so it would rot silently. The same test lives where it belongs: asm_optitrack/tests/system/, exercised by the module CI that is now served by the multi-repo orchestrator (#400). The mark stays registered in tests/pytest.ini for module-CI runs. - Drop tests/report-requirements.txt: develop's metrics job already installs tests/requirements.txt (#407); a second requirements file for the same tree is a drift hazard. The workflow contract now pins the #407 behavior. - Drop harness/image_prep.py and the image-preparation.json workflow instrumentation: metadata plumbing whose value doesn't carry its weight — the associative-array bash was the most fragile code in the PR. The image-prep step returns to develop's plain version. - diagnostics: drop LAUNCH_NATNET from SAFE_ENV_KEYS (dead); keep PX4_PARAM_SET (live compose env-file selector). - Un-clobber develop content the branch predated: run-system-tests SKILL description (waypoint_flight, wiring marks), BSD-3-Clause-Clear license, `airstack images build` spelling. - Reference docs refreshed to where CI stands now: AGENTS.md test-suite paragraph (schema-v2 run_meta, advisory metric deltas, diagnostics bundle, fingerprint-only comparisons) and --config-only intent flag; module_ci.md + module-system-tests.yml header (orchestrator polls a repos: list — asm_optitrack included today); Release Notes entry for the trustworthy-outcomes policy. Full unit suite on this branch: 419 passed, 0 failed. Co-Authored-By: Claude Fable 5 --- .agents/skills/run-system-tests/SKILL.md | 6 +- .github/workflows/module-system-tests.yml | 7 +- .github/workflows/system-tests.yml | 41 --- AGENTS.md | 4 +- docs/development/module_ci.md | 8 +- docs/release_notes/index.md | 18 + tests/harness/diagnostics.py | 1 - tests/harness/image_prep.py | 66 ---- .../meta/test_campaign_reporting_contract.py | 27 -- tests/meta/test_diagnostics_contract.py | 15 - tests/meta/test_workflow_contract.py | 14 +- tests/report-requirements.txt | 3 - tests/system/test_optitrack_e2e.py | 339 ------------------ 13 files changed, 37 insertions(+), 512 deletions(-) delete mode 100644 tests/harness/image_prep.py delete mode 100644 tests/report-requirements.txt delete mode 100644 tests/system/test_optitrack_e2e.py diff --git a/.agents/skills/run-system-tests/SKILL.md b/.agents/skills/run-system-tests/SKILL.md index 5b7ebd6de..262b4019f 100644 --- a/.agents/skills/run-system-tests/SKILL.md +++ b/.agents/skills/run-system-tests/SKILL.md @@ -1,7 +1,7 @@ --- 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 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 +description: Run, interpret, and extend AirStack's pytest system test suite (build_packages, build_docker, liveliness, wiring, sensors, takeoff_hover_land, autonomy, waypoint_flight), 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: BSD-3-Clause-Clear metadata: author: AirLab CMU repository: AirStack @@ -196,7 +196,7 @@ Total parametrize cardinality for sim tests = `len(sims) × len(num_robots) × s - For `liveliness` / `sensors` / `takeoff_hover_land`: NVIDIA driver + `nvidia-container-toolkit` - For `isaacsim`: `simulation/isaac-sim/docker/omni_pass.env` populated with Omniverse credentials (CI generates a `guest`/`guest` version automatically) - `airstack setup` already run so `airstack` is on `PATH` -- All required compose images present locally — `airstack_env` calls `missing_images()` and fails fast otherwise. Build them first via `airstack test -m build_docker` or `airstack image-build `. +- All required compose images present locally — `airstack_env` calls `missing_images()` and fails fast otherwise. Build them first via `airstack test -m build_docker` or `airstack images build `. ## Running Tests via PR Comment diff --git a/.github/workflows/module-system-tests.yml b/.github/workflows/module-system-tests.yml index 6f767bdc6..781298ab6 100644 --- a/.github/workflows/module-system-tests.yml +++ b/.github/workflows/module-system-tests.yml @@ -32,9 +32,10 @@ name: Module System Tests # -f airstack_ref=develop -f marks="build_packages or liveliness" # # GPU runner note: with the default runs_on, the job queues for the ephemeral -# OSMO-backed runners (.github/orchestrator/). The orchestrator polls one repo -# per instance, so a module repo must be added to the poll list before its -# calls can be picked up — see .github/orchestrator/README.md "Module repos". +# OSMO-backed runners (.github/orchestrator/). The orchestrator polls a +# `repos:` list, so a module repo must be added to that list (and covered by +# the orchestrator PAT) before its calls can be picked up — see +# .github/orchestrator/README.md "Module repos". # # Docs: docs/development/module_ci.md diff --git a/.github/workflows/system-tests.yml b/.github/workflows/system-tests.yml index 99d828893..bf7d99cbf 100644 --- a/.github/workflows/system-tests.yml +++ b/.github/workflows/system-tests.yml @@ -386,38 +386,17 @@ jobs: SIM_INPUT: ${{ steps.parse.outputs.sim }} NO_IMAGE_BUILD: ${{ steps.parse.outputs.no_image_build }} run: | - mkdir -p tests/results - image_result=tests/results/image-preparation.json - image_outcome=already-present - pulled=() - retagged=() - built=() - missing=() profiles=desktop [[ ",$SIM_INPUT," == *,msairsim,* ]] && profiles="$profiles,ms-airsim" [[ ",$SIM_INPUT," == *,isaacsim,* ]] && profiles="$profiles,isaac-sim" export COMPOSE_PROFILES="$profiles" echo "Pulling images for COMPOSE_PROFILES=$COMPOSE_PROFILES (no_image_build=$NO_IMAGE_BUILD)" - declare -A present_before=() - while IFS= read -r img; do - [[ -z "$img" ]] && continue - if docker image inspect "$img" --format '{{.Id}}' >/dev/null 2>&1; then - present_before["$img"]=1 - fi - done < <(docker compose -f docker-compose.yaml config --images) # Pull from registry; tolerate per-image failures so we can detect # what's still missing afterwards instead of aborting on the first # gap. `--progress=quiet` suppresses per-layer progress; errors # still surface on stderr. ./airstack.sh --progress=quiet images pull --ignore-pull-failures || true - while IFS= read -r img; do - [[ -z "$img" || -n "${present_before[$img]:-}" ]] && continue - if docker image inspect "$img" --format '{{.Id}}' >/dev/null 2>&1; then - pulled+=("$img") - image_outcome=pulled-versioned - fi - done < <(docker compose -f docker-compose.yaml config --images) # VERSION tags miss on every PR. Seed from floating cache_* tags. cache_tag="$(grep -E '^CACHE_TAG=' .env 2>/dev/null | cut -d= -f2 | tr -d '"' || true)" @@ -433,8 +412,6 @@ jobs: if docker pull --quiet "$cache_img"; then docker tag "$cache_img" "$img" echo "Retagged $cache_img -> $img" - retagged+=("$img") - image_outcome=cache-retagged else echo "Cache tag pull failed for $cache_img" fi @@ -452,32 +429,14 @@ jobs: echo "Images still missing after pull/retag:" printf ' - %s\n' "${missing[@]}" if [[ "$NO_IMAGE_BUILD" == "true" ]]; then - IMAGE_OUTCOME=missing IMAGE_RESULT="$image_result" \ - MISSING_IMAGES="$(printf '%s\n' "${missing[@]}")" \ - PYTHONPATH=tests python3 -m harness.image_prep "$image_result" echo "::error::Pull-only mode (--no-image-build or -m build_packages) will not run 'images build'. Run /pytest -m build_docker once, or omit --no-image-build." exit 1 fi echo "Falling back to images build" ./airstack.sh --progress=quiet images build - built=("${missing[@]}") - image_outcome=locally-built else echo "All required images present after pull/retag — skipping build." fi - IMAGE_OUTCOME="$image_outcome" IMAGE_RESULT="$image_result" \ - PULLED_IMAGES="$(printf '%s\n' "${pulled[@]}")" \ - RETAGGED_IMAGES="$(printf '%s\n' "${retagged[@]}")" \ - BUILT_IMAGES="$(printf '%s\n' "${built[@]}")" \ - PYTHONPATH=tests python3 -m harness.image_prep "$image_result" - echo "### Image preparation: $image_outcome" >> "$GITHUB_STEP_SUMMARY" - - - name: Record test-owned image preparation - if: ${{ steps.parse.outputs.skip_image_prep == 'true' }} - run: | - IMAGE_OUTCOME=delegated-to-build-docker PYTHONPATH=tests \ - python3 -m harness.image_prep tests/results/image-preparation.json - echo "### Image preparation: delegated to build_docker tests" >> "$GITHUB_STEP_SUMMARY" - name: Run tests env: diff --git a/AGENTS.md b/AGENTS.md index 14302b322..e0180bac3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -176,7 +176,7 @@ airstack install # Install Docker and dependencies # Container management airstack up [service] # Start services (robot, isaac-sim, gcs) -airstack up --sim isaac|airsim --robots N # Intent flags: derive profiles/URDF/sim script (add --headless, --play/--no-play, --no-autolaunch, --wait, --dry-run) +airstack up --sim isaac|airsim --robots N # Intent flags: derive profiles/URDF/sim script (add --headless, --play/--no-play, --no-autolaunch, --wait, --dry-run, --config-only) airstack up --stack [:] --sim isaac # Stack launch (RFC #379): stacks//launch/.launch.xml — the ONLY dispatch (no --stack = full_default; legacy AUTONOMY_ROLE was removed); see docs/development/stacks.md airstack up --fleet --sim isaac # Fleet launch (RFC #380): config/fleets/.yaml drives identity/placement/spawns; see docs/development/fleets.md airstack fleet list|generate # Fleet files: table / per-robot compose for heterogeneous fleets @@ -271,7 +271,7 @@ Pytest-based system tests live under [`tests/system/`](tests/system/). They brin | [`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 | | [`tests/system/test_waypoint_flight.py`](tests/system/test_waypoint_flight.py) | `waypoint_flight` | 4-phase flight chain (PX4 ready → takeoff → NavigateTask waypoint route → land) per (sim, num_robots, iter); pass/fail judged on the odometry track by the standalone [`tests/waypoint_checker.py`](tests/waypoint_checker.py) (in-order corridor arrival within `--waypoint-tolerance`, final goal within `--goal-tolerance`, per-waypoint `--waypoint-timeout`) | Docker, GPU, sim license | -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`), `run_meta`, `test_ids`, `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`, `run_meta.json`, 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) compares only matching, complete simulation campaigns and exits 1 on a genuine metric 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`), `run_meta`, `test_ids`, `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`, `run_meta.json` (schema v2: completion state, `failure_class`, campaign fingerprint over tests + behavior-changing CLI config), `metrics.json`, and — on bring-up/readiness failures — a bounded `diagnostics/` bundle (no per-test log files — live output streams to the terminal via `log_cli`). [`tests/parse_metrics.py`](tests/parse_metrics.py) compares only fingerprint-identical, complete simulation campaigns; numeric metric deltas are **advisory** (exit 0) and never fail CI — only real test failures, infrastructure/prerequisite failures, and report-integrity errors (exit 2) do. **Run via the CLI** (containerized runner — no local Python needed): diff --git a/docs/development/module_ci.md b/docs/development/module_ci.md index 56a412599..58ba70eb2 100644 --- a/docs/development/module_ci.md +++ b/docs/development/module_ci.md @@ -179,7 +179,9 @@ gh workflow run module-system-tests.yml --repo castacks/AirStack \ ## GPU runners for module repos With the default `runs_on`, jobs queue for the label pair -`[self-hosted, airstack-ephemeral]` — the ephemeral OSMO-backed runners. The -orchestrator polls **one repo per instance**, so a module repo must be added -to the poll list before its jobs are picked up: see +`[self-hosted, airstack-ephemeral]` — the ephemeral OSMO-backed runners. One +orchestrator instance polls a **`repos:` list** (trunk plus module repos), so +a module repo must be added to that list — and be covered by the +orchestrator's GitHub PAT — before its jobs are picked up: see [CI/CD Orchestrator → Module repos](../../tests/ci-cd-orchestrator.md#module-repos). +`castacks/asm_optitrack` is polled today. diff --git a/docs/release_notes/index.md b/docs/release_notes/index.md index aad379af4..6c3632ca8 100644 --- a/docs/release_notes/index.md +++ b/docs/release_notes/index.md @@ -19,6 +19,24 @@ its own notes. --> ## 0.20.0 (Unreleased) +- **Trustworthy system-test outcomes.** A red system-tests run now always + means the code under test got worse, never that CI infrastructure hiccuped: + `run_meta.json` (schema v2) classifies every failure as + `assertion` / `infrastructure` / `collection` / `ci_integrity`, and CI fails + only on those — comparable numeric metric deltas (Hz, CPU, error metrics) + are **advisory** in the report and no longer fail the PR. Metric + comparisons only happen between fingerprint-identical campaigns (same + tests **and** same behavior-changing CLI config: sim, robot count, + trajectories, velocities, tolerances), with the baseline selected from + recent base-branch artifacts by fingerprint instead of "newest artifact of + any shape". Bring-up/readiness failures fail fast when the simulator + process dies (instead of burning the full `/clock` timeout on an ephemeral + GPU pod) and capture a bounded, secret-free `diagnostics/` bundle before + the pod is destroyed. Maintainers can dispatch focused flight campaigns + (`trajectory_types`, `takeoff_velocities`) from `workflow_dispatch`. New + `airstack up --config-only`: dry-run restricted to logical launch-config + contracts (no Docker/credentials/image/submodule prerequisites). + - **CI un-redded: Metrics Report and Unit Tests fixed.** Every PR had been failing since ~2026-08-20 for reasons unrelated to the code under test. The system-tests **Metrics Report** job installed only `tabulate`, so diff --git a/tests/harness/diagnostics.py b/tests/harness/diagnostics.py index 08abb9baa..554120669 100644 --- a/tests/harness/diagnostics.py +++ b/tests/harness/diagnostics.py @@ -22,7 +22,6 @@ "MS_AIRSIM_HEADLESS", "MS_AIRSIM_ENV_DIR", "MS_AIRSIM_BINARY_PATH", - "LAUNCH_NATNET", "PX4_PARAM_SET", ) diff --git a/tests/harness/image_prep.py b/tests/harness/image_prep.py deleted file mode 100644 index b41e78d0f..000000000 --- a/tests/harness/image_prep.py +++ /dev/null @@ -1,66 +0,0 @@ -"""Structured Docker image-preparation outcomes for CI artifacts.""" - -from __future__ import annotations - -import argparse -import json -import os -from pathlib import Path - -OUTCOMES = { - "already-present", - "pulled-versioned", - "cache-retagged", - "locally-built", - "missing", - "delegated-to-build-docker", -} - - -def build_image_preparation( - outcome: str, - *, - pulled=(), - retagged=(), - built=(), - missing=(), -) -> dict: - if outcome not in OUTCOMES: - raise ValueError(f"unknown image preparation outcome: {outcome}") - return { - "schema_version": 1, - "outcome": outcome, - "versioned_pulled": sorted(filter(None, pulled)), - "cache_retagged": sorted(filter(None, retagged)), - "locally_built": sorted(filter(None, built)), - "missing": sorted(filter(None, missing)), - } - - -def write_from_environment(path: Path) -> Path: - def lines(name): - return os.environ.get(name, "").splitlines() - - payload = build_image_preparation( - os.environ["IMAGE_OUTCOME"], - pulled=lines("PULLED_IMAGES"), - retagged=lines("RETAGGED_IMAGES"), - built=lines("BUILT_IMAGES"), - missing=lines("MISSING_IMAGES"), - ) - path = Path(path) - path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps(payload, indent=2, sort_keys=True) + "\n") - return path - - -def main() -> int: - parser = argparse.ArgumentParser() - parser.add_argument("output", type=Path) - args = parser.parse_args() - write_from_environment(args.output) - return 0 - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/tests/meta/test_campaign_reporting_contract.py b/tests/meta/test_campaign_reporting_contract.py index 1522b5071..24fa19882 100644 --- a/tests/meta/test_campaign_reporting_contract.py +++ b/tests/meta/test_campaign_reporting_contract.py @@ -7,7 +7,6 @@ import pytest from harness.baseline import select_baseline -from harness.image_prep import build_image_preparation from harness.run_meta import build_run_meta, campaign_fingerprint import parse_metrics from parse_metrics import _score @@ -125,32 +124,6 @@ def test_timeout_or_missing_data_is_never_numeric_regression(): assert _score(None, numeric, 20)[1] == "" -@pytest.mark.parametrize( - "outcome,field", - [ - ("already-present", None), - ("pulled-versioned", "versioned_pulled"), - ("cache-retagged", "cache_retagged"), - ("locally-built", "locally_built"), - ("missing", "missing"), - ], -) -def test_image_preparation_paths_have_explicit_outcomes(outcome, field): - kwargs = {} - if field: - argument = { - "versioned_pulled": "pulled", - "cache_retagged": "retagged", - "locally_built": "built", - "missing": "missing", - }[field] - kwargs[argument] = ["registry/image:tag"] - payload = build_image_preparation(outcome, **kwargs) - assert payload["outcome"] == outcome - if field: - assert payload[field] == ["registry/image:tag"] - - def test_metric_delta_cli_is_advisory(monkeypatch, tmp_path): output = tmp_path / "report.md" monkeypatch.setattr( diff --git a/tests/meta/test_diagnostics_contract.py b/tests/meta/test_diagnostics_contract.py index 0abcb477b..6a063bcdf 100644 --- a/tests/meta/test_diagnostics_contract.py +++ b/tests/meta/test_diagnostics_contract.py @@ -7,7 +7,6 @@ from harness import diagnostics from harness.sim import SimulatorHealthError, wait_for_first_message -from system import test_optitrack_e2e pytestmark = pytest.mark.unit @@ -50,17 +49,3 @@ def test_message_wait_aborts_immediately_on_dead_process(): health_check=lambda: (False, "pane exited"), health_grace=0, ) - - -def test_optitrack_missing_sdk_is_explicit_infrastructure(monkeypatch): - missing = SimpleNamespace(returncode=1, stdout="", stderr="") - healthy = SimpleNamespace(returncode=0, stdout="", stderr="") - monkeypatch.setattr(test_optitrack_e2e, "ros2_exec", lambda *a, **k: missing) - monkeypatch.setattr(test_optitrack_e2e, "docker_exec", lambda *a, **k: healthy) - monkeypatch.setattr( - test_optitrack_e2e, - "collect_failure_diagnostics", - lambda *a, **k: "diagnostics.json", - ) - with pytest.raises(pytest.fail.Exception, match="licensed NatNet SDK"): - test_optitrack_e2e._check_optitrack_prerequisites("robot") diff --git a/tests/meta/test_workflow_contract.py b/tests/meta/test_workflow_contract.py index 08ccde40d..91a4e40d1 100644 --- a/tests/meta/test_workflow_contract.py +++ b/tests/meta/test_workflow_contract.py @@ -25,21 +25,17 @@ def test_comment_runs_cancel_older_run_for_same_pr(): def test_report_job_installs_declared_dependencies(): + # parse_metrics.py imports the tests/harness package, so the report job + # must install the full test requirements — a bare `pip install tabulate` + # crashed every report with ModuleNotFoundError: yaml (issue behind #407). report = _workflow().split("\n report:", 1)[1] - assert "pip install -r tests/report-requirements.txt" in report + assert "pip install -r tests/requirements.txt" in report assert "pip install tabulate" not in report - requirements = repo_path("tests", "report-requirements.txt").read_text().lower() + requirements = repo_path("tests", "requirements.txt").read_text().lower() assert "pyyaml" in requirements assert "tabulate" in requirements -def test_image_preparation_is_structured_and_uploaded(): - workflow = _workflow() - assert "image-preparation.json" in workflow - assert "python3 -m harness.image_prep" in workflow - assert "path: tests/results/" in workflow - - def test_metric_deltas_are_advisory_but_parser_errors_block(): workflow = _workflow() assert "- name: Fail on report integrity error" in workflow diff --git a/tests/report-requirements.txt b/tests/report-requirements.txt deleted file mode 100644 index 58220af7d..000000000 --- a/tests/report-requirements.txt +++ /dev/null @@ -1,3 +0,0 @@ -# Dependencies imported by the standalone metrics/report job. -PyYAML -tabulate diff --git a/tests/system/test_optitrack_e2e.py b/tests/system/test_optitrack_e2e.py deleted file mode 100644 index 1382ca049..000000000 --- a/tests/system/test_optitrack_e2e.py +++ /dev/null @@ -1,339 +0,0 @@ -"""OptiTrack NatNet end-to-end (sim). - -A single dedicated bring-up that exercises the whole OptiTrack path in Isaac Sim: -the in-sim NatNet **emulator** streams rigid-body poses → ``natnet_ros2`` publishes -the drone pose → the ``vision_pose`` bridge feeds MAVROS → PX4 EKF2 fuses it. - -This brings the NatNet stack up **once** and asserts only one NatNet-specific test. -The cheap, GPU-free half of this (host emulator → ``natnet_ros2`` Hz) lives in ``tests/integration/natnet/``. - -Mark: ``optitrack``. Needs Docker + GPU + Isaac Sim license; missing images/SDK -are classified as infrastructure prerequisite failures before topic waits. -""" -import os -import re -import time - -import pytest - -from conftest import ( # noqa: E402 — pytest adds tests/ to sys.path - airstack_cmd, - collect_failure_diagnostics, - container_running, - docker_exec, - find_container, - get_metrics, - get_robot_containers, - logger, - missing_images, - read_log_tail, - ros2_exec, - sample_hz, - wait_for_container, - wait_for_first_message, -) -from system.test_fixed_trajectory import ( - TARGET_ALTITUDE_M, - _landing_one_robot, - _run_parallel, - _takeoff_one_robot, - _trajectory_one_robot, -) - -pytestmark = pytest.mark.optitrack - -# Single-drone NatNet Isaac stack: the natnet Pegasus script spawns the emulator -# alongside PX4, and LAUNCH_NATNET=true brings up natnet_ros2 + the vision_pose / -# gp_origin / param bridges on the robot. -# -# PX4_PARAM_SET selects simulation/isaac-sim/docker/px4-params/external-vision.env, which -# switches PX4 SITL's EKF2 to mocap external vision and turns GPS, baro and range aiding -# OFF, so the OptiTrack stream is the vehicle's ONLY position source. PX4's rcS applies -# those PX4_PARAM_* entries at boot; they mirror the deployment-validated set in -# robot/ros_ws/src/perception/natnet_ros2/config/px4_params.yaml. -# -# Without it EKF2_EV_CTRL is 0, PX4 silently discards the vision and flies on sim GPS — -# which is what made the previous version of this module's fusion check vacuous. -_E2E_ENV = { - "NUM_ROBOTS": "1", - "COMPOSE_PROFILES": "desktop,isaac-sim", - "AUTOLAUNCH": "true", - "ISAAC_SIM_USE_STANDALONE": "true", - "ISAAC_SIM_SCRIPT_NAME": "example_one_px4_pegasus_natnet_launch_script.py", - "PLAY_SIM_ON_START": "true", - "LAUNCH_NATNET": "true", - # EKF2 external-vision (mocap) configuration — see comment above. Selects - # simulation/isaac-sim/docker/px4-params/external-vision.env, which turns GPS, baro - # and range aiding off, leaving mocap as the vehicle's only position source. - "PX4_PARAM_SET": "external-vision", - # Headless: no X on the CI runner. - "QT_QPA_PLATFORM": "offscreen", -} - -# The trajectory flown to prove fusion. Circle is the enforced PR gate: sustained lateral -# motion is where a wrong EV delay or a too-tight innovation gate actually shows up, which -# a stationary hover would never reveal. -_E2E_TRAJECTORY = "Circle" - -_ROBOT_PATTERN = "robot.*desktop" -_ROBOT_SETUP_BASH = "/root/AirStack/robot/ros_ws/install/setup.bash" -_ROBOT_DOMAIN = 1 -# Drone body's relative pose topic from natnet_config.yaml, namespaced per robot. -_NATNET_POSE_TOPIC = os.environ.get("NATNET_POSE_TOPIC", "perception/optitrack/drone") -_NATNET_MIN_HZ = 5.0 -# PX4 fused local position (proves EKF2 accepted the external vision). odom, not pose: -# it goes live only once EKF2 has converged and home is set, which is what PX4's arming -# preflight requires. See test_px4_ready in test_takeoff_hover_land.py — pose-era signals -# fire ~25s earlier, and arming in that window returns "failed to arm". -_PX4_LOCAL_POSE_TOPIC = "interface/mavros/local_position/odom" -# MAVROS param plugin node; hosts the FCU param table as ROS 2 parameters. Same -# service px4_param_setter reads through. -_EV_PARAM_NODE = "interface/mavros/param" -# One proves vision fusion is on, the other proves GPS aiding is off — together they -# are the precondition every test below assumes but none of them check. -_EV_EXPECTED = {"EKF2_EV_CTRL": 11, "EKF2_GPS_CTRL": 0} -# MAVROS pulls the param table lazily after FCU connect; px4_param_setter budgets -# settle_sec 10 + 30 retries for the same reason. -_EV_PARAM_TIMEOUT = 90 -# `ros2 param get` prints "Integer value is: 11" / "Double value is: 7.0". -_PARAM_VALUE_RE = re.compile(r"value is:\s*(-?[\d.]+)") -# Cold Isaac boot: Pegasus load + Play + emulator UDP connect. -_FIRST_MSG_TIMEOUT = 180 - -# Belt-and-braces behind the odom gate: EV-only convergence is slower and less predictable -# than the GPS case the autonomy suites were tuned against, and TakeoffTask does not retry -# its own ARM (takeoff_landing_task.cpp send_robot_command). -_ARM_ATTEMPTS = 5 -_ARM_RETRY_S = 2.0 -_ARM_SERVICE = "interface/robot_command" -_ARM_COMMAND = 1 # airstack_msgs/srv/RobotCommand.Request.ARM - -# The flight helpers imported from test_fixed_trajectory take an `airstack_env`-style cfg; -# `robot_setup_bash` is the only key any of them reads. -_TRAJ_CFG = {"robot_setup_bash": _ROBOT_SETUP_BASH} - - -def _check_optitrack_prerequisites(robot_container: str) -> None: - """Fail before topic waits when licensed/build/runtime inputs are absent.""" - node = ros2_exec( - robot_container, - "prefix=$(ros2 pkg prefix natnet_ros2 2>/dev/null) && " - "test -x \"$prefix/lib/natnet_ros2/natnet_ros2_node\"", - domain_id=_ROBOT_DOMAIN, - setup_bash=_ROBOT_SETUP_BASH, - timeout=20, - ) - emulator = docker_exec( - "isaac-sim", - "test -d /isaac-sim/AirStack/simulation/isaac-sim/extensions/" - "optitrack.natnet.emulator && " - "pgrep -fa 'example_one_px4_pegasus_natnet|isaac-sim' >/dev/null", - timeout=20, - ) - missing = [] - if node.returncode != 0: - missing.append( - "natnet_ros2_node is not installed (the licensed NatNet SDK was " - "not provisioned when the robot image was built)" - ) - if emulator.returncode != 0: - missing.append("Isaac NatNet emulator extension/process is unavailable") - if missing: - reason = "OptiTrack infrastructure prerequisite failed: " + "; ".join(missing) - diagnostics = collect_failure_diagnostics( - _E2E_ENV, reason, "optitrack-prerequisites" - ) - pytest.fail(f"{reason}; diagnostics: {diagnostics}") - - -def _arm_with_retries(container: str) -> None: - """Arm the vehicle, retrying while PX4's preflight is still rejecting it. - - Uses the same robot_command service TakeoffTask arms through, so a successful - call here leaves TakeoffTask's `is_armed_` set and it skips its own arming. - """ - service = f"/robot_{_ROBOT_DOMAIN}/{_ARM_SERVICE}" - last = "" - started = time.time() - for attempt in range(1, _ARM_ATTEMPTS + 1): - result = ros2_exec( - container, - f'timeout 10 ros2 service call {service} ' - f'airstack_msgs/srv/RobotCommand "{{command: {_ARM_COMMAND}}}"', - domain_id=_ROBOT_DOMAIN, setup_bash=_ROBOT_SETUP_BASH, timeout=20, - ) - last = (result.stdout or "") + (result.stderr or "") - if "success=True" in last.replace(" ", ""): - logger.info("armed on attempt %d/%d", attempt, _ARM_ATTEMPTS) - return - logger.info("arm attempt %d/%d refused (PX4 preflight not ready yet)", - attempt, _ARM_ATTEMPTS) - if attempt < _ARM_ATTEMPTS: - time.sleep(_ARM_RETRY_S) - - pytest.fail( - f"could not arm after {_ARM_ATTEMPTS} attempts over " - f"{time.time() - started:.0f}s — PX4 preflight still rejecting. " - "With GPS/baro/range aiding off, this means EKF2 has not converged on the " - f"vision estimate. Last response:\n{last.strip()[-400:]}" - ) - - -@pytest.fixture(scope="module") -def optitrack_sim_stack(request): - """Bring the NatNet Isaac stack up once for the module; tear it down after. - - Reuses an already-running robot-desktop container (fast local iteration); - otherwise brings the stack up. Missing images fail as infrastructure. - """ - existing = find_container(_ROBOT_PATTERN) - if existing and container_running(existing): - _check_optitrack_prerequisites(existing) - yield {"container": existing, "brought_up": False} - return - - missing = missing_images(env=_E2E_ENV) - if missing: - pytest.fail( - "OptiTrack infrastructure prerequisite failed: required images are " - "missing: " + ", ".join(missing) - ) - - airstack_cmd("down", timeout=120, log_name="optitrack_e2e") - result = airstack_cmd("up", env_overrides=_E2E_ENV, timeout=300, log_name="optitrack_e2e") - if result.returncode != 0: - pytest.fail(f"`airstack up` (natnet isaac) failed:\n{read_log_tail('optitrack_e2e')}") - - container = wait_for_container(_ROBOT_PATTERN, timeout=180) - assert container, "robot-desktop container not Running after 180s" - _check_optitrack_prerequisites(container) - try: - yield {"container": container, "brought_up": True} - finally: - airstack_cmd("down", timeout=120, log_name="optitrack_e2e") - - -def _robot_container(stack): - # robot_1 lives on the first (index-1) replica. - return get_robot_containers(_ROBOT_PATTERN)[0] if not stack["brought_up"] \ - else wait_for_container(_ROBOT_PATTERN, timeout=60) - - -class TestOptitrackE2E: - - @pytest.mark.dependency(name="natnet_pose") - def test_natnet_pose_alive(self, optitrack_sim_stack): - """Emulator → natnet_ros2 → vision_pose: the drone pose_cov streams >= 5 Hz.""" - container = _robot_container(optitrack_sim_stack) - topic = f"/robot_{_ROBOT_DOMAIN}/{_NATNET_POSE_TOPIC}/pose_cov" - - first = wait_for_first_message( - container, topic, domain_id=_ROBOT_DOMAIN, - setup_bash=_ROBOT_SETUP_BASH, timeout=_FIRST_MSG_TIMEOUT, - ) - assert first is not None, ( - f"no NatNet pose on {topic} within {_FIRST_MSG_TIMEOUT}s " - "(emulator → natnet_ros2 path down)" - ) - hz = sample_hz(container, topic, domain_id=_ROBOT_DOMAIN, - setup_bash=_ROBOT_SETUP_BASH, duration=5, window=20) - get_metrics().record("test_optitrack_e2e.natnet_pose_hz", - "natnet_pose_hz", hz if hz is not None else "none", unit="Hz") - assert hz is not None and hz >= _NATNET_MIN_HZ, \ - f"{topic} at {hz} Hz (< {_NATNET_MIN_HZ})" - - @pytest.mark.dependency(name="ev_params", depends=["natnet_pose"]) - def test_ev_params_applied(self, optitrack_sim_stack): - """The external-vision param set actually reached the FCU. - - Everything below assumes PX4_PARAM_SET=external-vision took effect. If it - silently did not, EKF2_EV_CTRL stays 0 and EKF2_GPS_CTRL stays 7, the vehicle - flies the Circle on sim GPS, and every other test here still passes. This reads - the live values back off the FCU, so it covers the whole chain: compose env_file - -> container env -> Pegasus -> PX4 rcS -> FCU. - """ - container = _robot_container(optitrack_sim_stack) - node = f"/robot_{_ROBOT_DOMAIN}/{_EV_PARAM_NODE}" - - unread = dict(_EV_EXPECTED) - actual = {} - deadline = time.time() + _EV_PARAM_TIMEOUT - while unread and time.time() < deadline: - for name in list(unread): - result = ros2_exec( - container, f"ros2 param get {node} {name}", - domain_id=_ROBOT_DOMAIN, setup_bash=_ROBOT_SETUP_BASH, timeout=20, - ) - # An unpulled param prints "Parameter not set." and still exits 0, so - # match on the value line rather than the return code. - match = _PARAM_VALUE_RE.search(result.stdout or "") - if match: - actual[name] = float(match.group(1)) - del unread[name] - if unread: - time.sleep(2.0) - - assert not unread, ( - f"{', '.join(sorted(unread))} never appeared in the MAVROS param table " - f"within {_EV_PARAM_TIMEOUT}s — the MAVROS/FCU link is down, which is a " - "different failure from a wrong parameter." - ) - wrong = {k: v for k, v in sorted(actual.items()) if v != _EV_EXPECTED[k]} - assert not wrong, ( - f"PX4 is not configured for external vision: {wrong} (expected " - f"{ {k: _EV_EXPECTED[k] for k in wrong} }). PX4_PARAM_SET=external-vision " - "did not reach the FCU, so the flight below would fly on GPS and pass anyway." - ) - logger.info("EV params confirmed on FCU: %s", actual) - - @pytest.mark.dependency(name="ev_ready", depends=["ev_params"]) - def test_px4_fuses_vision(self, optitrack_sim_stack): - """PX4 publishes local_position/odom, so EKF2 has converged and home is set. - - This only establishes that a converged estimate EXISTS — it is deliberately not - the proof that vision is being fused, because odom publishes off any aiding - source. The flight below is the proof: with GPS, baro and range aiding disabled in - _E2E_ENV, mocap is the only thing that can produce this estimate at all. - """ - container = _robot_container(optitrack_sim_stack) - topic = f"/robot_{_ROBOT_DOMAIN}/{_PX4_LOCAL_POSE_TOPIC}" - - first = wait_for_first_message( - container, topic, domain_id=_ROBOT_DOMAIN, - setup_bash=_ROBOT_SETUP_BASH, timeout=_FIRST_MSG_TIMEOUT, - ) - assert first is not None, ( - f"no PX4 local_position/odom on {topic} within {_FIRST_MSG_TIMEOUT}s — " - "EKF2 never converged or never set a home position. With GPS/baro/range " - "aiding off, that means the external-vision path never reached it." - ) - - @pytest.mark.dependency(name="ev_takeoff", depends=["ev_ready"]) - @pytest.mark.timeout(2400) - def test_takeoff(self, optitrack_sim_stack): - """Take off to TARGET_ALTITUDE_M flying on the mocap-fused estimate.""" - container = _robot_container(optitrack_sim_stack) - _arm_with_retries(container) - _run_parallel(1, lambda n: _takeoff_one_robot( - n, container, _TRAJ_CFG, TARGET_ALTITUDE_M)) - - @pytest.mark.dependency(name="ev_circle", depends=["ev_takeoff"]) - @pytest.mark.timeout(2400) - def test_circle_trajectory(self, optitrack_sim_stack): - """Fly a Circle with mocap as the only position source. - - This is the end-to-end proof: emulator → natnet_ros2 → vision_pose → MAVROS → - EKF2 → controller → airframe. Cross-track error is scored by the same code the - autonomy benchmark uses, so a mocap regression shows up as path deviation rather - than as a topic that merely exists. - """ - container = _robot_container(optitrack_sim_stack) - _run_parallel(1, lambda n: _trajectory_one_robot( - n, container, _TRAJ_CFG, _E2E_TRAJECTORY)) - - @pytest.mark.dependency(name="ev_land", depends=["ev_takeoff"]) - @pytest.mark.timeout(2400) - def test_landing(self, optitrack_sim_stack): - """Land the drone; runs even when the trajectory phase fails.""" - container = _robot_container(optitrack_sim_stack) - _run_parallel(1, lambda n: _landing_one_robot(n, container, _TRAJ_CFG))