diff --git a/.buildkite/common/ci_mirror_hardwares.yml b/.buildkite/common/ci_mirror_hardwares.yml index 00c6e4f82..e68cbd382 100644 --- a/.buildkite/common/ci_mirror_hardwares.yml +++ b/.buildkite/common/ci_mirror_hardwares.yml @@ -9,6 +9,11 @@ # a2b3_npu_1 — ascend-a2b3, resource_class npu-1 (A2/B3 CI image) # a2b3_npu_4 — ascend-a2b3, resource_class npu-4 # a3_npu_2 — ascend-a3, resource_class npu-2 +# +# The ``l4_*`` presets run under the ``docker`` plugin, which does not put +# ``buildkite-agent`` on PATH inside the container the way the ``kubernetes`` +# presets do, so they set ``mount-buildkite-agent: true``; without it any step +# calling ``buildkite-agent`` (artifact upload, annotate, meta-data) fails there. x-npu-k8s-plugins: &npu_k8s_plugins - kubernetes: @@ -26,6 +31,7 @@ mirror_hardwares: image: public.ecr.aws/q9t5s3a7/vllm-ci-test-repo:$BUILDKITE_COMMIT always-pull: true propagate-environment: true + mount-buildkite-agent: true shm-size: "8gb" environment: - HF_HOME=/fsx/hf_cache @@ -41,6 +47,7 @@ mirror_hardwares: image: public.ecr.aws/q9t5s3a7/vllm-ci-test-repo:$BUILDKITE_COMMIT always-pull: true propagate-environment: true + mount-buildkite-agent: true shm-size: "8gb" environment: - HF_HOME=/fsx/hf_cache diff --git a/.buildkite/common/scripts/run_cov_split.sh b/.buildkite/common/scripts/run_cov_split.sh new file mode 100755 index 000000000..fe068519c --- /dev/null +++ b/.buildkite/common/scripts/run_cov_split.sh @@ -0,0 +1,82 @@ +#!/usr/bin/env bash +# Run a model's E2E tests once per entry mode as separate coverage runs and upload +# the reports, so per-mode attribution survives (see vllm-project/vllm-omni#5332). +# +# Usage: run_cov_split.sh --model-id [--offline ] [--online ] \ +# -- +# +# --model-id Names the artifacts: coverage--{offline,online}-.xml.gz. +# Use the model's directory name under vllm_omni/*/models/. +# --offline Pytest path(s) for the offline half. Repeatable. +# --online Pytest path(s) for the online half. Repeatable. +# -- Everything after this is forwarded verbatim to every pytest run, +# quoting preserved: the job's own -m expression, --run-level, and +# anything else it needs (e.g. --test-config-file). +# +# At least one of --offline/--online is required, so a job with only one mode runs +# just that half. Total runtime is bounded by the step's timeout_in_minutes. +# +# Exits non-zero if any half or the upload failed. +set -uo pipefail + +MODEL_ID="" +OFFLINE=() +ONLINE=() +PYTEST_ARGS=() + +while [[ $# -gt 0 ]]; do + case "$1" in + --model-id) MODEL_ID="$2"; shift 2 ;; + --offline) OFFLINE+=("$2"); shift 2 ;; + --online) ONLINE+=("$2"); shift 2 ;; + --) shift; PYTEST_ARGS=("$@"); break ;; + *) echo "run_cov_split.sh: unknown argument: $1" >&2; exit 2 ;; + esac +done + +if [[ -z "${MODEL_ID}" ]]; then + echo "run_cov_split.sh: missing --model-id" >&2 + exit 2 +fi + +if ((${#OFFLINE[@]} == 0 && ${#ONLINE[@]} == 0)); then + echo "run_cov_split.sh: need at least one of --offline / --online" >&2 + exit 2 +fi + +# Two steps can test the same model, and artifacts are scoped to the build (a +# nightly one carries the ready, merge and nightly tiers at once), so unsuffixed +# reports would share a path -- a glob download then races to rename onto one file. +# Not BUILDKITE_JOB_ID: the step id survives retries, so attempts collapse under +# Buildkite's latest-attempt lookup instead of accumulating. A parallel step would +# need BUILDKITE_PARALLEL_JOB as well; no pilot is one. +REPORT_SUFFIX="${BUILDKITE_STEP_ID:-local}" +OFFLINE_REPORT="coverage-${MODEL_ID}-offline-${REPORT_SUFFIX}.xml" +ONLINE_REPORT="coverage-${MODEL_ID}-online-${REPORT_SUFFIX}.xml" + +run_half() { + local mode="$1" + shift + local report="coverage-${MODEL_ID}-${mode}-${REPORT_SUFFIX}.xml" + echo "--- coverage: ${MODEL_ID} ${mode}" + pytest -s -v "$@" "${PYTEST_ARGS[@]}" --cov=vllm_omni --cov-report="xml:${report}" + local status=$? + # Cobertura lists every statement in vllm_omni whether or not it ran, so the + # report is ~7MB no matter how little the job covers. The repetition is what + # dominates it, and gzip takes that to ~400KB. + gzip -9 -f "${report}" || status=1 + return "${status}" +} + +# Clear both modes, not just the ones being run: the upload glob matches both, so +# a report left by an earlier run would otherwise be published as if the mode it +# belongs to had run this time. +rm -f "${OFFLINE_REPORT}" "${ONLINE_REPORT}" "${OFFLINE_REPORT}.gz" "${ONLINE_REPORT}.gz" + +EXIT=0 +((${#OFFLINE[@]})) && { run_half offline "${OFFLINE[@]}" || EXIT=1; } +((${#ONLINE[@]})) && { run_half online "${ONLINE[@]}" || EXIT=1; } + +buildkite-agent artifact upload "coverage-${MODEL_ID}-*-${REPORT_SUFFIX}.xml.gz" || EXIT=1 + +exit "${EXIT}" diff --git a/.buildkite/cuda/test-merge.yml b/.buildkite/cuda/test-merge.yml index 363e8fbdc..970ab5a5a 100644 --- a/.buildkite/cuda/test-merge.yml +++ b/.buildkite/cuda/test-merge.yml @@ -94,13 +94,19 @@ steps: - vllm_omni/model_executor/models/qwen3_tts/ - vllm_omni/model_executor/stage_input_processors/qwen3_tts.py - vllm_omni/deploy/qwen3_tts.yaml + - .buildkite/common/scripts/run_cov_split.sh + - pyproject.toml + # 40 -> 60: the coverage split runs the model twice. + timeout_in_minutes: 60 commands: - | - timeout 40m bash -c " - export VLLM_LOGGING_LEVEL=DEBUG - export VLLM_ALLOW_LONG_MAX_MODEL_LEN=1 - pytest -s -v tests/e2e/online_serving/test_qwen3_tts_base.py tests/e2e/offline_inference/test_qwen3_tts_base.py -m 'advanced_model and cuda' --run-level 'advanced_model' - " + export VLLM_LOGGING_LEVEL=DEBUG + export VLLM_ALLOW_LONG_MAX_MODEL_LEN=1 + .buildkite/common/scripts/run_cov_split.sh \ + --model-id qwen3_tts \ + --offline tests/e2e/offline_inference/test_qwen3_tts_base.py \ + --online tests/e2e/online_serving/test_qwen3_tts_base.py \ + -- -m 'advanced_model and cuda' --run-level 'advanced_model' mirror_hardwares: l4_1 - label: "TTS · Higgs-Audio-V3 Test" @@ -203,9 +209,17 @@ steps: - vllm_omni/diffusion/models/bagel/ - vllm_omni/model_executor/stage_input_processors/bagel.py - vllm_omni/deploy/bagel.yaml - timeout_in_minutes: 20 + - .buildkite/common/scripts/run_cov_split.sh + - pyproject.toml + # 20 -> 40: the coverage split runs the model twice. + timeout_in_minutes: 40 commands: - - pytest -s -v tests/e2e/offline_inference/test_bagel.py tests/e2e/online_serving/test_bagel.py -m 'advanced_model and cuda' --run-level 'advanced_model' + - | + .buildkite/common/scripts/run_cov_split.sh \ + --model-id bagel \ + --offline tests/e2e/offline_inference/test_bagel.py \ + --online tests/e2e/online_serving/test_bagel.py \ + -- -m 'advanced_model and cuda' --run-level 'advanced_model' mirror_hardwares: h100_1 - label: "Diffusion · Qwen Image Test" diff --git a/.gitignore b/.gitignore index 3c24e40ae..f11b57388 100644 --- a/.gitignore +++ b/.gitignore @@ -46,6 +46,8 @@ htmlcov/ .cache nosetests.xml coverage.xml +coverage-*.xml +coverage-*.xml.gz *.cover *.py,cover .hypothesis/ diff --git a/docs/contributing/ci/ci_settings.md b/docs/contributing/ci/ci_settings.md index ff4b8e107..62717ddc9 100644 --- a/docs/contributing/ci/ci_settings.md +++ b/docs/contributing/ci/ci_settings.md @@ -327,6 +327,96 @@ On PR builds, `upload_pipeline.py` logs `skip '…' (no changes under …)` for - Use **groups** for dashboard readability; keep **E2E Test** as the group name CUDA diff filtering expects for `--e2e` nightly runs on main. - Prefix labels by model domain: **Omni ·**, **TTS ·**, **Diffusion ·**, **Simple ·**, etc., matching existing steps. +### Per-model coverage + +Pilot (v1): two Merge-tier (L3) jobs in `.buildkite/cuda/test-merge.yml` upload +per-model, per-entry-mode coverage as Buildkite artifacts — "Diffusion · Bagel +Test" and "TTS · Qwen3-TTS Base Test". Both run on a single GPU so the pilot is +cheap to reproduce. This is a pilot, not a rollout: no dashboard/visualization +lives in this repo, and nothing is gated on coverage. + +**Naming convention**: `coverage---.xml.gz`, where +`` is the model's directory name under +`vllm_omni/diffusion/models//` or +`vllm_omni/model_executor/models//` (e.g. `z_image`, `qwen3_omni`), +`` is `online` or `offline`, and `` is `BUILDKITE_STEP_ID` +(`local` outside Buildkite). `gunzip` before feeding a report to a Cobertura +consumer. + +Reports are gzipped because `--cov=vllm_omni` sets coverage's `source`, which +makes it walk the package and emit one `` element per statement for every +file — including the ones a single model's tests never import. That inventory, +not the coverage, is the size: a Bagel report is 7.1 MB, of which 96.5% is +`` elements and 77% belongs to the 840 of 1169 files that run never +imported. A run covering zero lines produces the same 7 MB. gzip takes it to +roughly 410 KB and changes nothing about the data. + +`` is what keeps two steps that cover the same model apart. Artifacts are +scoped to the build, and a nightly build carries the ready, merge and nightly tiers +at once, so several same-model steps land in one namespace — `qwen3_tts` already has +both a Base and a CustomVoice job, and `minicpmo_4_5` a base and a duplex one. Two +uploads on one path do not fail loudly: an exact-name download can report the path +as ambiguous, and a glob download fetches every match concurrently and renames each +onto the same destination, so whichever finishes last wins. The step id remains the +logical step identifier across retries, while Buildkite's default artifact lookup +selects only the latest attempt — `BUILDKITE_JOB_ID` would instead give every +attempt its own filename, leaving a failed attempt's partial report to be picked up +alongside the real one. A step using `parallelism`/matrix would need +`BUILDKITE_PARALLEL_JOB` added, since its jobs share a step id; neither pilot is +one. + +**Opting in a new model**: replace the job's combined `pytest` command with +[`run_cov_split.sh`](https://github.com/vllm-project/vllm-omni/blob/main/.buildkite/common/scripts/run_cov_split.sh), +which runs each mode, names the reports, uploads them, and fails the step if +either half or the upload failed: + +```yaml +commands: + - | + .buildkite/common/scripts/run_cov_split.sh \ + --model-id \ + --offline \ + --online \ + -- +``` + +Everything after `--` is forwarded verbatim to both runs with quoting preserved, +so an expression like `-m 'advanced_model and cuda'` survives intact and a job can +pass anything else it needs. `--offline`/`--online` are repeatable and only one is +required, so a job with tests in a single mode runs just that half. Runtime is +bounded solely by the step's `timeout_in_minutes` — raise it, since the split +loads the model once per mode. + +Before splitting, confirm each half actually collects at least one test under the job's forwarded filters — pytest exits with code 5 ("no tests collected") if a half is empty, which the script reports as a failure, red-ing a job that used to pass as one combined invocation. + +Also check the tests' `num_cards` against the job's `mirror_hardwares` preset. +`cuda_marks` turns `num_cards > 1` into `skipif(device_count() < num_cards)`, so a +test asking for more GPUs than the preset provides is silently skipped and its +coverage artifact comes back empty. Splitting such a job produces a file that +looks fine and measures nothing — "Diffusion · Z Image Test" is the current +example (online tests want 4 cards, the job runs on `l4_1`), which is why it is +not a coverage pilot. + +Online-mode coverage depends on the `[tool.coverage.run]` `patch`/`sigterm` +settings in `pyproject.toml`: online tests launch the server with +`subprocess.Popen` and stop it with SIGTERM, and without those settings the XML +reflects only the pytest parent process, not the server's code paths. + +The upload depends on `buildkite-agent` being callable inside the container. The +`kubernetes` presets (`h100_*`, `*_npu_*`) provide it; the `docker` ones (`l4_*`) +only do because they set `mount-buildkite-agent: true`. A new docker preset that +runs a coverage job needs the same. + +List both `run_cov_split.sh` and `pyproject.toml` in every opted-in job's +`source_file_dependencies` — both change what the job measures, so without them a +change there is filtered out of normal PR builds and only surfaces in a later +nightly. `tests/buildkite/test_upload_pipeline.py` asserts this for the pilots. +Editing only the surrounding CI YAML still does not schedule them, so a PR that +touches just the wiring needs a full E2E run (or the commands run on a GPU host) +to produce artifacts. When checking a new model's +artifacts, compare `lines-covered` between the online and offline XML rather than +just confirming both files exist. + ### Validation checklist | Check | Command / location | diff --git a/pyproject.toml b/pyproject.toml index 0e91bfae6..48273fd20 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -301,6 +301,21 @@ filterwarnings = [ "ignore:.*does not have '__bases__' attribute.*:UserWarning", ] +[tool.coverage.run] +core = "sysmon" +source = ["vllm_omni"] +# Online-serving tests launch the server via subprocess.Popen and stop it with +# SIGTERM. pytest-cov no longer instruments subprocesses implicitly, and a +# SIGTERM-ed process discards its data, so without patch/sigterm a coverage run +# over an online test measures only the pytest parent. +patch = ["subprocess"] +concurrency = ["multiprocessing", "thread"] +sigterm = true + +[tool.coverage.report] +show_missing = true +skip_covered = true + [tool.typos.default] extend-ignore-identifiers-re = [ ".*_thw", diff --git a/tests/buildkite/test_upload_pipeline.py b/tests/buildkite/test_upload_pipeline.py index 24a2a731e..ee45a64d3 100644 --- a/tests/buildkite/test_upload_pipeline.py +++ b/tests/buildkite/test_upload_pipeline.py @@ -5,6 +5,7 @@ import sys from pathlib import Path import pytest +import yaml sys.path.insert(0, str(Path(__file__).resolve().parents[2] / ".buildkite" / "common" / "scripts")) @@ -139,3 +140,48 @@ def test_mirror_hardwares_a2b3_npu_4_expands_agents_image_and_plugins() -> None: assert step["plugins"][0]["kubernetes"]["podSpecPatch"]["imagePullSecrets"] == [ {"name": "swr-secret"}, ] + + +COVERAGE_PILOT_LABELS = ("Diffusion · Bagel Test", "TTS · Qwen3-TTS Base Test") +MERGE_PIPELINE = Path(".buildkite/cuda/test-merge.yml") + + +def _surviving_labels(changed_files: list[str]) -> set[str]: + doc = yaml.safe_load(MERGE_PIPELINE.read_text(encoding="utf-8")) + rendered = _render_test_pipeline(doc, changed_files=changed_files) + labels: set[str] = set() + + def walk(steps: list | None) -> None: + for step in steps or []: + if not isinstance(step, dict): + continue + if "label" in step: + labels.add(step["label"]) + walk(step.get("steps")) + + walk(rendered.get("steps")) + return labels + + +@pytest.mark.parametrize( + "changed_file", + [ + ".buildkite/common/scripts/run_cov_split.sh", + "pyproject.toml", + ], +) +def test_coverage_pilots_selected_by_their_shared_inputs(changed_file: str) -> None: + """The split helper and the coverage config both change what these jobs measure. + + Without them in source_file_dependencies the jobs are filtered out of normal + PR builds, so a regression would only surface in a later nightly. + """ + labels = _surviving_labels([changed_file]) + for pilot in COVERAGE_PILOT_LABELS: + assert pilot in labels, f"{pilot} dropped when {changed_file} changed" + + +def test_coverage_pilots_not_selected_by_unrelated_change() -> None: + labels = _surviving_labels(["vllm_omni/entrypoints/openai/serving_chat.py"]) + for pilot in COVERAGE_PILOT_LABELS: + assert pilot not in labels