diff --git a/Cargo.toml b/Cargo.toml index 259d8478..8da5e69f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,7 +60,9 @@ enum_dispatch = "0.3.13" # binary does not grow a wasm client; `improvise-wasm-client` builds # natively, which is how its own tests already run. improvise-wasm-client = { path = "crates/improvise-wasm-client" } -improvise-protocol = { path = "crates/improvise-protocol" } +# `test-support` gives `FormulaEntry: From<&str>`, which the tests use; asked for +# here, or `-p improvise` alone does not compile (improvise-a61f). +improvise-protocol = { path = "crates/improvise-protocol", features = ["test-support"] } # `tests/headless_cli.rs` detaches the binary from any terminal with `setsid`. libc = "0.2" pest = "2.9.2" diff --git a/context/test-evidence/clients.md b/context/test-evidence/clients.md index 1b893acb..8c097931 100644 --- a/context/test-evidence/clients.md +++ b/context/test-evidence/clients.md @@ -5117,3 +5117,29 @@ Rival injected: `claim_as_read` ignoring the captured generation and claiming the current one. That is the pre-fix behaviour, restated after the fix. Only this test failed in the whole workspace. The layout tier staying current is still pinned by `an_edit_writes_the_cell_the_user_clicked_after_the_rows_renumber`. + +### a_folded_browser_commit_waits_for_the_host + +improvise-aopa. The helpers in `resolve.rs`'s tests fold what `resolve` +composed, which is what a browser **sends**. Eight of them folded it through +`ClientState::fold`, the door for what the **host** sends. That door does not +record a frame in `in_flight`, so a `Command::Model` folded there is never +awaited. Any assertion on rollback or settle state made through those helpers +would describe a client that cannot exist: the vacuity improvise-xoxh found in +its renaming test. All eight now use `fold_outgoing`, one frame per key, as +`web/app.js` and `lib.rs` do. + +The view moves the same way through either door, so no existing test could +tell them apart. This test is the cheap one that can. Through `press_folded` it +types `i` and `9`, neither of which composes a `Model`, so `in_flight` must be +empty. Then it presses `Enter`, which commits `9`, and requires exactly one +awaited frame whose commands are the commit's. + +Rival: `press_folded` folding through `fold` again. This test fails, and the +other 186 protocol tests pass. + +The other seven helpers are not pinned the same way: `heard`, `press`, the +sequence loop, the `enter-mode` and `append-char` folds, `arm` and the `yy` +fold. None of the tests that use them reads `in_flight` or `settled`, which is +why the bug was latent. A test that does read them now gets the true door +without having to know about it. diff --git a/crates/improvise-protocol/src/resolve.rs b/crates/improvise-protocol/src/resolve.rs index 981b5227..d9f4a0b0 100644 --- a/crates/improvise-protocol/src/resolve.rs +++ b/crates/improvise-protocol/src/resolve.rs @@ -2293,7 +2293,7 @@ mod tests { ..view.clone() }; let before = quiet(&state.view); - commands.iter().for_each(|c| state.fold(c)); + state.fold_outgoing(&commands); let sent = commands.iter().any(|c| matches!(c, Command::Model { .. })); let said = commands.iter().any(|c| matches!(c, Command::SetStatus(_))); match (sent || quiet(&state.view) != before, said) { @@ -2353,7 +2353,7 @@ mod tests { let (last, prefix) = sequence.split_last().expect("a sequence has a key"); for key in prefix { if let KeyOutcome::Local(commands) = resolve(&state, *key) { - commands.iter().for_each(|c| state.fold(c)); + state.fold_outgoing(&commands); } } let outcome = resolve(&state, *last); @@ -2520,7 +2520,7 @@ mod tests { let view = state.view.active_view.clone(); for key in &sequence { if let KeyOutcome::Local(commands) = resolve(&state, *key) { - commands.iter().for_each(|c| state.fold(c)); + state.fold_outgoing(&commands); } } let landed = state.view.cursor().selected; @@ -3448,9 +3448,7 @@ mod tests { let KeyOutcome::Local(commands) = resolve(client, ev(code)) else { panic!("{code:?} must resolve here"); }; - for command in &commands { - client.fold(command); - } + client.fold_outgoing(&commands); commands } @@ -3497,6 +3495,27 @@ mod tests { ); } + /// A frame this client composed goes through `fold_outgoing`, so a commit + /// waits for the host (improvise-aopa). See `context/test-evidence/clients.md`. + #[test] + fn a_folded_browser_commit_waits_for_the_host() { + let mut client = client_on_row_one("7"); + press_folded(&mut client, KeyCode::Char('i')); + press_folded(&mut client, KeyCode::Char('9')); + assert!( + client.in_flight.is_empty(), + "no Model yet, so nothing waits" + ); + let commit = press_folded(&mut client, KeyCode::Enter); + assert!( + commit.iter().any(|c| matches!(c, Command::Model { .. })), + "a typed commit writes: {commit:?}" + ); + assert_eq!(client.in_flight.len(), 1, "the commit is unanswered"); + assert!(client.in_flight[0].awaited); + assert_eq!(client.in_flight[0].cmds, commit); + } + /// improvise-465t: the editor an advance reopens starts untouched, even /// after a typed commit. See `context/test-evidence/clients.md`. #[test] @@ -3608,9 +3627,7 @@ mod tests { &["editing".into()], ) .expect("enter-mode editing composes here"); - for command in &entered { - client.fold(command); - } + client.fold_outgoing(&entered); assert!( client.view.mode.is_editing(), "enter-mode did not open an editor" @@ -3726,9 +3743,7 @@ mod tests { let KeyOutcome::Local(typed) = resolve(&client, ev(KeyCode::Char('5'))) else { panic!("a character in Editing is append-char"); }; - for command in &typed { - client.fold(command); - } + client.fold_outgoing(&typed); assert!( matches!( committed(&client), @@ -3824,9 +3839,7 @@ mod tests { let KeyOutcome::Local(commands) = resolve(state, prefix) else { panic!("{prefix:?} is a prefix; arming it must resolve locally"); }; - for command in &commands { - state.fold(command); - } + state.fold_outgoing(&commands); assert_eq!( state.view.transient_prefix, Some((prefix.code, prefix.modifiers)), @@ -3870,9 +3883,7 @@ mod tests { "the cell under the cursor reads 1200: {yank:?}" ); - for command in &yank { - state.fold(command); - } + state.fold_outgoing(&yank); assert_eq!( state.view.transient_prefix, None, "and the y prefix is spent" diff --git a/crates/improvise-wasm-client/Cargo.toml b/crates/improvise-wasm-client/Cargo.toml index 1abfbb14..34738d92 100644 --- a/crates/improvise-wasm-client/Cargo.toml +++ b/crates/improvise-wasm-client/Cargo.toml @@ -40,6 +40,9 @@ wasm-bindgen = "0.2" [dev-dependencies] improvise-command = { path = "../improvise-command" } improvise-core = { path = "../improvise-core" } +# The tests build `FormulaEntry` from `&str`, which only `test-support` provides. +# Asked for here, or `-p improvise-wasm-client` alone does not compile (a61f). +improvise-protocol = { path = "../improvise-protocol", features = ["test-support"] } improvise-ws-server = { path = "../improvise-ws-server" } # The `wasm-opt` wasm-pack downloads predates bulk-memory being enabled by