diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 316ffb2a..0cfde27f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,6 +22,53 @@ env: RUST_BACKTRACE: "1" jobs: + # Decides whether the Rust jobs need to run at all. A pull request that only touches prose + # (Markdown at the top level, `docs/` apart from the files tests read, `SKILLS/`, `work/`) + # cannot change a build or a test result, so it skips the build, test, clippy and coverage + # jobs; fmt and the static checks (links, text style, PII) still run on it. Pushes to main + # and manual runs always run everything. + changes: + name: changes + runs-on: ubuntu-24.04 + timeout-minutes: 5 + permissions: + contents: read + pull-requests: read + outputs: + code: ${{ steps.filter.outputs.code }} + steps: + - id: filter + env: + GH_TOKEN: ${{ github.token }} + EVENT: ${{ github.event_name }} + PR: ${{ github.event.pull_request.number }} + REPO: ${{ github.repository }} + run: | + set -euo pipefail + if [[ "$EVENT" != "pull_request" ]]; then + echo "code=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + # Both sides of a rename count: moving code into docs/ is a code change. + files=$(gh api --paginate "repos/$REPO/pulls/$PR/files" --jq '.[] | .filename, (.previous_filename // empty)') + count=$(printf '%s\n' "$files" | grep -c . || true) + code=false + # The files API stops at 3000 entries; a list that long or an empty one is not trusted. + if [[ "$count" -eq 0 || "$count" -ge 3000 ]]; then code=true; fi + while IFS= read -r f; do + [[ -z "$f" ]] && continue + case "$f" in + # Read by tests: the demo notebooks, the generated command list, the coverage baseline. + docs/demo/* | docs/commands.md | docs/coverage.json) code=true ;; + docs/* | SKILLS/* | work/*) ;; + */*) code=true ;; + *.md) ;; + *) code=true ;; + esac + done <<< "$files" + echo "$count changed paths; Rust jobs needed: $code" + echo "code=$code" >> "$GITHUB_OUTPUT" + fmt: name: fmt runs-on: ubuntu-24.04 @@ -34,6 +81,8 @@ jobs: clippy: name: clippy + needs: changes + if: needs.changes.outputs.code == 'true' runs-on: ubuntu-24.04 timeout-minutes: 60 steps: @@ -51,18 +100,28 @@ jobs: platform-build-test: name: build/test (${{ matrix.platform }}) + needs: changes + if: needs.changes.outputs.code == 'true' strategy: fail-fast: false matrix: include: + # Linux stays at two compile jobs: more ran the runner out of disk and memory + # (see the infrastructure retry below). The macOS (3 cores) and Windows (4 cores) + # runners have no such trouble, and two jobs left a core idle for every build. - platform: linux runner: ubuntu-24.04 + build-jobs: "2" - platform: windows runner: windows-2025 + build-jobs: "4" - platform: macos runner: macos-15 + build-jobs: "3" runs-on: ${{ matrix.runner }} timeout-minutes: 90 + env: + CARGO_BUILD_JOBS: ${{ matrix.build-jobs }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Install Linux build dependencies @@ -129,6 +188,8 @@ jobs: coverage: name: coverage (warns, never blocks) + needs: changes + if: needs.changes.outputs.code == 'true' runs-on: ubuntu-24.04 timeout-minutes: 60 continue-on-error: true @@ -157,13 +218,32 @@ jobs: ci-status: name: CI runs-on: ubuntu-latest - needs: [fmt, clippy, platform-build-test, static, coverage] + needs: [changes, fmt, clippy, platform-build-test, static, coverage] if: always() steps: - name: Check CI result + env: + CHANGES: ${{ needs.changes.result }} + CODE: ${{ needs.changes.outputs.code }} + FMT: ${{ needs.fmt.result }} + CLIPPY: ${{ needs.clippy.result }} + BUILD_TEST: ${{ needs.platform-build-test.result }} + STATIC: ${{ needs.static.result }} run: | - if [[ "${{ needs.fmt.result }}" == "failure" || "${{ needs.clippy.result }}" == "failure" || "${{ needs.platform-build-test.result }}" == "failure" || "${{ needs.static.result }}" == "failure" ]]; then - echo "Quality checks failed" + # The change filter must have succeeded: if it failed, the jobs it gates were skipped, + # and a skip must never pass as a green run. + if [[ "$CHANGES" != "success" ]]; then + echo "The change filter did not succeed ($CHANGES)" exit 1 fi + for result in "$FMT" "$STATIC"; do + if [[ "$result" != "success" ]]; then echo "Quality checks failed"; exit 1; fi + done + for result in "$CLIPPY" "$BUILD_TEST"; do + if [[ "$CODE" == "true" && "$result" != "success" ]] || [[ "$result" == "failure" || "$result" == "cancelled" ]]; then + echo "Quality checks failed" + exit 1 + fi + done + if [[ "$CODE" != "true" ]]; then echo "Only prose changed: build, test, clippy and coverage were skipped"; fi echo "CI checks passed" diff --git a/crates/ui-egui/tests/support/gpu.rs b/crates/ui-egui/tests/support/gpu.rs new file mode 100644 index 00000000..bfdc637d --- /dev/null +++ b/crates/ui-egui/tests/support/gpu.rs @@ -0,0 +1,25 @@ +//! One offscreen GPU device for a whole test binary. +//! +//! `Harness::builder().wgpu()` creates a new wgpu instance, adapter, device and queue for every +//! window. The UI suite opens some two hundred windows, so it spent much of its time asking the +//! driver for devices: on the hosted macOS runners that made the suite several times slower than +//! on Linux. The same fix the icon tests use (`icons_visual.rs`) applies here: the adapter, +//! device and queue are shared, and each window still gets its own `Renderer` with its own +//! textures and pipelines, so nothing drawn in one test can leak into another. + +use egui_kittest::wgpu::{WgpuTestRenderer, create_render_state, default_wgpu_setup}; +use egui_wgpu::{RenderState, Renderer, RendererOptions}; +use std::sync::{Arc, OnceLock}; + +fn shared() -> &'static RenderState { + static STATE: OnceLock = OnceLock::new(); + STATE.get_or_init(|| create_render_state(default_wgpu_setup(), RendererOptions::PREDICTABLE)) +} + +/// A fresh renderer on the shared device, for `HarnessBuilder::renderer`. It renders exactly as +/// `HarnessBuilder::wgpu()` does: same adapter choice, same `RendererOptions::PREDICTABLE`. +pub fn renderer() -> WgpuTestRenderer { + let base = shared(); + let renderer = Renderer::new(&base.device, base.target_format, RendererOptions::PREDICTABLE); + WgpuTestRenderer::from_render_state(RenderState { renderer: Arc::new(egui::mutex::RwLock::new(renderer)), ..base.clone() }) +} diff --git a/crates/ui-egui/tests/support/mod.rs b/crates/ui-egui/tests/support/mod.rs index 6ec8995f..b9948f5f 100644 --- a/crates/ui-egui/tests/support/mod.rs +++ b/crates/ui-egui/tests/support/mod.rs @@ -1,3 +1,4 @@ //! Helpers shared by the UI tests. Test code only. pub mod clipboard; +pub mod gpu; diff --git a/crates/ui-egui/tests/ui.rs b/crates/ui-egui/tests/ui.rs index 1a194e42..2afbf2dc 100644 --- a/crates/ui-egui/tests/ui.rs +++ b/crates/ui-egui/tests/ui.rs @@ -98,7 +98,11 @@ fn harness(app: NoteCraftApp, theme: Theme) -> Harness<'static, NoteCraftApp> { /// be clicked. fn sized_harness(mut app: NoteCraftApp, theme: Theme, size: egui::Vec2) -> Harness<'static, NoteCraftApp> { app.set_theme(theme); - Harness::builder().with_size(size).with_accessibility_check(false).wgpu().build_ui_state(|ui, app: &mut NoteCraftApp| app.show(ui), app) + Harness::builder() + .with_size(size) + .with_accessibility_check(false) + .renderer(support::gpu::renderer()) + .build_ui_state(|ui, app: &mut NoteCraftApp| app.show(ui), app) } /// Feeds the clicks, keys and text that control requests queued into the next frame, then runs it. @@ -115,10 +119,26 @@ fn pump(h: &mut Harness<'_, NoteCraftApp>) { /// that until its autosave fires, so any test that had typed, drawn or inserted anything /// tripped over `run` - four of them did, on four separate days, each looking like a /// flake because it passed when run alone. Not calling `run` here is what stops the fifth. +/// +/// Background work gets half a second of wall-clock time, measured with a clock, and the window +/// steps 26 frames. It used to sleep 20 ms twenty-five times instead, which is the same half +/// second only where a sleep lasts as long as it was asked to. On the hosted macOS runners a +/// 20 ms sleep lasts about 100 ms, so every call took two and a half seconds, and the UI suite +/// spent 98% of its macOS time asleep. Now no sleep runs past the half second; the frames that +/// are left are stepped at once. fn settle(h: &mut Harness<'_, NoteCraftApp>) { + const BACKGROUND: Duration = Duration::from_millis(500); + let deadline = std::time::Instant::now() + BACKGROUND; for _ in 0..25 { pump(h); - std::thread::sleep(Duration::from_millis(20)); + let left = deadline.saturating_duration_since(std::time::Instant::now()); + if !left.is_zero() { + std::thread::sleep(left.min(Duration::from_millis(20))); + } + } + let left = deadline.saturating_duration_since(std::time::Instant::now()); + if !left.is_zero() { + std::thread::sleep(left); } pump(h); } @@ -1892,7 +1912,7 @@ fn the_control_port_works_end_to_end() { #[test] fn display_math_paints_ink_inside_its_box() { - let mut h = Harness::builder().with_size(egui::vec2(400.0, 160.0)).wgpu().build_ui(|ui| { + let mut h = Harness::builder().with_size(egui::vec2(400.0, 160.0)).renderer(support::gpu::renderer()).build_ui(|ui| { let full = ui.max_rect(); ui.painter().rect_filled(full, 0.0, egui::Color32::WHITE); notecraft_ui_egui::fonts::install_once(ui.ctx()); @@ -1949,7 +1969,7 @@ fn picking_a_swatch_changes_the_theme_and_is_remembered() { let mut h2 = Harness::builder() .with_size(egui::vec2(1200.0, 760.0)) .with_accessibility_check(false) - .wgpu() + .renderer(support::gpu::renderer()) .build_ui_state(|ui, a: &mut NoteCraftApp| a.show(ui), again); h2.run(); assert_eq!(h2.state().theme(), Theme { mode: Mode::Dark, accent: Accent::Purple }); @@ -2401,7 +2421,7 @@ fn the_rack_can_be_arranged_and_is_remembered() { let mut h2 = Harness::builder() .with_size(egui::vec2(1200.0, 760.0)) .with_accessibility_check(false) - .wgpu() + .renderer(support::gpu::renderer()) .build_ui_state(|ui, a: &mut NoteCraftApp| a.show(ui), again); h2.run(); let reopened = ask(&mut h2, "ui.inspect", json!({})).expect("inspect")["ink"]["rack"].as_str().unwrap_or_default().to_string(); diff --git a/docs/testing.md b/docs/testing.md index d6c40efc..9e648e0f 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -1,6 +1,6 @@ # Testing NoteCraft -Last reviewed: 2026-11-07. +Last reviewed: 2026-11-12. Quality is not negotiable here, so the project is tested on several levels. The UI testing follows the method of the other Craft apps: no committed golden pictures, paired comparisons @@ -102,6 +102,24 @@ not part of `cargo xtask ci` or the pre-push hook, because a coverage run rebuil When a change raises coverage, run `--update` in the same pull request, so the new figure lands with the code that earned it and git keeps the history. +## Continuous integration + +[`ci.yml`](../.github/workflows/ci.yml) runs the same gates on every pull request and on main: +fmt, clippy, the static checks, coverage, and a build and test on Linux, Windows and macOS. + +- **Prose-only pull requests skip the Rust jobs.** When every changed path is top-level + Markdown, `SKILLS/`, `work/`, or `docs/` other than the files tests read (`docs/demo/`, + `docs/commands.md`, `docs/coverage.json`), the build, test, clippy and coverage jobs are + skipped. fmt and the static checks (links, text style, PII) still run. Pushes to main always + run everything. The `CI` status job fails if the change filter itself fails, so a skip can + never pass by accident. +- **UI tests wait by the clock, not by counting sleeps.** `settle` in + `crates/ui-egui/tests/ui.rs` gives background work half a second measured with a clock. The + hosted macOS runners oversleep about five times, so counting twenty-five 20 ms sleeps made the + UI suite take 14 minutes there instead of 3. +- **UI tests share one offscreen GPU device per test binary** (`tests/support/gpu.rs`), as the + icon tests do. Each window still gets its own renderer and textures. + ## Clipboard in tests No test reads or writes the system clipboard. The app takes its clipboard as a boundary (see