From abe1eb2d7b9bb0779ab14c9dfb7c68746b5b6854 Mon Sep 17 00:00:00 2001 From: timohueser Date: Wed, 16 Sep 2026 11:16:53 +0200 Subject: [PATCH 1/7] test: make bounded fixture tests fail with the sync command The fixture loader returned `None` for a missing package and only panicked with a hint when `OBC_REQUIRE_FIXTURES` was set, so every one of the sixteen call sites turned that `None` into an `.expect` message that named no setup command. `file()` now returns the path or panics with the exact `obc fixtures sync` command, `file_in` names a profile outside `test`, and the environment variable is gone from the loader, the justfile, the workflow, the registry command and the assistant-places verifier. `sim-peak-view` leaves the `test` sync profile: no test reads it, only the simulator does at runtime, and it stays in the `sim` profile. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 1 - apps/obc-sim/tests/present_fixtures.rs | 8 ++--- firmware/obc-reader/tests/poi_fixtures.rs | 2 +- firmware/obc-route/tests/nav_fixtures.rs | 12 +++---- fixtures/catalog.toml | 2 +- fixtures/sources/peak-view/README.md | 4 +-- fixtures/verify-assistant-places.py | 3 +- host/obc-dem/tests/assets.rs | 2 +- host/obc-fixtures/src/lib.rs | 39 +++++++++------------ host/obc-host-core/tests/altitude_fusion.rs | 6 ++-- host/obc-pack/tests/assistant_places.rs | 3 +- testing/suites.toml | 2 +- tools/justfile | 1 - 13 files changed, 37 insertions(+), 48 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 363896eee..b841e7f46 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -299,7 +299,6 @@ jobs: id: nextest-fixtures env: NEXTEST_PROFILE: fixtures - OBC_REQUIRE_FIXTURES: "1" run: | eval "$(cargo llvm-cov show-env --sh)" rm -f target/nextest/fixtures/junit.xml diff --git a/apps/obc-sim/tests/present_fixtures.rs b/apps/obc-sim/tests/present_fixtures.rs index 6ad6858d3..9b534fff7 100644 --- a/apps/obc-sim/tests/present_fixtures.rs +++ b/apps/obc-sim/tests/present_fixtures.rs @@ -24,7 +24,7 @@ fn app_scenarios_idle_is_free_tick_is_small_pan_is_most() { // The device resolution is the single ls021 authority, not a re-declared literal. const W: u32 = FRAME_W as u32; const H: u32 = FRAME_H as u32; - let bytes = obc_fixtures::read("sim-grimsel", "grimsel.obcm").expect("full fixture suite requires map"); + let bytes = obc_fixtures::read("sim-grimsel", "grimsel.obcm"); let tables = MapTables::parse(&SliceSource(&bytes)).expect("valid demo map"); let cache = MapCache::new(); let src = SliceSource(&bytes); @@ -93,7 +93,7 @@ fn app_scenarios_idle_is_free_tick_is_small_pan_is_most() { fn monaco_fixture_parses_populated_poi_and_nav_sections() { use obc_reader::{MapCache, MapTables, Reader, SliceSource}; - let bytes = obc_fixtures::read("sim-monaco", "monaco.obcm").expect("full fixture suite requires map"); + let bytes = obc_fixtures::read("sim-monaco", "monaco.obcm"); let src = SliceSource(&bytes); let tables = MapTables::parse(&src).expect("monaco.obcm parses as a valid current map"); let cache = MapCache::new(); @@ -261,7 +261,7 @@ fn tour_screens_dwell_with_no_present_miss() { const W: u32 = FRAME_W as u32; const H: u32 = FRAME_H as u32; - let bytes = obc_fixtures::read("sim-grimsel", "grimsel.obcm").expect("full fixture suite requires map"); + let bytes = obc_fixtures::read("sim-grimsel", "grimsel.obcm"); let tables = MapTables::parse(&SliceSource(&bytes)).expect("valid demo map"); let owner = HostStore::memory().unwrap(); let map = obc_host_core::flat_map::FlatMap::from_bytes_in(&owner, &bytes).unwrap(); @@ -484,7 +484,7 @@ fn demo_reset_rebuild_and_seek_present_clean() { const W: u32 = FRAME_W as u32; const H: u32 = FRAME_H as u32; - let bytes = obc_fixtures::read("sim-grimsel", "grimsel.obcm").expect("full fixture suite requires map"); + let bytes = obc_fixtures::read("sim-grimsel", "grimsel.obcm"); let tables = MapTables::parse(&SliceSource(&bytes)).expect("valid demo map"); let owner = HostStore::memory().unwrap(); let map = obc_host_core::flat_map::FlatMap::from_bytes_in(&owner, &bytes).unwrap(); diff --git a/firmware/obc-reader/tests/poi_fixtures.rs b/firmware/obc-reader/tests/poi_fixtures.rs index 805661698..f2c4ad35e 100644 --- a/firmware/obc-reader/tests/poi_fixtures.rs +++ b/firmware/obc-reader/tests/poi_fixtures.rs @@ -8,7 +8,7 @@ use obc_reader::{MapCache, MapTables, Poi, PoiCategory, Reader, SliceSource, MAX /// nearest-16 query fills and the results are the closest 16. #[test] fn monaco_water_query_smoke() { - let bytes = obc_fixtures::read("sim-monaco", "monaco.obcm").expect("full fixture suite requires map"); + let bytes = obc_fixtures::read("sim-monaco", "monaco.obcm"); let src = SliceSource(&bytes); let tables = MapTables::parse(&src).unwrap(); let cache = MapCache::new(); diff --git a/firmware/obc-route/tests/nav_fixtures.rs b/firmware/obc-route/tests/nav_fixtures.rs index 0e421ffaa..4e0bd5af4 100644 --- a/firmware/obc-route/tests/nav_fixtures.rs +++ b/firmware/obc-route/tests/nav_fixtures.rs @@ -30,7 +30,7 @@ use obc_route::nav::{plan_route, NavScratch}; #[cfg_attr(miri, ignore)] #[test] fn road_vs_mtb_diverge_over_grimsel() { - let bytes = obc_fixtures::read("sim-grimsel", "grimsel.obcm").expect("full fixture suite requires map"); + let bytes = obc_fixtures::read("sim-grimsel", "grimsel.obcm"); let from = (8_169_610, 46_694_536); let to = (8_217_309, 46_706_261); @@ -54,8 +54,8 @@ fn road_vs_mtb_diverge_over_grimsel() { #[cfg_attr(miri, ignore)] #[test] fn a_real_grimsel_plan_carries_the_pass_road_profile() { - let map = obc_fixtures::read("sim-grimsel", "grimsel.obcm").expect("full fixture suite requires map"); - let dem = obc_fixtures::read("sim-grimsel", "grimsel.obcd").expect("full fixture suite requires terrain"); + let map = obc_fixtures::read("sim-grimsel", "grimsel.obcm"); + let dem = obc_fixtures::read("sim-grimsel", "grimsel.obcd"); let terrain_src = SliceSource(&dem); let mut terrain = obc_elevation::TerrainElevation::<{ obc_elevation::DEFAULT_TILE_SLOTS }>::parse(&terrain_src) .expect("the baked terrain parses"); @@ -95,8 +95,8 @@ fn a_real_grimsel_plan_carries_the_pass_road_profile() { #[cfg_attr(miri, ignore)] // reads the committed fixtures from disk — see the note above #[test] fn a_planned_route_exported_to_gpx_and_reimported_keeps_its_climb() { - let map = obc_fixtures::read("sim-grimsel", "grimsel.obcm").expect("full fixture suite requires map"); - let dem = obc_fixtures::read("sim-grimsel", "grimsel.obcd").expect("full fixture suite requires terrain"); + let map = obc_fixtures::read("sim-grimsel", "grimsel.obcm"); + let dem = obc_fixtures::read("sim-grimsel", "grimsel.obcd"); let terrain_src = SliceSource(&dem); let mut terrain = obc_elevation::TerrainElevation::<{ obc_elevation::DEFAULT_TILE_SLOTS }>::parse(&terrain_src).unwrap(); @@ -140,7 +140,7 @@ fn a_planned_route_exported_to_gpx_and_reimported_keeps_its_climb() { #[cfg_attr(miri, ignore)] #[test] fn the_registered_grimsel_fixture_routes_byte_identically_on_every_profile() { - let bytes = obc_fixtures::read("sim-grimsel", "grimsel.obcm").expect("full fixture suite requires map"); + let bytes = obc_fixtures::read("sim-grimsel", "grimsel.obcm"); let (from, to) = ((8_169_610, 46_694_536), (8_217_309, 46_706_261)); let actual = core::array::from_fn::<_, 4, _>(|idx| { let (res, obcr, _) = plan_p(&bytes, from, to, "Grimsel", idx as u8); diff --git a/fixtures/catalog.toml b/fixtures/catalog.toml index 5ca4a1e9a..3f169ffc7 100644 --- a/fixtures/catalog.toml +++ b/fixtures/catalog.toml @@ -51,7 +51,7 @@ scenarios = ["grimsel", "monaco-upahead", "assistant-out-and-back", "assistant-f [profiles.test] summary = "All external fixture packages exercised by the full repository test suite" scenarios = ["grimsel", "monaco-upahead"] -packages = ["sim-peak-view"] +packages = [] [packages.assistant-osm] summary = "Pinned OSM sources for Ride Assistant: Switzerland, Monaco and regional West Cork" diff --git a/fixtures/sources/peak-view/README.md b/fixtures/sources/peak-view/README.md index a6eee7400..8262e748a 100644 --- a/fixtures/sources/peak-view/README.md +++ b/fixtures/sources/peak-view/README.md @@ -10,8 +10,8 @@ reconstructed from them. Production map summit records retain exact OSM coordina ## Obtain -Run `obc fixtures sync sim-peak-view`. The package is also part of the `sim` and `test` -profiles. The simulator resolves the package through the standard fixture cache. +Run `obc fixtures sync sim-peak-view`. The package is also part of the `sim` +profile. The simulator resolves the package through the standard fixture cache. `OBC_PEAK_TERRAIN_DIR` overrides it for a local bake. ## Generate diff --git a/fixtures/verify-assistant-places.py b/fixtures/verify-assistant-places.py index c55410f21..cb763919b 100644 --- a/fixtures/verify-assistant-places.py +++ b/fixtures/verify-assistant-places.py @@ -1,6 +1,5 @@ #!/usr/bin/env python3 """Verify captured Ride Assistant places through the production OBCM path.""" -import os from pathlib import Path import subprocess @@ -8,4 +7,4 @@ subprocess.run([ "cargo", "test", "--locked", "-p", "obc-pack", "--features", "external-fixtures", "--test", "assistant_places", "--", "--ignored", "--nocapture", -], cwd=ROOT, env={**os.environ, "OBC_REQUIRE_FIXTURES": "1"}, check=True) +], cwd=ROOT, check=True) diff --git a/host/obc-dem/tests/assets.rs b/host/obc-dem/tests/assets.rs index 295ca5600..0481e8671 100644 --- a/host/obc-dem/tests/assets.rs +++ b/host/obc-dem/tests/assets.rs @@ -41,7 +41,7 @@ fn with_sampler(bytes: &[u8], body: impl FnOnce(&mut dyn FnMut(f64, f64) -> Opti #[test] #[cfg(feature = "external-fixtures")] fn the_grimsel_sidecar_covers_its_map_and_reads_as_the_grimsel() { - let bytes = obc_fixtures::read("sim-grimsel", "grimsel.obcd").expect("full fixture suite requires terrain"); + let bytes = obc_fixtures::read("sim-grimsel", "grimsel.obcd"); assert_eq!(bytes.len(), 786_560, "24 cells of 32 KiB behind a 4 × 6 directory"); with_sampler(&bytes, |at| { diff --git a/host/obc-fixtures/src/lib.rs b/host/obc-fixtures/src/lib.rs index 3babfdb60..35428aef4 100644 --- a/host/obc-fixtures/src/lib.rs +++ b/host/obc-fixtures/src/lib.rs @@ -23,13 +23,18 @@ pub fn root() -> PathBuf { PathBuf::from(home).join(".cache/openbikecomputer/fixtures/by-id") } -/// Resolve a file in a logical package, returning `None` when it is not synced. +/// Resolve a file in a logical package, or fail with the command that syncs it. /// -/// Tests should return early on `None` during an ordinary `cargo test`. The -/// canonical `obc test` command sets `OBC_REQUIRE_FIXTURES=1`, turning a missing -/// package into a useful hard failure instead of a silent skip. +/// Fixtures are never downloaded by a test. A package that is not synced is a +/// setup error, and the panic names the exact `obc fixtures sync` command. #[must_use] -pub fn file(package: &str, relative: impl AsRef) -> Option { +pub fn file(package: &str, relative: impl AsRef) -> PathBuf { + file_in("test", package, relative) +} + +/// [`file`] for a package outside the `test` profile; `profile` is the one that provides it. +#[must_use] +pub fn file_in(profile: &str, package: &str, relative: impl AsRef) -> PathBuf { assert_id(package); let relative = relative.as_ref(); assert!( @@ -37,27 +42,15 @@ pub fn file(package: &str, relative: impl AsRef) -> Option { "OBC fixtures: relative paths may not escape a package" ); let path = root().join(package).join(relative); - if path.is_file() { - Some(path) - } else if std::env::var_os("OBC_REQUIRE_FIXTURES").is_some() { - panic!("OBC fixture is missing: {}. Run `obc fixtures sync test`.", path.display()); - } else { - eprintln!( - "skipping external-fixture assertion (missing {}); run `obc test` for the full suite", - path.display() - ); - None - } + assert!(path.is_file(), "OBC fixture is missing: {}. Run `obc fixtures sync {profile}`.", path.display()); + path } -/// Read a fixture file with the same optional/full-suite behavior as [`file`]. +/// Read a fixture file, with the same missing-package failure as [`file`]. #[must_use] -pub fn read(package: &str, relative: impl AsRef) -> Option> { - let path = file(package, relative)?; - Some( - std::fs::read(&path) - .unwrap_or_else(|error| panic!("OBC fixture became unreadable at {}: {error}", path.display())), - ) +pub fn read(package: &str, relative: impl AsRef) -> Vec { + let path = file(package, relative); + std::fs::read(&path).unwrap_or_else(|error| panic!("OBC fixture became unreadable at {}: {error}", path.display())) } fn assert_id(package: &str) { diff --git a/host/obc-host-core/tests/altitude_fusion.rs b/host/obc-host-core/tests/altitude_fusion.rs index 24f79bff3..45f3d3139 100644 --- a/host/obc-host-core/tests/altitude_fusion.rs +++ b/host/obc-host-core/tests/altitude_fusion.rs @@ -29,9 +29,9 @@ use obc_replay::{gpx::Track, BaroSensor, GpxPlayer}; const FIX_DT_S: f64 = 1.0; fn fixtures() -> (Vec, Vec, String) { - let map = obc_fixtures::read("sim-grimsel", "grimsel.obcm").expect("full fixture suite requires map"); - let terrain = obc_fixtures::read("sim-grimsel", "grimsel.obcd").expect("full fixture suite requires terrain"); - let gpx = obc_fixtures::read("sim-grimsel", "tracks/grimsel-climb.gpx").expect("full fixture suite requires track"); + let map = obc_fixtures::read("sim-grimsel", "grimsel.obcm"); + let terrain = obc_fixtures::read("sim-grimsel", "grimsel.obcd"); + let gpx = obc_fixtures::read("sim-grimsel", "tracks/grimsel-climb.gpx"); (map, terrain, String::from_utf8(gpx).expect("fixture GPX is UTF-8")) } diff --git a/host/obc-pack/tests/assistant_places.rs b/host/obc-pack/tests/assistant_places.rs index 48d8ae768..4db9ef283 100644 --- a/host/obc-pack/tests/assistant_places.rs +++ b/host/obc-pack/tests/assistant_places.rs @@ -12,8 +12,7 @@ use obc_reader::{MapCache, MapTables, PoiCategorySet, Reader, SliceSource}; #[test] #[ignore = "manual captured-source suite: fixtures/verify-assistant-places.py"] fn explicit_gletsch_access_survives_packing_and_nearby_roads_do_not_create_access() { - let source = obc_fixtures::file("assistant-osm", "switzerland.osm.pbf") - .expect("run tools/obc fixtures sync assistant-inputs"); + let source = obc_fixtures::file_in("assistant-inputs", "assistant-osm", "switzerland.osm.pbf"); let config = Config::parse(include_str!("../../../builder/presets/schema.json")).unwrap(); let ing = ingest_osm( &[source.display().to_string()], diff --git a/testing/suites.toml b/testing/suites.toml index e8769066f..1227d8061 100644 --- a/testing/suites.toml +++ b/testing/suites.toml @@ -1082,7 +1082,7 @@ ownership = [{ kind = "path", source = "xcuitest", pattern = "companion-ios/OBCC id = "ci.rust-fixture-tests" surface = "ci" level = "fixture" -command = 'python3 tools/fixtures.py sync test && OBC_REQUIRE_FIXTURES=1 OBC_FIXTURE_ROOT="$(python3 tools/fixtures.py root)" cargo nextest run --workspace --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fixtures)"' +command = 'python3 tools/fixtures.py sync test && OBC_FIXTURE_ROOT="$(python3 tools/fixtures.py root)" cargo nextest run --workspace --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fixtures)"' fixtures = ["test"] pull_request = "affected" scheduled = "none" diff --git a/tools/justfile b/tools/justfile index fac938566..5dfd1c2e1 100644 --- a/tools/justfile +++ b/tools/justfile @@ -569,7 +569,6 @@ test *args: _run python3 "$OBC_TOOLS/fixtures.py" sync test export OBC_FIXTURE_ROOT OBC_FIXTURE_ROOT="$(python3 "$OBC_TOOLS/fixtures.py" root)" - export OBC_REQUIRE_FIXTURES=1 } case "$mode" in full) From 75e4e999033d1783188d6ff8f904742565b7748a Mon Sep 17 00:00:00 2001 From: timohueser Date: Wed, 16 Sep 2026 11:23:29 +0200 Subject: [PATCH 2/7] ci: compile and run the packages the selection plan selected Both nextest steps and the doctest step ran `--workspace --all-features` and then filtered execution, so a three-crate change still built every crate. The plan already names the selected suites; `tools/ci/rust_packages.py` turns it into `-p NAME` flags for one tier, prints `--workspace` when the change can alter what any package compiles to, and prints nothing when the tier selected no package. An empty set skips the tier; it never falls back to the workspace. `tools/ci/test.sh` carries the job's body, one section per step, so the same file serves CI and a local run. The registry reads a `tools/ci/*.sh` script as the job body it is, and a computed package list counts as the whole root workspace for job coverage, which is what it is on a foundation change. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 50 ++++++---------------- testing/suites.toml | 8 ++-- tools/ci/rust_packages.py | 77 ++++++++++++++++++++++++++++++++++ tools/ci/test.sh | 87 +++++++++++++++++++++++++++++++++++++++ tools/suite_registry.py | 26 ++++++++++-- 5 files changed, 203 insertions(+), 45 deletions(-) create mode 100644 tools/ci/rust_packages.py create mode 100644 tools/ci/test.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b841e7f46..1d1d8b971 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -251,6 +251,8 @@ jobs: test: env: CARGO_LLVM_COV_TARGET_DIR: ${{ github.workspace }}/target + OBC_COVERAGE: "1" + OBC_SELECTION_PLAN: ${{ needs.selection.outputs.plan }} needs: selection if: contains(fromJSON(needs.selection.outputs.jobs), 'test') runs-on: ubuntu-latest @@ -285,39 +287,19 @@ jobs: with: path: ~/.cache/openbikecomputer/fixtures key: obc-fixtures-${{ hashFiles('fixtures/catalog.toml') }} + # Each section is one step so it keeps its own name and result artifact; the file is + # the same one `obc check test` runs. The package set comes from the selection plan, + # so a three-crate change compiles three crates and not the workspace. - name: cargo nextest (fast hermetic binaries) id: nextest - env: - NEXTEST_PROFILE: ci - run: | - mkdir -p target - eval "$(cargo llvm-cov show-env --sh)" - find target -maxdepth 1 -name '*.profraw' -delete - rm -f target/nextest/ci/junit.xml - cargo nextest run --workspace --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fast)" + run: bash tools/ci/test.sh nextest-fast - name: cargo nextest (captured fixture binaries) id: nextest-fixtures - env: - NEXTEST_PROFILE: fixtures - run: | - eval "$(cargo llvm-cov show-env --sh)" - rm -f target/nextest/fixtures/junit.xml - python3 tools/fixtures.py sync test - OBC_FIXTURE_ROOT="$(python3 tools/fixtures.py root)" - export OBC_FIXTURE_ROOT - cargo nextest run --workspace --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fixtures)" - # nextest runs no doctests, so the whole workspace's doctests run here as one cheap step - # rather than being silently dropped from the gate. - - name: cargo test --doc (workspace, all features) - shell: bash - run: | - mkdir -p .artifacts/rust - cargo test --workspace --all-features --locked --doc 2>&1 | tee .artifacts/rust/doctests.log + run: bash tools/ci/test.sh nextest-fixtures + - name: cargo test --doc (selected packages, all features) + run: bash tools/ci/test.sh doctests - name: cargo test (obc-formats, default features = the device's shape) - shell: bash - run: | - mkdir -p .artifacts/rust - cargo test -p obc-formats --locked 2>&1 | tee .artifacts/rust/formats-default.log + run: bash tools/ci/test.sh formats-default - uses: actions/setup-python@v5 with: python-version: "3.12" @@ -331,14 +313,8 @@ jobs: python3 -m coverage run --data-file=.artifacts/coverage/firmware-tools/.coverage --source=firmware/tools -m xmlrunner discover -s firmware/tools/tests -v -o .artifacts/python/firmware-tools - name: firmware production dependency direction run: python3 firmware/tools/check_dependencies.py - # Render + read-path golden gate (issues #327, #1467): re-run the 7 fixed bench scenes and - # the 9 route-corridor cases over the deterministic testkit fixtures and fail if any frame - # hash *or* any read counter (chunks visited, cache hits/misses, SD reads, bytes) drifts from - # the committed golden file — so a cache change that halves the hit rate cannot pass on - # identical pixels. Timings are printed to the log but never gated (shared runners are noisy); - # an intentional change regenerates golden.txt (--write-golden) in the same PR. - name: obc-bench --check (render + read-counter golden gate) - run: cargo run -p obc-bench --release --locked -- --check host/obc-bench/golden.txt + run: bash tools/ci/test.sh bench-golden # Screen pixel-hash tripwire (#1441): re-render every UI screen through the headless # simulator and compare the frames to the committed digest manifest. The bench tripwire above # covers the map renderer's scenes; this covers the screens drawn over them, which is what a @@ -434,14 +410,14 @@ jobs: .artifacts/rust/ if-no-files-found: error - name: Upload Rust test results - if: ${{ !cancelled() && (steps.nextest.outcome == 'success' || steps.nextest.outcome == 'failure') }} + if: ${{ !cancelled() && steps.nextest.outputs.report == 'true' }} uses: actions/upload-artifact@v4 with: name: rust-test-${{ github.run_attempt }} path: target/nextest/ci/junit.xml if-no-files-found: error - name: Upload Rust fixture test results - if: ${{ !cancelled() && (steps.nextest-fixtures.outcome == 'success' || steps.nextest-fixtures.outcome == 'failure') }} + if: ${{ !cancelled() && steps.nextest-fixtures.outputs.report == 'true' }} uses: actions/upload-artifact@v4 with: name: rust-fixtures-${{ github.run_attempt }} diff --git a/testing/suites.toml b/testing/suites.toml index 1227d8061..049e99915 100644 --- a/testing/suites.toml +++ b/testing/suites.toml @@ -633,8 +633,8 @@ pull_request = "affected" scheduled = "none" extra_triggers = ["Cargo.toml", "Cargo.lock", "rust-toolchain.toml", "tools/fixtures.py"] ownership = [ - { kind = "workflow", pattern = 'cargo nextest run --workspace --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fast)"' }, - { kind = "workflow", pattern = "cargo test --workspace --all-features --locked --doc*" }, + { kind = "workflow", pattern = 'cargo nextest run $packages --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fast)"' }, + { kind = "workflow", pattern = "cargo test $packages --all-features --locked --doc*" }, { kind = "workflow", pattern = "python3 tools/coverage_report.py --scope rust *" }, { kind = "workflow", pattern = "cargo test -p obc-formats --locked*" }, { kind = "workflow", pattern = "cargo nextest run --release --locked --config-file ../../.config/nextest.toml --no-tests fail" }, @@ -1082,12 +1082,12 @@ ownership = [{ kind = "path", source = "xcuitest", pattern = "companion-ios/OBCC id = "ci.rust-fixture-tests" surface = "ci" level = "fixture" -command = 'python3 tools/fixtures.py sync test && OBC_FIXTURE_ROOT="$(python3 tools/fixtures.py root)" cargo nextest run --workspace --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fixtures)"' +command = "bash tools/ci/test.sh nextest-fixtures" fixtures = ["test"] pull_request = "affected" scheduled = "none" extra_triggers = ["fixtures/catalog.toml", "tools/fixtures.py"] ownership = [ - { kind = "workflow", pattern = 'cargo nextest run --workspace --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fixtures)"' }, + { kind = "workflow", pattern = 'cargo nextest run $packages --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fixtures)"' }, { kind = "workflow", pattern = "python3 tools/fixtures.py sync test" }, ] diff --git a/tools/ci/rust_packages.py b/tools/ci/rust_packages.py new file mode 100644 index 000000000..afbdb20f6 --- /dev/null +++ b/tools/ci/rust_packages.py @@ -0,0 +1,77 @@ +#!/usr/bin/env python3 +"""Print the Cargo package flags one tier of the `test` job compiles. + +The selection plan already says which suites a change requires. This turns that +plan into `-p NAME` flags, so the job compiles the selected packages instead of +the whole workspace. It prints `--workspace` when the change can alter what any +package compiles to, and nothing at all when the tier selected no Rust package. +Without a plan (a local `obc check test`) the answer is the whole workspace. +""" + +from __future__ import annotations + +import argparse +import json +import os +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +import suite_registry as registry # noqa: E402 + +# A change to one of these can change what every package compiles to, or which +# suites the selector itself produces, so no narrowed package set is trustworthy. +FOUNDATION = registry.RUST_FOUNDATION_PATHS | { + ".config/nextest.toml", + ".github/workflows/ci.yml", + "testing/suites.toml", + "tools/suite_registry.py", +} + + +def _is_foundation(path: str) -> bool: + return path in FOUNDATION or path.startswith("tools/ci/") + + +def packages(plan: dict, tier: str, root: Path) -> list[str]: + """The root-workspace packages whose binaries this tier's filter can name.""" + + inventory = registry.load_inventory(root) + graph = registry.build_cargo_graph(root) + levels = registry.cargo_tier_levels(tier) + selected = set(plan.get("selected_suite_ids", [])) + names: set[str] = set() + for suite in inventory.suites: + if suite["id"] not in selected or suite["level"] not in levels or suite["pull_request"] == "never": + continue + for item in inventory.matches[suite["id"]]: + if item.kind != "rust-target" or item.detail.endswith(":example"): + continue + name = item.name.split(":", 1)[0] + package = graph.packages.get(name) + if package and package.root_workspace: + names.add(name) + return sorted(names) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--tier", choices=["fast", "fixtures"], default="fast") + parser.add_argument("--plan", help="selection-plan JSON file; defaults to OBC_SELECTION_PLAN") + args = parser.parse_args(argv) + + raw = Path(args.plan).read_text(encoding="utf-8") if args.plan else os.environ.get("OBC_SELECTION_PLAN", "") + if not raw.strip(): + print("--workspace") + return 0 + plan = json.loads(raw) + if any(_is_foundation(path) for path in plan.get("changed_paths", [])): + print("--workspace") + return 0 + print(" ".join(f"-p {name}" for name in packages(plan, args.tier, registry.repository_root()))) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tools/ci/test.sh b/tools/ci/test.sh new file mode 100644 index 000000000..266cf6558 --- /dev/null +++ b/tools/ci/test.sh @@ -0,0 +1,87 @@ +#!/usr/bin/env bash +# The body of the CI `test` job. CI runs one section per step so each keeps its own +# name and result artifact; `obc check test` runs the same sections from this file. +# +# Usage: tools/ci/test.sh SECTION... +# nextest-fast fast hermetic binaries over the selected packages +# nextest-fixtures captured-fixture binaries, after an explicit fixture sync +# doctests doctests for the same package set +# formats-default obc-formats in the device's default feature shape +# bench-golden render + read-counter golden gate +# +# `--all-features` is deliberate: it is the shape the host tests are written for. The +# device's feature shape is checked by the separate formats-default section. +# Set OBC_COVERAGE=1 to run the sections under llvm-cov instrumentation. +set -euo pipefail +cd "$(dirname "$0")/../.." + +if [ "${OBC_COVERAGE:-}" = "1" ]; then + eval "$(cargo llvm-cov show-env --sh)" +fi + +# An empty package set means the tier selected nothing. That is not a reason to fall +# back to the whole workspace, and the tier then writes no report for CI to upload. +report() { + if [ -n "${GITHUB_OUTPUT:-}" ]; then echo "report=true" >> "$GITHUB_OUTPUT"; fi +} + +nextest_fast() { + local packages + packages="$(python3 tools/ci/rust_packages.py --tier fast)" + if [ -z "$packages" ]; then echo "no Rust packages selected for the fast tier"; return 0; fi + export NEXTEST_PROFILE="${NEXTEST_PROFILE:-ci}" + mkdir -p target + find target -maxdepth 1 -name '*.profraw' -delete + rm -f target/nextest/ci/junit.xml + report + # shellcheck disable=SC2086 + cargo nextest run $packages --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fast)" +} + +nextest_fixtures() { + local packages + packages="$(python3 tools/ci/rust_packages.py --tier fixtures)" + if [ -z "$packages" ]; then echo "no Rust packages selected for the fixtures tier"; return 0; fi + export NEXTEST_PROFILE=fixtures + rm -f target/nextest/fixtures/junit.xml + report + python3 tools/fixtures.py sync test + OBC_FIXTURE_ROOT="$(python3 tools/fixtures.py root)" + export OBC_FIXTURE_ROOT + # shellcheck disable=SC2086 + cargo nextest run $packages --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fixtures)" +} + +# nextest runs no doctests, so they run here rather than falling out of the gate. +doctests() { + local packages + packages="$(python3 tools/ci/rust_packages.py --tier fast)" + if [ -z "$packages" ]; then echo "no Rust packages selected for the fast tier"; return 0; fi + mkdir -p .artifacts/rust + # shellcheck disable=SC2086 + cargo test $packages --all-features --locked --doc 2>&1 | tee .artifacts/rust/doctests.log +} + +formats_default() { + mkdir -p .artifacts/rust + cargo test -p obc-formats --locked 2>&1 | tee .artifacts/rust/formats-default.log +} + +# Re-render the fixed bench scenes and route-corridor cases and fail if any frame hash +# or read counter drifts from the committed golden file, so a cache change that halves +# the hit rate cannot pass on identical pixels. Timings print but are never gated. +bench_golden() { + cargo run -p obc-bench --release --locked -- --check host/obc-bench/golden.txt +} + +[ "$#" -gt 0 ] || { echo "tools/ci/test.sh: name at least one section" >&2; exit 2; } +for section in "$@"; do + case "$section" in + nextest-fast) nextest_fast ;; + nextest-fixtures) nextest_fixtures ;; + doctests) doctests ;; + formats-default) formats_default ;; + bench-golden) bench_golden ;; + *) echo "tools/ci/test.sh: unknown section $section" >&2; exit 2 ;; + esac +done diff --git a/tools/suite_registry.py b/tools/suite_registry.py index 1226b9cdc..fc1323e83 100644 --- a/tools/suite_registry.py +++ b/tools/suite_registry.py @@ -285,6 +285,19 @@ def discover_swift(root: Path) -> list[Discovered]: found.append(Discovered("swift-package", manifest.parent.name, relative)) return sorted(found) +SHELL_ASSIGNMENT_RE = re.compile(r"[A-Za-z_][A-Za-z0-9_]*=(?:\"[^\"]*\"|'[^']*'|\S*)") +JOB_BODY_RE = re.compile(r"(?:^|\s)(tools/ci/[\w.-]+\.sh)(?:\s|$)") + +def _job_body_lines(root: Path, command: str) -> list[str]: + """A `tools/ci/*.sh` script is a job body: its commands are the job's commands.""" + + lines: list[str] = [] + for match in JOB_BODY_RE.finditer(command): + script = root / match.group(1) + if script.is_file(): + lines.extend(line.strip() for line in script.read_text(encoding="utf-8").splitlines()) + return lines + def _strip_yaml_scalar(value: str) -> str: value = value.strip() if len(value) >= 2 and value[0] == value[-1] and value[0] in "\"'": @@ -306,8 +319,11 @@ def scan_workflow(root: Path) -> list[WorkflowStep]: in_jobs = False def record(command: str) -> None: - if any(marker in command for marker in WORKFLOW_MARKERS): - steps.append(WorkflowStep(command, current_job, step_directory or job_directory)) + for line in _job_body_lines(root, command) or [command]: + if line.startswith(("#", "export ")) or SHELL_ASSIGNMENT_RE.fullmatch(line): + continue + if any(marker in line for marker in WORKFLOW_MARKERS): + steps.append(WorkflowStep(line, current_job, step_directory or job_directory)) for raw in workflow.read_text(encoding="utf-8").splitlines(): indent = len(raw) - len(raw.lstrip()) @@ -327,7 +343,7 @@ def record(command: str) -> None: continue if stripped.startswith("export "): continue - if re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*=(?:\"[^\"]*\"|'[^']*'|\S*)", stripped): + if SHELL_ASSIGNMENT_RE.fullmatch(stripped): # A whole-line variable assignment is shell state, not a command to route. continue if block_indent is not None: @@ -780,7 +796,9 @@ def _cargo_packages(args: Sequence[str], directory: str, graph: CargoGraph) -> s continue if word.startswith("--package="): named.add(word.split("=", 1)[1]) - elif word in {"--workspace", "--all"}: + elif word in {"--workspace", "--all"} or (word.startswith("$") and not word.startswith("$(")): + # A package set computed at run time can name any root-workspace package, and on a + # foundation change it is `--workspace`, so the command covers the whole workspace. workspace = True elif word == "--manifest-path" and following: manifest = following From b4c99c41f417186a9891ad14c2e49d25b800dda0 Mon Sep 17 00:00:00 2001 From: timohueser Date: Wed, 16 Sep 2026 11:28:50 +0200 Subject: [PATCH 3/7] ci: one guards job, and the sweep and builder pytest off the test job Seven guard jobs each paid for a checkout to run one fast Python script. They are now seven steps of one `guards` job, which also carries the registry drift check, its unit tests and the fixture policy. No guard's logic moved and each step keeps its own name, so a failure still reads the same. The UI snapshot sweep and the builder pytest leave the `test` job's serial path for `ui-snapshots` and `builder-python`, each gated on its own job name and each restoring the shared host-tests cache. The builder job now fails, rather than silently skipping its packer half, when the binary or corpus is missing. Routing expectations that changed, all from the split: - python.repository-tools now routes to `guards`, python.builder to `builder-python`. - `builder-python` compiles obc-pack and `ui-snapshots` compiles obc-sim, so both are routes for those packages, exactly as `clippy` and `fmt` are. A foundational Rust change therefore starts both jobs. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 206 +++++++++++++++-------------- tools/justfile | 7 +- tools/tests/test_suite_registry.py | 14 +- 3 files changed, 120 insertions(+), 107 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1d1d8b971..ff3a8c4b4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -92,7 +92,10 @@ jobs: echo "jobs=$(cat selection-jobs.json)" >> "$GITHUB_OUTPUT" echo "required jobs: $(cat selection-jobs.json)" - retired-map-stack: + # Every guard is a fast Python check over the source tree. They share nothing but runner + # setup, so one job runs them all; each keeps its own step name and failure message. The + # job is unconditional: every guard suite is required on every pull request. + guards: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -100,58 +103,16 @@ jobs: persist-credentials: false - name: Deleted catalog and bake APIs stay deleted run: python3 tools/check_retired_map_stack.py - - card-scheduler-guard: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - persist-credentials: false - name: Host-pushed cards are built only by the card scheduler run: python3 tools/check_card_scheduler.py - - render-key-guard: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - persist-credentials: false - name: Deleted repaint mirrors stay deleted run: python3 tools/check_render_keys.py - - catalog-ownership-guard: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - persist-credentials: false - name: Only CatalogMachine orders a catalog re-read run: python3 tools/check_catalog_ownership.py - - one-home-guard: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - persist-credentials: false - name: A contextual setting has one home run: python3 tools/check_one_home.py - - screen-vocabulary-guard: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - persist-credentials: false - name: Shared screen drawing helpers live in screen/vocab run: python3 tools/check_screen_vocabulary.py - - fixture-registry: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - persist-credentials: false - name: Test-suite registry drift run: python3 tools/suite_registry.py check - uses: actions/setup-python@v5 @@ -315,42 +276,6 @@ jobs: run: python3 firmware/tools/check_dependencies.py - name: obc-bench --check (render + read-counter golden gate) run: bash tools/ci/test.sh bench-golden - # Screen pixel-hash tripwire (#1441): re-render every UI screen through the headless - # simulator and compare the frames to the committed digest manifest. The bench tripwire above - # covers the map renderer's scenes; this covers the screens drawn over them, which is what a - # screen refactor moves. The debug binary is used deliberately — its output is byte-identical - # to release and the tests above have already built it, so this adds a sweep, not a link. - # `update` is a developer command and never runs here: a self-regenerating manifest records - # nothing. - - name: ui-snapshots manifest check - id: ui-snapshots - if: contains(fromJSON(needs.selection.outputs.plan).selected_suite_ids, 'ci.ui-snapshots') - run: | - cargo build -p obc-sim --locked - SIM="$PWD/target/debug/obc-sim" firmware/ui-snapshots.sh "$RUNNER_TEMP/ui-snapshots" - python3 firmware/tools/ui_snapshot_manifest.py check firmware/ui-snapshots.sha256 "$RUNNER_TEMP/ui-snapshots" - - name: Upload UI review frames - if: ${{ !cancelled() && (steps.ui-snapshots.outcome == 'success' || steps.ui-snapshots.outcome == 'failure') }} - uses: actions/upload-artifact@v4 - with: - name: ui-snapshots-${{ github.run_attempt }} - path: ${{ runner.temp }}/ui-snapshots/*.png - if-no-files-found: ignore - # Routing-profile round-trips through the builder's pack path. These shell out to the - # real obc-pack binary over the tiny corpus and byte-compare the §8.6 profile table - # against independently quantized expectations — so they pin the packer's writer - # against the spec rather than against itself — plus the pure /api/schema and - # /api/presets endpoints via FastAPI's TestClient. The suite existed and passed but no - # workflow ran it; requirements-dev.txt is what makes it installable. - - name: Prepare web-builder packer tests - run: | - pip install -r builder/requirements-dev.txt - cargo build -p obc-pack --release --locked - - name: web-builder packer tests (pytest) - id: python-builder - run: | - rm -f .artifacts/python/builder.xml - PYTHONPATH=. python3 -m pytest builder/tests/ -v --junitxml=.artifacts/python/builder.xml --cov=builder/server --cov-report=lcov:.artifacts/coverage/builder/lcov.info --cov-report=term - name: firmware-tools coverage if: ${{ !cancelled() && (steps.python-firmware-tools.outcome == 'success' || steps.python-firmware-tools.outcome == 'failure') }} run: | @@ -372,25 +297,6 @@ jobs: name: python-firmware-tools-${{ github.run_attempt }} path: .artifacts/python/firmware-tools/*.xml if-no-files-found: error - - name: Builder Python coverage - if: ${{ !cancelled() && (steps.python-builder.outcome == 'success' || steps.python-builder.outcome == 'failure') }} - run: | - python3 tools/coverage_report.py --scope python --lcov .artifacts/coverage/builder/lcov.info --output .artifacts/coverage/builder --tool coverage.py=7.16.1 --tool pytest-cov=7.1.0 - cat .artifacts/coverage/builder/summary.md >> "$GITHUB_STEP_SUMMARY" - - name: Upload builder Python coverage - if: ${{ !cancelled() }} - uses: actions/upload-artifact@v4 - with: - name: coverage-builder-${{ github.run_attempt }} - path: .artifacts/coverage/builder/ - if-no-files-found: error - - name: Upload builder Python test results - if: ${{ !cancelled() && (steps.python-builder.outcome == 'success' || steps.python-builder.outcome == 'failure') }} - uses: actions/upload-artifact@v4 - with: - name: python-builder-${{ github.run_attempt }} - path: .artifacts/python/builder.xml - if-no-files-found: error - name: Rust component coverage if: ${{ !cancelled() && (steps.nextest.outcome == 'success' || steps.nextest.outcome == 'failure') }} run: | @@ -424,6 +330,108 @@ jobs: path: target/nextest/fixtures/junit.xml if-no-files-found: error + # Screen pixel-hash tripwire: re-render every UI screen through the headless simulator and + # compare the frames to the committed digest manifest. The bench golden gate in `test` covers + # the map renderer's scenes; this covers the screens drawn over them, which is what a screen + # refactor moves. It is its own job so the sweep is off the `test` job's serial path. The debug + # binary is deliberate: its output is byte-identical to release. `update` is a developer command + # and never runs here — a self-regenerating manifest records nothing. + ui-snapshots: + needs: selection + if: contains(fromJSON(needs.selection.outputs.jobs), 'ui-snapshots') + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + persist-credentials: false + - uses: actions-rust-lang/setup-rust-toolchain@v1 + with: + cache: false + - uses: Swatinem/rust-cache@v2 + with: + workspaces: . + shared-key: host-tests + save-if: false + - uses: ./.github/actions/setup-host-deps + - name: Cache content-addressed developer fixtures + uses: actions/cache@v4 + with: + path: ~/.cache/openbikecomputer/fixtures + key: obc-fixtures-${{ hashFiles('fixtures/catalog.toml') }} + - name: ui-snapshots manifest check + id: ui-snapshots + run: | + cargo build -p obc-sim --locked + SIM="$PWD/target/debug/obc-sim" firmware/ui-snapshots.sh "$RUNNER_TEMP/ui-snapshots" + python3 firmware/tools/ui_snapshot_manifest.py check firmware/ui-snapshots.sha256 "$RUNNER_TEMP/ui-snapshots" + - name: Upload UI review frames + if: ${{ !cancelled() && (steps.ui-snapshots.outcome == 'success' || steps.ui-snapshots.outcome == 'failure') }} + uses: actions/upload-artifact@v4 + with: + name: ui-snapshots-${{ github.run_attempt }} + path: ${{ runner.temp }}/ui-snapshots/*.png + if-no-files-found: ignore + + # Routing-profile round-trips through the builder's pack path. These shell out to the real + # obc-pack binary over the tiny corpus and byte-compare the §8.6 profile table against + # independently quantized expectations — so they pin the packer's writer against the spec + # rather than against itself — plus the pure /api/schema and /api/presets endpoints via + # FastAPI's TestClient. The preflight is what stops a missing binary or corpus from turning + # the packer half of the suite into a silent skip. + builder-python: + needs: selection + if: contains(fromJSON(needs.selection.outputs.jobs), 'builder-python') + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + persist-credentials: false + - uses: actions-rust-lang/setup-rust-toolchain@v1 + with: + cache: false + - uses: Swatinem/rust-cache@v2 + with: + workspaces: . + shared-key: host-tests + save-if: false + - uses: ./.github/actions/setup-host-deps + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + - name: Prepare web-builder packer tests + run: | + pip install -r builder/requirements-dev.txt + cargo build -p obc-pack --release --locked + - name: Packer binary and corpus are present + run: | + test -x target/release/obc-pack + test -f builder/tests/corpus/data/tiny.osm.pbf + - name: web-builder packer tests (pytest) + id: python-builder + run: | + mkdir -p .artifacts/python + rm -f .artifacts/python/builder.xml + PYTHONPATH=. python3 -m pytest builder/tests/ -v --junitxml=.artifacts/python/builder.xml --cov=builder/server --cov-report=lcov:.artifacts/coverage/builder/lcov.info --cov-report=term + - name: Builder Python coverage + if: ${{ !cancelled() && (steps.python-builder.outcome == 'success' || steps.python-builder.outcome == 'failure') }} + run: | + python3 tools/coverage_report.py --scope python --lcov .artifacts/coverage/builder/lcov.info --output .artifacts/coverage/builder --tool coverage.py=7.16.1 --tool pytest-cov=7.1.0 + cat .artifacts/coverage/builder/summary.md >> "$GITHUB_STEP_SUMMARY" + - name: Upload builder Python coverage + if: ${{ !cancelled() }} + uses: actions/upload-artifact@v4 + with: + name: coverage-builder-${{ github.run_attempt }} + path: .artifacts/coverage/builder/ + if-no-files-found: error + - name: Upload builder Python test results + if: ${{ !cancelled() && (steps.python-builder.outcome == 'success' || steps.python-builder.outcome == 'failure') }} + uses: actions/upload-artifact@v4 + with: + name: python-builder-${{ github.run_attempt }} + path: .artifacts/python/builder.xml + if-no-files-found: error + embedded: needs: selection if: contains(fromJSON(needs.selection.outputs.jobs), 'embedded') @@ -1230,7 +1238,7 @@ jobs: # publishes no plan, which fails this gate rather than silently passing. ci: if: always() - needs: [selection, retired-map-stack, card-scheduler-guard, render-key-guard, catalog-ownership-guard, one-home-guard, screen-vocabulary-guard, fixture-registry, fmt, clippy, test, embedded, boot, device, deny, wasm, wasm-bridges, docs, ios-unit, ios-app, web, desktop-frontend, desktop, desktop-launch] + needs: [selection, guards, fmt, clippy, test, ui-snapshots, builder-python, embedded, boot, device, deny, wasm, wasm-bridges, docs, ios-unit, ios-app, web, desktop-frontend, desktop, desktop-launch] runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 diff --git a/tools/justfile b/tools/justfile index 5dfd1c2e1..d087d2d66 100644 --- a/tools/justfile +++ b/tools/justfile @@ -685,9 +685,10 @@ check *args: if want test; then rm -rf .artifacts/python/repository-tools step "fixture registry unit tests" python3 -m xmlrunner discover -s tools/tests -v -o .artifacts/python/repository-tools - step "test (fast hermetic binaries)" cargo nextest run --workspace --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fast)" - step "doctests (workspace)" cargo test --workspace --all-features --locked --doc - step "bench golden gate (pixels + read counters)" cargo run -p obc-bench --release --locked -- --check host/obc-bench/golden.txt + step "test (fast hermetic binaries)" bash tools/ci/test.sh nextest-fast + step "doctests (selected packages)" bash tools/ci/test.sh doctests + step "obc-formats (default features = the device's shape)" bash tools/ci/test.sh formats-default + step "bench golden gate (pixels + read counters)" bash tools/ci/test.sh bench-golden fi want device && step "device build (obc-app @ thumbv8m)" \ cargo build -p obc-app --target thumbv8m.main-none-eabihf --locked diff --git a/tools/tests/test_suite_registry.py b/tools/tests/test_suite_registry.py index dc0b93a17..d7fc117ab 100644 --- a/tools/tests/test_suite_registry.py +++ b/tools/tests/test_suite_registry.py @@ -799,9 +799,13 @@ def test_every_suite_routes_to_the_job_that_executes_it(self) -> None: "swift.obckit-host": ["ios-unit"], "ci.docs": ["docs"], "web.builder-vitest": ["web"], - "python.repository-tools": ["fixture-registry", "selection"], + "python.repository-tools": ["guards", "selection"], "python.firmware-tools": ["test"], - "python.builder": ["test"], + "python.builder": ["builder-python"], + "ci.ui-snapshots": ["ui-snapshots"], + # The sweep job compiles the simulator, so it is a route for the simulator's own + # suite as well: a job that builds a package runs whenever that package is selected. + "rust.obc-sim": ["clippy", "fmt", "test", "ui-snapshots"], "web.demo-browser": ["wasm"], } for suite_id, jobs in expected.items(): @@ -828,7 +832,7 @@ def test_selected_job_set_per_change_class(self) -> None: ( "foundational Rust crate", ["firmware/obc-crc/src/lib.rs"], - ["boot", "clippy", "desktop", "desktop-frontend", "device", "embedded", "fmt", "test", "wasm", "wasm-bridges"], + ["boot", "builder-python", "clippy", "desktop", "desktop-frontend", "device", "embedded", "fmt", "test", "ui-snapshots", "wasm", "wasm-bridges"], ), ( "shared vectors", @@ -849,12 +853,12 @@ def test_selected_job_set_per_change_class(self) -> None: ( "workflow", [".github/workflows/ci.yml"], - ["boot", "clippy", "deny", "desktop", "desktop-frontend", "desktop-launch", "device", "docs", "embedded", "fmt", "ios-app", "ios-unit", "test", "wasm", "wasm-bridges", "web"], + ["boot", "builder-python", "clippy", "deny", "desktop", "desktop-frontend", "desktop-launch", "device", "docs", "embedded", "fmt", "ios-app", "ios-unit", "test", "ui-snapshots", "wasm", "wasm-bridges", "web"], ), ( "nextest configuration", [".config/nextest.toml"], - ["boot", "clippy", "deny", "desktop", "desktop-frontend", "desktop-launch", "device", "docs", "embedded", "fmt", "ios-app", "ios-unit", "test", "wasm", "wasm-bridges", "web"], + ["boot", "builder-python", "clippy", "deny", "desktop", "desktop-frontend", "desktop-launch", "device", "docs", "embedded", "fmt", "ios-app", "ios-unit", "test", "ui-snapshots", "wasm", "wasm-bridges", "web"], ), # The web demo is built only by `trunk build`, the OBCKit package is compiled into the # app only by `xcodebuild`, and tools/fixtures.py is run only by a workflow step. From 2522a816ee6475a9298e836e5cb249ed5327c1b8 Mon Sep 17 00:00:00 2001 From: timohueser Date: Wed, 16 Sep 2026 11:33:03 +0200 Subject: [PATCH 4/7] test: run focused scopes on nextest and move the Copernicus credit `obc test -p PKG` ran Cargo's harness while CI ran nextest, so a developer and CI could disagree about a merged target. The focused arm now runs nextest and then the same scope's doctests. `obc test -p PKG --lib` is the library-only loop: one target, no doctests, and Cargo itself rejects a package with no library. `COPERNICUS_ATTRIBUTION` and `SOURCE_DATASET` move from the host DEM tool to the `obc-elevation` leaf every consumer already depends on. The application drops its `obc-dem` dev-dependency, which existed only so one unit test could read the wording. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.lock | 2 +- firmware/README.md | 2 +- firmware/obc-app/Cargo.toml | 5 --- firmware/obc-app/src/screen/settings/about.rs | 9 +++-- firmware/obc-elevation/src/lib.rs | 33 +++++++++++++++++++ host/obc-bake/Cargo.toml | 3 ++ host/obc-bake/assets/README.md | 2 +- host/obc-bake/src/main.rs | 8 ++--- host/obc-bake/src/terrain.rs | 2 +- host/obc-bake/tests/cell_bakery.rs | 8 +++-- host/obc-dem/src/lib.rs | 33 ++----------------- host/obc-dem/src/main.rs | 3 +- host/obc-pack/src/catalog/terrain.rs | 2 +- host/obc-pack/src/contour.rs | 6 +++- tools/justfile | 15 ++++++++- 15 files changed, 79 insertions(+), 54 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d324fd42b..64a0df58c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2663,7 +2663,6 @@ dependencies = [ "embedded-graphics", "heapless 0.8.0", "libm", - "obc-dem", "obc-elevation", "obc-formats", "obc-map-scene", @@ -2682,6 +2681,7 @@ dependencies = [ "heapless 0.8.0", "image", "obc-dem", + "obc-elevation", "obc-formats", "obc-map-scene", "obc-pack", diff --git a/firmware/README.md b/firmware/README.md index e2954c5cc..dbaa9c2f5 100644 --- a/firmware/README.md +++ b/firmware/README.md @@ -326,7 +326,7 @@ contract**, pinned by a digest test. A source void becomes `NODATA`, an uncovere lattice point becomes `NODATA`, and nothing is ever inpainted. Anything derived from GLO-30 must carry the Copernicus credit; `bake` prints it, -and `obc_dem::COPERNICUS_ATTRIBUTION` is its single copy in the repo. +and `obc_elevation::COPERNICUS_ATTRIBUTION` is its single copy in the repo. The two committed terrain sidecars are regenerated by [`fixtures/build-map-package.sh terrain`](../fixtures/build-map-package.sh), the diff --git a/firmware/obc-app/Cargo.toml b/firmware/obc-app/Cargo.toml index 5805dfa1c..80a513938 100644 --- a/firmware/obc-app/Cargo.toml +++ b/firmware/obc-app/Cargo.toml @@ -34,8 +34,3 @@ libm = { version = "0.2", default-features = false } # no arch asm: identical co # The hand-written OBCM v6 byte builder shared by the reader/renderer tests — the POIs-browser # integration tests (#425) use `build_poi_map` to synthesise a v6 map with real POIs to snapshot. obcm-testkit = { path = "../../host/obcm-testkit" } -# The About page (#1149) hand-wraps the Copernicus attribution into on-glass lines; the parity -# test re-joins them and compares against `obc_dem::COPERNICUS_ATTRIBUTION` — the one copy of the -# wording in the repository — so the device text cannot drift from the host's. Dev-only: the -# `no_std` device build never sees the host crate. -obc-dem = { path = "../../host/obc-dem" } diff --git a/firmware/obc-app/src/screen/settings/about.rs b/firmware/obc-app/src/screen/settings/about.rs index aa083cc04..fb4b1d2ff 100644 --- a/firmware/obc-app/src/screen/settings/about.rs +++ b/firmware/obc-app/src/screen/settings/about.rs @@ -13,9 +13,8 @@ //! The copy is **hand-wrapped constant lines**, not runtime-wrapped text: the legal formulas are //! fixed strings, so pre-wrapping makes the exact on-glass layout reviewable in the source, and //! the tests below enforce the two properties that matter — every line fits the panel width -//! (the never-ellipsize rule), and the Copernicus lines re-join to `obc_dem`'s canonical -//! [`COPERNICUS_ATTRIBUTION`] word for word (a dev-dependency, so the device build never sees the -//! host crate). +//! (the never-ellipsize rule), and the Copernicus lines re-join to `obc_elevation`'s canonical +//! [`COPERNICUS_ATTRIBUTION`] word for word. //! //! The page is taller than the panel, so it **scrolls by line**: Rotate moves the window, Back //! climbs out, and a right-edge scrollbar shows where you are. Press does nothing — there is @@ -53,7 +52,7 @@ const LINE_CHARS: usize = 17; const OSM_LINES: &[&str] = &["\u{00a9} OpenStreetMap", "contributors", "Open Database", "License (ODbL)", "openstreetmap", ".org/copyright"]; -/// `obc_dem::COPERNICUS_ATTRIBUTION`, pre-wrapped. The parity test re-joins these with single +/// `obc_elevation::COPERNICUS_ATTRIBUTION`, pre-wrapped. The parity test re-joins these with single /// spaces and compares against the host crate's const, so the wording cannot drift and the wraps /// can only fall on word boundaries. const COPERNICUS_LINES: &[&str] = &[ @@ -208,7 +207,7 @@ mod tests { } joined.push_str(line); } - assert_eq!(joined, obc_dem::COPERNICUS_ATTRIBUTION); + assert_eq!(joined, obc_elevation::COPERNICUS_ATTRIBUTION); } /// Every pre-wrapped content line fits the Label-width budget, and every caption does so in diff --git a/firmware/obc-elevation/src/lib.rs b/firmware/obc-elevation/src/lib.rs index 554e53af5..311132d09 100644 --- a/firmware/obc-elevation/src/lib.rs +++ b/firmware/obc-elevation/src/lib.rs @@ -43,7 +43,40 @@ pub use integrator::ProfileIntegrator; pub use reader::{TerrainHeader, TerrainReader, TerrainTables}; pub use source::{ElevationSource, NullElevation, TerrainElevation}; +/// The credit the Copernicus DEM licence requires on any product derived from the dataset, verbatim. +/// +/// The licence ("Copernicus DEM Instance COP-DEM-GLO-30-F") requires this exact notice wherever the +/// data have been adapted or modified — which a resample to a different lattice certainly is. It is +/// not a courtesy and it is not paraphrasable: the baker stamps it into the catalog, the builder +/// surfaces it in the browser, and the About page shows it on glass. It lives in this leaf crate +/// because every one of those consumers already depends on it, and none of them should have to +/// take a host DEM tool just to read a string. +pub const COPERNICUS_ATTRIBUTION: &str = "produced using Copernicus WorldDEM-30 © DLR e.V. 2010-2014 \ +and © Airbus Defence and Space GmbH 2014-2018 provided under COPERNICUS by the European Union and \ +ESA; all rights reserved"; + +/// The dataset the terrain tiles are derived from, as the catalog names it. +pub const SOURCE_DATASET: &str = "Copernicus DEM GLO-30"; + /// The v1 tile-cache depth: four 512 B tiles ≈ 2.1 KB. Four because a single bilinear query can /// straddle a tile corner and touch exactly four tiles — anything less would thrash on the one /// access pattern the sampler is guaranteed to make. pub const DEFAULT_TILE_SLOTS: usize = 4; + +#[cfg(test)] +mod tests { + use super::COPERNICUS_ATTRIBUTION; + + #[test] + fn the_attribution_is_the_wording_the_licence_names() { + // Pinned as one line: the `const` is written with continuations, and a stray newline in it + // would travel into the catalog and the builder. + assert_eq!( + COPERNICUS_ATTRIBUTION, + "produced using Copernicus WorldDEM-30 © DLR e.V. 2010-2014 and © Airbus Defence and \ + Space GmbH 2014-2018 provided under COPERNICUS by the European Union and ESA; all \ + rights reserved" + ); + assert!(!COPERNICUS_ATTRIBUTION.contains('\n')); + } +} diff --git a/host/obc-bake/Cargo.toml b/host/obc-bake/Cargo.toml index bfb4b8fc7..5b5aeeb71 100644 --- a/host/obc-bake/Cargo.toml +++ b/host/obc-bake/Cargo.toml @@ -31,6 +31,9 @@ obc-formats = { path = "../../firmware/obc-formats" } # into this tree. Declared here rather than inherited by workspace feature unification, so a # standalone `cargo check -p obc-bake` matches what the workspace builds. obc-dem = { path = "../obc-dem", default-features = false, features = ["geotiff", "fetch"] } +# The Copernicus credit the terrain stage stamps into every catalog entry. It is a `const` in the +# elevation leaf, which this crate reads through rather than copying the wording. +obc-elevation = { path = "../../firmware/obc-elevation" } # Skin previews are not screenshots of a second renderer. The bakery restamps a # canonical tiny OBCM through the assembly engine's production skin resolver and # renders it through the same `RenderScratch` the device runs. diff --git a/host/obc-bake/assets/README.md b/host/obc-bake/assets/README.md index 92417d77b..26464e1b7 100644 --- a/host/obc-bake/assets/README.md +++ b/host/obc-bake/assets/README.md @@ -89,7 +89,7 @@ parses and covers the published camera centre. carry *"produced using Copernicus WorldDEM-30 © DLR e.V. 2010-2014 and © Airbus Defence and Space GmbH 2014-2018 provided under COPERNICUS by the European Union and ESA; all rights reserved"*. The string lives once, in -`obc_dem::COPERNICUS_ATTRIBUTION`. +`obc_elevation::COPERNICUS_ATTRIBUTION`. Refresh this fixture whenever the schema's style-id assignment or OBCM version changes. `obc-bake` checks the assignment before starting a region bake and diff --git a/host/obc-bake/src/main.rs b/host/obc-bake/src/main.rs index 9bc5cb4b9..b1b9017c0 100644 --- a/host/obc-bake/src/main.rs +++ b/host/obc-bake/src/main.rs @@ -275,7 +275,7 @@ fn run_cell_bake( posting_log2: obc_dem::bake::V1_POSTING_LOG2, cell_log2: obc_dem::bake::V1_CELL_LOG2, revision: 1, - attribution: obc_dem::COPERNICUS_ATTRIBUTION.to_string(), + attribution: obc_elevation::COPERNICUS_ATTRIBUTION.to_string(), } }; let sources = match flags.get("dem-sources") { @@ -293,7 +293,7 @@ fn run_cell_bake( .run(&obc_pack::progress::Progress::stdout())?; print!("{}", summary.render()); // The credit is a licence obligation, printed wherever the dataset was used. - println!("{}\n", obc_dem::COPERNICUS_ATTRIBUTION); + println!("{}\n", obc_elevation::COPERNICUS_ATTRIBUTION); } let cutter = obc_bake::cells::ObcCutter { @@ -529,7 +529,7 @@ fn run_terrain(args: &[String]) -> Result<(), String> { revision: number("terrain-revision", 1)?, // The credit is a licence obligation and is never retyped here: it comes from the one // `const` in `obc-dem`, travels into the catalog, and a consumer reads it from there. - attribution: obc_dem::COPERNICUS_ATTRIBUTION.to_string(), + attribution: obc_elevation::COPERNICUS_ATTRIBUTION.to_string(), }; let cache = flags.get("cache").map(PathBuf::from).unwrap_or_else(default_cache_dir); @@ -564,7 +564,7 @@ fn run_terrain(args: &[String]) -> Result<(), String> { }; // Unconditional, and before the `?`: the credit is a licence obligation of the data that was just // written, so it cannot be something only a fully successful catalog pass gets to print. - println!("\n{}", obc_dem::COPERNICUS_ATTRIBUTION); + println!("\n{}", obc_elevation::COPERNICUS_ATTRIBUTION); finished } diff --git a/host/obc-bake/src/terrain.rs b/host/obc-bake/src/terrain.rs index dc12dd994..bf87c2c2f 100644 --- a/host/obc-bake/src/terrain.rs +++ b/host/obc-bake/src/terrain.rs @@ -671,7 +671,7 @@ mod tests { posting_log2: 9, cell_log2: 19, revision: 1, - attribution: obc_dem::COPERNICUS_ATTRIBUTION.into(), + attribution: obc_elevation::COPERNICUS_ATTRIBUTION.into(), } } diff --git a/host/obc-bake/tests/cell_bakery.rs b/host/obc-bake/tests/cell_bakery.rs index 8099adef1..c53f1d524 100644 --- a/host/obc-bake/tests/cell_bakery.rs +++ b/host/obc-bake/tests/cell_bakery.rs @@ -427,7 +427,7 @@ fn terrain_doc(revision: u32, dataset_version: &str) -> TerrainDoc { revision, // The credit comes from `obc-dem`'s own `const` and is never retyped, here or anywhere: // this assertion is the whole reason the bakery reaches for the library rather than a CLI. - attribution: obc_dem::COPERNICUS_ATTRIBUTION.into(), + attribution: obc_elevation::COPERNICUS_ATTRIBUTION.into(), } } @@ -527,7 +527,11 @@ fn a_terrain_bake_publishes_cells_ocean_runs_and_a_priced_region_selection() { assert_eq!(terrain.terrain_revision, 1); assert_eq!(terrain.dataset_id, "copernicus-glo-30"); assert_eq!((terrain.posting_log2, terrain.cell_log2), (TERRAIN_POSTING_LOG2, TERRAIN_CELL_LOG2)); - assert_eq!(terrain.attribution, obc_dem::COPERNICUS_ATTRIBUTION, "§13.5: the credit comes from obc-dem's const"); + assert_eq!( + terrain.attribution, + obc_elevation::COPERNICUS_ATTRIBUTION, + "§13.5: the credit comes from obc-elevation's const" + ); assert_eq!((terrain.cell_index.cell_count, terrain.cell_index.known_empty_count), (5, 1)); assert!(terrain.cell_index.url.contains(&terrain.cell_index.sha256), "the index is addressed by its own digest"); diff --git a/host/obc-dem/src/lib.rs b/host/obc-dem/src/lib.rs index 4b3b71e91..ac6c2d8ec 100644 --- a/host/obc-dem/src/lib.rs +++ b/host/obc-dem/src/lib.rs @@ -46,9 +46,9 @@ //! //! ## Attribution is a licence obligation //! -//! [`COPERNICUS_ATTRIBUTION`] must travel with anything derived from GLO-30. `bake` prints it, and -//! the catalog (EL3) and the builder (EL4) carry it onward to a rider. It is a `const` here so -//! there is one copy of the wording in the repository. +//! `obc_elevation::COPERNICUS_ATTRIBUTION` must travel with anything derived from GLO-30. `bake` +//! prints it, and the catalog and the builder carry it onward to a rider. It is a `const` in the +//! elevation leaf so there is one copy of the wording in the repository. /// The producer half — a GeoTIFF decoder (`geotiff`) and an HTTP client (`fetch`), together the /// default `dem` feature. [`container`] stands without either, so the assembler (EL4) can reuse the @@ -62,20 +62,6 @@ pub mod fetch; pub mod geotiff; pub mod surface; -/// The credit the Copernicus DEM licence requires on any product derived from the dataset, verbatim. -/// -/// The licence ("Copernicus DEM Instance COP-DEM-GLO-30-F") requires this exact notice wherever the -/// data have been adapted or modified — which a resample to a different lattice certainly is. It is -/// not a courtesy and it is not paraphrasable: EL3 stamps it into the catalog, EL4 surfaces it in -/// the builder, and `obc-dem bake` prints it at the end of every run so an operator producing cells -/// cannot fail to have seen it. -pub const COPERNICUS_ATTRIBUTION: &str = "produced using Copernicus WorldDEM-30 © DLR e.V. 2010-2014 \ -and © Airbus Defence and Space GmbH 2014-2018 provided under COPERNICUS by the European Union and \ -ESA; all rights reserved"; - -/// The dataset this tool is built for, as the catalog will name it (EL3). -pub const SOURCE_DATASET: &str = "Copernicus DEM GLO-30"; - /// A geographic box in integer microdegrees — the unit every OBC coordinate is in, so the box that /// selects cells is exact rather than a float that nearly lands on a cell boundary. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -125,19 +111,6 @@ impl BboxUdeg { mod tests { use super::*; - #[test] - fn the_attribution_is_the_wording_the_licence_names() { - // Pinned as one line: the `const` is written with continuations, and a stray newline in it - // would travel into the catalog and the builder. - assert_eq!( - COPERNICUS_ATTRIBUTION, - "produced using Copernicus WorldDEM-30 © DLR e.V. 2010-2014 and © Airbus Defence and \ - Space GmbH 2014-2018 provided under COPERNICUS by the European Union and ESA; all \ - rights reserved" - ); - assert!(!COPERNICUS_ATTRIBUTION.contains('\n')); - } - #[test] fn a_bbox_is_latitude_first_and_exact_in_microdegrees() { let b = BboxUdeg::parse("46.48261,8.15034,46.72070,8.46007").unwrap(); diff --git a/host/obc-dem/src/main.rs b/host/obc-dem/src/main.rs index 50c3a9ddc..a52d54a11 100644 --- a/host/obc-dem/src/main.rs +++ b/host/obc-dem/src/main.rs @@ -10,7 +10,8 @@ use std::process::ExitCode; use obc_dem::bake::{bake_cells, bake_shard, BakeParams, BakeReport, V1_CELL_LOG2, V1_POSTING_LOG2}; use obc_dem::fetch::{fetch_tiles, Fetched}; use obc_dem::geotiff::DemMosaic; -use obc_dem::{BboxUdeg, COPERNICUS_ATTRIBUTION, SOURCE_DATASET}; +use obc_dem::BboxUdeg; +use obc_elevation::{COPERNICUS_ATTRIBUTION, SOURCE_DATASET}; fn main() -> ExitCode { let args: Vec = std::env::args().skip(1).collect(); diff --git a/host/obc-pack/src/catalog/terrain.rs b/host/obc-pack/src/catalog/terrain.rs index 326527155..d5d7e9492 100644 --- a/host/obc-pack/src/catalog/terrain.rs +++ b/host/obc-pack/src/catalog/terrain.rs @@ -35,7 +35,7 @@ pub(super) struct TerrainDoc { /// The terrain store's own revision. Nothing here is `schema_revision`. pub(super) revision: u32, /// The source licence's required credit, verbatim. The bakery stamps - /// `obc_dem::COPERNICUS_ATTRIBUTION` here; this crate never hard-codes it, because + /// `obc_elevation::COPERNICUS_ATTRIBUTION` here; this crate never hard-codes it, because /// a generic producer publishing another dataset owes a different notice. pub(super) attribution: String, } diff --git a/host/obc-pack/src/contour.rs b/host/obc-pack/src/contour.rs index 5b3d0b82e..dae031e33 100644 --- a/host/obc-pack/src/contour.rs +++ b/host/obc-pack/src/contour.rs @@ -169,7 +169,11 @@ pub(crate) fn add_contours( )); // The credit is a licence obligation and travels with the data, never retyped: it is one `const` // in `obc-dem`, and this is the point at which a `.obcm` starts carrying GLO-30-derived geometry. - progress.log(format!(" contours derived from {}: {}", obc_dem::SOURCE_DATASET, obc_dem::COPERNICUS_ATTRIBUTION)); + progress.log(format!( + " contours derived from {}: {}", + obc_elevation::SOURCE_DATASET, + obc_elevation::COPERNICUS_ATTRIBUTION + )); Ok(()) } diff --git a/tools/justfile b/tools/justfile index d087d2d66..c828e061c 100644 --- a/tools/justfile +++ b/tools/justfile @@ -533,6 +533,8 @@ build: cd "$OBC_ROOT"; _run cargo build --release # Run focused host tests by default. A scope is required: `obc test -p obc-pack`. +# The focused arm runs nextest and then the same scope's doctests; `--lib` runs the library +# target alone and no doctests. # The registry selectors run whole suites and print why each one was selected: # # obc test affected --base origin/develop [--head REF] [--dry-run] @@ -565,6 +567,13 @@ test *args: done return 1 } + library_only() { + local arg + for arg in "$@"; do + case "$arg" in --lib) return 0 ;; esac + done + return 1 + } fixture_env() { _run python3 "$OBC_TOOLS/fixtures.py" sync test export OBC_FIXTURE_ROOT @@ -601,7 +610,11 @@ test *args: _hint "example: obc test -p obc-app" exit 2 } - _run cargo test --locked "$@" + _run cargo nextest run --locked "$@" + # `--lib` is the library-only loop: nextest builds one target and doctests, which are a + # separate Cargo run over the whole package, stay out of it. Cargo rejects `--lib` on a + # package with no library, so an unbuildable scope fails before anything runs. + library_only "$@" || _run cargo test --locked --doc "$@" ;; esac From a6d51b8c56610aebba19cebe0e0c2a7e9f29e2ee Mon Sep 17 00:00:00 2001 From: timohueser Date: Wed, 16 Sep 2026 11:38:52 +0200 Subject: [PATCH 5/7] docs: describe the routes, the package-scoped test job and the new jobs Names the one explicit route for each of the three ignored tests and the five manual commands, and says there is no expensive tier. Records that bounded fixture suites are ordinary work that fails with the sync command, that the `test` job compiles the packages the plan selected, that the sweep is its own job with a wider job-level trigger than its suite, and that a focused `obc test -p` runs on nextest with a `--lib` form. Co-Authored-By: Claude Opus 5 (1M context) --- CONTRIBUTING.md | 8 +++++--- docs/testing.md | 38 +++++++++++++++++++++++++++++++++----- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8683d94db..df45fad5a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -84,9 +84,11 @@ being opened, or another agent also working in the repository is not by itself a Concurrent full runs are allowed; the discipline is to start one only when its coverage is needed. `obc test full` runs fast binaries, captured-fixture binaries and doctests in separate commands. -`obc check test` runs the fast tier, doctests and the render contract. These workspace commands -need the CI-pinned runner (`cargo install cargo-nextest --version 0.9.143 --locked`). Focused -`obc test -p ` commands continue to use Cargo's test harness. +`obc check test` runs `tools/ci/test.sh`, the same file the CI `test` job runs: the fast tier, +doctests, the default-feature formats shape and the render contract. Every command here, focused +ones included, needs the CI-pinned runner (`cargo install cargo-nextest --version 0.9.143 +--locked`). `obc test -p ` runs that scope on nextest and then its doctests; add `--lib` +for the library target alone. ## Reclaiming stale development state diff --git a/docs/testing.md b/docs/testing.md index d05b9ef11..d8ed05b3a 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -60,6 +60,20 @@ Vector and assembly writers are Cargo examples. They run only through the manual `manual.obc-display` runs the row-hash timing probe. These commands can write fixtures or print measurements; they are not required test passes. +There is no expensive tier. Every test that is not on the ordinary route has one explicit route, +and this table is the complete list: + +| Item | Route | +| --- | --- | +| `host/obc-dem` `decode` and `real_tile` (ignored, live Copernicus download) | `live.copernicus`, explicit command only | +| `host/obc-pack` `assistant_places` (ignored, pinned 549 MB source) | `fixtures.assistant-places`, through `fixtures/verify-assistant-places.py` | +| `manual.obc-display`, `manual.obc-vectors`, `manual.obc-link`, `manual.obc-web-assemble` | their own explicit commands; a generator writes, it never verifies | +| `ios.application-weekly`, the weekly `rust.obc-storage` run | `test-weekly.yml` | + +A captured fixture is not on this list. Bounded fixture suites are ordinary work: they run in CI +after an explicit sync, and a missing package fails with the exact `obc fixtures sync` command +rather than skipping. No environment variable turns that failure on. + The required iOS suite owns `WebsiteScreenshotTests.swift`. The separate weekly application suite owns the other XCUITest classes. `test-weekly.yml` runs the registered weekly iOS and storage suites each Monday and on manual dispatch. Run the same cadence locally with: @@ -88,9 +102,11 @@ fails the upload; setup failure can leave no bundle. The screenshot script also `OBC_XCRESULT_PATH` for local retention. A workflow declaration alone does not establish a passing run. TS6 still owns the remaining critical application journeys. -The UI snapshot sweep runs only when its registry input triggers match the change. A broad -coverage or workflow change does not cause an unrelated sweep. Its existing rendering, screen -and snapshot-input triggers remain the single selection source. +The UI snapshot sweep is its own `ui-snapshots` job, off the `test` job's serial path. Its +`ci.ui-snapshots` suite still selects only on its own rendering, screen and snapshot-input +triggers, and a broad coverage or policy change does not select it. The job itself also starts +whenever the simulator is selected, because the job compiles the simulator and a job that +compiles a package is a route for it — the same rule that starts `clippy` and `fmt`. ## Suite registry fields @@ -154,7 +170,7 @@ unmeasured files. Unmeasured informational source stays visible and does not bec | Maintained invocation | Collector | Coverage artifact | | --- | --- | --- | -| Workspace nextest | cargo-llvm-cov 0.9.1, current Rust LLVM | `coverage-rust-ATTEMPT` | +| Selected-package nextest | cargo-llvm-cov 0.9.1, current Rust LLVM | `coverage-rust-ATTEMPT` | | Repository and firmware Python | coverage.py 7.16.1 around xmlrunner | `coverage-repository-tools-ATTEMPT`, `coverage-firmware-tools-ATTEMPT` | | Builder Python | pytest-cov 7.1.0, coverage.py 7.16.1 | `coverage-builder-ATTEMPT` | | Builder Vitest | Vitest and V8 provider 3.2.6 | `coverage-web-ATTEMPT` | @@ -217,6 +233,7 @@ selected suites without an executable CI route are errors; a selection error nev obc test affected --base origin/develop [--head REF] [--dry-run] obc test unit|component|contract|fixtures|e2e [--surface NAME] [--dry-run] obc test -p obc-app # focused package work, no registry involved +obc test -p obc-app --lib # the library target alone, no doctests obc test fixtures -p obc-route obc test full # cross-cutting changes only ``` @@ -229,6 +246,10 @@ current host is reported as skipped with that restriction, never as passed. `obc keeps its scoped meaning whenever a Cargo scope is present, and that path needs neither the registry, Git, nor Cargo metadata. +A focused `obc test -p PACKAGE` runs that scope on nextest, the runner CI uses, and then the +same scope's doctests. `--lib` runs the library test target alone and no doctests; Cargo rejects +it on a package with no library. + ### Reproducing CI gates locally `obc check ` runs the primitive commands of the named gates and prints the registry suites @@ -238,6 +259,12 @@ request that the run did not reproduce, with the reason. It makes no unqualified ## Rust CI result artifacts +`tools/ci/test.sh` is the body of the `test` job, one section per CI step, and `obc check test` +runs the same file. Each section asks `tools/ci/rust_packages.py` which packages its tier +selected and compiles those with `-p`. A change to the root manifest, the lockfile, the +toolchain, the registry or the workflow compiles the whole workspace instead. A tier that +selected no package prints that and runs nothing; it never falls back to the workspace. + The two `cargo nextest run` commands in `test` use `NEXTEST_PROFILE=ci` for fast binaries and `NEXTEST_PROFILE=fixtures` for captured fixtures. The profiles in `.config/nextest.toml` write native JUnit XML to `target/nextest/ci/junit.xml` and `target/nextest/fixtures/junit.xml`. Each test case retains its binary and test identity, result, and @@ -257,7 +284,8 @@ Each command removes an old report before it starts. A skipped job uploads nothi fixture setup failure remains a failure and may produce no report; the upload step reports a missing file as an error. CI does not create an empty report or run the tests again for reporting. -These JUnit artifacts cover nextest invocations. Workspace doctests and the default-feature +These JUnit artifacts cover nextest invocations. A tier that selected no package writes no +report and uploads none. Doctests for the selected packages and the default-feature `obc-formats` Cargo command retain their native text in `coverage-rust-ATTEMPT` (`doctests.log` and `formats-default.log`). Those logs contain native identities and outcomes, with aggregate durations; Cargo does not emit individual case durations for these commands. No synthetic durations or passes From e10eeca09885409a3fa318a01e82bd19d5467a28 Mon Sep 17 00:00:00 2001 From: timohueser Date: Wed, 16 Sep 2026 12:19:12 +0200 Subject: [PATCH 6/7] ci: keep the test job on the whole workspace for the coverage ratchet The per-pull-request ratchet reads one LCOV report over the whole workspace and fails any critical file it never compiled, so compiling only the selected packages made `tools/coverage_report.py` report the five critical components as unmeasured and fail the job. A leaf-crate plan omits obc-link and obc-storage outright. Both nextest sections and the doctest section go back to `--workspace --all-features` with the tier filter, `tools/ci/rust_packages.py` is deleted, and the registry's unexpanded-variable accommodation goes with it. Reading a `tools/ci/*.sh` script as a job body stays: the commands live there. llvm-cov instrumentation is now on the two nextest steps only. It was job-wide, so the doctests, the default-feature formats run and the bench golden gate were instrumented too, and the bench release binary's profile data would have merged into the ratchet's evidence. Each section also runs in a subshell, so one section's environment cannot reach the next. The sweep step carries its own `ci.ui-snapshots` condition again. The job still gates on its own name, so routing, `validate-filters` and the aggregate are unchanged, but the sweep itself runs only for the suite that owns the rendering triggers. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 24 ++++-- firmware/obc-app/src/screen/settings/about.rs | 8 +- host/obc-bake/src/main.rs | 2 +- host/obc-pack/src/contour.rs | 2 +- testing/suites.toml | 6 +- tools/ci/rust_packages.py | 77 ------------------- tools/ci/test.sh | 61 +++++++-------- tools/suite_registry.py | 4 +- 8 files changed, 53 insertions(+), 131 deletions(-) delete mode 100644 tools/ci/rust_packages.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ff3a8c4b4..112718ca5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -212,8 +212,6 @@ jobs: test: env: CARGO_LLVM_COV_TARGET_DIR: ${{ github.workspace }}/target - OBC_COVERAGE: "1" - OBC_SELECTION_PLAN: ${{ needs.selection.outputs.plan }} needs: selection if: contains(fromJSON(needs.selection.outputs.jobs), 'test') runs-on: ubuntu-latest @@ -248,16 +246,21 @@ jobs: with: path: ~/.cache/openbikecomputer/fixtures key: obc-fixtures-${{ hashFiles('fixtures/catalog.toml') }} - # Each section is one step so it keeps its own name and result artifact; the file is - # the same one `obc check test` runs. The package set comes from the selection plan, - # so a three-crate change compiles three crates and not the workspace. + # Each section is one step so it keeps its own step name and result artifact; the file + # is the same one `obc check test` runs. Only the two nextest steps are instrumented: + # their lcov is what the coverage ratchet reads, and the bench gate's release binary + # would otherwise merge its profile data into that evidence. - name: cargo nextest (fast hermetic binaries) id: nextest + env: + OBC_COVERAGE: "1" run: bash tools/ci/test.sh nextest-fast - name: cargo nextest (captured fixture binaries) id: nextest-fixtures + env: + OBC_COVERAGE: "1" run: bash tools/ci/test.sh nextest-fixtures - - name: cargo test --doc (selected packages, all features) + - name: cargo test --doc (workspace, all features) run: bash tools/ci/test.sh doctests - name: cargo test (obc-formats, default features = the device's shape) run: bash tools/ci/test.sh formats-default @@ -316,14 +319,14 @@ jobs: .artifacts/rust/ if-no-files-found: error - name: Upload Rust test results - if: ${{ !cancelled() && steps.nextest.outputs.report == 'true' }} + if: ${{ !cancelled() && (steps.nextest.outcome == 'success' || steps.nextest.outcome == 'failure') }} uses: actions/upload-artifact@v4 with: name: rust-test-${{ github.run_attempt }} path: target/nextest/ci/junit.xml if-no-files-found: error - name: Upload Rust fixture test results - if: ${{ !cancelled() && steps.nextest-fixtures.outputs.report == 'true' }} + if: ${{ !cancelled() && (steps.nextest-fixtures.outcome == 'success' || steps.nextest-fixtures.outcome == 'failure') }} uses: actions/upload-artifact@v4 with: name: rust-fixtures-${{ github.run_attempt }} @@ -336,6 +339,10 @@ jobs: # refactor moves. It is its own job so the sweep is off the `test` job's serial path. The debug # binary is deliberate: its output is byte-identical to release. `update` is a developer command # and never runs here — a self-regenerating manifest records nothing. + # + # The job starts for every suite that routes to it, which includes each package it compiles. + # The sweep itself still runs only for `ci.ui-snapshots`, the suite that owns the rendering + # input triggers, so its selection budget is exactly what it was. ui-snapshots: needs: selection if: contains(fromJSON(needs.selection.outputs.jobs), 'ui-snapshots') @@ -360,6 +367,7 @@ jobs: key: obc-fixtures-${{ hashFiles('fixtures/catalog.toml') }} - name: ui-snapshots manifest check id: ui-snapshots + if: contains(fromJSON(needs.selection.outputs.plan).selected_suite_ids, 'ci.ui-snapshots') run: | cargo build -p obc-sim --locked SIM="$PWD/target/debug/obc-sim" firmware/ui-snapshots.sh "$RUNNER_TEMP/ui-snapshots" diff --git a/firmware/obc-app/src/screen/settings/about.rs b/firmware/obc-app/src/screen/settings/about.rs index fb4b1d2ff..43e2109c9 100644 --- a/firmware/obc-app/src/screen/settings/about.rs +++ b/firmware/obc-app/src/screen/settings/about.rs @@ -195,11 +195,11 @@ mod tests { assert!(matches!(run(&mut scr, Gesture::Back), Transition::Pop)); } - /// The Copernicus lines re-join to `obc_dem`'s canonical attribution word for word — the - /// single-copy-of-the-wording rule, held across the firmware/host boundary by a dev-dep the - /// device build never sees. If the wording ever changes in `obc-dem`, this fails here. + /// The Copernicus lines re-join to `obc_elevation`'s canonical attribution word for word — + /// the single-copy-of-the-wording rule. The credit lives in the elevation leaf this crate + /// already depends on, so the device text cannot drift from the one the bakery stamps. #[test] - fn copernicus_wording_matches_obc_dem() { + fn copernicus_wording_matches_obc_elevation() { let mut joined = std::string::String::new(); for (i, line) in COPERNICUS_LINES.iter().enumerate() { if i > 0 { diff --git a/host/obc-bake/src/main.rs b/host/obc-bake/src/main.rs index b1b9017c0..b7d4254fb 100644 --- a/host/obc-bake/src/main.rs +++ b/host/obc-bake/src/main.rs @@ -528,7 +528,7 @@ fn run_terrain(args: &[String]) -> Result<(), String> { cell_log2: log2("cell-log2", obc_dem::bake::V1_CELL_LOG2)?, revision: number("terrain-revision", 1)?, // The credit is a licence obligation and is never retyped here: it comes from the one - // `const` in `obc-dem`, travels into the catalog, and a consumer reads it from there. + // `const` in `obc-elevation`, travels into the catalog, and a consumer reads it from there. attribution: obc_elevation::COPERNICUS_ATTRIBUTION.to_string(), }; diff --git a/host/obc-pack/src/contour.rs b/host/obc-pack/src/contour.rs index dae031e33..038c21e3a 100644 --- a/host/obc-pack/src/contour.rs +++ b/host/obc-pack/src/contour.rs @@ -168,7 +168,7 @@ pub(crate) fn add_contours( cfg.simplify_m )); // The credit is a licence obligation and travels with the data, never retyped: it is one `const` - // in `obc-dem`, and this is the point at which a `.obcm` starts carrying GLO-30-derived geometry. + // in `obc-elevation`, and this is where a `.obcm` starts carrying GLO-30-derived geometry. progress.log(format!( " contours derived from {}: {}", obc_elevation::SOURCE_DATASET, diff --git a/testing/suites.toml b/testing/suites.toml index 049e99915..c1e6a2074 100644 --- a/testing/suites.toml +++ b/testing/suites.toml @@ -633,8 +633,8 @@ pull_request = "affected" scheduled = "none" extra_triggers = ["Cargo.toml", "Cargo.lock", "rust-toolchain.toml", "tools/fixtures.py"] ownership = [ - { kind = "workflow", pattern = 'cargo nextest run $packages --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fast)"' }, - { kind = "workflow", pattern = "cargo test $packages --all-features --locked --doc*" }, + { kind = "workflow", pattern = 'cargo nextest run --workspace --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fast)"' }, + { kind = "workflow", pattern = "cargo test --workspace --all-features --locked --doc*" }, { kind = "workflow", pattern = "python3 tools/coverage_report.py --scope rust *" }, { kind = "workflow", pattern = "cargo test -p obc-formats --locked*" }, { kind = "workflow", pattern = "cargo nextest run --release --locked --config-file ../../.config/nextest.toml --no-tests fail" }, @@ -1088,6 +1088,6 @@ pull_request = "affected" scheduled = "none" extra_triggers = ["fixtures/catalog.toml", "tools/fixtures.py"] ownership = [ - { kind = "workflow", pattern = 'cargo nextest run $packages --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fixtures)"' }, + { kind = "workflow", pattern = 'cargo nextest run --workspace --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fixtures)"' }, { kind = "workflow", pattern = "python3 tools/fixtures.py sync test" }, ] diff --git a/tools/ci/rust_packages.py b/tools/ci/rust_packages.py deleted file mode 100644 index afbdb20f6..000000000 --- a/tools/ci/rust_packages.py +++ /dev/null @@ -1,77 +0,0 @@ -#!/usr/bin/env python3 -"""Print the Cargo package flags one tier of the `test` job compiles. - -The selection plan already says which suites a change requires. This turns that -plan into `-p NAME` flags, so the job compiles the selected packages instead of -the whole workspace. It prints `--workspace` when the change can alter what any -package compiles to, and nothing at all when the tier selected no Rust package. -Without a plan (a local `obc check test`) the answer is the whole workspace. -""" - -from __future__ import annotations - -import argparse -import json -import os -import sys -from pathlib import Path - -sys.path.insert(0, str(Path(__file__).resolve().parents[1])) - -import suite_registry as registry # noqa: E402 - -# A change to one of these can change what every package compiles to, or which -# suites the selector itself produces, so no narrowed package set is trustworthy. -FOUNDATION = registry.RUST_FOUNDATION_PATHS | { - ".config/nextest.toml", - ".github/workflows/ci.yml", - "testing/suites.toml", - "tools/suite_registry.py", -} - - -def _is_foundation(path: str) -> bool: - return path in FOUNDATION or path.startswith("tools/ci/") - - -def packages(plan: dict, tier: str, root: Path) -> list[str]: - """The root-workspace packages whose binaries this tier's filter can name.""" - - inventory = registry.load_inventory(root) - graph = registry.build_cargo_graph(root) - levels = registry.cargo_tier_levels(tier) - selected = set(plan.get("selected_suite_ids", [])) - names: set[str] = set() - for suite in inventory.suites: - if suite["id"] not in selected or suite["level"] not in levels or suite["pull_request"] == "never": - continue - for item in inventory.matches[suite["id"]]: - if item.kind != "rust-target" or item.detail.endswith(":example"): - continue - name = item.name.split(":", 1)[0] - package = graph.packages.get(name) - if package and package.root_workspace: - names.add(name) - return sorted(names) - - -def main(argv: list[str] | None = None) -> int: - parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("--tier", choices=["fast", "fixtures"], default="fast") - parser.add_argument("--plan", help="selection-plan JSON file; defaults to OBC_SELECTION_PLAN") - args = parser.parse_args(argv) - - raw = Path(args.plan).read_text(encoding="utf-8") if args.plan else os.environ.get("OBC_SELECTION_PLAN", "") - if not raw.strip(): - print("--workspace") - return 0 - plan = json.loads(raw) - if any(_is_foundation(path) for path in plan.get("changed_paths", [])): - print("--workspace") - return 0 - print(" ".join(f"-p {name}" for name in packages(plan, args.tier, registry.repository_root()))) - return 0 - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/tools/ci/test.sh b/tools/ci/test.sh index 266cf6558..3e6d34b09 100644 --- a/tools/ci/test.sh +++ b/tools/ci/test.sh @@ -3,63 +3,56 @@ # name and result artifact; `obc check test` runs the same sections from this file. # # Usage: tools/ci/test.sh SECTION... -# nextest-fast fast hermetic binaries over the selected packages +# nextest-fast fast hermetic binaries # nextest-fixtures captured-fixture binaries, after an explicit fixture sync -# doctests doctests for the same package set +# doctests workspace doctests # formats-default obc-formats in the device's default feature shape # bench-golden render + read-counter golden gate # +# Every section runs in a subshell, so one section's exported environment cannot reach +# the next when several run in one invocation. +# # `--all-features` is deliberate: it is the shape the host tests are written for. The # device's feature shape is checked by the separate formats-default section. -# Set OBC_COVERAGE=1 to run the sections under llvm-cov instrumentation. +# +# Compilation stays workspace-wide. The per-pull-request coverage ratchet reads one lcov +# report over the whole workspace and fails any critical file it never compiled, so a +# narrowed package set would fail the ratchet rather than save time. set -euo pipefail cd "$(dirname "$0")/../.." -if [ "${OBC_COVERAGE:-}" = "1" ]; then - eval "$(cargo llvm-cov show-env --sh)" -fi - -# An empty package set means the tier selected nothing. That is not a reason to fall -# back to the whole workspace, and the tier then writes no report for CI to upload. -report() { - if [ -n "${GITHUB_OUTPUT:-}" ]; then echo "report=true" >> "$GITHUB_OUTPUT"; fi +# Instrumentation belongs to the two nextest sections only: they produce the ratchet's +# evidence. The bench golden gate is a release binary whose profile data would otherwise +# merge into that evidence, and doctests are not part of it either. +coverage_env() { + if [ "${OBC_COVERAGE:-}" = "1" ]; then + eval "$(cargo llvm-cov show-env --sh)" + fi } nextest_fast() { - local packages - packages="$(python3 tools/ci/rust_packages.py --tier fast)" - if [ -z "$packages" ]; then echo "no Rust packages selected for the fast tier"; return 0; fi - export NEXTEST_PROFILE="${NEXTEST_PROFILE:-ci}" + coverage_env + export NEXTEST_PROFILE=ci mkdir -p target find target -maxdepth 1 -name '*.profraw' -delete rm -f target/nextest/ci/junit.xml - report - # shellcheck disable=SC2086 - cargo nextest run $packages --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fast)" + cargo nextest run --workspace --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fast)" } nextest_fixtures() { - local packages - packages="$(python3 tools/ci/rust_packages.py --tier fixtures)" - if [ -z "$packages" ]; then echo "no Rust packages selected for the fixtures tier"; return 0; fi + coverage_env export NEXTEST_PROFILE=fixtures rm -f target/nextest/fixtures/junit.xml - report python3 tools/fixtures.py sync test OBC_FIXTURE_ROOT="$(python3 tools/fixtures.py root)" export OBC_FIXTURE_ROOT - # shellcheck disable=SC2086 - cargo nextest run $packages --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fixtures)" + cargo nextest run --workspace --all-features --locked --no-tests fail --filter-expr "$(python3 tools/suite_registry.py cargo-filter --tier fixtures)" } # nextest runs no doctests, so they run here rather than falling out of the gate. doctests() { - local packages - packages="$(python3 tools/ci/rust_packages.py --tier fast)" - if [ -z "$packages" ]; then echo "no Rust packages selected for the fast tier"; return 0; fi mkdir -p .artifacts/rust - # shellcheck disable=SC2086 - cargo test $packages --all-features --locked --doc 2>&1 | tee .artifacts/rust/doctests.log + cargo test --workspace --all-features --locked --doc 2>&1 | tee .artifacts/rust/doctests.log } formats_default() { @@ -77,11 +70,11 @@ bench_golden() { [ "$#" -gt 0 ] || { echo "tools/ci/test.sh: name at least one section" >&2; exit 2; } for section in "$@"; do case "$section" in - nextest-fast) nextest_fast ;; - nextest-fixtures) nextest_fixtures ;; - doctests) doctests ;; - formats-default) formats_default ;; - bench-golden) bench_golden ;; + nextest-fast) ( nextest_fast ) ;; + nextest-fixtures) ( nextest_fixtures ) ;; + doctests) ( doctests ) ;; + formats-default) ( formats_default ) ;; + bench-golden) ( bench_golden ) ;; *) echo "tools/ci/test.sh: unknown section $section" >&2; exit 2 ;; esac done diff --git a/tools/suite_registry.py b/tools/suite_registry.py index fc1323e83..7b1a24801 100644 --- a/tools/suite_registry.py +++ b/tools/suite_registry.py @@ -796,9 +796,7 @@ def _cargo_packages(args: Sequence[str], directory: str, graph: CargoGraph) -> s continue if word.startswith("--package="): named.add(word.split("=", 1)[1]) - elif word in {"--workspace", "--all"} or (word.startswith("$") and not word.startswith("$(")): - # A package set computed at run time can name any root-workspace package, and on a - # foundation change it is `--workspace`, so the command covers the whole workspace. + elif word in {"--workspace", "--all"}: workspace = True elif word == "--manifest-path" and following: manifest = following From 3094435b22a685223db286597dda608b9a520858 Mon Sep 17 00:00:00 2001 From: timohueser Date: Wed, 16 Sep 2026 12:19:17 +0200 Subject: [PATCH 7/7] docs: the test job compiles the workspace, and the sweep gates on its suite Replaces the package-narrowing description with the reason it cannot narrow: the coverage ratchet needs one report over the whole workspace. Records that only the two nextest sections are instrumented, and that the sweep step runs for `ci.ui-snapshots` alone. Co-Authored-By: Claude Opus 5 (1M context) --- docs/testing.md | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/docs/testing.md b/docs/testing.md index d8ed05b3a..e7ba6165e 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -102,11 +102,10 @@ fails the upload; setup failure can leave no bundle. The screenshot script also `OBC_XCRESULT_PATH` for local retention. A workflow declaration alone does not establish a passing run. TS6 still owns the remaining critical application journeys. -The UI snapshot sweep is its own `ui-snapshots` job, off the `test` job's serial path. Its -`ci.ui-snapshots` suite still selects only on its own rendering, screen and snapshot-input -triggers, and a broad coverage or policy change does not select it. The job itself also starts -whenever the simulator is selected, because the job compiles the simulator and a job that -compiles a package is a route for it — the same rule that starts `clippy` and `fmt`. +The UI snapshot sweep is its own `ui-snapshots` job, off the `test` job's serial path. The +sweep step runs only when `ci.ui-snapshots` is selected, and that suite still selects only on +its own rendering, screen and snapshot-input triggers. A broad coverage or policy change does +not run a sweep. ## Suite registry fields @@ -170,7 +169,7 @@ unmeasured files. Unmeasured informational source stays visible and does not bec | Maintained invocation | Collector | Coverage artifact | | --- | --- | --- | -| Selected-package nextest | cargo-llvm-cov 0.9.1, current Rust LLVM | `coverage-rust-ATTEMPT` | +| Workspace nextest | cargo-llvm-cov 0.9.1, current Rust LLVM | `coverage-rust-ATTEMPT` | | Repository and firmware Python | coverage.py 7.16.1 around xmlrunner | `coverage-repository-tools-ATTEMPT`, `coverage-firmware-tools-ATTEMPT` | | Builder Python | pytest-cov 7.1.0, coverage.py 7.16.1 | `coverage-builder-ATTEMPT` | | Builder Vitest | Vitest and V8 provider 3.2.6 | `coverage-web-ATTEMPT` | @@ -260,10 +259,10 @@ request that the run did not reproduce, with the reason. It makes no unqualified ## Rust CI result artifacts `tools/ci/test.sh` is the body of the `test` job, one section per CI step, and `obc check test` -runs the same file. Each section asks `tools/ci/rust_packages.py` which packages its tier -selected and compiles those with `-p`. A change to the root manifest, the lockfile, the -toolchain, the registry or the workflow compiles the whole workspace instead. A tier that -selected no package prints that and runs nothing; it never falls back to the workspace. +runs the same file. Compilation is workspace-wide. The per-pull-request coverage ratchet reads +one LCOV report over the whole workspace and fails any critical file it never compiled, so a +narrowed package set would fail the ratchet instead of saving time. Only the two nextest +sections run under llvm-cov instrumentation, because their report is that evidence. The two `cargo nextest run` commands in `test` use `NEXTEST_PROFILE=ci` for fast binaries and `NEXTEST_PROFILE=fixtures` for captured fixtures. The profiles in `.config/nextest.toml` write @@ -284,8 +283,7 @@ Each command removes an old report before it starts. A skipped job uploads nothi fixture setup failure remains a failure and may produce no report; the upload step reports a missing file as an error. CI does not create an empty report or run the tests again for reporting. -These JUnit artifacts cover nextest invocations. A tier that selected no package writes no -report and uploads none. Doctests for the selected packages and the default-feature +These JUnit artifacts cover nextest invocations. Workspace doctests and the default-feature `obc-formats` Cargo command retain their native text in `coverage-rust-ATTEMPT` (`doctests.log` and `formats-default.log`). Those logs contain native identities and outcomes, with aggregate durations; Cargo does not emit individual case durations for these commands. No synthetic durations or passes