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..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: "" @@ -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 @@ -369,17 +386,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 +433,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 +452,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 +546,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 +566,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 +592,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 +625,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 +676,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 +727,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..18c411b0c 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. | --- @@ -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 @@ -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..372e47c56 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. --- @@ -561,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 @@ -570,14 +577,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..0b13665b2 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: @@ -157,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) @@ -239,8 +264,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..5d8a8ab67 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 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..1522b5071 --- /dev/null +++ b/tests/meta/test_campaign_reporting_contract.py @@ -0,0 +1,187 @@ +"""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 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] == "" + + +@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..08ccde40d --- /dev/null +++ b/tests/meta/test_workflow_contract.py @@ -0,0 +1,71 @@ +"""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 + + +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 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/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..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_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..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"])