From da907c0f4670849b009c621838ba1432633eafb8 Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Fri, 27 Feb 2026 05:39:33 +0000 Subject: [PATCH] Prevent tests from writing to user data --- ...6-prevent-writing-to-user-data-in-tests.md | 35 ++++++ src/app.rs | 119 ++++++++++++++++-- src/main.rs | 77 ++++-------- src/ui/components/dashboard.rs | 43 +++++-- src/ui/components/skill_tree.rs | 46 +------ src/ui/components/stats_dashboard.rs | 119 ++++++++++++------ src/ui/layout.rs | 46 +++++++ 7 files changed, 330 insertions(+), 155 deletions(-) create mode 100644 docs/plans/2026-02-26-prevent-writing-to-user-data-in-tests.md diff --git a/docs/plans/2026-02-26-prevent-writing-to-user-data-in-tests.md b/docs/plans/2026-02-26-prevent-writing-to-user-data-in-tests.md new file mode 100644 index 0000000..34b0473 --- /dev/null +++ b/docs/plans/2026-02-26-prevent-writing-to-user-data-in-tests.md @@ -0,0 +1,35 @@ +# Prevent Tests from Writing to Real User Data + +## Context + +Two tests in `src/app.rs` (`adaptive_auto_continue_arms_input_lock` and `adaptive_does_not_auto_continue_with_milestones`) call `App::new()` which connects to the real `JsonStore` at `~/.local/share/keydr/`. When they call `finish_drill()` → `save_data()`, fake drill results get persisted to the user's actual history file. All other app tests also use `App::new()` but happen to not call `finish_drill()`. + +## Changes + +### 1. Add `#[cfg(not(test))]` gate on `App::new()` (`src/app.rs:293`) + +Mark `App::new()` with `#[cfg(not(test))]` so it cannot be called from test code at all. This is a compile-time guarantee — any future test that tries `App::new()` will fail to compile. + +### 2. Add `App::new_test()` (`src/app.rs`, in `#[cfg(test)]` block) + +Add a `pub fn new_test()` constructor inside a `#[cfg(test)] impl App` block that mirrors `App::new()` but sets `store: None`. This prevents any persistence to disk. All existing fields get their default/empty values (no loading from disk either). + +Since most test fields just need defaults and a started drill, the test constructor can be minimal: +- `Config::default()`, `Theme::default()` (leaked), `Menu::new()`, `store: None` +- Default key stats, skill tree, profile, empty drill history +- `Dictionary::load()`, `TransitionTable`, `KeyboardModel` — same as production (needed for `start_drill()`) +- Call `start_drill()` at the end (same as `App::new()`) + +### 3. Update all existing tests to use `App::new_test()` + +Replace every `App::new()` call in the test module with `App::new_test()`. This covers all 7 tests in `#[cfg(test)] mod tests`. + +## File to Modify + +- `src/app.rs` — gate `new()`, add `new_test()`, update test calls + +## Verification + +1. `cargo test` — all tests pass +2. `cargo build` — production build still compiles (ungated `new()` available) +3. Temporarily add `App::new()` in a test → should fail to compile diff --git a/src/app.rs b/src/app.rs index 1cbd2de..b7e8c79 100644 --- a/src/app.rs +++ b/src/app.rs @@ -2171,6 +2171,111 @@ fn insert_line_breaks(text: &str) -> String { result } +#[cfg(test)] +impl App { + pub fn new_test() -> Self { + let config = Config::default(); + let theme: &'static Theme = Box::leak(Box::new(Theme::default())); + let menu = Menu::new(theme); + let dictionary = Dictionary::load(); + let transition_table = TransitionTable::build_from_words(&dictionary.words_list()); + let keyboard_model = KeyboardModel::from_name(&config.keyboard_layout); + + let mut app = Self { + screen: AppScreen::Menu, + drill_mode: DrillMode::Adaptive, + drill_scope: DrillScope::Global, + drill: None, + drill_events: Vec::new(), + last_result: None, + drill_history: Vec::new(), + menu, + theme, + config, + key_stats: KeyStatsStore::default(), + ranked_key_stats: KeyStatsStore::default(), + skill_tree: SkillTree::default(), + profile: ProfileData::default(), + store: None, + should_quit: false, + settings_selected: 0, + settings_editing_download_dir: false, + stats_tab: 0, + depressed_keys: HashSet::new(), + last_key_time: None, + history_selected: 0, + history_confirm_delete: false, + skill_tree_selected: 0, + skill_tree_detail_scroll: 0, + drill_source_info: None, + code_language_selected: 0, + code_language_scroll: 0, + passage_book_selected: 0, + passage_intro_selected: 0, + passage_intro_downloads_enabled: false, + passage_intro_download_dir: String::new(), + passage_intro_paragraph_limit: 0, + passage_intro_downloading: false, + passage_intro_download_total: 0, + passage_intro_downloaded: 0, + passage_intro_current_book: String::new(), + passage_intro_download_bytes: 0, + passage_intro_download_bytes_total: 0, + passage_download_queue: Vec::new(), + passage_drill_selection_override: None, + last_passage_drill_selection: None, + passage_download_action: PassageDownloadCompleteAction::StartPassageDrill, + code_intro_selected: 0, + code_intro_downloads_enabled: false, + code_intro_download_dir: String::new(), + code_intro_snippets_per_repo: 0, + code_intro_downloading: false, + code_intro_download_total: 0, + code_intro_downloaded: 0, + code_intro_current_repo: String::new(), + code_intro_download_bytes: 0, + code_intro_download_bytes_total: 0, + code_download_queue: Vec::new(), + code_drill_language_override: None, + last_code_drill_language: None, + code_download_attempted: false, + code_download_action: CodeDownloadCompleteAction::StartCodeDrill, + shift_held: false, + caps_lock: false, + keyboard_model, + milestone_queue: VecDeque::new(), + settings_confirm_import: false, + settings_export_conflict: false, + settings_status_message: None, + settings_export_path: default_export_path(), + settings_import_path: default_export_path(), + settings_editing_export_path: false, + settings_editing_import_path: false, + keyboard_explorer_selected: None, + explorer_accuracy_cache_overall: None, + explorer_accuracy_cache_ranked: None, + bigram_stats: BigramStatsStore::default(), + ranked_bigram_stats: BigramStatsStore::default(), + trigram_stats: TrigramStatsStore::default(), + ranked_trigram_stats: TrigramStatsStore::default(), + user_median_transition_ms: 0.0, + transition_buffer: Vec::new(), + trigram_gain_history: Vec::new(), + current_focus: None, + post_drill_input_lock_until: None, + adaptive_word_history: VecDeque::new(), + rng: SmallRng::from_entropy(), + transition_table, + dictionary, + passage_download_job: None, + code_download_job: None, + }; + + app.start_drill(); + app + } +} + #[cfg(test)] mod tests { use super::*; @@ -2178,7 +2283,7 @@ mod tests { #[test] fn adaptive_word_history_clears_on_code_mode_switch() { - let mut app = App::new(); + let mut app = App::new_test(); // App starts in Adaptive/Global; new() calls start_drill() which populates history assert_eq!(app.drill_mode, DrillMode::Adaptive); @@ -2200,7 +2305,7 @@ mod tests { #[test] fn adaptive_word_history_clears_on_passage_mode_switch() { - let mut app = App::new(); + let mut app = App::new_test(); assert_eq!(app.drill_mode, DrillMode::Adaptive); assert!(!app.adaptive_word_history.is_empty()); @@ -2219,7 +2324,7 @@ mod tests { #[test] fn adaptive_word_history_clears_on_scope_change() { - let mut app = App::new(); + let mut app = App::new_test(); // Start in Adaptive/Global — drill already started in new() assert_eq!(app.drill_scope, DrillScope::Global); @@ -2266,7 +2371,7 @@ mod tests { #[test] fn adaptive_word_history_persists_within_same_context() { - let mut app = App::new(); + let mut app = App::new_test(); // Adaptive/Global: run multiple drills, history should accumulate let history_after_first = app.adaptive_word_history.len(); @@ -2287,7 +2392,7 @@ mod tests { #[test] fn adaptive_word_history_not_cleared_on_same_branch_redrill() { - let mut app = App::new(); + let mut app = App::new_test(); // Start a branch drill app.start_branch_drill(BranchId::Lowercase); @@ -2319,7 +2424,7 @@ mod tests { #[test] fn adaptive_auto_continue_arms_input_lock() { - let mut app = App::new(); + let mut app = App::new_test(); assert_eq!(app.drill_mode, DrillMode::Adaptive); assert_eq!(app.screen, AppScreen::Drill); assert!(app.drill.is_some()); @@ -2340,7 +2445,7 @@ mod tests { #[test] fn adaptive_does_not_auto_continue_with_milestones() { - let mut app = App::new(); + let mut app = App::new_test(); assert_eq!(app.drill_mode, DrillMode::Adaptive); // Push a milestone before finishing the drill diff --git a/src/main.rs b/src/main.rs index a28fb88..ba5d70a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -36,6 +36,7 @@ use generator::passage::{is_book_cached, passage_options}; use keyboard::display::key_display_name; use keyboard::finger::Hand; use ui::components::dashboard::Dashboard; +use ui::layout::{pack_hint_lines, wrapped_line_count}; use ui::components::keyboard_diagram::KeyboardDiagram; use ui::components::skill_tree::{SkillTreeWidget, detail_line_count, selectable_branches}; use ui::components::stats_dashboard::{AnomalyBigramRow, NgramTabData, StatsDashboard}; @@ -1083,12 +1084,23 @@ fn render_menu(frame: &mut ratatui::Frame, app: &App) { let area = frame.area(); let colors = &app.theme.colors; + let menu_hints = [ + "[1-3] Start", + "[t] Skill Tree", + "[b] Keyboard", + "[s] Stats", + "[c] Settings", + "[q] Quit", + ]; + let footer_lines_vec = pack_hint_lines(&menu_hints, area.width as usize); + let footer_line_count = footer_lines_vec.len().max(1) as u16; + let layout = Layout::default() .direction(Direction::Vertical) .constraints([ Constraint::Length(3), Constraint::Min(0), - Constraint::Length(1), + Constraint::Length(footer_line_count), ]) .split(area); @@ -1125,10 +1137,16 @@ fn render_menu(frame: &mut ratatui::Frame, app: &App) { let menu_area = ui::layout::centered_rect(50, 80, layout[1]); frame.render_widget(&app.menu, menu_area); - let footer = Paragraph::new(Line::from(vec![Span::styled( - " [1-3] Start [t] Skill Tree [b] Keyboard [s] Stats [c] Settings [q] Quit ", - Style::default().fg(colors.text_pending()), - )])); + let footer_lines: Vec = footer_lines_vec + .into_iter() + .map(|line| { + Line::from(Span::styled( + line, + Style::default().fg(colors.text_pending()), + )) + }) + .collect(); + let footer = Paragraph::new(footer_lines); frame.render_widget(footer, layout[2]); } @@ -1529,9 +1547,7 @@ mod review_tests { /// Create an App for testing with the store disabled so tests never /// read or write the user's real data files. fn test_app() -> App { - let mut app = App::new(); - app.store = None; - app + App::new_test() } fn test_result(ts_offset_secs: i64) -> DrillResult { @@ -2832,51 +2848,6 @@ fn render_settings(frame: &mut ratatui::Frame, app: &App) { } } -fn wrapped_line_count(text: &str, width: usize) -> usize { - if width == 0 { - return 0; - } - let chars = text.chars().count().max(1); - chars.div_ceil(width) -} - -fn pack_hint_lines(hints: &[&str], width: usize) -> Vec { - if width == 0 || hints.is_empty() { - return Vec::new(); - } - - let prefix = " "; - let separator = " "; - let mut out: Vec = Vec::new(); - let mut current = prefix.to_string(); - let mut has_hint = false; - - for hint in hints { - if hint.is_empty() { - continue; - } - let candidate = if has_hint { - format!("{current}{separator}{hint}") - } else { - format!("{current}{hint}") - }; - if candidate.chars().count() <= width { - current = candidate; - has_hint = true; - } else { - if has_hint { - out.push(current); - } - current = format!("{prefix}{hint}"); - has_hint = true; - } - } - - if has_hint { - out.push(current); - } - out -} fn render_code_language_select(frame: &mut ratatui::Frame, app: &App) { let area = frame.area(); diff --git a/src/ui/components/dashboard.rs b/src/ui/components/dashboard.rs index f6ea3d9..0138241 100644 --- a/src/ui/components/dashboard.rs +++ b/src/ui/components/dashboard.rs @@ -5,6 +5,7 @@ use ratatui::text::{Line, Span}; use ratatui::widgets::{Block, Paragraph, Widget}; use crate::session::result::DrillResult; +use crate::ui::layout::pack_hint_lines; use crate::ui::theme::Theme; pub struct Dashboard<'a> { @@ -38,6 +39,19 @@ impl Widget for Dashboard<'_> { let inner = block.inner(area); block.render(area, buf); + let footer_line_count = if self.input_lock_remaining_ms.is_some() { + 1u16 + } else { + let hints = [ + "[c/Enter/Space] Continue", + "[r] Retry", + "[q] Menu", + "[s] Stats", + "[x] Delete", + ]; + pack_hint_lines(&hints, inner.width as usize).len().max(1) as u16 + }; + let layout = Layout::default() .direction(Direction::Vertical) .constraints([ @@ -47,7 +61,7 @@ impl Widget for Dashboard<'_> { Constraint::Length(3), Constraint::Length(3), Constraint::Min(0), - Constraint::Length(2), + Constraint::Length(footer_line_count), ]) .split(inner); @@ -137,16 +151,23 @@ impl Widget for Dashboard<'_> { ), ])) } else { - Paragraph::new(Line::from(vec![ - Span::styled( - " [c/Enter/Space] Continue ", - Style::default().fg(colors.accent()), - ), - Span::styled("[r] Retry ", Style::default().fg(colors.accent())), - Span::styled("[q] Menu ", Style::default().fg(colors.accent())), - Span::styled("[s] Stats ", Style::default().fg(colors.accent())), - Span::styled("[x] Delete", Style::default().fg(colors.accent())), - ])) + let hints = [ + "[c/Enter/Space] Continue", + "[r] Retry", + "[q] Menu", + "[s] Stats", + "[x] Delete", + ]; + let lines: Vec = pack_hint_lines(&hints, inner.width as usize) + .into_iter() + .map(|line| { + Line::from(Span::styled( + line, + Style::default().fg(colors.accent()), + )) + }) + .collect(); + Paragraph::new(lines) }; help.render(layout[6], buf); } diff --git a/src/ui/components/skill_tree.rs b/src/ui/components/skill_tree.rs index fc6e953..07f2162 100644 --- a/src/ui/components/skill_tree.rs +++ b/src/ui/components/skill_tree.rs @@ -8,6 +8,7 @@ use crate::engine::key_stats::KeyStatsStore; use crate::engine::skill_tree::{ BranchId, BranchStatus, DrillScope, SkillTree as SkillTreeEngine, get_branch_definition, }; +use crate::ui::layout::{pack_hint_lines, wrapped_line_count}; use crate::ui::theme::Theme; pub struct SkillTreeWidget<'a> { @@ -437,48 +438,3 @@ fn dual_progress_bar_parts( ) } -fn wrapped_line_count(text: &str, width: usize) -> usize { - if width == 0 { - return 0; - } - let chars = text.chars().count().max(1); - chars.div_ceil(width) -} - -fn pack_hint_lines(hints: &[&str], width: usize) -> Vec { - if width == 0 || hints.is_empty() { - return Vec::new(); - } - - let prefix = " "; - let separator = " "; - let mut out: Vec = Vec::new(); - let mut current = prefix.to_string(); - let mut has_hint = false; - - for hint in hints { - if hint.is_empty() { - continue; - } - let candidate = if has_hint { - format!("{current}{separator}{hint}") - } else { - format!("{current}{hint}") - }; - if candidate.chars().count() <= width { - current = candidate; - has_hint = true; - } else { - if has_hint { - out.push(current); - } - current = format!("{prefix}{hint}"); - has_hint = true; - } - } - - if has_hint { - out.push(current); - } - out -} diff --git a/src/ui/components/stats_dashboard.rs b/src/ui/components/stats_dashboard.rs index 6fc566e..8569fbd 100644 --- a/src/ui/components/stats_dashboard.rs +++ b/src/ui/components/stats_dashboard.rs @@ -11,6 +11,7 @@ use crate::keyboard::display::{self, BACKSPACE, ENTER, MODIFIER_SENTINELS, SPACE use crate::keyboard::model::KeyboardModel; use crate::session::result::DrillResult; use crate::ui::components::activity_heatmap::ActivityHeatmap; +use crate::ui::layout::pack_hint_lines; use crate::ui::theme::Theme; // --------------------------------------------------------------------------- @@ -106,17 +107,8 @@ impl Widget for StatsDashboard<'_> { return; } - let layout = Layout::default() - .direction(Direction::Vertical) - .constraints([ - Constraint::Length(2), - Constraint::Min(10), - Constraint::Length(2), - ]) - .split(inner); - - // Tab header - let tabs = [ + // Tab header — width-aware wrapping + let tab_labels = [ "[1] Dashboard", "[2] History", "[3] Activity", @@ -124,10 +116,20 @@ impl Widget for StatsDashboard<'_> { "[5] Timing", "[6] N-grams", ]; - let tab_spans: Vec = tabs - .iter() - .enumerate() - .flat_map(|(i, &label)| { + let tab_separator = " "; + let width = inner.width as usize; + let mut tab_lines: Vec = Vec::new(); + { + let mut current_spans: Vec = Vec::new(); + let mut current_width: usize = 0; + for (i, &label) in tab_labels.iter().enumerate() { + let styled_label = format!(" {label} "); + let item_width = styled_label.chars().count() + tab_separator.len(); + if current_width > 0 && current_width + item_width > width { + tab_lines.push(Line::from(current_spans)); + current_spans = Vec::new(); + current_width = 0; + } let style = if i == self.active_tab { Style::default() .fg(colors.accent()) @@ -135,25 +137,56 @@ impl Widget for StatsDashboard<'_> { } else { Style::default().fg(colors.text_pending()) }; - vec![Span::styled(format!(" {label} "), style), Span::raw(" ")] - }) - .collect(); - Paragraph::new(Line::from(tab_spans)).render(layout[0], buf); + current_spans.push(Span::styled(styled_label, style)); + current_spans.push(Span::raw(tab_separator)); + current_width += item_width; + } + if !current_spans.is_empty() { + tab_lines.push(Line::from(current_spans)); + } + } + let tab_line_count = tab_lines.len().max(1) as u16; + + // Footer — width-aware wrapping + let footer_hints: Vec<&str> = if self.active_tab == 1 { + vec![ + "[ESC] Back", + "[Tab] Next tab", + "[1-6] Switch tab", + "[j/k] Navigate", + "[x] Delete", + ] + } else { + vec!["[ESC] Back", "[Tab] Next tab", "[1-6] Switch tab"] + }; + let footer_lines_vec = pack_hint_lines(&footer_hints, width); + let footer_line_count = footer_lines_vec.len().max(1) as u16; + + let layout = Layout::default() + .direction(Direction::Vertical) + .constraints([ + Constraint::Length(tab_line_count), + Constraint::Min(10), + Constraint::Length(footer_line_count), + ]) + .split(inner); + + Paragraph::new(tab_lines).render(layout[0], buf); // Render only one tab at a time so each tab gets full breathing room. self.render_tab(self.active_tab, layout[1], buf); // Footer - let footer_text = if self.active_tab == 1 { - " [ESC] Back [Tab] Next tab [1-6] Switch tab [j/k] Navigate [x] Delete" - } else { - " [ESC] Back [Tab] Next tab [1-6] Switch tab" - }; - let footer = Paragraph::new(Line::from(Span::styled( - footer_text, - Style::default().fg(colors.accent()), - ))); - footer.render(layout[2], buf); + let footer_lines: Vec = footer_lines_vec + .into_iter() + .map(|line| { + Line::from(Span::styled( + line, + Style::default().fg(colors.accent()), + )) + }) + .collect(); + Paragraph::new(footer_lines).render(layout[2], buf); // Confirmation dialog overlay if self.history_confirm_delete && self.active_tab == 1 { @@ -1591,26 +1624,34 @@ impl StatsDashboard<'_> { fn render_ngram_summary(&self, data: &NgramTabData, area: Rect, buf: &mut Buffer) { let colors = &self.theme.colors; + let w = area.width as usize; let gain_str = match data.latest_trigram_gain { Some(g) => format!("{:.1}%", g * 100.0), None => "--".to_string(), }; + + // Build segments from most to least important, progressively drop from the right + let scope = format!(" {}", data.scope_label); + let bigrams = format!(" | Bi: {}", data.total_bigrams); + let trigrams = format!(" | Tri: {}", data.total_trigrams); + let hesitation = format!(" | Hes: >{:.0}ms", data.hesitation_threshold_ms); + let gain = format!(" | Gain: {}", gain_str); let gain_note = if data.latest_trigram_gain.is_none() { - " (computed every 50 drills)" + " (every 50)" } else { "" }; - let line = format!( - " Scope: {} | Bigrams: {} | Trigrams: {} | Hesitation: >{:.0}ms | Tri-gain: {}{}", - data.scope_label, - data.total_bigrams, - data.total_trigrams, - data.hesitation_threshold_ms, - gain_str, - gain_note, - ); + let segments: &[&str] = &[&scope, &bigrams, &trigrams, &hesitation, &gain, gain_note]; + let mut line = String::new(); + for seg in segments { + if line.len() + seg.len() <= w { + line.push_str(seg); + } else { + break; + } + } buf.set_string( area.x, diff --git a/src/ui/layout.rs b/src/ui/layout.rs index 5d61f28..b8d5c66 100644 --- a/src/ui/layout.rs +++ b/src/ui/layout.rs @@ -81,6 +81,52 @@ impl AppLayout { } } +pub fn wrapped_line_count(text: &str, width: usize) -> usize { + if width == 0 { + return 0; + } + let chars = text.chars().count().max(1); + chars.div_ceil(width) +} + +pub fn pack_hint_lines(hints: &[&str], width: usize) -> Vec { + if width == 0 || hints.is_empty() { + return Vec::new(); + } + + let prefix = " "; + let separator = " "; + let mut out: Vec = Vec::new(); + let mut current = prefix.to_string(); + let mut has_hint = false; + + for hint in hints { + if hint.is_empty() { + continue; + } + let candidate = if has_hint { + format!("{current}{separator}{hint}") + } else { + format!("{current}{hint}") + }; + if candidate.chars().count() <= width { + current = candidate; + has_hint = true; + } else { + if has_hint { + out.push(current); + } + current = format!("{prefix}{hint}"); + has_hint = true; + } + } + + if has_hint { + out.push(current); + } + out +} + pub fn centered_rect(percent_x: u16, percent_y: u16, area: Rect) -> Rect { const MIN_POPUP_WIDTH: u16 = 72; const MIN_POPUP_HEIGHT: u16 = 18;