diff --git a/context/test-evidence/command.md b/context/test-evidence/command.md index 35456b6f..efcda9b0 100644 --- a/context/test-evidence/command.md +++ b/context/test-evidence/command.md @@ -4852,3 +4852,54 @@ The test also asserts `RemoveItem::kind()` is `Model(ModelChange::CellData)` before applying it, so reclassifying that effect fails here rather than silently turning this into a test of the `LayoutGeometry` arm that its sibling already covers. + +### ui::host::tests::a_rolled_back_load_tells_nobody_and_puts_every_mirror_back + +improvise-k84e, scoped to what remained after PR #109: the `Applied::Rebased` +path proper. A frame whose `LoadModel` **succeeds**, emits a `Snapshot` to every +client, and is *then* rolled back by a later command in the same frame. + +`a_rolled_back_load_leaves_no_ghost_view_and_no_false_status` already covers the +document half — `active_view` and `status_msg` go back. This covers the +**`Sessions`** half, which nothing touched: `mutating_frame` clones the whole +`Sessions` before the batch and `undo_frame` puts the clone back, so the reseed +`rebase` performed is undone with it and the computed `Snapshot`s are discarded +rather than sent. + +It reproduced nothing — the mechanism is sound, which is what the bead predicted +("should be sound", the phrasing this repo's gotcha list is made of). It is a +regression guard, and its value is entirely in the fault it can see. + +Fault injected: `self.sessions = mirrors;` replaced by `let _ = mirrors;` in +`undo_frame`. The test failed on the **projection** assertion, not its setup, +naming the asker's mirror and printing the whole correcting batch — a `RowFrame` +of `Region`/`North`/`South` and a `Document` of `"Test"`, i.e. the restored +document being *re-sent* to a client whose mirror had been left holding the +discarded one. That is precisely the user-visible symptom: a client hearing +about a load that never happened. Reverted. + +Three things make it non-vacuous: + +- **The control is the load-bearing half** + (`the_load_this_fixture_performs_rebases_every_client`). If the fixture's + `LoadModel` silently failed, `Applied::Rebased` would never run and every + assertion here would pass over nothing. The control asserts the generation + moves and that *both* clients are owed a `Snapshot`. +- **The rollback is asserted before anything is concluded from it** — the + `document_generation` equality is checked first, so "the frame was rejected" + cannot be confused with "the frame never got that far". +- **A bystander is seated where a wrong relay could move it.** A single-client + fixture cannot tell "the mirrors were restored" from "there was one mirror and + it happened to be the asker's". + +The projections are compared as an emptiness claim over `Projection`s rather +than over `Addressed`, because `project_all` can address a client with an empty +`ServerFrame::Commands`; asserting on the `Addressed` list would pass for the +wrong reason. + +### ui::host::tests::the_load_this_fixture_performs_rebases_every_client + +The control for the test above, and it is stated separately rather than folded +in so that a change making the fixture's load stop rebasing fails *here*, where +the message says what actually broke, rather than turning the rollback test +green by emptying it. diff --git a/crates/improvise-command/src/ui/host.rs b/crates/improvise-command/src/ui/host.rs index 8044f881..50d7b392 100644 --- a/crates/improvise-command/src/ui/host.rs +++ b/crates/improvise-command/src/ui/host.rs @@ -1097,6 +1097,106 @@ mod tests { ); } + /// Everything one client was told, flattened to the projections it carries. + fn projections(frames: &[Addressed], id: SessionId) -> Vec { + to(frames, id) + .into_iter() + .flat_map(|msg| match msg { + ServerFrame::Commands(cmds) => cmds, + _ => Vec::new(), + }) + .filter_map(|cmd| match cmd { + Command::Project(p) => Some(p), + _ => None, + }) + .collect() + } + + /// The control for the test below: the fixture's load really does rebase, + /// so a rollback of it has something to undo. + #[test] + fn the_load_this_fixture_performs_rebases_every_client() { + let (mut host, path) = host_over_a_loadable_document(); + let (asker, _) = host.connect(10, 10); + let (bystander, _) = host.connect(10, 10); + let was = host.app().document_generation(); + + let answer = host.model(asker, ModelCommand::LoadModel(path)); + + assert_eq!( + host.app().document_generation(), + was + 1, + "the fixture's load did not replace the document: {answer:?}" + ); + for (who, id) in [("asker", asker), ("bystander", bystander)] { + assert!( + to(&answer, id) + .iter() + .any(|m| matches!(m, ServerFrame::Snapshot(_))), + "{who} was owed a fresh Snapshot by the rebase: {answer:?}" + ); + } + } + + /// Clause 4 over the `Applied::Rebased` path: the Snapshots were computed + /// and are discarded. Evidence: `context/test-evidence/command.md`. + #[test] + fn a_rolled_back_load_tells_nobody_and_puts_every_mirror_back() { + let (mut host, path) = host_over_a_loadable_document(); + let (asker, _) = host.connect(10, 10); + let (bystander, _) = host.connect(10, 10); + let was_generation = host.app().document_generation(); + let basis = frame_basis(&host, &["North"]); + + // The load lands and rebases every mirror; the write then names a + // record the incoming document has never had, so the frame fails. + let answer = host.handle( + asker, + ClientFrame::Commands(vec![ + Command::Model { + basis: basis.clone(), + cmd: ModelCommand::LoadModel(path), + }, + Command::Model { + basis, + cmd: ModelCommand::SetCell { + cell: addressed("North"), + value: CellValue::Number(5.0), + }, + }, + ]), + ); + + assert_eq!( + host.app().document_generation(), + was_generation, + "the document did not go back, so nothing below is about a rollback" + ); + assert!( + !answer + .iter() + .any(|a| matches!(a.msg, ServerFrame::Snapshot(_))), + "a rolled-back load let its Snapshots out: {answer:?}" + ); + assert!( + to(&answer, bystander).is_empty(), + "a client that asked for nothing was told about a frame that did \ + nothing: {answer:?}" + ); + + // Nothing moved, so the next fan-out owes nobody anything. An + // unrestored mirror is corrected here instead — test-evidence. + let after = host.project_all(); + for (who, id) in [("asker", asker), ("bystander", bystander)] { + assert!( + projections(&after, id).is_empty(), + "{who}'s mirror was left describing the discarded document, so \ + the next projection corrects it: {:?}", + projections(&after, id) + ); + } + } + /// `batch_aborted` is reset only inside `apply_effects`, so a frame that /// runs none reads a STALE flag. Evidence: `context/test-evidence/command.md`. #[test]