From b08280515835cbc3c04e47b3fa7de648b800a7dd Mon Sep 17 00:00:00 2001 From: H-Chris233 Date: Fri, 8 May 2026 15:35:34 +0800 Subject: [PATCH] Close backend test blind spots from review The first #295 pass made cargo tests visible but still left the reviewer-requested state-machine, modifier-edge, and insertion fallback contracts too implicit. This adds narrow unit coverage around those contracts without refactoring runtime flow.\n\nConstraint: Address review compliance with minimal production changes.\nRejected: Split coordinator or hotkey adapters for testability | broader than the issue and riskier for the active PR.\nConfidence: high\nScope-risk: narrow\nTested: cargo test --manifest-path openless-all/app/src-tauri/Cargo.toml --lib; cargo check --manifest-path src-tauri/Cargo.toml; npm run build\nNot-tested: Live macOS/Windows native hotkey hooks and clipboard/AX behavior. --- openless-all/app/src-tauri/src/coordinator.rs | 94 ++++++ openless-all/app/src-tauri/src/hotkey.rs | 270 +++++++++++++++++- openless-all/app/src-tauri/src/insertion.rs | 81 +++++- 3 files changed, 435 insertions(+), 10 deletions(-) diff --git a/openless-all/app/src-tauri/src/coordinator.rs b/openless-all/app/src-tauri/src/coordinator.rs index 2bc1b26d..fd85a4cf 100644 --- a/openless-all/app/src-tauri/src/coordinator.rs +++ b/openless-all/app/src-tauri/src/coordinator.rs @@ -4047,6 +4047,9 @@ fn resolve_ark_endpoint_with_policy( mod tests { use super::*; use crate::types::HotkeyTrigger; + use once_cell::sync::Lazy; + + static ENV_LOCK: Lazy> = Lazy::new(|| tokio::sync::Mutex::new(())); fn session_id(n: u128) -> SessionId { Uuid::from_u128(n) @@ -4058,6 +4061,7 @@ mod tests { .filter_level(log::LevelFilter::Info) .is_test(false) .try_init(); + let _guard = ENV_LOCK.lock().await; std::env::set_var("OPENLESS_HOTKEY_INJECTION_DRY_RUN", "1"); let coordinator = Coordinator::new(); @@ -4067,6 +4071,52 @@ mod tests { std::env::remove_var("OPENLESS_HOTKEY_INJECTION_DRY_RUN"); } + #[tokio::test] + async fn begin_session_dry_run_enters_listening_and_clears_stale_edges() { + let _guard = ENV_LOCK.lock().await; + std::env::set_var("OPENLESS_HOTKEY_INJECTION_DRY_RUN", "1"); + + let coordinator = Coordinator::new(); + let old_session_id = coordinator.inner.state.lock().session_id; + { + let mut state = coordinator.inner.state.lock(); + state.pending_stop = true; + state.cancelled = true; + } + + coordinator.start_dictation().await.unwrap(); + + let state = coordinator.inner.state.lock(); + assert_eq!(state.phase, SessionPhase::Listening); + assert!(!state.pending_stop); + assert!(!state.cancelled); + assert_ne!(state.session_id, old_session_id); + + std::env::remove_var("OPENLESS_HOTKEY_INJECTION_DRY_RUN"); + } + + #[tokio::test] + async fn begin_session_ignores_non_idle_phase() { + let _guard = ENV_LOCK.lock().await; + std::env::set_var("OPENLESS_HOTKEY_INJECTION_DRY_RUN", "1"); + + let coordinator = Coordinator::new(); + let old_session_id = { + let mut state = coordinator.inner.state.lock(); + state.phase = SessionPhase::Processing; + state.session_id = session_id(99); + state.session_id + }; + + coordinator.start_dictation().await.unwrap(); + + let state = coordinator.inner.state.lock(); + assert_eq!(state.phase, SessionPhase::Processing); + assert_eq!(state.session_id, old_session_id); + + std::env::remove_var("OPENLESS_HOTKEY_INJECTION_DRY_RUN"); + } + #[test] fn window_key_matcher_mirrors_windows_trigger_aliases() { let cases = [ @@ -4245,6 +4295,50 @@ mod tests { assert!(state.pending_stop); } + #[tokio::test] + async fn stop_dictation_from_listening_without_asr_returns_idle() { + let coordinator = Coordinator::new(); + { + let mut state = coordinator.inner.state.lock(); + state.phase = SessionPhase::Listening; + state.session_id = session_id(123); + } + + coordinator.stop_dictation().await.unwrap(); + + assert_eq!(coordinator.inner.state.lock().phase, SessionPhase::Idle); + } + + #[test] + fn cancel_session_state_machine_is_table_driven() { + let cases = [ + (SessionPhase::Idle, SessionPhase::Idle, false), + (SessionPhase::Starting, SessionPhase::Idle, true), + (SessionPhase::Listening, SessionPhase::Idle, true), + (SessionPhase::Processing, SessionPhase::Processing, true), + (SessionPhase::Inserting, SessionPhase::Inserting, false), + ]; + + for (initial, expected_phase, expected_cancelled) in cases { + let coordinator = Coordinator::new(); + { + let mut state = coordinator.inner.state.lock(); + state.phase = initial; + state.cancelled = false; + state.focus_target = Some(1); + } + + coordinator.cancel_dictation(); + + let state = coordinator.inner.state.lock(); + assert_eq!(state.phase, expected_phase, "initial={initial:?}"); + assert_eq!(state.cancelled, expected_cancelled, "initial={initial:?}"); + if matches!(initial, SessionPhase::Starting | SessionPhase::Listening) { + assert!(state.focus_target.is_none(), "initial={initial:?}"); + } + } + } + #[test] fn recorder_runtime_error_aborts_active_session() { let coordinator = Coordinator::new(); diff --git a/openless-all/app/src-tauri/src/hotkey.rs b/openless-all/app/src-tauri/src/hotkey.rs index afbf8c66..38bc516f 100644 --- a/openless-all/app/src-tauri/src/hotkey.rs +++ b/openless-all/app/src-tauri/src/hotkey.rs @@ -20,7 +20,7 @@ use parking_lot::RwLock; use crate::types::HotkeyTrigger; use crate::types::{HotkeyAdapterKind, HotkeyBinding, HotkeyCapability, HotkeyInstallError}; -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum HotkeyEvent { Pressed, Released, @@ -592,6 +592,93 @@ mod platform { HotkeyTrigger::Custom => unreachable!("custom combo hotkeys use ComboHotkeyMonitor"), } } + + #[cfg(test)] + mod tests { + use super::*; + use parking_lot::RwLock; + use std::sync::atomic::AtomicBool; + use std::sync::mpsc; + + fn shared(trigger: HotkeyTrigger) -> Arc { + Arc::new(Shared { + binding: RwLock::new(HotkeyBinding { + trigger, + mode: crate::types::HotkeyMode::Toggle, + keys: None, + }), + trigger_held: AtomicBool::new(false), + qa_trigger: RwLock::new(None), + qa_trigger_held: AtomicBool::new(false), + translation_trigger: RwLock::new(None), + translation_trigger_held: AtomicBool::new(false), + translation_modifier_held: AtomicBool::new(false), + }) + } + + fn callback_context(shared: Arc) -> (CallbackContext, mpsc::Receiver) { + let (tx, rx) = mpsc::channel(); + ( + CallbackContext { + shared, + tx, + tap: std::sync::Mutex::new(None), + }, + rx, + ) + } + + fn drain(rx: &mpsc::Receiver) -> Vec { + rx.try_iter().collect() + } + + #[test] + fn mac_optional_modifier_edges_are_deduped_from_mock_flags() { + let shared = shared(HotkeyTrigger::RightControl); + let (ctx, rx) = callback_context(Arc::clone(&shared)); + + handle_optional_modifier_trigger( + &ctx, + trigger_to_keycode(HotkeyTrigger::RightCommand), + trigger_to_flag_mask(HotkeyTrigger::RightCommand), + Some(HotkeyTrigger::RightCommand), + &shared.qa_trigger_held, + HotkeyEvent::QaShortcutPressed, + ); + handle_optional_modifier_trigger( + &ctx, + trigger_to_keycode(HotkeyTrigger::RightCommand), + trigger_to_flag_mask(HotkeyTrigger::RightCommand), + Some(HotkeyTrigger::RightCommand), + &shared.qa_trigger_held, + HotkeyEvent::QaShortcutPressed, + ); + handle_optional_modifier_trigger( + &ctx, + trigger_to_keycode(HotkeyTrigger::RightCommand), + 0, + Some(HotkeyTrigger::RightCommand), + &shared.qa_trigger_held, + HotkeyEvent::QaShortcutPressed, + ); + handle_optional_modifier_trigger( + &ctx, + trigger_to_keycode(HotkeyTrigger::RightCommand), + trigger_to_flag_mask(HotkeyTrigger::RightCommand), + Some(HotkeyTrigger::RightCommand), + &shared.qa_trigger_held, + HotkeyEvent::QaShortcutPressed, + ); + + assert_eq!( + drain(&rx), + vec![ + HotkeyEvent::QaShortcutPressed, + HotkeyEvent::QaShortcutPressed, + ] + ); + } + } } // ─────────────────────────── Windows implementation ─────────────────────────── @@ -899,6 +986,88 @@ mod platform { fn accept_injected_events() -> bool { std::env::var(ACCEPT_INJECTED_ENV).ok().as_deref() == Some("1") } + + #[cfg(test)] + mod tests { + use super::*; + use parking_lot::RwLock; + use std::sync::atomic::AtomicBool; + use std::sync::mpsc; + + fn shared(trigger: HotkeyTrigger) -> Arc { + Arc::new(Shared { + binding: RwLock::new(HotkeyBinding { + trigger, + mode: crate::types::HotkeyMode::Toggle, + keys: None, + }), + trigger_held: AtomicBool::new(false), + qa_trigger: RwLock::new(None), + qa_trigger_held: AtomicBool::new(false), + translation_trigger: RwLock::new(None), + translation_trigger_held: AtomicBool::new(false), + translation_modifier_held: AtomicBool::new(false), + }) + } + + fn callback_context(shared: Arc) -> (CallbackContext, mpsc::Receiver) { + let (tx, rx) = mpsc::channel(); + ( + CallbackContext { + shared, + tx, + hook: std::sync::Mutex::new(None), + }, + rx, + ) + } + + fn drain(rx: &mpsc::Receiver) -> Vec { + rx.try_iter().collect() + } + + #[test] + fn windows_modifier_edges_are_deduped_from_mock_hook_events() { + let shared = shared(HotkeyTrigger::RightControl); + let (ctx, rx) = callback_context(shared); + + assert!(dispatch_keyboard_event(&ctx, VK_RCONTROL, WM_KEYDOWN)); + assert!(dispatch_keyboard_event(&ctx, VK_RCONTROL, WM_KEYDOWN)); + assert!(dispatch_keyboard_event(&ctx, VK_RCONTROL, WM_KEYUP)); + assert!(dispatch_keyboard_event(&ctx, VK_RCONTROL, WM_KEYUP)); + + assert_eq!( + drain(&rx), + vec![HotkeyEvent::Pressed, HotkeyEvent::Released] + ); + } + + #[test] + fn windows_optional_modifier_shortcuts_use_independent_latches() { + let shared = shared(HotkeyTrigger::RightControl); + *shared.qa_trigger.write() = Some(HotkeyTrigger::RightCommand); + *shared.translation_trigger.write() = Some(HotkeyTrigger::LeftOption); + let (ctx, rx) = callback_context(shared); + + dispatch_keyboard_event(&ctx, VK_RWIN, WM_KEYDOWN); + dispatch_keyboard_event(&ctx, VK_RWIN, WM_KEYDOWN); + dispatch_keyboard_event(&ctx, VK_RMENU, WM_KEYDOWN); + dispatch_keyboard_event(&ctx, VK_LSHIFT, WM_KEYDOWN); + dispatch_keyboard_event(&ctx, VK_LSHIFT, WM_KEYDOWN); + dispatch_keyboard_event(&ctx, VK_RWIN, WM_KEYUP); + dispatch_keyboard_event(&ctx, VK_RWIN, WM_KEYDOWN); + + assert_eq!( + drain(&rx), + vec![ + HotkeyEvent::QaShortcutPressed, + HotkeyEvent::TranslationModifierPressed, + HotkeyEvent::TranslationModifierPressed, + HotkeyEvent::QaShortcutPressed, + ] + ); + } + } } // ─────────────────────────── Linux / other implementation ─────────────────────────── @@ -1114,4 +1283,103 @@ mod platform { HotkeyTrigger::Custom => unreachable!("custom combo hotkeys use ComboHotkeyMonitor"), } } + + #[cfg(test)] + mod tests { + use super::*; + use parking_lot::RwLock; + use std::sync::atomic::AtomicBool; + use std::sync::mpsc; + use std::time::SystemTime; + + fn shared(trigger: HotkeyTrigger) -> Shared { + Shared { + binding: RwLock::new(HotkeyBinding { + trigger, + mode: crate::types::HotkeyMode::Toggle, + keys: None, + }), + trigger_held: AtomicBool::new(false), + qa_trigger: RwLock::new(None), + qa_trigger_held: AtomicBool::new(false), + translation_trigger: RwLock::new(None), + translation_trigger_held: AtomicBool::new(false), + translation_modifier_held: AtomicBool::new(false), + } + } + + fn key_event(event_type: EventType) -> Event { + Event { + time: SystemTime::UNIX_EPOCH, + name: None, + event_type, + } + } + + fn drain(rx: &mpsc::Receiver) -> Vec { + rx.try_iter().collect() + } + + #[test] + fn rdev_modifier_edges_are_deduped_from_mock_events() { + let shared = shared(HotkeyTrigger::RightControl); + let (tx, rx) = mpsc::channel(); + + dispatch_event( + &shared, + &tx, + key_event(EventType::KeyPress(Key::ControlRight)), + ); + dispatch_event( + &shared, + &tx, + key_event(EventType::KeyPress(Key::ControlRight)), + ); + dispatch_event( + &shared, + &tx, + key_event(EventType::KeyRelease(Key::ControlRight)), + ); + dispatch_event( + &shared, + &tx, + key_event(EventType::KeyRelease(Key::ControlRight)), + ); + + assert_eq!( + drain(&rx), + vec![HotkeyEvent::Pressed, HotkeyEvent::Released] + ); + } + + #[test] + fn rdev_optional_modifier_shortcuts_use_independent_latches() { + let shared = shared(HotkeyTrigger::RightControl); + *shared.qa_trigger.write() = Some(HotkeyTrigger::RightCommand); + *shared.translation_trigger.write() = Some(HotkeyTrigger::LeftOption); + let (tx, rx) = mpsc::channel(); + + dispatch_event(&shared, &tx, key_event(EventType::KeyPress(Key::MetaRight))); + dispatch_event(&shared, &tx, key_event(EventType::KeyPress(Key::MetaRight))); + dispatch_event(&shared, &tx, key_event(EventType::KeyPress(Key::Alt))); + dispatch_event(&shared, &tx, key_event(EventType::KeyPress(Key::ShiftLeft))); + dispatch_event(&shared, &tx, key_event(EventType::KeyPress(Key::ShiftLeft))); + dispatch_event( + &shared, + &tx, + key_event(EventType::KeyRelease(Key::MetaRight)), + ); + dispatch_event(&shared, &tx, key_event(EventType::KeyPress(Key::MetaRight))); + + assert_eq!( + drain(&rx), + vec![ + HotkeyEvent::QaShortcutPressed, + HotkeyEvent::TranslationModifierPressed, + HotkeyEvent::TranslationModifierPressed, + HotkeyEvent::QaShortcutPressed, + ] + ); + } + } } diff --git a/openless-all/app/src-tauri/src/insertion.rs b/openless-all/app/src-tauri/src/insertion.rs index 04fee283..da36ccd4 100644 --- a/openless-all/app/src-tauri/src/insertion.rs +++ b/openless-all/app/src-tauri/src/insertion.rs @@ -78,11 +78,7 @@ impl TextInserter { if !copy_to_clipboard(text) { return InsertStatus::Failed; } - if let Err(err) = simulate_paste() { - log::warn!("[insertion] simulated paste failed: {}", err); - return InsertStatus::CopiedFallback; - } - insertion_success_status() + macos_insert_status_after_paste(simulate_paste()) } /// Copy text without attempting a synthetic paste. Used when the platform cannot @@ -99,6 +95,17 @@ impl TextInserter { } } +#[cfg(target_os = "macos")] +fn macos_insert_status_after_paste(result: Result<(), String>) -> InsertStatus { + match result { + Ok(()) => insertion_success_status(), + Err(err) => { + log::warn!("[insertion] simulated paste failed: {}", err); + InsertStatus::CopiedFallback + } + } +} + impl Default for TextInserter { fn default() -> Self { Self::new() @@ -187,22 +194,29 @@ fn insert_with_clipboard_restore(text: &str, restore_clipboard_after_paste: bool #[cfg(not(target_os = "macos"))] fn schedule_clipboard_restore(plan: ClipboardRestorePlan) { + let (restore_id, original_text) = + remember_pending_clipboard_restore(plan.previous_text.clone()); + std::thread::spawn(move || { + restore_clipboard_after_delay(plan, original_text, restore_id, CLIPBOARD_RESTORE_DELAY) + }); +} + +#[cfg(not(target_os = "macos"))] +fn remember_pending_clipboard_restore(previous_text: Option) -> (u64, Option) { let restore_id = NEXT_CLIPBOARD_RESTORE_ID.fetch_add(1, Ordering::SeqCst); let original_text = { let mut pending = PENDING_CLIPBOARD_RESTORE.lock(); let original = pending .as_ref() .map(|batch| batch.original_text.clone()) - .unwrap_or_else(|| plan.previous_text.clone()); + .unwrap_or(previous_text); *pending = Some(PendingClipboardRestore { latest_restore_id: restore_id, original_text: original.clone(), }); original }; - std::thread::spawn(move || { - restore_clipboard_after_delay(plan, original_text, restore_id, CLIPBOARD_RESTORE_DELAY) - }); + (restore_id, original_text) } #[cfg(not(target_os = "macos"))] @@ -468,6 +482,55 @@ mod tests { assert_eq!(inserter.copy_fallback(""), InsertStatus::CopiedFallback); } + #[test] + #[cfg(not(target_os = "macos"))] + fn pending_clipboard_restore_keeps_first_original_until_latest_restore() { + *PENDING_CLIPBOARD_RESTORE.lock() = None; + + let (first_id, first_original) = + remember_pending_clipboard_restore(Some("user clipboard".to_string())); + let (second_id, second_original) = + remember_pending_clipboard_restore(Some("first dictated text".to_string())); + + assert_ne!(first_id, second_id); + assert_eq!(first_original.as_deref(), Some("user clipboard")); + assert_eq!(second_original.as_deref(), Some("user clipboard")); + assert!(!is_latest_clipboard_restore(first_id)); + assert!(is_latest_clipboard_restore(second_id)); + + clear_pending_clipboard_restore(first_id); + assert!(is_latest_clipboard_restore(second_id)); + clear_pending_clipboard_restore(second_id); + assert!(!is_latest_clipboard_restore(second_id)); + } + + #[test] + #[cfg(not(target_os = "macos"))] + fn clipboard_restore_skips_when_clipboard_no_longer_matches_inserted_text() { + assert!(should_restore_clipboard( + Some("dictated text"), + "dictated text" + )); + assert!(!should_restore_clipboard( + Some("user edited clipboard"), + "dictated text" + )); + assert!(!should_restore_clipboard(None, "dictated text")); + } + + #[test] + #[cfg(target_os = "macos")] + fn macos_paste_failure_keeps_copied_fallback_available() { + assert_eq!( + macos_insert_status_after_paste(Ok(())), + InsertStatus::Inserted + ); + assert_eq!( + macos_insert_status_after_paste(Err("AX write unavailable".to_string())), + InsertStatus::CopiedFallback + ); + } + #[test] #[cfg(target_os = "windows")] fn delayed_terminal_paste_must_see_dictated_text_before_clipboard_restore() {