ci: run the PyO3 crates' unit tests instead of only compiling their libs - #3124
Merged
Conversation
`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) <noreply@anthropic.com>
Contributor
|
👍 Pull request will be merged soon because tests have passed on #2859, which was a pull request behind this one (test branch SHA) - details. |
trunk-io
Bot
added
trunk-not-ready
trunk-queued
trunk-testing
and removed
trunk-not-ready
trunk-queued
labels
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The three PyO3 crates have 46
#[cfg(test)]unit tests between them that no CIjob has ever run. They're excluded from
cargo test(ci.yml,nightly.yml)and from clippy; the only job that touches them is
cargo check --all, whichbuilds lib targets only. So the test modules weren't even type-checked — a
syntax error inside one could ship, and any regression test added there carries
no enforcement (noticed while reviewing #3039, which adds four).
Two gates now cover them:
check(every PR) addscargo check --all-targetsfor the three crates.rmeta-only against a dependency graph the previous step already checked with
the same features, so it costs seconds. Verified it catches the gap: a
deliberate type error inside
seqlock_testspassescargo check --allandfails this step.
contract-tests(merge gate) runs the newmake qa-test-python. That job isthe one with
actions/setup-python, which these test binaries need: cargobuilds the crates without
pyo3/extension-module, so unlike the maturinwheel they link libpython directly. It runs before the wheel build — the two
feature sets can't share pyo3 artifacts anyway, so a failing unit test
surfaces without waiting on maturin.
make qa-test-pythonstays out ofqa-test: folding it in would break theeveryday local gate on a machine without a shared libpython ≥ 3.11.
All 46 pass today (33
dora-node-api-python, 7dora-operator-api-python, 6dora-ros2-bridge-python), so this only locks in current behavior.Linux-only, matching the rest of PR CI. Extending it to nightly's macOS and
Windows legs would mean a cold pyo3 build on each, which isn't worth the ~3-4h
budget there.