diff --git a/.config/nextest.toml b/.config/nextest.toml index 5e746034972..cd588f2899a 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -19,9 +19,8 @@ slow-timeout = { period = "30s", terminate-after = 2 } # race until proven otherwise (see Root Cause First in AGENTS.md). retries = 0 -# `--all-targets` still compiles the criterion benches, which is what catches a -# bench that no longer builds. Running them in test mode only replays the -# workload once, and the large origin fan-outs take ~20s each for no signal. +# PR recipes build library, binary, and integration-test targets. Nightly keeps +# the exhaustive target compile, but executes benches through cargo bench smokes. default-filter = "not kind(bench)" # Limit ring concurrency within one invocation. Unit-test workers use small diff --git a/quest/m0/ci/README.md b/quest/m0/ci/README.md index 4674b0d3472..280eebc4191 100644 --- a/quest/m0/ci/README.md +++ b/quest/m0/ci/README.md @@ -59,4 +59,3 @@ Measure each change against the numbers above and record the result in its PR. - [Nix cache](/quest/m0/ci/nix-cache.md) - `nix.moq.dev` on R2 replaces Cachix for CI and releases - [Nightly finishes](/quest/m0/ci/nightly-split.md) - every Nightly step runs within its timeout - [FFI artifact reuse](/quest/m0/ci/ffi-reuse.md) - identical FFI release builds are built once and shared across the language packages -- [Feature permutations](/quest/m0/ci/features.md) - PR feature checks and test builds compile only what they run, measured first diff --git a/quest/m0/ci/features.md b/quest/m0/ci/features.md deleted file mode 100644 index 5b6c9629ef3..00000000000 --- a/quest/m0/ci/features.md +++ /dev/null @@ -1,14 +0,0 @@ -# [M] PR builds compile only what they check - -## Goal - -A PR's feature checks cover the feature sets its diff can affect, and its test -compile builds only the targets it runs. Nightly keeps the exhaustive -feature coverage. - -## Plan - -A transitive change triggers many isolated feature builds, and ordinary test -runs compile benchmarks they never execute. Measure which feature builds and -bench compiles a typical PR pays for before moving any of them, and record -where the moved coverage now runs. diff --git a/rs/justfile b/rs/justfile index bd463e0354a..4886877e885 100644 --- a/rs/justfile +++ b/rs/justfile @@ -81,9 +81,10 @@ fix-changed $LIST: test-changed $LIST: sh/rs/select.sh test "$LIST" -# Compile moq-tokio by itself at the feature extremes and with each crypto provider. -tokio-features: - sh/rs/tokio-features.sh +# Compile moq-tokio at its feature extremes; --all also isolates crypto and transport modes. +[positional-arguments] +tokio-features *args: + sh/rs/tokio-features.sh "$@" # Runs the `#[cfg(windows)]` code past the compiler, which the Linux gate # skips; platform.yml runs it on a Windows runner, scoped like `check-changed` @@ -113,9 +114,9 @@ uring-changed base="": [positional-arguments] uring *args: just rs uring-check "$@" - bash sh/rs/nextest.sh cargo nextest run --locked -p moq-uring -p moq-relay --features moq-relay/io-uring --all-targets "$@" + bash sh/rs/nextest.sh cargo nextest run --locked -p moq-uring -p moq-relay --features moq-relay/io-uring --tests "$@" cargo clippy --locked -p moq-uring -p moq-relay --features moq-relay/io-uring,moq-relay/qlog --all-targets "$@" -- -D warnings - bash sh/rs/nextest.sh cargo nextest run --locked -p moq-uring --features qlog --all-targets "$@" + bash sh/rs/nextest.sh cargo nextest run --locked -p moq-uring --features qlog --tests "$@" # Compile the io_uring relay path, off the default feature set. [positional-arguments] @@ -225,6 +226,8 @@ c-tests: # Compile the workspace at the feature extremes. features: just rs net-features + just rs tokio-features --all + just rs media-features --all cargo check --locked --workspace {{ no_fuzz }} --all-targets --no-default-features cargo hack check --locked -p moq-tokio --each-feature --exclude-features _transport --features aws-lc-rs cargo check --locked -p moq-relay --no-default-features @@ -241,21 +244,23 @@ features: net-features: bash sh/rs/net-features.sh -# Compile the media crates at supported feature boundaries and prove expensive +# Compile media boundaries (--all isolates every mode) and prove expensive # native/render dependencies are absent when their features are excluded. -media-features: - sh/rs/media-features.sh +[positional-arguments] +media-features *args: + sh/rs/media-features.sh "$@" # nextest kills a wedged test (`.config/nextest.toml` sets the timeout), but # skips doctests; `just rs doctest` covers those. clippy-driver as the workspace -# wrapper enforces lint findings from the same compile, including benches and -# examples. `build.warnings` denies warnings without changing cargo's artifact -# fingerprint, unlike RUSTFLAGS. +# wrapper enforces lint findings from the same compile. `--tests` builds library, +# binary, and integration-test targets, including binary-only crates; benches and +# examples compile and lint in Nightly's `features`. `build.warnings` denies +# warnings without changing cargo's artifact fingerprint, unlike RUSTFLAGS. -# Run the test suite and enforce clippy findings from the same compile. +# Run library, binary, and integration tests, enforcing clippy from the same compile. [positional-arguments] test *args: - CARGO_BUILD_WARNINGS=deny RUSTC_WORKSPACE_WRAPPER={{ quote(require("clippy-driver")) }} bash sh/rs/nextest.sh cargo nextest run --locked --all-targets "$@" + CARGO_BUILD_WARNINGS=deny RUSTC_WORKSPACE_WRAPPER={{ quote(require("clippy-driver")) }} bash sh/rs/nextest.sh cargo nextest run --locked --tests "$@" # Exercise the ignored hardware tests for every GPU vendor on this Linux host. gpu: diff --git a/sh/rs/media-features.sh b/sh/rs/media-features.sh index 8887fc691a7..a2148513c26 100755 --- a/sh/rs/media-features.sh +++ b/sh/rs/media-features.sh @@ -1,6 +1,14 @@ #!/usr/bin/env bash set -euo pipefail +all=0 +if [[ $# == 1 && $1 == --all ]]; then + all=1 +elif [[ $# != 0 ]]; then + echo 'usage: just rs media-features [--all]' >&2 + exit 2 +fi + check() { echo "media features: cargo check $*" cargo check --locked --all-targets "$@" @@ -29,9 +37,10 @@ forbid_crate() { fi } -# Each shape is compiled independently so workspace feature unification cannot -# hide a missing gate. The normal dependency graph then proves the expensive -# stacks are absent rather than merely unused by this target. +# Minimal shapes and the off-default VPX backend always compile and test. The +# other positive paths already compile in default, capture, and play checks; +# isolate them when their feature owner changes, and exhaustively in Nightly. +# Dependency absence contracts stay on every affected PR, including transitive edits. check -p moq-video --no-default-features minimal=$(tree -p moq-video --no-default-features) for crate in openh264 openh264-sys2 wgpu cpal; do @@ -39,13 +48,13 @@ for crate in openh264 openh264-sys2 wgpu cpal; do done cargo nextest run --locked -p moq-video --no-default-features -check -p moq-video --no-default-features --features openh264 +if ((all)); then check -p moq-video --no-default-features --features openh264; fi software=$(tree -p moq-video --no-default-features --features openh264) require_crate "$software" openh264 "moq-video software-only" require_crate "$software" openh264-sys2 "moq-video software-only" forbid_crate "$software" wgpu "moq-video software-only" -check -p moq-video --no-default-features --features nvidia +if ((all)); then check -p moq-video --no-default-features --features nvidia; fi native=$(tree -p moq-video --no-default-features --features nvidia) require_crate "$native" moq-nvenc "moq-video native-only" for crate in openh264 openh264-sys2 wgpu; do @@ -63,7 +72,7 @@ for crate in openh264 openh264-sys2 wgpu; do done cargo nextest run --locked -p moq-video --no-default-features --features vpx -E 'test(/::vpx::/)' -check -p moq-video --no-default-features --features render +if ((all)); then check -p moq-video --no-default-features --features render; fi render=$(tree -p moq-video --no-default-features --features render) require_crate "$render" wgpu "moq-video rendering" for crate in openh264 openh264-sys2; do @@ -76,12 +85,12 @@ for crate in openh264 openh264-sys2 wgpu moq-nvenc; do forbid_crate "$transcode_minimal" "$crate" "moq-transcode minimal" done -check -p moq-transcode --no-default-features --features openh264 +if ((all)); then check -p moq-transcode --no-default-features --features openh264; fi transcode_software=$(tree -p moq-transcode --no-default-features --features openh264) require_crate "$transcode_software" openh264 "moq-transcode software-only" forbid_crate "$transcode_software" wgpu "moq-transcode software-only" -check -p moq-transcode --no-default-features --features nvidia +if ((all)); then check -p moq-transcode --no-default-features --features nvidia; fi transcode_native=$(tree -p moq-transcode --no-default-features --features nvidia) require_crate "$transcode_native" moq-nvenc "moq-transcode native-only" for crate in openh264 openh264-sys2 wgpu; do @@ -92,7 +101,7 @@ check -p moq-audio --no-default-features --features pipewire audio_host=$(tree -p moq-audio --no-default-features --features pipewire) forbid_crate "$audio_host" cpal "moq-audio host-only" -check -p moq-audio --no-default-features --features capture,pipewire +if ((all)); then check -p moq-audio --no-default-features --features capture,pipewire; fi audio_capture=$(tree -p moq-audio --no-default-features --features capture,pipewire) require_crate "$audio_capture" cpal "moq-audio PipeWire capture" diff --git a/sh/rs/net-features.sh b/sh/rs/net-features.sh index dcd71025e4d..f2be50fcf7c 100644 --- a/sh/rs/net-features.sh +++ b/sh/rs/net-features.sh @@ -11,7 +11,7 @@ fi cargo clippy --locked -p moq-net --no-default-features --all-targets -- -D warnings RUSTDOCFLAGS="-D warnings" cargo doc --locked -p moq-net --no-default-features --no-deps -bash sh/rs/nextest.sh cargo nextest run --locked -p moq-net --no-default-features --all-targets +bash sh/rs/nextest.sh cargo nextest run --locked -p moq-net --no-default-features --tests # A browser leaf chooses its entropy backend. Build a consumer instead of moq-net's # native-only bench dev-dependencies, with the same dependency versions as the repo. diff --git a/sh/rs/select.sh b/sh/rs/select.sh index 555d540f617..d18ead747af 100755 --- a/sh/rs/select.sh +++ b/sh/rs/select.sh @@ -95,14 +95,25 @@ wants() { # The feature contract scripts are no crate, so their own diff selects # nothing and still has to reach the gates below. media_script= -if [[ -f "$list" ]] && grep -qx 'sh/rs/media-features.sh' "$list"; then media_script=1; fi +tokio_script= net_script= -if [[ -f "$list" ]] && grep -qx 'sh/rs/net-features.sh' "$list"; then net_script=1; fi +if [[ -f "$list" ]]; then + if grep -qx 'sh/rs/media-features.sh' "$list"; then media_script=1; fi + if grep -qx 'sh/rs/tokio-features.sh' "$list"; then tokio_script=1; fi + if grep -qx 'sh/rs/net-features.sh' "$list"; then net_script=1; fi +fi + +# Global build inputs select ALL. Otherwise isolate permutations when their +# owning crate changes; dependency-only diffs keep the cheaper boundary checks. +feature_owner_changed() { + [[ "$packages" == ALL ]] || grep -qE "$1" "$list" +} case "$packages" in "") - if [[ ("$action" == check* || "$action" == lint) && -n "$media_script$net_script" ]]; then - if [[ -n "$media_script" ]]; then just rs media-features; fi + if [[ ("$action" == check* || "$action" == lint) && -n "$media_script$tokio_script$net_script" ]]; then + if [[ -n "$media_script" ]]; then just rs media-features --all; fi + if [[ -n "$tokio_script" ]]; then just rs tokio-features --all; fi if [[ -n "$net_script" ]]; then just rs net-features; fi else echo "rs: no crates affected; skipping." @@ -130,7 +141,11 @@ case "$action" in just rs "$action" "${flags[@]}" # Workspace feature unification hides a broken moq-tokio feature set # whenever any dependent enables a transport. - if wants moq-tokio; then just rs tokio-features; fi + if [[ -n "$tokio_script" ]] || wants moq-tokio; then + args=() + if feature_owner_changed '^(rs/moq-tokio/|sh/rs/tokio-features\.sh$)'; then args+=(--all); fi + just rs tokio-features "${args[@]}" + fi # Likewise every dependent turns moq-net's default `async` feature back on. if [[ -n "$net_script" ]] || wants moq-net; then just rs net-features; fi # moq-wasm's crate root is `#![cfg(target_arch = "wasm32")]`, so only a @@ -147,7 +162,11 @@ case "$action" in # The relay's io_uring listener, off the default feature set. if wants moq-relay; then just rs uring-check; fi # Each media feature shape compiles on its own, about two minutes. - if [[ -n "$media_script" ]] || wants '(moq-video|moq-audio|moq-transcode)'; then just rs media-features; fi + if [[ -n "$media_script" ]] || wants '(moq-video|moq-audio|moq-transcode)'; then + args=() + if feature_owner_changed '^(rs/moq-(video|audio|transcode)/|sh/rs/media-features\.sh$)'; then args+=(--all); fi + just rs media-features "${args[@]}" + fi ;; fix) just rs fix "${flags[@]}" diff --git a/sh/rs/select.test.py b/sh/rs/select.test.py index 573d39509ab..71efcd099de 100644 --- a/sh/rs/select.test.py +++ b/sh/rs/select.test.py @@ -1,4 +1,4 @@ -"""Verify shared Rust test inputs reach the workspace recipes.""" +"""Verify Rust selection and test target boundaries.""" import os from pathlib import Path @@ -11,7 +11,8 @@ class SelectTest(unittest.TestCase): - def test_nextest_launcher_selects_workspace(self): + # CI's Check job runs `lint`; `check` is its local equivalent. + def selected(self, *paths, action="lint"): with tempfile.TemporaryDirectory() as directory: scratch = Path(directory) calls = scratch / "calls" @@ -19,25 +20,128 @@ def test_nextest_launcher_selects_workspace(self): just.write_text('#!/bin/sh\nprintf "%s\\n" "$*" >> "$SELECT_CALLS"\n') just.chmod(0o755) changed = scratch / "changed" - changed.write_text("sh/rs/nextest.sh\n") - env = dict(os.environ, PATH=f"{scratch}:{os.environ['PATH']}", SELECT_CALLS=str(calls), - BASH_ENV=os.devnull) - for action in ("check", "check-test", "lint", "test"): - with self.subTest(action=action): - calls.write_text("") - subprocess.run( - ["bash", "sh/rs/select.sh", action, str(changed)], - cwd=ROOT, env=env, check=True, - ) - self.assertIn( - f"rs {action} --workspace --exclude moq-net-fuzz", - calls.read_text().splitlines(), - ) - if action == "lint": - selected = calls.read_text().splitlines() - for recipe in ("tokio-features", "wasm", "uring-check", "media-features"): - self.assertIn(f"rs {recipe}", selected) - self.assertNotIn("rs capture", selected) + changed.write_text("\n".join(paths) + "\n") + env = dict(os.environ, PATH=f"{scratch}:{os.environ['PATH']}", + SELECT_CALLS=str(calls), BASH_ENV=os.devnull) + subprocess.run(["bash", "sh/rs/select.sh", action, str(changed)], + cwd=ROOT, env=env, check=True, capture_output=True, text=True) + return calls.read_text().splitlines() + + def test_nextest_launcher_selects_workspace(self): + for action in ("check", "check-test", "lint", "test"): + with self.subTest(action=action): + self.assertIn(f"rs {action} --workspace --exclude moq-net-fuzz", + self.selected("sh/rs/nextest.sh", action=action)) + selected = self.selected("sh/rs/nextest.sh") + for recipe in ("tokio-features --all", "net-features", "wasm", "uring-check", "media-features --all"): + self.assertIn(f"rs {recipe}", selected) + self.assertNotIn("rs capture", selected) + + def test_transitive_change_keeps_boundary_checks(self): + calls = self.selected("rs/kio/src/lib.rs") + self.assertIn("rs tokio-features", calls) + self.assertIn("rs media-features", calls) + self.assertNotIn("rs tokio-features --all", calls) + self.assertNotIn("rs media-features --all", calls) + + def test_feature_owner_gets_isolated_permutations(self): + self.assertIn("rs tokio-features --all", self.selected("rs/moq-tokio/src/lib.rs")) + self.assertIn("rs media-features --all", self.selected("rs/moq-video/src/lib.rs")) + + def test_feature_script_diff_checks_its_own_modes(self): + for action in ("check", "lint"): + for owner in ("tokio", "media"): + with self.subTest(action=action, owner=owner): + self.assertEqual(self.selected(f"sh/rs/{owner}-features.sh", action=action), + [f"rs {owner}-features --all"]) + with self.subTest(action=action, owner="net"): + self.assertEqual(self.selected("sh/rs/net-features.sh", action=action), + ["rs net-features"]) + + def test_shared_build_input_gets_all_permutations(self): + calls = self.selected("Cargo.lock") + self.assertIn("rs tokio-features --all", calls) + self.assertIn("rs media-features --all", calls) + + +class RecipeTest(unittest.TestCase): + def test_unexecuted_bench_is_not_compiled(self): + for source in ("lib.rs", "main.rs"): + with self.subTest(source=source), tempfile.TemporaryDirectory() as directory: + scratch = Path(directory) + (scratch / "src").mkdir() + (scratch / "benches").mkdir() + (scratch / "Cargo.toml").write_text( + '[package]\nname = "nextest-target-probe"\nversion = "0.0.0"\nedition = "2024"\n' + '[workspace]\n[[bench]]\nname = "unexecuted"\nharness = false\n' + ) + (scratch / "Cargo.lock").write_text( + 'version = 4\n[[package]]\nname = "nextest-target-probe"\nversion = "0.0.0"\n' + ) + (scratch / "src" / source).write_text( + '#[test] fn runs() { assert_eq!(1 + 1, 2); }\n' + + ('fn main() {}\n' if source == "main.rs" else '') + ) + (scratch / "benches/unexecuted.rs").write_text( + 'compile_error!("nextest must not compile a benchmark it never runs");\nfn main() {}\n' + ) + env = dict(os.environ, BASH_ENV=os.devnull, TMPDIR="/tmp") + result = subprocess.run( + ["just", "rs", "test", "--manifest-path", str(scratch / "Cargo.toml")], + cwd=ROOT, env=env, capture_output=True, text=True, + ) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + +class FeatureTest(unittest.TestCase): + def trace(self, owner, *args): + with tempfile.TemporaryDirectory() as directory: + scratch = Path(directory) + calls = scratch / "calls" + cargo = scratch / "cargo" + cargo.write_text(CARGO_STUB) + cargo.chmod(0o755) + env = dict(os.environ, PATH=f"{scratch}:{os.environ['PATH']}", + FEATURE_CALLS=str(calls), BASH_ENV=os.devnull) + result = subprocess.run(["bash", f"sh/rs/{owner}-features.sh", *args], + cwd=ROOT, env=env, capture_output=True, text=True) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + return calls.read_text().splitlines() + + def test_tokio_keeps_extremes_and_crypto_contracts(self): + boundary = self.trace("tokio") + exhaustive = self.trace("tokio", "--all") + self.assertEqual(sum(call.startswith("clippy ") for call in boundary), 2) + self.assertEqual(sum(call.startswith("clippy ") for call in exhaustive), 9) + self.assertEqual([call for call in boundary if not call.startswith("clippy ")], + [call for call in exhaustive if not call.startswith("clippy ")]) + + def test_media_keeps_dependency_contracts_and_vpx_tests(self): + boundary = self.trace("media") + exhaustive = self.trace("media", "--all") + self.assertEqual(sum(call.startswith("check ") for call in boundary), 4) + self.assertEqual(sum(call.startswith("check ") for call in exhaustive), 10) + self.assertEqual([call for call in boundary if not call.startswith("check ")], + [call for call in exhaustive if not call.startswith("check ")]) + + +CARGO_STUB = r"""#!/bin/bash +printf '%s\n' "$*" >> "$FEATURE_CALLS" +if [[ "$*" == 'check --locked -p moq-tokio --no-default-features --features noq' ]]; then + echo 'a rustls QUIC backend requires a crypto provider: enable either the `aws-lc-rs` or `ring` feature' >&2 + exit 1 +fi +if [[ $1 == tree ]]; then + echo 'root v1' + case "$*" in + *'--features openh264'*) printf 'openh264 v1\nopenh264-sys2 v1\n' ;; + *'--features nvidia'*) echo 'moq-nvenc v1' ;; + *'--features vpx'*) echo 'libvpx-native-sys v1' ;; + *'--features render'*) echo 'wgpu v1' ;; + *'--features capture,pipewire'*) echo 'cpal v1' ;; + esac +fi +""" if __name__ == "__main__": diff --git a/sh/rs/tokio-features.sh b/sh/rs/tokio-features.sh index 1a0a4604995..8727b4a233a 100755 --- a/sh/rs/tokio-features.sh +++ b/sh/rs/tokio-features.sh @@ -1,19 +1,31 @@ #!/usr/bin/env bash -# Compile moq-tokio by itself at the feature extremes and with each crypto -# provider. Its default-feature build is already part of the ordinary clippy -# pass; this catches what workspace feature unification hides. +# Compile moq-tokio by itself at the feature extremes; --all also isolates each +# crypto and transport permutation. The default-feature build is already in the +# ordinary gate, which can hide missing feature gates through unification. set -euo pipefail +all=0 +if [[ $# == 1 && $1 == --all ]]; then + all=1 +elif [[ $# != 0 ]]; then + echo 'usage: just rs tokio-features [--all]' >&2 + exit 2 +fi + cd "$(git rev-parse --show-toplevel)" cargo clippy --locked -p moq-tokio --no-default-features -- -D warnings cargo clippy --locked -p moq-tokio --all-features -- -D warnings -cargo clippy --locked -p moq-tokio --no-default-features --features noq,aws-lc-rs -- -D warnings -cargo clippy --locked -p moq-tokio --no-default-features --features noq,ring -- -D warnings +# Only a feature-owner edit needs every isolated permutation on a PR. Shared +# dependencies still compile at both extremes; Nightly runs every permutation. +if ((all)); then + cargo clippy --locked -p moq-tokio --no-default-features --features noq,aws-lc-rs -- -D warnings + cargo clippy --locked -p moq-tokio --no-default-features --features noq,ring -- -D warnings -for feature in aws-lc-rs ring tcp uds websocket; do - cargo clippy --locked -p moq-tokio --no-default-features --features "$feature" -- -D warnings -done + for feature in aws-lc-rs ring tcp uds websocket; do + cargo clippy --locked -p moq-tokio --no-default-features --features "$feature" -- -D warnings + done +fi # shellcheck disable=SC2016 # the backticks are literal, as rustc prints them want='a rustls QUIC backend requires a crypto provider: enable either the `aws-lc-rs` or `ring` feature'