From 307ba7c4bc4abacb352324cd3ae82ce4e020f5f5 Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Tue, 11 Aug 2026 14:24:51 +0000 Subject: [PATCH] ci: run the PyO3 crates' unit tests instead of only compiling their libs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `dora-node-api-python`, `dora-operator-api-python` and `dora-ros2-bridge-python` are excluded from `cargo test` (ci.yml, nightly.yml) and from clippy. The only job that touched them was `cargo check --all`, which builds lib targets only — so their 46 `#[cfg(test)]` unit tests (seqlock generation counters, transport classification, pinning thresholds, DORADMA read-after-free tracking) were never compiled, let alone run, and a syntax error inside one of those modules could ship. Two gates now cover them: - `check` (every PR) adds `cargo check --all-targets` for the three crates. rmeta-only on an already-checked dependency graph, so it costs seconds and catches test modules that no longer compile. - `contract-tests` (merge gate) runs `make qa-test-python`. It is the one job that sets up a Python interpreter, which these test binaries need: cargo builds the crates without `pyo3/extension-module`, so unlike the maturin wheel they link libpython directly. The step runs before the wheel build, since the two feature sets cannot share pyo3 artifacts anyway. `make qa-test-python` stays out of `qa-test` on purpose — it would make the everyday local gate fail on a machine without a shared libpython >= 3.11. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 39 +++++++++++++++++++++++++++++++++++++++ CLAUDE.md | 9 ++++++++- Makefile | 24 +++++++++++++++++++++++- 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0fce04df35..69a6d624b3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -176,6 +176,19 @@ jobs: # surface and deserve their own compile gate. - name: Check examples compile run: cargo check --examples + # ...nor does it build test targets, so the `#[cfg(test)]` modules of + # the three PyO3 crates were type-checked nowhere: they're excluded + # from `cargo test` (see the `test` job) and from clippy above, and + # `cargo check --all` only covers their lib targets. A syntax error in + # one of those test modules could ship. This is rmeta-only and `Check` + # above already checked the whole dependency graph with the same + # features, so it costs seconds. contract-tests actually *runs* them. + - name: Check PyO3 crate test targets compile + run: > + cargo check --all-targets + -p dora-node-api-python + -p dora-operator-api-python + -p dora-ros2-bridge-python test: # Linux-only on PR CI (#1716). macOS + Windows coverage runs in nightly. @@ -223,6 +236,10 @@ jobs: --exclude dora-node-api-python --exclude dora-operator-api-python --exclude dora-ros2-bridge-python + # The PyO3 crates stay excluded here — their test binaries link + # libpython, and this job sets up no interpreter. Their unit tests run + # in `contract-tests`, which does; keep that step in sync if this list + # changes. - name: Test run: > cargo test --all @@ -502,6 +519,28 @@ jobs: - uses: actions/setup-python@v6 with: python-version: "3.12" + # The three PyO3 crates are excluded from the workspace `cargo test` + # in the `test` job, so until this step their `#[cfg(test)]` modules + # ran nowhere at all — pure-Rust logic tests (seqlock generation + # counters, transport classification, pinning thresholds, the + # `FREED_POOL_IDS` cap) that carried no enforcement. They run here + # because this is the one job with a Python interpreter set up. + # + # Deliberately BEFORE the maturin install below. A test binary is an + # executable and has to link libpython, so this build must NOT enable + # `pyo3/extension-module` (which suppresses that link — right for a + # wheel, a link error for anything else). `[tool.maturin] features` + # in apis/python/node/pyproject.toml enables it for the wheel, so the + # two builds can't share pyo3 artifacts either way; running the tests + # first keeps a unit-test failure from waiting behind the wheel build. + - name: Test PyO3 crates (unit tests) + run: make qa-test-python + # These binaries link libpython dynamically, so the loader needs the + # tool-cache CPython's lib dir. setup-python exports exactly this + # already; restating it keeps the step from depending on that + # implementation detail (LD_LIBRARY_PATH only adds a search path). + env: + LD_LIBRARY_PATH: ${{ env.pythonLocation }}/lib - uses: astral-sh/setup-uv@v8.1.0 with: enable-cache: true diff --git a/CLAUDE.md b/CLAUDE.md index 03fb7f7138..00d358b414 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -141,6 +141,12 @@ cargo test --all \ # can ship silently otherwise. CI gates on this (#1680). cargo check --examples +# 5. Only if you touched a PyO3 crate (apis/python/node, apis/python/operator, +# libraries/extensions/ros2-bridge/python): their unit tests are excluded from +# the `cargo test --all` above because the test binaries link libpython, so run +# them explicitly. CI runs the same command in ci.yml's `contract-tests` job. +make qa-test-python + # Quick single-crate check while iterating: # cargo test -p ``` @@ -177,8 +183,9 @@ The deeper QA gates — `make qa-full`, `make qa-deep`, `make qa-nightly`, `make - `cargo fmt --all -- --check` - `cargo clippy --all -- -D warnings` (excluding Python packages) - `cargo test --all` on **ubuntu-latest only** (excluding Python packages) +- `cargo check --all-targets` on the three PyO3 crates, so their `#[cfg(test)]` modules are at least type-checked on every PR (`cargo check --all` covers lib targets only) - E2E tests: `ws-cli-e2e` + `fault-tolerance-e2e` -- Semantic contract tests (`tests/example-smoke.rs::contract_*`) +- Semantic contract tests (`tests/example-smoke.rs::contract_*`), plus the PyO3 crates' unit tests (`make qa-test-python` — this is the one job that sets up a Python interpreter, so it's where those tests run) - Benchmark regression check (criterion baseline caching) - Typo checking via `crate-ci/typos` (config: `_typos.toml`) - Supply-chain audit (`cargo-audit` + `cargo-deny`) diff --git a/Makefile b/Makefile index a04ebc8051..2101767981 100644 --- a/Makefile +++ b/Makefile @@ -52,11 +52,20 @@ # excludes dora-examples tests to # keep per-commit budgets tight. # +# make qa-test-python ~1 min warm unit tests of the three PyO3 crates, +# which every other qa-* target +# excludes. Needs a Python >= 3.11 +# with a shared libpython; run it +# after touching apis/python/* or +# libraries/extensions/ros2-bridge/ +# python. CI runs the same command in +# ci.yml's contract-tests job. +# # `make qa-tier1` is a back-compat alias for `make qa-deep`. .PHONY: qa qa-fast qa-full qa-deep qa-tier1 qa-nightly qa-release-gate qa-mutation-audit \ qa-examples qa-cluster-e2e qa-cluster-record-replay ros2-zenoh-humble ros2-zenoh-kilted \ - qa-fmt qa-audit qa-unwrap qa-clippy qa-test qa-coverage qa-mutants qa-semver \ + qa-fmt qa-audit qa-unwrap qa-clippy qa-test qa-test-python qa-coverage qa-mutants qa-semver \ qa-adversarial qa-kani qa-pgo qa-install qa-pgo-install qa-kani-install qa: qa-fast @@ -139,6 +148,19 @@ qa-test: --exclude dora-cli-api-python \ --exclude dora-examples +# The unit tests of the three PyO3 crates `qa-test` excludes. Kept a separate +# target, not folded into `qa-test`: cargo builds these crates without +# `pyo3/extension-module` (unlike the maturin wheel), so the test binaries +# link libpython directly and need an interpreter >= 3.11 with a shared +# library — a machine set up only for Rust work would start failing the +# everyday gate. CI runs this same target in ci.yml's `contract-tests` job, +# which sets Python up explicitly. +qa-test-python: + @cargo test --lib \ + -p dora-node-api-python \ + -p dora-operator-api-python \ + -p dora-ros2-bridge-python + qa-coverage: @scripts/qa/coverage.sh