diff --git a/.agents/skills/run-system-tests/SKILL.md b/.agents/skills/run-system-tests/SKILL.md index 6e2940f8b..262b4019f 100644 --- a/.agents/skills/run-system-tests/SKILL.md +++ b/.agents/skills/run-system-tests/SKILL.md @@ -232,7 +232,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 @@ -284,7 +284,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/ \ @@ -298,7 +298,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. @@ -453,7 +457,7 @@ python tests/parse_metrics.py \ - `tests/harness/` — helpers split by concern: `session`, `discovery`, `commands`, `containers`, `metrics` (`MetricsRecorder`), `run_meta`, `test_ids`, `sim`, `collection` (ordering) - `tests/meta/` — fast contract tests (`unit` mark) pinning CLI/docs/stack contracts - `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/.env b/.env index e956c9c58..efc34f962 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.19" +VERSION="0.20.0-alpha.20" # 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/.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 804538520..bf7d99cbf 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: "" @@ -58,8 +66,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 +119,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 +128,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" @@ -141,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 @@ -158,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 @@ -424,6 +441,8 @@ jobs: - 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 +505,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 @@ -533,15 +553,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) @@ -562,29 +586,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 @@ -604,8 +637,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 @@ -655,17 +688,13 @@ 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' - # parse_metrics.py writes report.md before exiting 1 on a regression; - # an uncaught crash also exits 1 but never writes the report. Require - # both so a parser crash cannot masquerade as a metric regression. + # Numeric metric deltas are advisory (parse_metrics.py exits 0 on + # them); this step fires only on parser/integrity failures (exit 2) + # or an uncaught crash, and never labels either a metric regression. run: | - if [ "${{ steps.report.outputs.parser_exit }}" = "1" ] && [ -f report.md ]; 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 b2c58d466..2372aabdc 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -55,6 +55,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/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/airstack.sh b/airstack.sh index 3ccf422e1..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," @@ -1135,6 +1138,7 @@ function parse_launch_intent { AIRSTACK_INTENT_SCENE="" AIRSTACK_FLEET_COMPOSE_FILE="" AIRSTACK_DRY_RUN="" + AIRSTACK_CONFIG_ONLY="" AIRSTACK_UP_WAIT="" local args=("$@") i=0 a @@ -1159,6 +1163,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)) @@ -1630,15 +1635,42 @@ 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 - # 5. Missing images: compose 'up' silently starts a very long build + # 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 + unset -f _pf_error + return $errors + fi + + # 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 @@ -1650,42 +1682,25 @@ 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 } 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=() @@ -2317,7 +2332,7 @@ function register_builtin_commands { COMMAND_HELP["install"]="Install dependencies (Docker Engine, NVIDIA Container Toolkit)" COMMAND_HELP["setup"]="Configure AirStack settings and add to shell profile" 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]" + 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 f67dcd243..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. | --- @@ -205,8 +205,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 @@ -384,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/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/README.md b/tests/README.md index 643c1e213..71caf65b9 100644 --- a/tests/README.md +++ b/tests/README.md @@ -144,23 +144,25 @@ Writes custom metrics to `tests/results//metrics.json` after each `re 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. +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 - ├── run_meta.json # Completion/outcome and campaign fingerprint + ├── 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 └── wiring/ # (wiring mark only) observed_.md graph snapshots ``` -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. --- @@ -536,17 +538,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: @@ -558,7 +562,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. --- @@ -591,6 +598,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 @@ -600,14 +609,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 8f11f0565..3a9e92498 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -154,12 +154,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: @@ -174,9 +188,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) @@ -303,8 +328,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..554120669 --- /dev/null +++ b/tests/harness/diagnostics.py @@ -0,0 +1,125 @@ +"""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", + "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/run_meta.py b/tests/harness/run_meta.py index 342ef3eb5..48b22cad8 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() @@ -70,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" @@ -86,7 +129,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 +160,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 +236,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 +306,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 +408,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 bc4d09e4a..e0cc76972 100644 --- a/tests/harness/sim.py +++ b/tests/harness/sim.py @@ -60,7 +60,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 @@ -73,6 +85,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..24fa19882 --- /dev/null +++ b/tests/meta/test_campaign_reporting_contract.py @@ -0,0 +1,160 @@ +"""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.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 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("") + (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] == "" + + +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 535925bb9..3b58e1e77 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..6a063bcdf --- /dev/null +++ b/tests/meta/test_diagnostics_contract.py @@ -0,0 +1,51 @@ +"""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 + +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, + ) 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 diff --git a/tests/meta/test_launch_intent_contract.py b/tests/meta/test_launch_intent_contract.py index ed36b8997..26fc63f9d 100644 --- a/tests/meta/test_launch_intent_contract.py +++ b/tests/meta/test_launch_intent_contract.py @@ -2,15 +2,14 @@ # SPDX-License-Identifier: BSD-3-Clause-Clear """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 @@ -36,7 +35,7 @@ def run_up_dry(*flags, env=None, check=True): # exactly that error. 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 @@ -160,12 +159,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..91a4e40d1 --- /dev/null +++ b/tests/meta/test_workflow_contract.py @@ -0,0 +1,67 @@ +"""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(): + # 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/requirements.txt" in report + assert "pip install tabulate" not in report + requirements = repo_path("tests", "requirements.txt").read_text().lower() + assert "pyyaml" in requirements + assert "tabulate" in requirements + + +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 + + +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 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/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 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..ac0675e96 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"] @@ -155,6 +183,7 @@ def _poll_until(predicate, timeout, interval, fail_msg): @pytest.mark.liveliness +@pytest.mark.infrastructure @pytest.mark.timeout(1800) class TestLiveliness: @@ -203,24 +232,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_sensors.py b/tests/system/test_sensors.py index 8170286d6..35e212e5d 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 @@ -28,24 +39,38 @@ 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"] 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"])