From 13470ca1618ffbe426a0f05a649a1c7bb4dceda7 Mon Sep 17 00:00:00 2001 From: eliasx45 Date: Tue, 10 Nov 2026 14:57:32 +0100 Subject: [PATCH] Add affected CI and replayable crash repros (#1224) * AG4: xtask ci --affected cargo xtask ci --affected [--base REV] [--dry-run] runs fmt, clippy, tests (nextest + doctests when available), layers, assets (only on asset changes) and wasm (only for affected L0-L4/web crates) for the packages a change can affect: changed files map to the owning package and to packages whose sources read them via ../ paths, expanded to reverse dependents. Workspace-wide inputs (root Cargo.toml, Cargo.lock, .cargo/, xtask/, toolchain, clippy.toml) fall back to full CI. Documented in docs/testing.md, AGENTS.md and CLAUDE.md. (cherry picked from commit f3ca84fc2ee3948582cbc94e4c99ee510e829540) (cherry picked from commit 8a2cdcadf447eafa57ab14016beae4420f002af0) (cherry picked from commit d8a4301bc10c7bbcc2c4e5c70980f4b774d6efe3) (cherry picked from commit 36402c61ac2f7e29ecd76a6cfc89365cc2572e0a) * AO1: fast CI profile, looser timing tests - [profile.ci] (release without LTO, 16 codegen units); `xtask ci` runs clippy and tests with --profile ci instead of --release. - Wall-clock asserts in engine memory_tick, gpu OpenGL decline and script socket tests keep their intent with bounds that survive a loaded machine. - docs/testing.md and CONTRIBUTING.md mention the profile. (cherry picked from commit b45c8a379351ebdfda0d7076ad4b740baf6f2087) (cherry picked from commit fa1ea6a107c4c1936cde89b518a05adc73f2b4b0) (cherry picked from commit e5d5ed80e9ead43c2ce62b75ec1902340826a8e1) (cherry picked from commit d2c67660c70764f9c56880a642ad7f0d004a7f92) * AG6: journal replay repros Bug reports become replay scripts and crash regression tests become data files: - engine::replay: a JSON-lines repro format (start line, command steps with expect_error, expect queries with JSON pointer + equals, expect_no_error), a never-panicking runner that reports per-step ok/error and the first failure, and journal export (from the last project open; saves left out; size-capped). - journal.export {path?} and help.exportRepro {path?} (not journaled). - Crash capture: when the last-resort guard catches a panic in the outermost command, the journal plus that command is saved to Repros/crash-*.jsonl in the settings store (newest 20 kept) and the error names the file. - Commands run by a journaled command (script steps) are no longer journaled twice. - effectcraft-cli replay SCRIPT.jsonl [--json] [--keep-going]: exits 1 on a failed step, unmet expectation or panic. - crates/host/tests/repros.rs replays every tests/repros/*.jsonl; three samples from past crash fixes (zero frame rate, Layer Settings with no selection, New Comp from Selection with hostile overlap). - Docs: docs/agents.md "Repro scripts"; AGENTS.md: crash fix regression test = a .jsonl in tests/repros. Co-Authored-By: Claude Opus 5.5 (cherry picked from commit c38509d7dafe507856c48c78db65e00b94b0cebc) (cherry picked from commit e55b56dbf76716adfcbfe4cc773df19a74edc419) (cherry picked from commit 96a63855a98b4a05c24f9095b949aebf29d8665a) (cherry picked from commit c59351ab3bd697f0868e41230dd883960d94a540) * xtask: ci and ci --affected run cargo test and nextest with --no-fail-fast One run now lists every failing crate instead of stopping at the first. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_019qXL3GPNnY9BSFF2XoU2Yu (cherry picked from commit e46c7debe738eca62b35bf7392ab77637f6a33ae) * Agents: confinement write-checks journal.export, help.exportRepro and file.exportTimeline Review on #859 (split into this PR): their output paths were only read-checked (relative ones not at all). Test: confinement_covers_every_command_that_writes_a_file. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014d65YdNxQmjRkbXRtP2NwH * #1224: handle bin-only CI targets and BOM repros without weakening timing tests * #1224: run the full gate when nextest policy changes --------- Co-authored-by: Elias Co-authored-by: Claude Co-authored-by: FredApps Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com> --- CONTRIBUTING.md | 2 +- apps/effectcraft-cli/src/main.rs | 61 +- apps/effectcraft-cli/tests/it/cli.rs | 36 + crates/automation/src/confine.rs | 7 + crates/automation/src/tests.rs | 25 + crates/engine/src/cmd.rs | 2 + crates/engine/src/cmd/journal.rs | 53 ++ crates/engine/src/lib.rs | 1 + crates/engine/src/replay.rs | 748 +++++++++++++++++ crates/engine/src/session/dispatch.rs | 16 +- crates/engine/src/system/guard.rs | 14 + crates/host/tests/it/main.rs | 2 + crates/host/tests/it/repros.rs | 31 + docs/agents.md | 69 ++ docs/testing.md | 42 + tests/repros/comp-zero-frame-rate.jsonl | 13 + .../layer-settings-empty-selection.jsonl | 8 + ...-comp-from-selection-hostile-overlap.jsonl | 17 + xtask/src/affected.rs | 764 ++++++++++++++++++ xtask/src/main.rs | 83 +- 20 files changed, 1970 insertions(+), 24 deletions(-) create mode 100644 crates/engine/src/cmd/journal.rs create mode 100644 crates/engine/src/replay.rs create mode 100644 crates/host/tests/it/repros.rs create mode 100644 tests/repros/comp-zero-frame-rate.jsonl create mode 100644 tests/repros/layer-settings-empty-selection.jsonl create mode 100644 tests/repros/new-comp-from-selection-hostile-overlap.jsonl create mode 100644 xtask/src/affected.rs diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 29521ad73..8ce4b85aa 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -26,7 +26,7 @@ that variable set. ```sh cargo run -p effectcraft -- --demo # the desktop app, with the demo project cargo test --workspace -cargo xtask ci # what every commit must pass +cargo xtask ci # what every commit must pass (dev-profile checks and tests) ``` The default `dev` profile keeps debug info to line tables: builds are fast and panics/backtraces diff --git a/apps/effectcraft-cli/src/main.rs b/apps/effectcraft-cli/src/main.rs index b8634178e..f52401a00 100644 --- a/apps/effectcraft-cli/src/main.rs +++ b/apps/effectcraft-cli/src/main.rs @@ -31,6 +31,7 @@ //! effectcraft-cli bench --roto [--frames N] [--sam2 model.safetensors] [--mobilesam mobile_sam.pt] [--fixes] [--clip FRAMES --truth MASKS] [--json] Roto Brush trackers on synthetic clips with known mattes //! (classical engine, MobileSAM and SAM 2.1 (any installed size) when installed; IoU, flicker, re-appearance, seconds per frame) //! effectcraft-cli script FILE.jsx [F.ecproj] | --eval CODE run an After Effects-style script +//! effectcraft-cli replay SCRIPT.jsonl [--keep-going] replay a repro script (docs/agents.md) //! effectcraft-cli mcp [--autosave | --bridge PORT] MCP server on stdio //! //! Project: --project F.ecproj (or a positional *.ecproj) | --demo | --empty (default: demo; mcp: empty) @@ -97,7 +98,7 @@ fn stdout_broken() -> bool { STDOUT_FAILED.get().is_some_and(|k| *k != std::io::ErrorKind::BrokenPipe) } -const USAGE: &str = "usage: effectcraft-cli [args] [--json] +const USAGE: &str = "usage: effectcraft-cli [args] [--json] info project + engine summary commands [--filter TEXT] [--enabled] list engine commands exec [--params JSON] run one engine command (exec --list: list them, as `commands`) @@ -138,6 +139,9 @@ const USAGE: &str = "usage: effectcraft-cli Result<(), Failure> { Ok(()) } +/// `replay SCRIPT.jsonl`: run a repro script against a headless session (its start line picks +/// the project) and report every step. Exits 1 when the replay fails. +fn replay_cmd(args: &Args, json_out: bool) -> Result<(), Failure> { + use effectcraft_engine::replay; + let Some(file) = args.pos.first() else { return usage_err("replay needs a .jsonl script") }; + if args.opt("--bridge").is_some() || args.project.is_some() || args.flag("--demo") || args.flag("--empty") { + return usage_err("replay runs headless from the script's start line ({\"start\": \"new\" | \"demo\" | {\"project\": path}})"); + } + use std::io::Read; + let path = std::path::Path::new(file); + let input = std::fs::File::open(path).map_err(|e| Failure::Error(format!("cannot read {file}: {e}")))?; + // Bound the read itself: metadata can be stale or describe a pipe rather than its contents. + let mut script = String::new(); + input.take(replay::MAX_SCRIPT_BYTES as u64 + 1).read_to_string(&mut script).map_err(|e| Failure::Error(format!("cannot read {file}: {e}")))?; + if script.len() > replay::MAX_SCRIPT_BYTES { + return Err(Failure::Error(format!("{file} is larger than {} MB", replay::MAX_SCRIPT_BYTES >> 20))); + } + let mut s = session(args)?; + let opts = replay::Options { keep_going: args.flag("--keep-going"), base_dir: path.parent().map(std::path::Path::to_path_buf) }; + let report = replay::run(&mut s, &script, &opts); + if json_out { + emit(&serde_json::to_value(&report).unwrap_or_else(|e| json!({"error": e.to_string()})), true); + } else { + for st in &report.steps { + let what = st.command.as_deref().unwrap_or(st.kind); + match (&st.error, st.ok) { + (None, _) => say!("line {:>4} ok {what}", st.line), + (Some(e), true) => say!("line {:>4} ok {what} (tolerated error: {e})", st.line), + (Some(e), false) => say!("line {:>4} FAIL {what}: {e}", st.line), + } + } + } + let Some(first) = &report.first_failure else { + if !json_out { + say!("replay ok: {} steps", report.steps.len()); + } + return Ok(()); + }; + let msg = format!( + "failed at line {} ({}){}: {}{}", + first.line, + first.command.as_deref().unwrap_or(first.kind), + if report.panicked { ", a command panicked" } else { "" }, + first.error.as_deref().unwrap_or(""), + if report.skipped > 0 { format!(" ({} later steps not run)", report.skipped) } else { String::new() } + ); + if json_out { + note!("effectcraft-cli replay: {msg}"); + std::process::exit(1); + } + Err(Failure::Error(msg)) +} + /// Print a result: compact JSON with `--json`, else pretty JSON. fn emit(v: &Value, json_out: bool) { if json_out { @@ -614,6 +672,7 @@ fn run(cmd: &str, args: &Args, json_out: bool) -> Result<(), Failure> { "commands" => list_commands_cmd(args, json_out)?, "exec" if args.flag("--list") => list_commands_cmd(args, json_out)?, "script" => script_cmd(args, json_out)?, + "replay" => replay_cmd(args, json_out)?, "exec" => { let Some(id) = args.pos.first().cloned() else { return usage_err("exec needs a command id") }; let params = match (args.opt("--params"), args.pos.get(1).map(String::as_str)) { diff --git a/apps/effectcraft-cli/tests/it/cli.rs b/apps/effectcraft-cli/tests/it/cli.rs index 363eb8e52..4510f16c9 100644 --- a/apps/effectcraft-cli/tests/it/cli.rs +++ b/apps/effectcraft-cli/tests/it/cli.rs @@ -1223,3 +1223,39 @@ fn render_exr_compression_and_depth() { assert!(reply["error"].as_str().unwrap().contains("exrBitDepth: half|float"), "{reply}"); let _ = std::fs::remove_dir_all(&root); } + +#[test] +fn replay_reports_steps_and_the_first_failure() { + let repro = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../../tests/repros/comp-zero-frame-rate.jsonl"); + let v = ok_json(&["replay", repro.to_str().unwrap()]); + assert_eq!(v["ok"], true, "{v}"); + assert!(v["steps"].as_array().unwrap().len() > 5); + let bad = tmp("bad-repro.jsonl"); + std::fs::write( + &bad, + "{\"start\": \"new\"}\n{\"command\": \"comp.new\"}\n{\"expect\": {\"query\": \"comp.info\", \"pointer\": \"/width\", \"equals\": 1}}\nnot json\n", + ) + .unwrap(); + let (code, v) = run_json(&["replay", bad.to_str().unwrap()]); + assert_eq!(code, 1); + assert_eq!(v["ok"], false); + assert_eq!(v["first_failure"]["line"], 3, "{v}"); + assert_eq!(v["skipped"], 1); + let out = bin().args(["replay", bad.to_str().unwrap(), "--keep-going"]).output().unwrap(); + assert_eq!(out.status.code(), Some(1)); + let text = String::from_utf8_lossy(&out.stdout); + assert!(text.contains("line 2 ok comp.new") && text.contains("line 4 FAIL invalid: not JSON"), "{text}"); + assert!(String::from_utf8_lossy(&out.stderr).contains("failed at line 3 (comp.info)")); + // The start line picks the project. + assert_eq!(bin().args(["replay", bad.to_str().unwrap(), "--demo"]).output().unwrap().status.code(), Some(2)); +} + +/// Text editors can prefix an exported JSON-lines repro with a UTF-8 BOM. +#[test] +fn replay_accepts_a_utf8_bom() { + let path = tmp("bom-repro.jsonl"); + std::fs::write(&path, "\u{feff}{\"command\":\"comp.new\"}\n").unwrap(); + let (code, report) = run_json(&["replay", path.to_str().unwrap()]); + assert_eq!(code, 0, "{report}"); + assert_eq!(report["ok"], true); +} diff --git a/crates/automation/src/confine.rs b/crates/automation/src/confine.rs index 570726932..a9fc4665f 100644 --- a/crates/automation/src/confine.rs +++ b/crates/automation/src/confine.rs @@ -143,6 +143,8 @@ const WRITES: &[(&str, &[&str])] = &[ ("shortcuts.export", &["path"]), ("renderQueue.add", &["output", "path"]), ("file.exportTimeline", &["path"]), + ("journal.export", &["path"]), + ("help.exportRepro", &["path"]), // `path` / `output` may also be a file-name template, as for render.preRender. ("file.createProxy", &["path", "output"]), ]; @@ -280,6 +282,11 @@ impl Confinement { if matches!(id, "file.openRecent" | "file.importRecent" | "anim.applyRecentPreset") && !given { return Err(Error::BadArgs(format!("`{id}`: give `path` while file access is confined"))); } + // The default repro destination is the settings store, not an automation write root. + // Require a single explicit path so missing, null or array parameters cannot bypass it. + if id == "help.exportRepro" && params.get("path").and_then(Value::as_str).is_none() { + return Err(Error::BadArgs("`help.exportRepro`: give `path` inside an automation write root while file access is confined".into())); + } let mut lookups = vec![]; // An output the agent did not name is derived from where the project sits: the Default // Output Folder preference when that is set, beside the project file otherwise — and the diff --git a/crates/automation/src/tests.rs b/crates/automation/src/tests.rs index 92b62da2f..8a1839ede 100644 --- a/crates/automation/src/tests.rs +++ b/crates/automation/src/tests.rs @@ -1865,3 +1865,28 @@ fn automation_roots_check_relinked_footage_again() { assert!(!(px[0] > 200 && px[3] > 200), "the link pointed outside afterwards was read: {px:?}"); let _ = std::fs::remove_dir_all(&base); } + +/// Exporting a repro must not write to the settings store outside the automation roots. +#[test] +fn confined_repro_export_requires_an_explicit_writable_path() { + use crate::confine::Confinement; + use effectcraft_engine::config::{ConfigStore, MemoryConfig}; + use std::sync::Arc; + + let dir = tempfile::tempdir().unwrap(); + let store = Arc::new(MemoryConfig::default()); + let session = Session { config: Some(store.clone()), ..Default::default() }; + let mut s = McpServer::new(Backend::headless(session)); + s.backend().confine(Confinement::new(&[], &[dir.path().to_path_buf()]).unwrap()); + for params in [json!({}), json!({"path": null}), json!({"path": []}), json!({"path": [dir.path().join("repro.jsonl")]})] { + let (content, error) = call(&mut s, "execute_command", json!({"command": "help.exportRepro", "params": params})); + assert!(error, "{content:?}"); + assert!(store.list("Repros").is_empty(), "implicit settings-store writes must be refused"); + } + let path = dir.path().join("repro.jsonl"); + call_json(&mut s, "execute_command", json!({"command": "help.exportRepro", "params": {"path": path}})); + assert!(path.is_file()); + let journal = call_json(&mut s, "execute_command", json!({"command": "journal.export"})); + assert!(journal["script"].is_string(), "in-memory exports need no write root"); + assert!(store.list("Repros").is_empty()); +} diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 40603e8cd..09caa7cf4 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -31,6 +31,7 @@ pub(crate) mod frame_export; mod frontend; mod help; mod import_project; +mod journal; mod key_labels; mod key_transform; mod keys_more; @@ -202,6 +203,7 @@ pub fn command_specs() -> &'static [CommandSpec] { v.extend(nav_more::specs()); v.extend(render_queue::specs()); v.extend(help::specs()); + v.extend(journal::specs()); v.extend(query::specs()); v.extend(batch::specs()); v.extend(layer_menu::specs()); diff --git a/crates/engine/src/cmd/journal.rs b/crates/engine/src/cmd/journal.rs new file mode 100644 index 000000000..533cc498c --- /dev/null +++ b/crates/engine/src/cmd/journal.rs @@ -0,0 +1,53 @@ +//! The command journal as a replay script (see [`crate::replay`]): `journal.export` for agents +//! and `help.exportRepro` for bug reports. Neither is journaled. + +use serde_json::{Value, json}; + +use super::{CommandSpec, always, str_p}; +use crate::{Result, Session, replay}; + +/// `journal.export {path?}`: the script itself, or written to `path`. +fn export(s: &mut Session, p: &Value) -> Result { + if let Some(path) = str_p(p, "path") { + let (path, steps) = replay::save_repro(s, Some(path))?; + return Ok(json!({"path": path, "steps": steps})); + } + let script = replay::export(s, None); + Ok(json!({"steps": script.lines().count().saturating_sub(1), "script": script})) +} + +/// `help.exportRepro {path?}`: save the steps so far for a bug report (default: the settings +/// folder's `Repros`). +fn export_repro(s: &mut Session, p: &Value) -> Result { + let (path, steps) = replay::save_repro(s, str_p(p, "path"))?; + Ok(json!({"path": path, "steps": steps})) +} + +const fn spec(id: &'static str, label: &'static str, params: &'static str, run: super::Run) -> CommandSpec { + CommandSpec { id, label, menu: &[], shortcut: None, alt_shortcuts: &[], params, enabled: always, run, journal: false } +} + +pub fn specs() -> Vec { + #[allow(unused_mut)] + let mut v = vec![ + spec( + "journal.export", + "Export Command Journal", + "{path? (write the script there)} → {script, steps} or {path, steps}: the commands run since the project was last opened, as a replay script (docs/agents.md, Repro scripts)", + export, + ), + spec( + "help.exportRepro", + "Save Bug Repro...", + "{path? (default: Repros in the settings folder)} → {path, steps}: the steps since the project was opened, for a bug report (commands and parameters only)", + export_repro, + ), + ]; + // A command with a bug, for the crash-repro tests. + #[cfg(test)] + v.push(spec("test.panic", "Panic (test)", "{why?}", |_, p| { + let why = str_p(p, "why").unwrap_or("test").to_string(); + std::panic::panic_any(why) + })); + v +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 599aba1b7..f8edfbe08 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -21,6 +21,7 @@ pub mod menus; mod output_guard; pub mod preview; pub mod render_queue; +pub mod replay; pub mod scriptui; mod settings; mod system; diff --git a/crates/engine/src/replay.rs b/crates/engine/src/replay.rs new file mode 100644 index 000000000..5cca36918 --- /dev/null +++ b/crates/engine/src/replay.rs @@ -0,0 +1,748 @@ +//! Replay scripts: a bug or crash as a list of commands an agent (or a test) can run again. +//! +//! The session's command journal ([`Session::journal`]) becomes a JSON-lines script +//! (`journal.export`, `help.exportRepro`, and automatically when the last-resort guard catches a +//! panic, see [`after_command`]); `effectcraft-cli replay script.jsonl` runs one headless and +//! `crates/host/tests/it/repros.rs` runs every `tests/repros/*.jsonl` as a regression test. +//! +//! Format (one JSON object per line; blank lines and lines starting with `//` or `#` are +//! comments): +//! +//! ```text +//! {"start": "new" | "demo" | {"project": "path.ecproj"}, "allow_errors"?: bool} optional, first +//! {"command": "comp.new", "params": {...}, "expect_error"?: true | "substring"} +//! {"expect": {"query": "prop.get", "params": {...}, "pointer"?: "/value", "equals"?: ...}} +//! {"expect_no_error": true} +//! ``` +//! +//! A command must succeed unless it says `expect_error` (then it must fail, with that text in +//! the message when it is a string), or the start line says `allow_errors` (errors are reported +//! but tolerated; crash repros say so, since after the fix the crashing step may fail cleanly). +//! A panic always fails. `expect` runs a command (normally a query) and compares the value at +//! the JSON `pointer` of its result with `equals` (numbers within 1e-6); `expect_no_error` +//! checks that the previous command succeeded. Other keys (`version`, `app`, `note`, `crash`…) +//! are information for people. Relative `project` paths are relative to the script. +//! +//! Privacy: a repro holds command ids and their parameters only, never project or media file +//! contents. Parameters can include file paths, layer names and text typed into text layers. + +use std::cell::Cell; +use std::path::{Path, PathBuf}; + +use serde::Serialize; +use serde_json::{Map, Value, json}; + +use crate::{EngineError, Result, Session, guard}; + +/// The format version written into exported headers. +pub const VERSION: u64 = 1; +/// Largest script [`run`] accepts (bytes). +pub const MAX_SCRIPT_BYTES: usize = 64 << 20; +/// Most lines [`run`] reads. +pub const MAX_LINES: usize = 100_000; +/// Exported scripts stay below this size: the oldest steps are dropped (and counted). +const MAX_EXPORT_BYTES: usize = 32 << 20; +/// Folder of saved repros in the settings store (next to `Logs`). +pub const REPRO_DIR: &str = "Repros"; +/// Automatic crash repros kept (the oldest are removed). +const KEEP_CRASH_REPROS: usize = 20; +/// Commands that replace the whole project: an export starts at the last one. +const PROJECT_STARTS: &[&str] = &["file.newProject", "file.open", "file.openDemoProject"]; +/// Error and value texts in reports are cut to this many characters. +const MAX_TEXT: usize = 600; + +// ---------- nesting and crash capture (Session::execute) ---------- + +thread_local! { + /// Commands running on this thread: (all, journaled ones). + static DEPTH: Cell<(u32, u32)> = const { Cell::new((0, 0)) }; + /// A replay is running on this thread (its panics are reported, not saved as repros). + static REPLAYING: Cell = const { Cell::new(false) }; +} + +/// One running command (see [`Session::execute`]): commands it runs itself (a script's steps) +/// aren't journaled again, and only the outermost command saves a crash repro. +pub struct Nesting { + outer: (u32, u32), +} + +impl Nesting { + /// Record nesting so commands dispatched by a script are not exported twice. + pub fn enter(journaled: bool) -> Nesting { + let outer = DEPTH.with(Cell::get); + DEPTH.with(|d| d.set((outer.0.saturating_add(1), outer.1.saturating_add(u32::from(journaled))))); + Nesting { outer } + } + /// Run by a journaled command (which replays it). + pub fn inside_journaled(&self) -> bool { + self.outer.1 > 0 + } + /// Whether this command is responsible for capturing a nested panic. + pub fn outermost(&self) -> bool { + self.outer.0 == 0 + } +} + +impl Drop for Nesting { + fn drop(&mut self) { + DEPTH.with(|d| d.set(self.outer)); + } +} + +/// After the outermost command: when the guard caught a panic while it ran (`caught` is +/// [`guard::caught`] from before), save the journal plus this command as a crash repro in the +/// settings store and name it in the error. +pub fn after_command(s: &Session, id: &str, params: &Value, nesting: &Nesting, caught: u64, r: Result) -> Result { + if !nesting.outermost() || guard::caught() == caught || REPLAYING.with(Cell::get) { + return r; + } + let error = match &r { + Err(e) => e.to_string(), + Ok(_) => "a step it ran failed with an internal error".to_string(), + }; + let saved = save_crash_repro(s, id, params, &error); + match (r, saved) { + (Err(EngineError::Other(m)), Some(path)) => Err(EngineError::Other(format!("{m} (the steps that led here were saved to {path})"))), + (r, _) => r, + } +} + +/// Wall-clock seconds used only to label exported repros. +fn unix_secs() -> u64 { + web_time::SystemTime::now().duration_since(web_time::UNIX_EPOCH).map(|d| d.as_secs()).unwrap_or(0) +} + +/// `Repros/--.jsonl`. +fn repro_name(prefix: &str, id: &str) -> String { + let id: String = id.chars().map(|c| if c.is_ascii_alphanumeric() || c == '.' { c } else { '_' }).take(60).collect(); + format!("{REPRO_DIR}/{prefix}-{:012}-{id}.jsonl", unix_secs()) +} + +/// Where a store entry is on disk (or its name in stores without a folder). +fn store_path(store: &dyn crate::config::ConfigStore, name: &str) -> String { + store.dir().map(|d| d.join(name).display().to_string()).unwrap_or_else(|| name.to_string()) +} + +/// Persist the failing command with its preceding journal and prune older crash repros. +fn save_crash_repro(s: &Session, id: &str, params: &Value, error: &str) -> Option { + let store = s.config.clone()?; + let mut text = export(s, Some(json!({"command": id, "error": cut(error)}))); + text.push_str(&command_line(id, params)); + text.push('\n'); + let name = repro_name("crash", id); + if let Err(e) = store.write(&name, &text) { + log::error!("cannot save the crash repro {name}: {e}"); + return None; + } + // Keep the newest few (names sort by time). + let mut old: Vec = store.list(REPRO_DIR).into_iter().filter(|n| n.starts_with("crash-")).collect(); + old.sort(); + let extra = old.len().saturating_sub(KEEP_CRASH_REPROS); + for n in old.into_iter().take(extra) { + let _ = store.remove(&format!("{REPRO_DIR}/{n}")); + } + let path = store_path(store.as_ref(), &name); + log::error!("crash repro saved to {path}"); + Some(path) +} + +/// Save the journal as a repro: at `path` (through the session's file services), else in the +/// settings store's `Repros` folder. Returns where it went and how many steps it holds. +pub fn save_repro(s: &Session, path: Option<&str>) -> Result<(String, usize)> { + let text = export(s, None); + let steps = text.lines().count().saturating_sub(1); + if let Some(path) = path { + s.services.write_file(path, text.as_bytes()).map_err(|e| EngineError::Other(format!("cannot write {path}: {e}")))?; + return Ok((path.to_string(), steps)); + } + let store = s.config.clone().ok_or_else(|| EngineError::Other("there is no settings folder to save the repro in; pass `path`".into()))?; + let name = repro_name("repro", "session"); + store.write(&name, &text).map_err(|e| EngineError::Other(format!("cannot save {name}: {e}")))?; + Ok((store_path(store.as_ref(), &name), steps)) +} + +// ---------- export ---------- + +/// Serialize one command without including an empty parameter object. +fn command_line(id: &str, params: &Value) -> String { + let mut o = Map::new(); + o.insert("command".into(), json!(id)); + if !(params.is_null() || params.as_object().is_some_and(Map::is_empty)) { + o.insert("params".into(), params.clone()); + } + Value::Object(o).to_string() +} + +/// Saving writes the user's files and doesn't change the project: left out of repros. +fn skipped_in_export(id: &str) -> bool { + id.starts_with("file.save") || id.starts_with("file.increment") +} + +/// The journal as a replay script: a start line, then the steps since the project was last +/// replaced (`file.newProject` / `file.open` / `file.openDemoProject`; successful Revert records a resolved `file.open`). `crash` (the failing +/// command and its error) marks a crash repro, which tolerates errors on replay. +pub fn export(s: &Session, crash: Option) -> String { + let from = s.journal.iter().rposition(|(id, _)| PROJECT_STARTS.contains(&id.as_str())).unwrap_or(0); + let mut lines: Vec = + s.journal.get(from..).unwrap_or_default().iter().filter(|(id, _)| !skipped_in_export(id)).map(|(id, p)| command_line(id, p)).collect(); + let mut size: usize = lines.iter().map(|l| l.len().saturating_add(1)).sum(); + let mut dropped = 0usize; + while size > MAX_EXPORT_BYTES && !lines.is_empty() { + size = size.saturating_sub(lines.remove(0).len().saturating_add(1)); + dropped += 1; + } + let mut head = Map::new(); + head.insert("start".into(), json!("new")); + head.insert("version".into(), json!(VERSION)); + head.insert("app".into(), json!(format!("EffectCraft {}", env!("CARGO_PKG_VERSION")))); + head.insert("os".into(), json!(std::env::consts::OS)); + head.insert("exported".into(), json!(unix_secs())); + if let Some(c) = crash { + head.insert("allow_errors".into(), json!(true)); + head.insert("crash".into(), c); + } + if dropped > 0 { + head.insert("dropped".into(), json!(dropped)); + } + head.insert("note".into(), json!("commands and parameters only (no file contents); parameters can include file paths and text")); + let mut out = Value::Object(head).to_string(); + out.push('\n'); + for l in lines { + out.push_str(&l); + out.push('\n'); + } + out +} + +// ---------- parsing ---------- + +#[derive(Clone, Debug, PartialEq)] +enum Start { + New, + Demo, + Project(String), +} + +#[derive(Clone, Debug)] +enum Step { + Start(Start), + Command { + id: String, + params: Value, + expect_error: Option>, + }, + Expect { + query: String, + params: Value, + pointer: Option, + equals: Option, + }, + ExpectNoError, + /// A line that isn't a valid step (reported as a failure). + Invalid(String), +} + +fn params_of(v: Option<&Value>) -> std::result::Result { + match v { + None | Some(Value::Null) => Ok(json!({})), + Some(p @ Value::Object(_)) => Ok(p.clone()), + Some(_) => Err("`params` must be an object".into()), + } +} + +/// Decode the supported initial project states. +fn parse_start(v: &Value) -> std::result::Result { + match v { + Value::String(s) if s == "new" => Ok(Start::New), + Value::String(s) if s == "demo" => Ok(Start::Demo), + Value::Object(o) => match o.get("project").and_then(Value::as_str) { + Some(p) if !p.trim().is_empty() => Ok(Start::Project(p.to_string())), + _ => Err("`start` object needs a `project` path".into()), + }, + _ => Err("`start` must be \"new\", \"demo\" or {\"project\": path}".into()), + } +} + +/// One line → a step (`None`: a blank or comment line). +fn parse_line(text: &str, first: bool, allow_errors: &mut bool) -> Option { + let t = text.trim(); + if t.is_empty() || t.starts_with("//") || t.starts_with('#') { + return None; + } + let v: Value = match serde_json::from_str(t) { + Ok(v) => v, + Err(e) => return Some(Step::Invalid(format!("not JSON: {e}"))), + }; + let Some(o) = v.as_object() else { return Some(Step::Invalid("a line must be a JSON object".into())) }; + let step = if let Some(st) = o.get("start") { + if !first { + return Some(Step::Invalid("the `start` line must come first".into())); + } + *allow_errors = o.get("allow_errors").and_then(Value::as_bool).unwrap_or(false); + parse_start(st).map(Step::Start) + } else if let Some(c) = o.get("command") { + parse_command(c, o) + } else if let Some(e) = o.get("expect") { + parse_expect(e) + } else if o.get("expect_no_error").and_then(Value::as_bool) == Some(true) { + Ok(Step::ExpectNoError) + } else if o.len() == 1 && o.contains_key("comment") { + return None; + } else { + Err("expected a `start`, `command`, `expect` or `expect_no_error` line".to_string()) + }; + Some(step.unwrap_or_else(Step::Invalid)) +} + +/// Parse one command and its optional expected error without executing it. +fn parse_command(c: &Value, o: &Map) -> std::result::Result { + let id = c.as_str().filter(|s| !s.is_empty()).ok_or("`command` must be a command id")?; + let expect_error = match o.get("expect_error") { + None | Some(Value::Null | Value::Bool(false)) => None, + Some(Value::Bool(true)) => Some(None), + Some(Value::String(s)) => Some(Some(s.clone())), + Some(_) => return Err("`expect_error` must be true or a text the error contains".into()), + }; + Ok(Step::Command { id: id.to_string(), params: params_of(o.get("params"))?, expect_error }) +} + +/// Parse a query assertion, retaining the distinction between no expected value and JSON null. +fn parse_expect(e: &Value) -> std::result::Result { + let query = e.get("query").and_then(Value::as_str).filter(|s| !s.is_empty()).ok_or("`expect` needs a `query` command id")?; + let pointer = e.get("pointer").map(|p| p.as_str().map(str::to_string).ok_or("`pointer` must be a JSON pointer such as /value")).transpose()?; + Ok(Step::Expect { query: query.to_string(), params: params_of(e.get("params"))?, pointer, equals: e.get("equals").cloned() }) +} + +// ---------- running ---------- + +/// How to run a script. +#[derive(Clone, Debug, Default)] +pub struct Options { + /// Keep running after a failed step (default: stop at the first failure). + pub keep_going: bool, + /// Folder relative `start.project` paths are resolved against (the script's). + pub base_dir: Option, +} + +/// What happened to one line. +#[derive(Clone, Debug, Serialize)] +pub struct StepReport { + /// 1-based line number (0: the implicit start). + pub line: usize, + /// `start`, `command`, `expect`, `expect_no_error` or `invalid`. + pub kind: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + pub command: Option, + pub ok: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub error: Option, + #[serde(skip_serializing_if = "std::ops::Not::not")] + pub panicked: bool, +} + +/// The outcome of [`run`]. +#[derive(Clone, Debug, Default, Serialize)] +pub struct Report { + pub ok: bool, + /// A step panicked (caught by the last-resort guard): always a failure. + pub panicked: bool, + pub steps: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub first_failure: Option, + /// Steps not run after the first failure. + pub skipped: usize, +} + +/// Cut a message to [`MAX_TEXT`] characters. +fn cut(s: &str) -> String { + match s.char_indices().nth(MAX_TEXT) { + Some((i, _)) => format!("{}…", s.get(..i).unwrap_or(s)), + None => s.to_string(), + } +} + +/// Run one command the way the app does, noting whether it panicked. +fn exec(s: &mut Session, id: &str, params: Value) -> (Result, bool) { + let before = guard::caught(); + let r = guard::guarded(&format!("replay of `{id}`"), || s.execute(id, params)); + s.events.clear(); + (r, guard::caught() != before) +} + +/// Numbers within 1e-6 (relative above 1), everything else exactly. +fn approx_eq(a: &Value, b: &Value) -> bool { + match (a, b) { + (Value::Number(x), Value::Number(y)) => match (x.as_f64(), y.as_f64()) { + (Some(x), Some(y)) => (x - y).abs() <= 1e-6 * x.abs().max(y.abs()).max(1.0), + _ => x == y, + }, + (Value::Array(x), Value::Array(y)) => x.len() == y.len() && x.iter().zip(y).all(|(a, b)| approx_eq(a, b)), + (Value::Object(x), Value::Object(y)) => x.len() == y.len() && x.iter().all(|(k, a)| y.get(k).is_some_and(|b| approx_eq(a, b))), + _ => a == b, + } +} + +/// Marks this thread as replaying while alive. +struct Replaying(bool); + +impl Replaying { + /// Suppress automatic crash exports for the duration of a replay. + fn enter() -> Replaying { + Replaying(REPLAYING.with(|r| r.replace(true))) + } +} + +impl Drop for Replaying { + fn drop(&mut self) { + REPLAYING.with(|r| r.set(self.0)); + } +} + +/// Create or open the initial project and retain whether its command panicked. +fn start_step(s: &mut Session, start: &Start, base: Option<&Path>) -> (String, Result, bool) { + match start { + Start::New => { + let (r, p) = exec(s, "file.newProject", json!({})); + ("file.newProject".into(), r, p) + } + Start::Demo => { + let (r, p) = exec(s, "file.openDemoProject", json!({})); + ("file.openDemoProject".into(), r, p) + } + Start::Project(path) => { + let full = match base { + Some(b) if Path::new(path).is_relative() => b.join(path).display().to_string(), + _ => path.clone(), + }; + let (r, p) = exec(s, "file.open", json!({"path": full})); + ("file.open".into(), r, p) + } + } +} + +/// Compare a query result and produce a bounded explanation when it fails. +fn expectation_error(r: Result, panicked: bool, pointer: Option<&str>, equals: Option<&Value>) -> Option { + let value = match r { + Ok(v) => v, + Err(e) => return Some(cut(&format!("{}{e}", if panicked { "panicked: " } else { "" }))), + }; + let got = match pointer { + Some(p) => value.pointer(p), + None => Some(&value), + }; + match (got, equals) { + (None, _) => Some(cut(&format!("no value at {} in {value}", pointer.unwrap_or("")))), + (Some(got), Some(want)) if !approx_eq(got, want) => Some(cut(&format!("expected {want}, got {got}"))), + _ => None, + } +} + +/// Execute one parsed line and remember whether the last command failed for `expect_no_error`. +fn run_step(s: &mut Session, line: usize, step: Step, opts: &Options, allow_errors: bool, last_command_failed: &mut Option) -> StepReport { + let mut sr = StepReport { line, kind: "command", command: None, ok: true, error: None, panicked: false }; + match step { + Step::Invalid(m) => { + sr.kind = "invalid"; + sr.ok = false; + sr.error = Some(m); + } + Step::Start(start) => { + sr.kind = "start"; + let (id, r, panicked) = start_step(s, &start, opts.base_dir.as_deref()); + sr.command = Some(id); + sr.panicked = panicked; + sr.ok = !panicked; + if let Err(e) = r { + sr.ok = false; + sr.error = Some(cut(&e.to_string())); + } + } + Step::Command { id, params, expect_error } => { + let (r, panicked) = exec(s, &id, params); + sr.command = Some(id); + sr.panicked = panicked; + *last_command_failed = Some(r.is_err()); + let err = r.err().map(|e| e.to_string()); + let (ok, error) = match (err, expect_error) { + (e, _) if panicked => (false, Some(format!("panicked: {}", e.unwrap_or_else(|| "a step failed with an internal error".into())))), + (None, None) => (true, None), + (None, Some(_)) => (false, Some("expected an error, but the command succeeded".into())), + (Some(e), Some(Some(want))) if !e.contains(want.as_str()) => (false, Some(format!("expected an error containing {want:?}, got: {e}"))), + (Some(_), Some(_)) => (true, None), + (Some(e), None) => (allow_errors, Some(e)), + }; + sr.ok = ok; + sr.error = error.as_deref().map(cut); + } + Step::Expect { query, params, pointer, equals } => { + sr.kind = "expect"; + let (r, panicked) = exec(s, &query, params); + sr.command = Some(query); + sr.panicked = panicked; + sr.error = expectation_error(r, panicked, pointer.as_deref(), equals.as_ref()); + sr.ok = sr.error.is_none() && !panicked; + } + Step::ExpectNoError => { + sr.kind = "expect_no_error"; + match last_command_failed { + Some(false) => {} + Some(true) => { + sr.ok = false; + sr.error = Some("the previous command failed".into()); + } + None => { + sr.ok = false; + sr.error = Some("no command before this line".into()); + } + } + } + } + sr +} + +/// Run a replay script against `s`. Never panics: malformed lines are failed steps. +pub fn run(s: &mut Session, script: &str, opts: &Options) -> Report { + let _replaying = Replaying::enter(); + let mut report = Report { ok: true, ..Default::default() }; + let fail = |report: &mut Report, sr: StepReport| { + report.ok = false; + report.panicked |= sr.panicked; + if report.first_failure.is_none() { + report.first_failure = Some(sr.clone()); + } + report.steps.push(sr); + }; + if script.len() > MAX_SCRIPT_BYTES { + let msg = format!("the script is larger than {} MB", MAX_SCRIPT_BYTES >> 20); + fail(&mut report, StepReport { line: 0, kind: "invalid", command: None, ok: false, error: Some(msg), panicked: false }); + return report; + } + // Hand-edited UTF-8 scripts may start with a BOM, especially on Windows. + let script = script.strip_prefix('\u{feff}').unwrap_or(script); + let mut allow_errors = false; + let mut steps: Vec<(usize, Step)> = vec![]; + for (i, line) in script.lines().enumerate().take(MAX_LINES) { + if let Some(step) = parse_line(line, steps.is_empty(), &mut allow_errors) { + steps.push((i + 1, step)); + } + } + if script.lines().nth(MAX_LINES).is_some() { + steps.push((MAX_LINES + 1, Step::Invalid(format!("the script has more than {MAX_LINES} lines; the rest was not read")))); + } + if !matches!(steps.first(), Some((_, Step::Start(_)) | (_, Step::Invalid(_)))) { + steps.insert(0, (0, Step::Start(Start::New))); + } + let mut last_command_failed: Option = None; + let mut stopped = false; + for (line, step) in steps { + if stopped { + report.skipped += 1; + continue; + } + let sr = run_step(s, line, step, opts, allow_errors, &mut last_command_failed); + if sr.ok { + report.steps.push(sr); + } else { + fail(&mut report, sr); + stopped = !opts.keep_going; + } + } + report +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use serde_json::json; + + use super::{Options, run}; + use crate::Session; + use crate::config::{ConfigStore, MemoryConfig}; + + fn replay(script: &str) -> super::Report { + run(&mut Session::new(), script, &Options::default()) + } + + #[test] + fn steps_expectations_and_failures_are_reported() { + let r = replay( + r#"{"start": "new"} +// a comment +{"command": "comp.new", "params": {"name": "A", "width": 320, "height": 180, "frameRate": 30, "duration": 2}} +{"expect_no_error": true} +{"command": "comp.new", "params": {"frameRate": 0}, "expect_error": "frame"} +{"expect": {"query": "comp.info", "pointer": "/width", "equals": 320}} +"#, + ); + assert!(r.ok, "{r:?}"); + assert_eq!(r.steps.len(), 5); + let r = replay( + "{\"command\": \"comp.new\"}\n{\"expect\": {\"query\": \"comp.info\", \"pointer\": \"/width\", \"equals\": 1}}\n{\"command\": \"comp.new\"}", + ); + assert!(!r.ok && !r.panicked && r.skipped == 1, "{r:?}"); + assert_eq!(r.first_failure.as_ref().map(|f| f.line), Some(2)); + // An unexpected success, an unknown command. + assert!(!replay(r#"{"command": "comp.new", "expect_error": true}"#).ok); + assert!(!replay(r#"{"command": "no.such.command"}"#).ok); + // Tolerated errors. + let r = replay("{\"start\": \"new\", \"allow_errors\": true}\n{\"command\": \"no.such.command\"}\n{\"expect_no_error\": true}"); + assert!(!r.ok && r.first_failure.as_ref().map(|f| f.kind) == Some("expect_no_error"), "{r:?}"); + } + + /// A UTF-8 BOM belongs to the text file, not the first JSON step. + #[test] + fn utf8_bom_preserves_explicit_and_implicit_start_lines() { + for body in ["{\"start\":\"new\"}\n{\"command\":\"comp.new\"}", "{\"command\":\"comp.new\"}"] { + let script = format!("\u{feff}{body}"); + let report = replay(&script); + assert!(report.ok, "{report:?}"); + assert_eq!(report.steps.last().unwrap().line, body.lines().count()); + } + } + + #[test] + fn malformed_lines_are_failures_not_crashes() { + for bad in [ + "not json", + "[1, 2]", + "{\"command\": 5}", + "{\"command\": \"comp.new\", \"params\": [1]}", + "{\"command\": \"comp.new\", \"expect_error\": 3}", + "{\"expect\": {}}", + "{\"expect\": {\"query\": \"comp.info\", \"pointer\": 4}}", + "{\"expect\": {\"query\": \"comp.info\", \"pointer\": \"/nope\"}}", + "{\"start\": 7}", + "{\"start\": {\"project\": \"\"}}", + "{\"command\": \"comp.new\"}\n{\"start\": \"new\"}", + "{\"what\": 1}", + "{\"expect_no_error\": true}", + "{\"start\": {\"project\": \"/no/such/file.ecproj\"}}", + ] { + let r = replay(bad); + assert!(!r.ok && !r.panicked, "{bad}: {r:?}"); + } + let r = replay(&format!("{{\"command\": \"{}\"}}", "\u{1F600}".repeat(1000))); + assert!(!r.ok && r.first_failure.and_then(|f| f.error).is_some_and(|e| e.ends_with('…'))); + assert!(replay("").ok, "an empty script is a new project"); + } + + #[test] + fn the_journal_exports_and_replays() { + let mut s = Session::new(); + s.execute("comp.new", json!({"name": "Old"})).unwrap(); + s.execute("file.openDemoProject", json!({})).unwrap(); + s.execute("layer.newSolid", json!({"color": "#ff0000", "name": "Red"})).unwrap(); + s.execute("layer.newSolid", json!({"color": "#00ff00", "name": "Green"})).unwrap(); + // A query isn't journaled; neither is saving. + s.execute("comp.info", json!({})).unwrap(); + let text = super::export(&s, None); + let lines: Vec<&str> = text.lines().collect(); + assert_eq!(lines.len(), 4, "{text}"); + assert!(lines[0].contains("\"start\":\"new\"") && lines[1].contains("file.openDemoProject"), "starts at the last project open: {text}"); + let mut again = Session::new(); + let r = run(&mut again, &text, &Options::default()); + assert!(r.ok, "{r:?}"); + let layers = |s: &Session| s.active_comp().map(|c| c.layers.len()); + assert_eq!(layers(&again), layers(&s)); + // journal.export returns it; with `path` it writes the file. + let v = s.execute("journal.export", json!({})).unwrap(); + assert_eq!(v["steps"], 3); + assert_eq!(v["script"].as_str().unwrap().lines().skip(1).collect::>(), lines[1..]); + let dir = std::env::temp_dir().join(format!("ec-replay-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("j.jsonl"); + s.execute("journal.export", json!({"path": path.to_str().unwrap()})).unwrap(); + assert!(std::fs::read_to_string(&path).unwrap().contains("layer.newSolid")); + let _ = std::fs::remove_dir_all(&dir); + assert_eq!(s.journal.iter().filter(|(id, _)| id.starts_with("journal.")).count(), 0); + } + + #[test] + fn help_export_repro_saves_into_the_settings_store() { + let mut s = Session::new(); + assert!(s.execute("help.exportRepro", json!({})).is_err(), "no settings folder and no path"); + let store = Arc::new(MemoryConfig::default()); + s.config = Some(store.clone()); + s.execute("comp.new", json!({})).unwrap(); + let v = s.execute("help.exportRepro", json!({})).unwrap(); + assert_eq!(v["steps"], 1); + let saved = store.list(super::REPRO_DIR); + assert_eq!(saved.len(), 1, "{saved:?}"); + assert!(store.read(v["path"].as_str().unwrap()).unwrap().contains("comp.new")); + } + + #[test] + fn a_panicking_command_saves_a_crash_repro_and_replays_as_a_failure() { + let mut s = Session::new(); + let store = Arc::new(MemoryConfig::default()); + s.config = Some(store.clone()); + s.execute("comp.new", json!({"name": "C"})).unwrap(); + // `test.panic` stands in for a command with a bug (see `commands::journal`). + let e = s.execute("test.panic", json!({"why": "boom"})).unwrap_err().to_string(); + assert!(e.contains("boom") && e.contains("Repros/crash-"), "{e}"); + let names = store.list(super::REPRO_DIR); + assert_eq!(names.len(), 1); + let text = store.read(&format!("{}/{}", super::REPRO_DIR, names[0])).unwrap(); + assert!(text.lines().next().unwrap().contains("\"crash\""), "{text}"); + assert!(text.lines().last().unwrap().contains("test.panic"), "{text}"); + // The replay fails on the panic (and doesn't save another repro). + let r = run(&mut Session::new(), &text, &Options::default()); + assert!(!r.ok && r.panicked, "{r:?}"); + assert_eq!(r.first_failure.as_ref().and_then(|f| f.command.as_deref()), Some("test.panic")); + // A failing replay inside a session with a store saves nothing more. + let mut t = Session::new(); + t.config = Some(store.clone()); + assert!(!run(&mut t, &text, &Options::default()).ok); + assert_eq!(store.list(super::REPRO_DIR).len(), 1); + // Only the newest crash repros are kept. + for i in 0..30 { + store.write(&format!("{}/crash-{i:012}-x.jsonl", super::REPRO_DIR), "{}").unwrap(); + } + let _ = s.execute("test.panic", json!({})); + assert_eq!(store.list(super::REPRO_DIR).len(), super::KEEP_CRASH_REPROS); + } + + #[test] + fn commands_run_by_a_journaled_command_are_not_journaled_twice() { + let mut s = Session::new(); + s.execute("comp.new", json!({})).unwrap(); + // engine.batch isn't journaled: its steps are. + s.execute("engine.batch", json!({"steps": [{"command": "layer.newNull"}, {"command": "layer.newNull"}]})).unwrap(); + assert_eq!(s.journal.iter().filter(|(id, _)| id == "layer.newNull").count(), 2); + // A journaled command's own steps replay with it. + s.execute("file.runScript", json!({"steps": [{"command": "layer.newNull"}]})).unwrap(); + let ids: Vec<&str> = s.journal.iter().map(|(id, _)| id.as_str()).collect(); + assert_eq!(ids, ["comp.new", "layer.newNull", "layer.newNull", "file.runScript"]); + } +} + +#[cfg(all(test, not(target_arch = "wasm32")))] +mod revert_tests { + use super::{Options, export, run}; + use crate::Session; + use serde_json::json; + + /// Saving is omitted, so a successful Revert must keep the file it read as an open boundary. + #[test] + fn save_as_then_revert_exports_a_replayable_open() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("saved.ecproj"); + let mut s = Session::new(); + s.execute("comp.new", json!({"name": "Saved"})).unwrap(); + s.execute("file.saveAs", json!({"path": path})).unwrap(); + s.execute("layer.newNull", json!({})).unwrap(); + s.execute("file.revert", json!({})).unwrap(); + s.execute("layer.newNull", json!({"name": "After Revert"})).unwrap(); + let script = export(&s, None); + assert!(!script.contains("file.save") && !script.contains("file.revert"), "{script}"); + assert!(script.lines().nth(1).unwrap().contains("file.open"), "{script}"); + let saved = std::fs::read(&path).unwrap(); + let mut again = Session::new(); + let report = run(&mut again, &script, &Options::default()); + assert!(report.ok, "{report:?}"); + assert_eq!(again.path, s.path); + assert_eq!(again.active_comp().unwrap().layers.len(), s.active_comp().unwrap().layers.len()); + assert_eq!(saved, std::fs::read(path).unwrap(), "replay must not rewrite the saved project"); + } +} diff --git a/crates/engine/src/session/dispatch.rs b/crates/engine/src/session/dispatch.rs index aa83ac09f..a3f3f05d5 100644 --- a/crates/engine/src/session/dispatch.rs +++ b/crates/engine/src/session/dispatch.rs @@ -22,10 +22,20 @@ impl Session { } } // Last-resort guard: a panicking command reports an error instead of crashing the app. - let r = guard::guarded(&format!("command `{id}`"), || (spec.run)(self, ¶ms))?; + let nesting = crate::replay::Nesting::enter(spec.journal); + let caught = guard::caught(); + let r = guard::guarded(&format!("command `{id}`"), || (spec.run)(self, ¶ms)); + let r = crate::replay::after_command(self, id, ¶ms, &nesting, caught, r)?; self.learn_observe(id, ¶ms); - if spec.journal { - self.journal.push((id.to_string(), params)); + if spec.journal && !nesting.inside_journaled() { + // Revert and Open Recent resolve a file the parameters may not name. Store + // the successful read as an explicit open boundary while its resolved path is known. + let entry = if matches!(id, "file.revert" | "file.openRecent") { + self.path.as_ref().map(|path| ("file.open".to_string(), serde_json::json!({"path": path}))) + } else { + None + }; + self.journal.push(entry.unwrap_or_else(|| (id.to_string(), params))); if self.journal.len() > 10_000 { self.journal.drain(..1000); } diff --git a/crates/engine/src/system/guard.rs b/crates/engine/src/system/guard.rs index 699e6474d..ec32e2545 100644 --- a/crates/engine/src/system/guard.rs +++ b/crates/engine/src/system/guard.rs @@ -4,14 +4,28 @@ //! panics leaves the project as it was. This is a safety net, not a substitute for returning //! errors (AGENTS.md, "Never crash"). +use std::cell::Cell; + use crate::{EngineError, Result}; +thread_local! { + /// Panics [`guarded`] caught on this thread (see [`caught`]). + static CAUGHT: Cell = const { Cell::new(0) }; +} + +/// How many panics [`guarded`] has caught on this thread so far: compare two readings to learn +/// whether a call panicked (crash repros, `effectcraft-cli replay`). +pub fn caught() -> u64 { + CAUGHT.with(Cell::get) +} + /// Run `f`; a panic inside it becomes an error naming `what`. The panic itself is logged by /// the panic hook ([`crate::logging::install_panic_hook`]). pub fn guarded(what: &str, f: impl FnOnce() -> Result) -> Result { match std::panic::catch_unwind(std::panic::AssertUnwindSafe(f)) { Ok(r) => r, Err(payload) => { + CAUGHT.with(|c| c.set(c.get().saturating_add(1))); let msg = payload .downcast_ref::<&str>() .map(|s| s.to_string()) diff --git a/crates/host/tests/it/main.rs b/crates/host/tests/it/main.rs index 168ec127a..ecc1abb0f 100644 --- a/crates/host/tests/it/main.rs +++ b/crates/host/tests/it/main.rs @@ -15,3 +15,5 @@ mod timeline_interop; mod vector_import; mod wav_markers; mod webm_metadata; + +mod repros; diff --git a/crates/host/tests/it/repros.rs b/crates/host/tests/it/repros.rs new file mode 100644 index 000000000..1b343603c --- /dev/null +++ b/crates/host/tests/it/repros.rs @@ -0,0 +1,31 @@ +//! Regression tests as data: every `tests/repros/*.jsonl` (repository root) is a replay script +//! (`effectcraft_engine::replay`, docs/agents.md "Repro scripts") run against a fully wired +//! headless session, like `effectcraft-cli replay`. A crash fix's regression test is a `.jsonl` +//! dropped there: it must replay without a panic and meet its expectations. + +use std::path::{Path, PathBuf}; + +use effectcraft_engine::replay; + +fn repros_dir() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")).join("../../tests/repros") +} + +#[test] +fn every_repro_replays_cleanly() { + let mut files: Vec = + std::fs::read_dir(repros_dir()).unwrap().map(|e| e.unwrap().path()).filter(|p| p.extension().is_some_and(|x| x == "jsonl")).collect(); + files.sort(); + assert!(!files.is_empty(), "no repros in {}", repros_dir().display()); + let mut failures = vec![]; + for f in &files { + let script = std::fs::read_to_string(f).unwrap(); + let mut s = effectcraft_host::session(); + let opts = replay::Options { keep_going: false, base_dir: f.parent().map(Path::to_path_buf) }; + let r = replay::run(&mut s, &script, &opts); + if !r.ok { + failures.push(format!("{}: {}", f.display(), serde_json::to_string(&r.first_failure).unwrap())); + } + } + assert!(failures.is_empty(), "{} of {} repros failed:\n{}", failures.len(), files.len(), failures.join("\n")); +} diff --git a/docs/agents.md b/docs/agents.md index ae9b1a6f6..06f9bedae 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -865,6 +865,75 @@ not read or written. `` is an id or name, or `-` for the active comp. `` is JSON (`50`, `[960,540]`, `"#ff0000"`) or a bare string. +## Repro scripts + +Every journaled command the app runs (menus, panels, shortcuts, agents, scripts) goes into the session's +command journal. A **repro script** is that journal as JSON lines, so a bug or crash becomes +something an agent can replay and a regression test becomes a data file. + +```text +// comments: lines starting with // or #; blank lines are skipped +{"start": "new" | "demo" | {"project": "rel/or/abs.ecproj"}, "allow_errors": false} optional, first line +{"command": "comp.new", "params": {"name": "Main", "frameRate": 30}} +{"command": "comp.settings", "params": {"frameRate": 0}, "expect_error": "frameRate"} +{"expect": {"query": "comp.info", "pointer": "/frameRate", "equals": 30}} +{"expect_no_error": true} +``` + +- `start`: the project to begin with (default `new`). A relative `project` path is relative to the + script. Other header keys (`version`, `app`, `os`, `crash`, `note`) are for people. +- `command`: run like the app runs it. It must succeed, unless it has `expect_error` (`true`, or a + text the error must contain) or the header says `allow_errors` (errors are reported but + tolerated). A panic always fails the replay. +- `expect`: run a command, normally a query (`prop.get`, `comp.info`, `layer.tree`, `project.summary`), + and compare the value at the JSON `pointer` of its result with `equals` (numbers within 1e-6). + Without `equals` it only checks that the query succeeds. +- `expect_no_error`: the previous command succeeded. + +Where scripts come from: + +- `journal.export {path?}` returns `{script, steps}` (or writes `path`): the commands since the + project was last replaced (`file.newProject`, `file.open`, `file.openDemoProject`). Queries and + saves are not in it; commands a script or `file.runScript` ran are covered by that one step. + A successful Revert starts a new replay boundary at the saved file it reopened. +- `help.exportRepro {path?}` saves the same for a bug report, by default to `Repros/` in the + settings folder (macOS `~/Library/Application Support/EffectCraft/Repros`, next to `Logs/`). + Confined automation must give an explicit path inside a write root; `journal.export {}` + remains available because it only returns the script in memory. +- **Crash repros**: when the last-resort guard catches a panic in a command, the app writes + `Repros/crash--.jsonl` there automatically (the newest 20 are kept): the + journal, then the command that panicked, with `"allow_errors": true` and the error in the header. + The error the user or agent sees names the file. Headless sessions without a settings folder + (the CLI, tests) don't write one. + +Privacy: a repro holds command ids and parameters only, never project, image or media file +contents. Parameters can include file paths, layer and item names and text typed into text +layers, so read a repro before sharing it. + +Replay one headless (a fully wired session, like the CLI's other subcommands): + +```sh +effectcraft-cli replay crash.jsonl # per-step ok/FAIL lines, then the first failure +effectcraft-cli replay crash.jsonl --json # {ok, panicked, steps, first_failure, skipped} +effectcraft-cli replay crash.jsonl --keep-going +``` + +Replaying a repro runs its commands with your privileges and file access, including commands +that write files or run scripts. Read repros from untrusted sources before running them, or use +a disposable environment. + +It exits with status 1 when a step fails, an expectation is not met or a command panics, and +stops at the first failure unless `--keep-going`. Steps that work on the user's files (`file.open` +of their project, imports) need those files; replace them with a `start` project or generated +content when you turn a report into a test. A repro only sees commands: UI state changed +without one (a hover, a half-finished drag) is not in it. + +**Regression tests are data files.** A crash fix's regression test is the repro, trimmed to the +steps that matter, with an `expect_error` or `expect` line where the fix changed the outcome, +dropped into [`tests/repros/`](../tests/repros). `crates/host/tests/it/repros.rs` replays every +`*.jsonl` there and fails on any panic or unmet expectation +(`cargo nextest run -p effectcraft-host --test it -E 'test(/^repros::/)'`). + ## Seeing the UI To work on the UI, start the app with `cargo run -p effectcraft -- --control 9877` (add `--demo` to diff --git a/docs/testing.md b/docs/testing.md index 4a7610d16..4c5bfddcb 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -3,6 +3,7 @@ ```sh cargo test --workspace # everything cargo xtask ci # fmt, clippy -D warnings, tests, pdf-deps, lock-patches, layers, assets, wasm +cargo xtask ci --affected # the same gates, scoped to crates your change can affect ``` The After Effects test projects of `crates/aep` are not in the repository; without them the @@ -32,6 +33,47 @@ Each crate with several integration-test files has one test binary, `tests/it/ma modules are the files in `tests/it/`. Run one file with `cargo test -p --test it ::` or `cargo nextest run -p -E 'binary(it) and test(/^::/)'`. +## Affected-only CI + +Use `cargo xtask ci --affected` to run the gates for the crates a change can affect. The full +`cargo xtask ci` remains the required check before a release and when affected mode falls back +because a workspace-wide input changed. + +```sh +cargo xtask ci --affected # base: the merge-base of HEAD with main +cargo xtask ci --affected --base HEAD~3 # any git revision +cargo xtask ci --affected --dry-run # print the plan only +``` + +It collects files that differ from the base (committed, staged, unstaged and untracked), preserving +Unicode and whitespace in their names. Each file maps to the workspace package whose directory +holds it and every package whose sources point at it with a `../` literal, such as +`include_bytes!("../../../assets/fonts/Inter-Regular.ttf")` or +`concat!(env!("CARGO_MANIFEST_DIR"), "/../../docs/preferences.md")`. Deleted and renamed inputs +still select their readers, including when the input's entire directory no longer exists. + +The affected set is those packages plus everything that depends on them (transitively through +normal and build dependencies) and every package that uses one of them as a dev-dependency. +Clippy and tests use the same dev profile as full CI. Both modes use the same test runner: +nextest plus doctests when installed, otherwise cargo test plus doctests. GPU tests run in one +libtest process, and the translation report reruns with visible output when ui-egui is affected. +The audio-backend probe runs before building affected crates. + +Formatting, PDF dependency policy, lock-patch and layer checks always run. Assets are checked +when an asset, an `.attribution` sidecar or `ATTRIBUTION.md` changes. The wasm check selects +affected L0–L4 and web crates. Every step runs even when an earlier step fails; the final result +names failed steps and includes timings. + +Changes to the root `Cargo.toml`, `Cargo.lock`, `.cargo/`, `xtask/`, `rust-toolchain(.toml)`, +`clippy.toml` or `.config/nextest.toml` can change every crate's build or test policy, so they run +the full CI instead and say why. Changes no crate reads (most of `docs/`, `README.md`, `.github/`, +`packaging/`) only get the cheap checks. + +Limits: a crate that reads a repo file at run time through a path that is not a `../` literal in its +sources is not detected, and building a subset of packages can unify features differently from the +whole workspace. Non-UTF-8 Git paths stop the command rather than silently omitting checks. +Run the full `cargo xtask ci` before a release. + ## What the tests cover - **Engine commands** (`crates/engine/src/tests*.rs`): every command family, including undo and diff --git a/tests/repros/comp-zero-frame-rate.jsonl b/tests/repros/comp-zero-frame-rate.jsonl new file mode 100644 index 000000000..5ce9dd873 --- /dev/null +++ b/tests/repros/comp-zero-frame-rate.jsonl @@ -0,0 +1,13 @@ +// comp.new / comp.settings took a frameRate of 0 (or below 0.0005) as a 0/1 rate: the next +// frame-duration computation divided by zero and killed the process with the unsaved project +// (fixed in 780419c, "Compositions: reject zero and negative frame rates instead of panicking"). +{"start": "new"} +{"command": "comp.new", "params": {"name": "Zero", "frameRate": 0}, "expect_error": "frameRate"} +{"command": "comp.new", "params": {"name": "Tiny", "frameRate": 0.0001}, "expect_error": "frameRate"} +{"command": "comp.new", "params": {"name": "Negative", "frameRate": -24}, "expect_error": "frameRate"} +{"command": "comp.new", "params": {"name": "Main", "width": 320, "height": 180, "frameRate": 30, "duration": 1}} +{"command": "comp.settings", "params": {"frameRate": 0}, "expect_error": "frameRate"} +{"expect": {"query": "comp.info", "pointer": "/frameRate", "equals": 30}} +{"command": "layer.newSolid", "params": {"color": "#ff0000"}} +{"command": "time.set", "params": {"time": 0.5}} +{"expect_no_error": true} diff --git a/tests/repros/layer-settings-empty-selection.jsonl b/tests/repros/layer-settings-empty-selection.jsonl new file mode 100644 index 000000000..71a0216ad --- /dev/null +++ b/tests/repros/layer-settings-empty-selection.jsonl @@ -0,0 +1,8 @@ +// Layer Settings' enablement read the first selected layer with `selected_layers[0]`: with layers +// in the comp but none selected it indexed an empty list and panicked (fixed in b21e04b, M4.12). +{"start": "demo"} +{"command": "edit.deselectAll"} +{"command": "layer.settings", "expect_error": "select a layer"} +{"command": "layer.newSolid", "params": {"color": "#3366ff", "name": "Blue"}} +{"command": "layer.settings", "params": {"name": "Renamed"}} +{"expect_no_error": true} diff --git a/tests/repros/new-comp-from-selection-hostile-overlap.jsonl b/tests/repros/new-comp-from-selection-hostile-overlap.jsonl new file mode 100644 index 000000000..07e96afc6 --- /dev/null +++ b/tests/repros/new-comp-from-selection-hostile-overlap.jsonl @@ -0,0 +1,17 @@ +// File > New Comp from Selection with Sequence Layers: a huge `overlapDuration` times the layer +// count overflowed the duration arithmetic (a panic with overflow checks, a wrapped, wrong comp +// length without them), and `dimensionsFrom` past the selection indexed out of bounds +// (fixed in b21e04b, M4.12). +{"start": "new"} +{"command": "comp.new", "params": {"name": "Holder", "width": 64, "height": 64, "frameRate": 30, "duration": 2}} +{"command": "layer.newSolid", "params": {"color": "#ff0000", "name": "a"}} +{"command": "layer.newSolid", "params": {"color": "#00ff00", "name": "b"}} +{"command": "layer.newSolid", "params": {"color": "#0000ff", "name": "c"}} +{"command": "project.select", "params": {"items": ["a", "b", "c"]}} +{"command": "file.newCompFromSelection", "params": {"single": true, "sequence": true, "overlap": true, "overlapDuration": 1e30, "duration": 2}} +{"expect": {"query": "comp.info", "pointer": "/width", "equals": 64}} +{"command": "project.select", "params": {"items": ["a", "b", "c"]}} +{"command": "file.newCompFromSelection", "params": {"single": true, "sequence": true, "overlap": true, "overlapDuration": -5, "duration": 2}} +{"expect": {"query": "comp.info", "pointer": "/duration", "equals": 4}} +{"command": "project.select", "params": {"items": ["a", "b"]}} +{"command": "file.newCompFromSelection", "params": {"single": true, "dimensionsFrom": 99}, "expect_error": "dimensionsFrom"} diff --git a/xtask/src/affected.rs b/xtask/src/affected.rs new file mode 100644 index 000000000..2ce407d27 --- /dev/null +++ b/xtask/src/affected.rs @@ -0,0 +1,764 @@ +//! `cargo xtask ci --affected [--base REV] [--dry-run]`: the CI gates, run only for the crates a +//! change can affect (docs/testing.md). +//! +//! 1. Changed files: everything that differs from the base (default: the merge-base with `main`), +//! committed, staged, unstaged or untracked. +//! 2. Files map to the workspace package whose directory holds them, and to every package whose +//! sources point at them with a `../` path (`include_bytes!("../../../assets/fonts/…")`, +//! `env!("CARGO_MANIFEST_DIR")` + `"/../../docs/effects.md"`, …). +//! 3. Affected = those packages, every package that depends on them (transitively, through +//! normal and build dependencies) and every package that has one of them as a dev-dependency. +//! 4. Workspace-wide inputs (root `Cargo.toml`, `Cargo.lock`, `.cargo/`, `xtask/`, the toolchain +//! file, `clippy.toml`, `.config/nextest.toml`) fall back to the full `cargo xtask ci`. + +use std::collections::BTreeSet; +use std::path::Path; +use std::process::Command; +use std::time::{Duration, Instant}; + +use serde_json::Value; + +/// A workspace package, as far as change mapping cares. +#[derive(Debug, Clone, Default)] +pub struct Pkg { + pub name: String, + /// Package directory relative to the workspace root, `/`-separated, no trailing slash. + pub dir: String, + /// Workspace (path) dependencies: (package name, is a dev-dependency). + pub deps: Vec<(String, bool)>, + /// Has a library target (`--lib` is valid even when doctests are disabled). + pub has_lib: bool, + /// Has a library target with doctests (`cargo test --doc` runs for it). + pub has_doctests: bool, + /// Repo paths outside `dir` that the sources read (files or directories). + pub inputs: Vec, +} + +/// Changes to these force the full CI: they can change every crate's build or test policy. +pub fn full_ci_reason(path: &str) -> Option<&'static str> { + match path { + "Cargo.toml" => Some("workspace Cargo.toml changed"), + "Cargo.lock" => Some("Cargo.lock changed"), + "rust-toolchain" | "rust-toolchain.toml" => Some("toolchain changed"), + "clippy.toml" => Some("clippy.toml changed"), + // Test selection and required runner versions apply to the whole workspace. + ".config/nextest.toml" => Some("nextest configuration changed"), + _ if path.starts_with(".cargo/") => Some(".cargo/ changed"), + _ if path.starts_with("xtask/") => Some("xtask changed"), + _ => None, + } +} + +/// Files the `assets` check looks at (see `assets()` in main.rs). +pub fn is_asset_related(path: &str) -> bool { + path == "ATTRIBUTION.md" || path.ends_with(".attribution") || crate::is_asset(path) +} + +fn under(path: &str, dir: &str) -> bool { + dir.is_empty() || path == dir || path.strip_prefix(dir).is_some_and(|r| r.starts_with('/')) +} + +/// The package whose directory holds `path` (the innermost one). +pub fn owner<'a>(path: &str, pkgs: &'a [Pkg]) -> Option<&'a Pkg> { + pkgs.iter().filter(|p| !p.dir.is_empty() && under(path, &p.dir)).max_by_key(|p| p.dir.len()) +} + +/// Packages whose sources read `path` from outside their own directory. +pub fn readers<'a>(path: &'a str, pkgs: &'a [Pkg]) -> impl Iterator + 'a { + pkgs.iter().filter(move |p| p.inputs.iter().any(|i| under(path, i))) +} + +/// `direct` plus everything that depends on it: transitively through normal/build edges, then +/// one hop through dev-dependency edges (a dev-dependent's tests rerun, its dependents don't). +pub fn expand(direct: &BTreeSet, pkgs: &[Pkg]) -> BTreeSet { + let mut out = direct.clone(); + let mut stack: Vec = direct.iter().cloned().collect(); + while let Some(n) = stack.pop() { + for p in pkgs { + if !out.contains(&p.name) && p.deps.iter().any(|(d, dev)| !dev && *d == n) { + out.insert(p.name.clone()); + stack.push(p.name.clone()); + } + } + } + let dev: Vec = pkgs.iter().filter(|p| p.deps.iter().any(|(d, dev)| *dev && out.contains(d))).map(|p| p.name.clone()).collect(); + out.extend(dev); + out +} + +/// What `ci --affected` will do for a set of changed files. +#[derive(Debug, Default, PartialEq)] +pub struct Plan { + /// Set when the full CI must run instead. + pub full: Option, + /// Packages that hold or read a changed file. + pub direct: BTreeSet, + /// `direct` and its dependents: clippy and tests run for these. + pub affected: BTreeSet, + /// Whether the assets check runs. + pub assets: bool, + /// Changed files no package reads (docs, CI configs, packaging…). + pub unmapped: Vec, +} + +pub fn plan(files: &[String], pkgs: &[Pkg]) -> Plan { + let mut plan = Plan::default(); + for f in files { + if let Some(why) = full_ci_reason(f) { + plan.full = Some(format!("{why} ({f})")); + return plan; + } + plan.assets |= is_asset_related(f); + let mut hit = false; + for p in owner(f, pkgs).into_iter().chain(readers(f, pkgs)) { + plan.direct.insert(p.name.clone()); + hit = true; + } + if !hit { + plan.unmapped.push(f.clone()); + } + } + plan.affected = expand(&plan.direct, pkgs); + plan +} + +/// Normalise `base/rel` (both `/`-separated, relative to the repo root). None if it leaves the repo. +pub fn join_rel(base: &str, rel: &str) -> Option { + let mut parts: Vec<&str> = base.split('/').filter(|s| !s.is_empty() && *s != ".").collect(); + for c in rel.split('/') { + match c { + "" | "." => {} + ".." => { + parts.pop()?; + } + c => parts.push(c), + } + } + Some(parts.join("/")) +} + +/// The `../…` paths in a line of Rust: the part of each string literal from its first `../`, +/// cut back to a directory when a format placeholder follows (`"{}/../model/fixtures/{n}"`). +pub fn rel_paths(line: &str) -> Vec { + let mut out = vec![]; + for lit in line.split('"').skip(1).step_by(2) { + let Some(i) = lit.find("../") else { continue }; + let Some(rest) = lit.get(i..) else { continue }; + let p = match rest.find('{') { + Some(j) => rest.get(..j).and_then(|s| s.rfind('/').and_then(|k| s.get(..k))).unwrap_or(""), + None => rest, + }; + let p = p.trim_end_matches('/'); + if !p.is_empty() { + out.push(p.to_string()); + } + } + out +} + +/// Repo paths outside `pkg_dir` that a source file at `file` (both repo-relative) points at. +/// A literal may be relative to the file (`include_bytes!`) or to the manifest dir +/// (`CARGO_MANIFEST_DIR`), so both are tried. Keep existing paths and paths containing a change, +/// including deleted files and directories that no longer exist in the working tree. +pub fn inputs_of(file: &str, pkg_dir: &str, text: &str, known_input: &dyn Fn(&str) -> bool) -> BTreeSet { + let file_dir = file.rsplit_once('/').map(|(d, _)| d).unwrap_or(""); + let mut out = BTreeSet::new(); + for line in text.lines().filter(|l| l.contains("../")) { + let t = line.trim_start(); + if t.starts_with("//") && !t.starts_with("///") && !t.starts_with("//!") { + continue; + } + for rel in rel_paths(line) { + for base in [file_dir, pkg_dir] { + let Some(p) = join_rel(base, &rel) else { continue }; + if p.is_empty() || p == "target" || p.starts_with("target/") || under(&p, pkg_dir) || !known_input(&p) { + continue; + } + out.insert(p); + } + } + } + out +} + +fn rs_files(dir: &Path, out: &mut Vec, depth: u32) { + if depth > 32 { + return; + } + for e in std::fs::read_dir(dir).into_iter().flatten().flatten() { + let p = e.path(); + let name = e.file_name(); + if p.is_dir() { + if name != "target" && name != ".git" { + rs_files(&p, out, depth + 1); + } + } else if p.extension().is_some_and(|x| x == "rs") { + out.push(p); + } + } +} + +fn rel_to(root: &Path, p: &Path) -> String { + p.strip_prefix(root).unwrap_or(p).components().map(|c| c.as_os_str().to_string_lossy().into_owned()).collect::>().join("/") +} + +/// Workspace packages (xtask excluded) with their path dependencies and outside inputs. +pub fn packages(root: &Path, md: &Value, changed: &[String]) -> Vec { + let ws = md["workspace_root"].as_str().map(Path::new).unwrap_or(root); + let names: BTreeSet<&str> = md["packages"].as_array().into_iter().flatten().filter_map(|p| p["name"].as_str()).collect(); + let mut pkgs = vec![]; + for p in md["packages"].as_array().into_iter().flatten() { + let Some(name) = p["name"].as_str().filter(|n| *n != "xtask") else { continue }; + let Some(dir) = p["manifest_path"].as_str().and_then(|m| Path::new(m).parent()) else { continue }; + let deps = p["dependencies"] + .as_array() + .into_iter() + .flatten() + .filter(|d| !d["path"].is_null()) + .filter_map(|d| d["name"].as_str().filter(|n| names.contains(n)).map(|n| (n.to_string(), d["kind"] == "dev"))) + .collect(); + let lib_kind = |k: &Value| ["lib", "rlib", "dylib", "cdylib", "staticlib", "proc-macro"].iter().any(|l| k == *l); + let libs: Vec<&Value> = p["targets"].as_array().into_iter().flatten().filter(|t| t["kind"].as_array().into_iter().flatten().any(lib_kind)).collect(); + let has_lib = !libs.is_empty(); + let has_doctests = libs.iter().any(|t| t["doctest"] != false); + let rel = rel_to(ws, dir); + let mut files = vec![]; + rs_files(dir, &mut files, 0); + let known_input = |p: &str| ws.join(p).exists() || changed.iter().any(|f| under(f, p)); + let mut inputs = BTreeSet::new(); + for f in files { + if let Ok(text) = std::fs::read_to_string(&f) { + inputs.extend(inputs_of(&rel_to(ws, &f), &rel, &text, &known_input)); + } + } + pkgs.push(Pkg { name: name.to_string(), dir: rel, deps, has_lib, has_doctests, inputs: inputs.into_iter().collect() }); + } + pkgs +} + +fn git(root: &Path, args: &[&str]) -> Result { + let out = Command::new("git").arg("-C").arg(root).args(args).output().map_err(|e| format!("git: {e}"))?; + if !out.status.success() { + return Err(format!("git {}: {}", args.join(" "), String::from_utf8_lossy(&out.stderr).trim())); + } + String::from_utf8(out.stdout).map_err(|e| format!("git {}: output is not UTF-8; cannot safely map changed paths: {e}", args.join(" "))) +} + +/// The base commit: `--base REV` or the merge-base of HEAD with `main` (or `origin/main`). +fn base(root: &Path, rev: Option<&str>) -> Result<(String, String), String> { + if let Some(r) = rev { + let sha = git(root, &["rev-parse", "--verify", &format!("{r}^{{commit}}")])?; + return Ok((sha.trim().to_string(), r.to_string())); + } + for m in ["main", "origin/main"] { + if let Ok(sha) = git(root, &["merge-base", "HEAD", m]) { + return Ok((sha.trim().to_string(), format!("merge-base with {m}"))); + } + } + Err("no merge-base with main or origin/main; pass --base REV".into()) +} + +/// Committed since `base`, staged, unstaged and untracked files (repo-relative, sorted). +fn changed_files(root: &Path, base: &str) -> Result, String> { + let diff = git(root, &["diff", "--name-only", "--no-renames", "-z", base, "--"])?; + let untracked = git(root, &["ls-files", "--others", "--exclude-standard", "-z"])?; + // Git otherwise C-quotes Unicode, tabs and newlines. Whitespace is part of the filename. + let set: BTreeSet = diff.split('\0').chain(untracked.split('\0')).filter(|p| !p.is_empty()).map(str::to_string).collect(); + Ok(set.into_iter().collect()) +} + +fn short_list(names: &BTreeSet) -> String { + if names.is_empty() { "none".into() } else { names.iter().map(|n| crate::short(n)).collect::>().join(", ") } +} + +fn first_few(files: &[String]) -> String { + let mut s = files.iter().take(3).cloned().collect::>().join(", "); + if files.len() > 3 { + s.push_str(&format!(", … (+{})", files.len() - 3)); + } + s +} + +/// One CI step: label and the cargo invocation (None = an in-process check). +struct Step { + label: String, + cmd: Option, + check: Option Result<(), String>>>, +} + +fn cargo(args: &[&str], pkgs: &[&String], tail: &[&str]) -> Command { + let mut c = Command::new(env!("CARGO")); + c.args(args); + for p in pkgs { + c.args(["-p", p.as_str()]); + } + c.args(tail); + c +} + +/// Library tests and doctests are independent: a library can opt out of doctests. +fn test_targets(plan: &Plan, pkgs: &[Pkg]) -> (bool, bool) { + pkgs.iter().filter(|p| plan.affected.contains(&p.name)).fold((false, false), |(lib, docs), p| (lib || p.has_lib, docs || p.has_doctests)) +} + +fn steps(plan: &Plan, pkgs: &[Pkg], nextest: bool) -> Vec { + let mut v = vec![Step { label: "fmt".into(), cmd: Some(cargo(&["fmt", "--check"], &[], &[])), check: None }]; + let aff: Vec<&String> = plan.affected.iter().collect(); + if !aff.is_empty() { + v.push(Step { label: "audio backend".into(), cmd: None, check: Some(Box::new(crate::audio_backend)) }); + let n = aff.len(); + v.push(Step { label: format!("clippy ({n})"), cmd: Some(cargo(&["clippy", "--all-targets"], &aff, &["--", "-D", "warnings"])), check: None }); + let (has_lib, doctests) = test_targets(plan, pkgs); + let selected: Vec = aff.iter().map(|p| p.to_string()).collect(); + v.push(Step { label: format!("tests ({n})"), cmd: None, check: Some(Box::new(move || crate::tests_for(&selected, nextest, has_lib, doctests))) }); + } + v.push(Step { label: "pdf-deps".into(), cmd: None, check: Some(Box::new(|| crate::pdf_deps::run(&crate::root()))) }); + v.push(Step { label: "lock-patches".into(), cmd: None, check: Some(Box::new(|| crate::lock_patches::run(&crate::root()))) }); + v.push(Step { label: "layers".into(), cmd: None, check: Some(Box::new(crate::layers)) }); + if plan.assets { + v.push(Step { label: "assets".into(), cmd: None, check: Some(Box::new(crate::assets)) }); + } + let wasm: Vec<&String> = aff.iter().copied().filter(|a| crate::layer_of(a).is_some_and(|l| l <= 4) || crate::WEB_CRATES.contains(&a.as_str())).collect(); + if !wasm.is_empty() { + v.push(Step { label: format!("wasm ({})", wasm.len()), cmd: Some(cargo(&["check", "--target", "wasm32-unknown-unknown"], &wasm, &[])), check: None }); + } + v +} + +fn fmt_dur(d: Duration) -> String { + let s = d.as_secs(); + if s >= 60 { format!("{}m{:02}s", s / 60, s % 60) } else { format!("{:.1}s", d.as_secs_f64()) } +} + +fn summary(times: &[(String, Duration, bool)]) { + let total: Duration = times.iter().map(|t| t.1).sum(); + eprintln!("\nci --affected timing:"); + for (label, d, ok) in times { + eprintln!(" {:<16} {:>8} {}", label, fmt_dur(*d), if *ok { "ok" } else { "FAILED" }); + } + eprintln!(" {:<16} {:>8}", "total", fmt_dur(total)); +} + +pub fn run(root: &Path, args: &[String]) -> Result<(), String> { + let dry = args.iter().any(|a| a == "--dry-run"); + let rev = match args.iter().position(|a| a == "--base") { + Some(i) => Some(args.get(i + 1).map(String::as_str).ok_or("--base needs a revision")?), + None => None, + }; + let (sha, why) = base(root, rev)?; + let files = changed_files(root, &sha)?; + let md = crate::metadata()?; + let pkgs = packages(root, &md, &files); + let plan = plan(&files, &pkgs); + let short_sha = sha.get(..9).unwrap_or(&sha); + println!("ci --affected: base {short_sha} ({why}), {} changed file(s)", files.len()); + if let Some(reason) = &plan.full { + println!(" full CI: {reason}"); + if dry { + return Ok(()); + } + let t = Instant::now(); + let r = crate::ci(); + summary(&[("full ci".into(), t.elapsed(), r.is_ok())]); + return r; + } + println!(" changed: {}", short_list(&plan.direct)); + println!(" affected: {} ({} of {} crates)", short_list(&plan.affected), plan.affected.len(), pkgs.len()); + if !plan.unmapped.is_empty() { + println!(" read by no crate: {}", first_few(&plan.unmapped)); + } + let nextest = Command::new(env!("CARGO")).args(["nextest", "--version"]).output().is_ok_and(|o| o.status.success()); + let steps = steps(&plan, &pkgs, nextest); + let labels: Vec<&str> = steps.iter().map(|s| s.label.as_str()).collect(); + println!(" steps: {}{}", labels.join(", "), if plan.assets { "" } else { " (assets skipped: no asset changes)" }); + if dry { + return Ok(()); + } + let mut times = vec![]; + let mut failed = vec![]; + for mut s in steps { + let t = Instant::now(); + let r = match (s.cmd.as_mut(), s.check) { + (Some(c), _) => crate::run(c), + (None, Some(f)) => f(), + (None, None) => Ok(()), + }; + if let Err(e) = &r { + eprintln!("ci --affected: {} failed: {e}", s.label); + failed.push(s.label.clone()); + } + times.push((s.label, t.elapsed(), r.is_ok())); + } + summary(×); + if failed.is_empty() { Ok(()) } else { Err(format!("ci --affected failed: {}", failed.join(", "))) } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn pkg(name: &str, dir: &str, deps: &[(&str, bool)], inputs: &[&str]) -> Pkg { + Pkg { + name: name.into(), + dir: dir.into(), + deps: deps.iter().map(|(d, dev)| (d.to_string(), *dev)).collect(), + has_lib: true, + has_doctests: true, + inputs: inputs.iter().map(|s| s.to_string()).collect(), + } + } + + /// time ← render ← engine ← ui ← app; host has engine as a dev-dependency; text reads fonts. + fn ws() -> Vec { + vec![ + pkg("effectcraft-time", "crates/time", &[], &[]), + pkg("effectcraft-text", "crates/text", &[], &["assets/fonts"]), + pkg("effectcraft-render", "crates/render", &[("effectcraft-time", false), ("effectcraft-text", false)], &[]), + pkg("effectcraft-engine", "crates/engine", &[("effectcraft-render", false)], &["docs/preferences.md"]), + pkg("effectcraft-host", "crates/host", &[("effectcraft-time", false), ("effectcraft-engine", true)], &["crates/svg/tests/fixtures"]), + pkg("effectcraft-ui-egui", "crates/ui-egui", &[("effectcraft-engine", false)], &[]), + pkg("effectcraft", "apps/effectcraft", &[("effectcraft-ui-egui", false)], &[]), + pkg("effectcraft-cli", "apps/effectcraft-cli", &[("effectcraft", false)], &[]), + pkg("effectcraft-svg", "crates/svg", &[], &[]), + ] + } + + fn set(v: &[&str]) -> BTreeSet { + v.iter().map(|s| s.to_string()).collect() + } + + fn files(v: &[&str]) -> Vec { + v.iter().map(|s| s.to_string()).collect() + } + + #[test] + fn file_maps_to_innermost_package() { + let mut p = ws(); + p.push(pkg("nested", "crates/time/nested", &[], &[])); + assert_eq!(owner("crates/time/src/lib.rs", &p).map(|p| p.name.as_str()), Some("effectcraft-time")); + assert_eq!(owner("crates/time/nested/src/lib.rs", &p).map(|p| p.name.as_str()), Some("nested")); + // a prefix of the dir name is not a match + assert_eq!(owner("crates/timeline/src/lib.rs", &p).map(|p| p.name.as_str()), None); + assert_eq!(owner("apps/effectcraft-cli/src/main.rs", &p).map(|p| p.name.as_str()), Some("effectcraft-cli")); + assert_eq!(owner("apps/effectcraft/src/main.rs", &p).map(|p| p.name.as_str()), Some("effectcraft")); + assert!(owner("docs/testing.md", &p).is_none()); + } + + #[test] + fn reverse_deps_follow_normal_edges_and_one_dev_hop() { + let p = ws(); + // time: render, engine, ui, app, cli via normal edges; host directly; host is not + // a dependency of anything. + let a = expand(&set(&["effectcraft-time"]), &p); + assert_eq!( + a, + set(&["effectcraft-time", "effectcraft-render", "effectcraft-engine", "effectcraft-host", "effectcraft-ui-egui", "effectcraft", "effectcraft-cli"]) + ); + // ui-egui: only its dependents + assert_eq!(expand(&set(&["effectcraft-ui-egui"]), &p), set(&["effectcraft-ui-egui", "effectcraft", "effectcraft-cli"])); + // engine: host only through its dev-dependency, and nothing beyond host + let e = expand(&set(&["effectcraft-engine"]), &p); + assert!(e.contains("effectcraft-host")); + assert!(!e.contains("effectcraft-time") && !e.contains("effectcraft-render")); + assert_eq!(expand(&set(&["effectcraft-cli"]), &p), set(&["effectcraft-cli"])); + assert!(expand(&BTreeSet::new(), &p).is_empty()); + } + + #[test] + fn workspace_inputs_fall_back_to_full_ci() { + let p = ws(); + for f in ["Cargo.toml", "Cargo.lock", ".cargo/config.toml", "xtask/src/main.rs", "rust-toolchain.toml", "clippy.toml"] { + let pl = plan(&files(&["crates/time/src/lib.rs", f]), &p); + assert!(pl.full.is_some(), "{f}"); + } + // a crate manifest is not workspace-wide + assert_eq!(plan(&files(&["crates/time/Cargo.toml"]), &p).full, None); + assert_eq!(full_ci_reason("docs/xtask.md"), None); + } + + #[test] + fn nextest_policy_alone_requires_full_ci() { + let pl = plan(&files(&[".config/nextest.toml"]), &ws()); + assert_eq!(pl.full.as_deref(), Some("nextest configuration changed (.config/nextest.toml)")); + // Only the actual shared test input triggers this fallback, not every config file. + assert_eq!(full_ci_reason(".config/editor.toml"), None); + assert_eq!(full_ci_reason("docs/nextest.toml"), None); + } + + #[test] + fn docs_only_changes_affect_nothing() { + let pl = plan(&files(&["docs/gaps.md", "README.md", ".github/workflows/release.yml"]), &ws()); + assert_eq!(pl.full, None); + assert!(pl.affected.is_empty()); + assert!(!pl.assets); + assert_eq!(pl.unmapped.len(), 3); + } + + #[test] + fn files_read_from_outside_map_to_their_readers() { + let p = ws(); + // docs/preferences.md is read by an engine test + let pl = plan(&files(&["docs/preferences.md"]), &p); + assert_eq!(pl.direct, set(&["effectcraft-engine"])); + assert!(pl.unmapped.is_empty()); + // a font: read by text (directory input), and an asset + let pl = plan(&files(&["assets/fonts/Inter-Regular.ttf"]), &p); + assert_eq!(pl.direct, set(&["effectcraft-text"])); + assert!(pl.assets); + assert!(pl.affected.contains("effectcraft-cli")); + // svg fixtures belong to svg and are read by host + let pl = plan(&files(&["crates/svg/tests/fixtures/basic-shapes.svg"]), &p); + assert_eq!(pl.direct, set(&["effectcraft-svg", "effectcraft-host"])); + } + + #[test] + fn asset_rules() { + assert!(is_asset_related("ATTRIBUTION.md")); + assert!(is_asset_related("crates/engine/scripts/Rename Layers.jsx.attribution")); + assert!(is_asset_related("docs/brand/artcraft-mark.png")); + assert!(!is_asset_related("docs/testing.md")); + assert!(!is_asset_related("crates/time/src/lib.rs")); + } + + #[test] + fn rel_path_extraction() { + assert_eq!(rel_paths(r#"static F: &[u8] = include_bytes!("../../../assets/fonts/Inter-Regular.ttf");"#), ["../../../assets/fonts/Inter-Regular.ttf"]); + assert_eq!(rel_paths(r#"let p = concat!(env!("CARGO_MANIFEST_DIR"), "/../../docs/effects.md");"#), ["../../docs/effects.md"]); + assert_eq!(rel_paths(r#"format!("{}/../model/tests/fixtures/{n}", env!("CARGO_MANIFEST_DIR"))"#), ["../model/tests/fixtures"]); + assert_eq!(rel_paths(r#"let a = "x"; let b = "../y/z{n}.png";"#), ["../y"]); + assert!(rel_paths(r#"let s = "no parent here";"#).is_empty()); + } + + #[test] + fn rel_path_joining() { + assert_eq!(join_rel("crates/text/src", "../../../assets/fonts/A.ttf").as_deref(), Some("assets/fonts/A.ttf")); + assert_eq!(join_rel("crates/host", "../model/./tests").as_deref(), Some("crates/model/tests")); + assert_eq!(join_rel("crates", "../../outside"), None); + } + + #[test] + fn inputs_try_file_and_manifest_dirs() { + let existing = set(&["assets/fonts/Inter-Regular.ttf", "docs/effects.md", "crates/model/tests/fixtures", "crates/svg/tests/fixtures/a.svg"]); + let exists = |p: &str| existing.contains(p); + let src = r#" +static F: &[u8] = include_bytes!("../../../assets/fonts/Inter-Regular.ttf"); +let p = concat!(env!("CARGO_MANIFEST_DIR"), "/../../docs/effects.md"); +let fx = format!("{}/../model/tests/fixtures/{n}", env!("CARGO_MANIFEST_DIR")); +let own = include_bytes!("../tests/fixtures/own.svg"); +let out = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../target/test-out"); +// let commented = include_bytes!("../../svg/tests/fixtures/a.svg"); +"#; + let got = inputs_of("crates/host/src/lib.rs", "crates/host", src, &exists); + assert_eq!(got, set(&["assets/fonts/Inter-Regular.ttf", "docs/effects.md", "crates/model/tests/fixtures"])); + // integration tests in tests/ resolve include_bytes! against tests/ + let got = inputs_of("crates/host/tests/v.rs", "crates/host", r#"include_bytes!("../../svg/tests/fixtures/a.svg")"#, &exists); + assert_eq!(got, set(&["crates/svg/tests/fixtures/a.svg"])); + } + + fn committed_repo() -> tempfile::TempDir { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + git(root, &["init", "-q"]).unwrap(); + git(root, &["config", "core.quotepath", "true"]).unwrap(); + std::fs::write(root.join("README.md"), "fixture\n").unwrap(); + commit_fixture(root); + dir + } + + fn commit_fixture(root: &Path) { + git(root, &["add", "--all"]).unwrap(); + git(root, &["-c", "user.name=Test", "-c", "user.email=test@example.invalid", "-c", "commit.gpgsign=false", "commit", "--no-verify", "-qm", "fixture"]) + .unwrap(); + } + + #[test] + fn changed_files_preserve_unicode_in_tracked_and_untracked_paths() { + let dir = committed_repo(); + let root = dir.path(); + let tracked = "crates/time/src/caf\u{e9}.rs"; + let untracked = "crates/time/src/\u{65b0}\u{898f}.rs"; + std::fs::create_dir_all(root.join("crates/time/src")).unwrap(); + std::fs::write(root.join(tracked), "before").unwrap(); + commit_fixture(root); + std::fs::write(root.join(tracked), "after").unwrap(); + std::fs::write(root.join(untracked), "new").unwrap(); + let changed = changed_files(root, "HEAD").unwrap(); + assert_eq!(changed, files(&[tracked, untracked])); + assert_eq!(plan(&changed, &ws()).direct, set(&["effectcraft-time"])); + } + + #[cfg(unix)] + #[test] + fn changed_files_preserve_whitespace_quotes_and_backslashes() { + let dir = committed_repo(); + let root = dir.path(); + let tracked = " tracked\t\"\\name\n "; + let untracked = " untracked\t\"\\name\n "; + std::fs::write(root.join(tracked), "before").unwrap(); + commit_fixture(root); + std::fs::write(root.join(tracked), "after").unwrap(); + std::fs::write(root.join(untracked), "new").unwrap(); + assert_eq!(changed_files(root, "HEAD").unwrap(), files(&[tracked, untracked])); + } + + #[cfg(unix)] + #[test] + fn changed_files_reject_non_utf8_instead_of_silently_omitting_it() { + use std::os::unix::ffi::OsStringExt; + let dir = committed_repo(); + std::fs::write(dir.path().join(std::ffi::OsString::from_vec(b"invalid-\xff.rs".to_vec())), "new").unwrap(); + assert!(changed_files(dir.path(), "HEAD").unwrap_err().contains("not UTF-8")); + } + + #[test] + fn deleted_and_renamed_shared_inputs_still_select_their_readers() { + for rename in [false, true] { + let dir = committed_repo(); + let root = dir.path(); + std::fs::create_dir_all(root.join("crates/engine/src")).unwrap(); + std::fs::create_dir_all(root.join("docs/fixtures")).unwrap(); + std::fs::write(root.join("docs/preferences.md"), "preferences").unwrap(); + std::fs::write(root.join("docs/fixtures/example.json"), "{}").unwrap(); + std::fs::write( + root.join("crates/engine/src/lib.rs"), + r#" +const PREFS: &str = include_str!("../../../docs/preferences.md"); +fn fixture(n: &str) -> String { format!("{}/../../docs/fixtures/{n}", env!("CARGO_MANIFEST_DIR")) } +"#, + ) + .unwrap(); + commit_fixture(root); + if rename { + std::fs::rename(root.join("docs/preferences.md"), root.join("docs/renamed.md")).unwrap(); + std::fs::rename(root.join("docs/fixtures"), root.join("docs/moved")).unwrap(); + } else { + std::fs::remove_file(root.join("docs/preferences.md")).unwrap(); + std::fs::remove_dir_all(root.join("docs/fixtures")).unwrap(); + } + let changed = changed_files(root, "HEAD").unwrap(); + let md = serde_json::json!({ + "workspace_root": root, + "packages": [{ + "name": "effectcraft-engine", + "manifest_path": root.join("crates/engine/Cargo.toml"), + "targets": [{"kind": ["lib"], "doctest": true}], + "dependencies": [] + }] + }); + let pkgs = packages(root, &md, &changed); + let engine = pkgs.first().unwrap(); + assert!(engine.inputs.contains(&"docs/preferences.md".into())); + assert!(engine.inputs.contains(&"docs/fixtures".into())); + for deleted in ["docs/preferences.md", "docs/fixtures/example.json"] { + assert!(changed.contains(&deleted.to_string())); + assert_eq!(plan(&files(&[deleted]), &pkgs).direct, set(&["effectcraft-engine"])); + } + } + } + + #[test] + fn test_runner_keeps_dev_profile_target_selection_and_gpu_isolation() { + for packages in [vec![], files(&["effectcraft-gpu", "effectcraft-ui-egui"])] { + for nextest in [false, true] { + let command = crate::test_command(&packages, nextest, true); + let args: Vec<_> = command.get_args().map(|a| a.to_string_lossy().into_owned()).collect(); + assert!(!args.iter().any(|a| a == "--release" || a == "--profile")); + for target in ["--lib", "--bins", "--tests", "--no-fail-fast"] { + assert!(args.iter().any(|a| a == target)); + } + assert_eq!(args.iter().any(|a| a == "--workspace"), packages.is_empty()); + for package in &packages { + assert!(args.windows(2).any(|w| w[0] == "-p" && w[1] == *package)); + } + assert_eq!(args.windows(2).any(|w| w == ["-E", "not package(effectcraft-gpu)"]), nextest); + assert_eq!(args.iter().any(|a| a == "--no-tests=pass"), nextest && !packages.is_empty()); + } + } + } + + #[test] + fn full_workspace_test_commands_keep_library_tests() { + for nextest in [false, true] { + let command = crate::test_command(&[], nextest, false); + assert!(command.get_args().any(|a| a == "--workspace")); + assert!(command.get_args().any(|a| a == "--lib")); + } + } + + #[test] + fn bin_only_and_doctest_disabled_packages_select_the_right_test_targets() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let md = serde_json::json!({ + "workspace_root": root, + "packages": [ + { + "name": "effectcraft-cli", + "manifest_path": root.join("apps/effectcraft-cli/Cargo.toml"), + "targets": [{"kind": ["bin"], "doctest": false}], + "dependencies": [] + }, + { + "name": "effectcraft-time", + "manifest_path": root.join("crates/time/Cargo.toml"), + "targets": [{"kind": ["lib"], "doctest": false}], + "dependencies": [] + }, + { + "name": "effectcraft-color", + "manifest_path": root.join("crates/color/Cargo.toml"), + "targets": [{"kind": ["lib"], "doctest": true}], + "dependencies": [] + } + ] + }); + let pkgs = packages(root, &md, &[]); + for (name, changed, want_lib, want_doctests) in [ + ("effectcraft-cli", "apps/effectcraft-cli/src/main.rs", false, false), + ("effectcraft-time", "crates/time/src/lib.rs", true, false), + ("effectcraft-color", "crates/color/src/lib.rs", true, true), + ] { + let plan = plan(&files(&[changed]), &pkgs); + assert_eq!(plan.affected, set(&[name])); + let (has_lib, doctests) = test_targets(&plan, &pkgs); + assert_eq!((has_lib, doctests), (want_lib, want_doctests)); + let selected = plan.affected.into_iter().collect::>(); + for nextest in [false, true] { + let command = crate::test_command(&selected, nextest, has_lib); + let args: Vec<_> = command.get_args().map(|a| a.to_string_lossy().into_owned()).collect(); + assert_eq!(args.iter().any(|a| a == "--lib"), want_lib); + assert!(args.windows(2).any(|w| w == ["-p", name])); + for target in ["--bins", "--tests", "--no-fail-fast"] { + assert!(args.iter().any(|a| a == target)); + } + } + } + } + + fn labels(s: &[Step]) -> Vec { + s.iter().map(|s| s.label.clone()).collect() + } + + #[test] + fn steps_follow_the_plan() { + let mut p = ws(); + if let Some(cli) = p.iter_mut().find(|p| p.name == "effectcraft-cli") { + cli.has_lib = false; + cli.has_doctests = false; + } + // docs only: cheap checks + assert_eq!(labels(&steps(&plan(&files(&["docs/gaps.md"]), &p), &p, true)), ["fmt", "pdf-deps", "lock-patches", "layers"]); + // ui-egui (L5, a web crate) and its two app dependents; cli has no doctests + let s = steps(&plan(&files(&["crates/ui-egui/src/dock.rs"]), &p), &p, true); + assert_eq!(labels(&s), ["fmt", "audio backend", "clippy (3)", "tests (3)", "pdf-deps", "lock-patches", "layers", "wasm (1)"]); + let args = |i: usize| s.get(i).and_then(|s| s.cmd.as_ref()).map(|c| c.get_args().map(|a| a.to_string_lossy().into_owned()).collect::>()); + let clippy = args(2).unwrap_or_default(); + assert!(!clippy.iter().any(|a| a == "--release" || a == "--profile")); + assert!(clippy.windows(2).any(|w| w == ["-p", "effectcraft-ui-egui"])); + assert!(clippy.ends_with(&["--".into(), "-D".into(), "warnings".into()])); + assert!(args(7).unwrap_or_default().windows(2).any(|w| w == ["-p", "effectcraft-ui-egui"])); + // without nextest: one cargo test step; an asset change adds the assets check + let s = steps(&plan(&files(&["assets/fonts/A.ttf"]), &p), &p, false); + assert!(labels(&s).contains(&"tests (7)".to_string()) && labels(&s).contains(&"assets".to_string())); + } +} diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 06d32e51a..16e28fa07 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -13,11 +13,14 @@ //! `crates/aep/tests/fixtures/corpus.lock`, verify every file's SHA-256, install into `corpus/aep`. //! - `ico …`: pack PNGs into a Windows `.ico` (used by `packaging/icons.sh`). //! - `ci`: fmt check, clippy -D warnings, tests (`cargo nextest run` when installed) and doctests, -//! PDF dependency policy, layers, assets, wasm. Everything builds in the dev profile. +//! PDF dependency policy, lock patches, layers, assets, wasm. Everything builds in the dev profile. +//! - `ci --affected [--base REV] [--dry-run]`: the same gates for only the crates the change since +//! REV (default: the merge-base with `main`) can affect; see `affected.rs` and docs/testing.md. //! - `bench [--bin CLI] [--quick] [--compare OLD.jsonl] …`: render benchmark of generated scenarios //! (memory, CPU, wall time) into `target/bench/results.jsonl` (`bench.rs`, docs/testing.md). mod aep_corpus; +mod affected; mod bench; mod ico; mod lock_patches; @@ -480,34 +483,73 @@ fn audio_backend() -> Result<(), String> { /// /// Each run goes ahead when an earlier one fails; the error names every run that failed. fn tests() -> Result<(), String> { + let nextest = Command::new(env!("CARGO")).args(["nextest", "--version"]).output().is_ok_and(|o| o.status.success()); + tests_for(&[], nextest, true, true) +} + +/// Keep the full and affected gates on the same test runner, including GPU isolation and the +/// translation report. An empty package list selects the whole workspace. +fn tests_for(packages: &[String], nextest: bool, has_lib: bool, doctests: bool) -> Result<(), String> { let cargo = env!("CARGO"); - let targets = ["--workspace", "--lib", "--bins", "--tests", "--no-fail-fast"]; - let nextest = Command::new(cargo).args(["nextest", "--version"]).output().is_ok_and(|o| o.status.success()); - let mut runs = Vec::new(); - if nextest { - runs.push(run(Command::new(cargo).args(["nextest", "run"]).args(targets).args(["-E", "not package(effectcraft-gpu)"]))); - // effectcraft-gpu's tests share one device, so they run in one libtest process: the - // binary nextest just built (`cargo test -p effectcraft-gpu` would rebuild ~2 minutes). - runs.push(lib_test_binary("effectcraft-gpu").and_then(|mut c| run(&mut c))); - } else { + let selection = test_selection(packages); + let includes = |name: &str| packages.is_empty() || packages.iter().any(|p| p == name); + if !nextest { println!("cargo-nextest not found, running cargo test (install: cargo install cargo-nextest --locked)"); - runs.push(run(Command::new(cargo).arg("test").args(targets))); + } + let mut runs = vec![run(&mut test_command(packages, nextest, has_lib))]; + if nextest && includes("effectcraft-gpu") { + // GPU tests share one device, so reuse the selected build in one libtest process. + runs.push(lib_test_binary("effectcraft-gpu", &selection).and_then(|mut c| run(&mut c))); } // The test runs above hide a passing test's output, so the always-passing report runs again // with --nocapture, from the binary already built. - runs.push(lib_test_binary("effectcraft-ui-egui").and_then(|mut c| run(c.args(["missing_translations_report", "--nocapture"])))); - runs.push(run(Command::new(cargo).args(["test", "--workspace", "--doc", "--no-fail-fast"]))); + if includes("effectcraft-ui-egui") { + runs.push(lib_test_binary("effectcraft-ui-egui", &selection).and_then(|mut c| run(c.args(["missing_translations_report", "--nocapture"])))); + } + if doctests { + runs.push(run(Command::new(cargo).arg("test").args(&selection).args(["--doc", "--no-fail-fast"]))); + } let failed: Vec = runs.into_iter().filter_map(Result::err).collect(); if failed.is_empty() { Ok(()) } else { Err(failed.join("; ")) } } +/// Select library tests only when the selected packages contain a library; bin-only package +/// sets otherwise fail before running any tests. The full workspace always has library targets. +fn test_command(packages: &[String], nextest: bool, has_lib: bool) -> Command { + let mut c = Command::new(env!("CARGO")); + if nextest { + c.args(["nextest", "run"]); + // A selected package can have no tests, or only GPU tests that run separately. + if !packages.is_empty() { + c.arg("--no-tests=pass"); + } + } else { + c.arg("test"); + } + c.args(test_selection(packages)); + if packages.is_empty() || has_lib { + c.arg("--lib"); + } + c.args(["--bins", "--tests", "--no-fail-fast"]); + if nextest { + c.args(["-E", "not package(effectcraft-gpu)"]); + } + c +} + +fn test_selection(packages: &[String]) -> Vec { + if packages.is_empty() { vec!["--workspace".into()] } else { packages.iter().flat_map(|p| ["-p".into(), p.clone()]).collect() } +} + /// A command running `package`'s lib test binary from the test build above, the way `cargo test` -/// runs it (from the package directory), found with `--no-run` and the same workspace selection +/// runs it (from the package directory), found with `--no-run` and the same package selection /// so nothing rebuilds: `cargo test -p ` resolves features for that package alone and /// recompiles much of the workspace. -fn lib_test_binary(package: &str) -> Result { +fn lib_test_binary(package: &str, selection: &[String]) -> Result { let out = Command::new(env!("CARGO")) - .args(["test", "--workspace", "--lib", "--bins", "--tests", "--no-run", "--message-format=json-render-diagnostics"]) + .arg("test") + .args(selection) + .args(["--lib", "--bins", "--tests", "--no-run", "--message-format=json-render-diagnostics"]) .stderr(std::process::Stdio::inherit()) .output() .map_err(|e| format!("cargo test --no-run: {e}"))?; @@ -560,7 +602,10 @@ fn main() -> ExitCode { "pdf-deps" => pdf_deps::run(&root()), "lock-patches" => lock_patches::run(&root()), "aep-corpus" => aep_corpus::run(&root(), &std::env::args().skip(2).collect::>()), - "ci" => ci(), + "ci" => { + let rest: Vec = std::env::args().skip(2).collect(); + if rest.iter().any(|a| a == "--affected") { affected::run(&root(), &rest) } else { ci() } + } "audio-backend" => audio_backend(), "web" => web(&std::env::args().skip(2).collect::>()), "bench" => bench::run(&root(), &std::env::args().skip(2).collect::>()), @@ -572,7 +617,7 @@ fn main() -> ExitCode { let rest: Vec = std::env::args().skip(2).collect(); version::run(&root(), &rest.iter().map(String::as_str).collect::>()) } - _ => Err("usage: cargo xtask ".into()), + _ => Err("usage: cargo xtask ".into()), }; match r { Ok(()) => ExitCode::SUCCESS, @@ -585,7 +630,7 @@ fn main() -> ExitCode { /// The workspace root (the parent of `xtask/`). pub fn root() -> std::path::PathBuf { - std::path::Path::new(env!("CARGO_MANIFEST_DIR")).parent().expect("xtask has a parent dir").to_path_buf() + std::path::Path::new(env!("CARGO_MANIFEST_DIR")).parent().map(std::path::Path::to_path_buf).unwrap_or_else(|| std::path::PathBuf::from(".")) } #[cfg(all(test, target_os = "linux"))]