Test the dispatch panic guard end to end; mark REL-3 done (#1803)

A cfg(test)-only test.panic command drives Session::execute into the catch_unwind
guard in jobs::dispatch; the test asserts an internal-error Err, an untouched document
and history, and a working session afterwards. REL-3 flips to done.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Reynold Hu 2026-10-09 23:51:51 +08:00 committed by GitHub
parent d8d2df4fba
commit d7d5c6069c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 35 additions and 4 deletions

View File

@ -1170,9 +1170,23 @@ fn build() -> Vec<CommandSpec> {
v.extend(crate::fx_visibility_cmds::specs());
v.extend(crate::mask_view_cmds::specs());
v.extend(crate::actions_cmds::specs());
// The dispatch guard's unit test needs a command that panics on purpose (REL-3).
#[cfg(test)]
v.push(test_panic_command());
v
}
/// A command whose `run` panics on purpose, so the last-resort guard in `jobs::dispatch`
/// can be tested (REL-3). Test builds only: `commands::build` pushes it under `cfg(test)`.
#[cfg(test)]
fn test_panic_command() -> CommandSpec {
#[allow(clippy::panic)]
fn run(_: &mut Session, _: &Value) -> Result<Value> {
panic!("test: this command panicked on purpose")
}
CommandSpec { id: "test.panic", label: "Test Panic", menu: &[], shortcut: None, params: r##"{}"##, enabled: |_: &Session| Ok(()), run, journal: false }
}
/// Commands generated from a table share one `run` fn; the kind is recovered from the id.
pub(crate) fn inject_kind(id: &str, params: Value) -> Value {
let kind = id.strip_prefix("layer.newAdjustmentLayer.").or_else(|| id.strip_prefix("image.adjustments."));

View File

@ -126,6 +126,23 @@ fn a_panic_in_the_worker_becomes_an_error_and_the_document_is_unchanged() {
s.execute("filter.blur.gaussianBlur", json!({"radius": 1})).unwrap();
}
#[test]
fn a_panic_in_a_command_becomes_an_error_and_the_document_is_unchanged() {
// REL-3: the last-resort guard in `jobs::dispatch` catches a panic that escapes `run`,
// reports it as an error and leaves the document (and its history) untouched.
let mut s = session(64, 64);
let doc_before = s.active().unwrap().doc.clone();
let (rev, steps) = (s.active().unwrap().revision, s.active().unwrap().history.past_len());
let msg = s.execute("test.panic", json!({})).unwrap_err().to_string();
assert!(msg.contains("internal error"), "{msg}");
assert!(msg.contains("test.panic"), "{msg}");
let st = s.active().unwrap();
assert!(Arc::ptr_eq(&st.doc, &doc_before), "the document snapshot was not replaced");
assert_eq!((st.revision, st.history.past_len()), (rev, steps));
// The session still works.
s.execute("filter.blur.gaussianBlur", json!({"radius": 1})).unwrap();
}
#[test]
fn progress_is_monotonic_and_reaches_one() {
let ctx = JobCtx::new();

View File

@ -12,7 +12,7 @@ Generated by `cargo xtask scorecard` from `perf/budgets.toml`, `perf/baseline.js
| [Workspace and UI](#workspace-and-ui) | 1 | 7 | 15 | settings that do nothing: 55 of 141 (target 0) |
| [Type](#type) | 0 | 3 | 6 | |
| [Automation](#automation) | 3 | 2 | 6 | 627 of 627 menu items live (100.0%) |
| [Reliability](#reliability) | 4 | 1 | 2 | never-crash lints on 28 of 28 crates; panic_hunt present |
| [Reliability](#reliability) | 5 | 0 | 2 | never-crash lints on 28 of 28 crates; panic_hunt present |
| [Distribution](#distribution) | 4 | 4 | 3 | |
| [Open bugs](#open-bugs) | 22 | 2 | 8 | |
@ -228,7 +228,7 @@ Never-crash attribute on 28 of 28 crate roots.
|---|---|---|---|---|
| REL-1 | panic_hunt fuzzes every command with adversarial params | done | | engine/tests/panic_hunt.rs: every command, own thread, 4 s timeout; #[ignore], run in the gate |
| REL-2 | Every workspace crate carries the never-crash deny(clippy::unwrap_used, ...) attribute | done | | Counted automatically above (crate roots under crates/ and apps/); testkit gained it in #221 |
| REL-3 | The app catches escaped panics around command dispatch and import/export | partial | | engine lib.rs:336 dispatch; crash_guard.rs:21 import/export; the dispatch guard is untested |
| REL-3 | The app catches escaped panics around command dispatch and import/export | done | | a_panic_in_a_command_... runs the test-only `test.panic` spec through Session::execute and asserts Err plus an unchanged document (engine jobs_tests.rs); the worker guard is covered by a_panic_in_the_worker_...; crash_guard.rs guard_turns_a_panic_into_an_error covers import/export (apps/photocraft) |
| REL-4 | Atomic save (temp, fsync, rename) for every document write | done | [#203](https://github.com/storytold/photocraft/issues/203) | photocraft_format::atomic_write (format/src/atomic.rs, #230): same-dir temp, sync_all, rename with Windows PermissionDenied retry, Unix dir fsync; every document write site routes through it and format/src/atomic/tests.rs failure-injects write, rename and read-only-dir errors |
| REL-212-1 | Fallible allocation for paste/erase/fill/transform/save/import, clear out-of-memory error | missing | [#212](https://github.com/storytold/photocraft/issues/212) | try_reserve only in io/flat.rs:221; codec max_alloc limits cover decode only |
| REL-212-2 | Export switches to PSB when the estimated file size exceeds 2 GB | done | [#212](https://github.com/storytold/photocraft/issues/212) | PSD export uses checked size estimates for raster, masks, channels, and retained metadata |

View File

@ -20,8 +20,8 @@ note = "Counted automatically above (crate roots under crates/ and apps/); testk
[[item]]
id = "REL-3"
target = "The app catches escaped panics around command dispatch and import/export"
status = "partial"
note = "engine lib.rs:336 dispatch; crash_guard.rs:21 import/export; the dispatch guard is untested"
status = "done"
note = "a_panic_in_a_command_... runs the test-only `test.panic` spec through Session::execute and asserts Err plus an unchanged document (engine jobs_tests.rs); the worker guard is covered by a_panic_in_the_worker_...; crash_guard.rs guard_turns_a_panic_into_an_error covers import/export (apps/photocraft)"
[[item]]
id = "REL-4"