From bbf42f4e128a1f0b50ce061bb24a9a48a9615f95 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Fri, 22 Aug 2025 14:03:58 -0700 Subject: [PATCH] improve performance of 'cargo test -p codex-tui' (#2593) before: ``` $ time cargo test -p codex-tui -q [...] cargo test -p codex-tui -q 39.89s user 10.77s system 98% cpu 51.328 total ``` after: ``` $ time cargo test -p codex-tui -q [...] cargo test -p codex-tui -q 1.37s user 0.64s system 29% cpu 6.699 total ``` the major offenders were the textarea fuzz test and the custom_terminal doctests. (i think the doctests were being recompiled every time which made them extra slow?) --- codex-rs/tui/src/bottom_pane/mod.rs | 5 - codex-rs/tui/src/bottom_pane/textarea.rs | 4 +- codex-rs/tui/src/custom_terminal.rs | 145 -------------------- codex-rs/tui/src/status_indicator_widget.rs | 19 ++- 4 files changed, 16 insertions(+), 157 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 04f5d4b9..e24975f1 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -492,8 +492,6 @@ mod tests { assert!(pane.active_view.is_some(), "active view should be present"); // Render and ensure the top row includes the Working header instead of the composer. - // Give the animation thread a moment to tick. - std::thread::sleep(std::time::Duration::from_millis(120)); let area = Rect::new(0, 0, 40, 3); let mut buf = Buffer::empty(area); (&pane).render_ref(area, &mut buf); @@ -525,9 +523,6 @@ mod tests { // Begin a task: show initial status. pane.set_task_running(true); - // Allow some frames so the animation thread ticks. - std::thread::sleep(std::time::Duration::from_millis(120)); - // Render and confirm the line contains the "Working" header. let area = Rect::new(0, 0, 40, 3); let mut buf = Buffer::empty(area); diff --git a/codex-rs/tui/src/bottom_pane/textarea.rs b/codex-rs/tui/src/bottom_pane/textarea.rs index 8379deb9..e2c83f55 100644 --- a/codex-rs/tui/src/bottom_pane/textarea.rs +++ b/codex-rs/tui/src/bottom_pane/textarea.rs @@ -1473,7 +1473,7 @@ mod tests { .timestamp() as u64; let mut rng = rand::rngs::StdRng::seed_from_u64(pst_today_seed); - for _case in 0..10_000 { + for _case in 0..500 { let mut ta = TextArea::new(); let mut state = TextAreaState::default(); // Track element payloads we insert. Payloads use characters '[' and ']' which @@ -1497,7 +1497,7 @@ mod tests { let mut width: u16 = rng.random_range(1..=12); let mut height: u16 = rng.random_range(1..=4); - for _step in 0..200 { + for _step in 0..60 { // Mostly stable width/height, occasionally change if rng.random_bool(0.1) { width = rng.random_range(1..=12); diff --git a/codex-rs/tui/src/custom_terminal.rs b/codex-rs/tui/src/custom_terminal.rs index ef642b1a..407ab329 100644 --- a/codex-rs/tui/src/custom_terminal.rs +++ b/codex-rs/tui/src/custom_terminal.rs @@ -70,20 +70,6 @@ impl Frame<'_> { /// Usually the area argument is the size of the current frame or a sub-area of the current /// frame (which can be obtained using [`Layout`] to split the total area). /// - /// # Example - /// - /// ```rust - /// # use ratatui::{backend::TestBackend, Terminal}; - /// # let backend = TestBackend::new(5, 5); - /// # let mut terminal = Terminal::new(backend).unwrap(); - /// # let mut frame = terminal.get_frame(); - /// use ratatui::{layout::Rect, widgets::Block}; - /// - /// let block = Block::new(); - /// let area = Rect::new(0, 0, 5, 5); - /// frame.render_widget(block, area); - /// ``` - /// /// [`Layout`]: crate::layout::Layout pub fn render_widget(&mut self, widget: W, area: Rect) { widget.render(area, self.buffer); @@ -93,22 +79,6 @@ impl Frame<'_> { /// /// Usually the area argument is the size of the current frame or a sub-area of the current /// frame (which can be obtained using [`Layout`] to split the total area). - /// - /// # Example - /// - /// ```rust - /// # #[cfg(feature = "unstable-widget-ref")] { - /// # use ratatui::{backend::TestBackend, Terminal}; - /// # let backend = TestBackend::new(5, 5); - /// # let mut terminal = Terminal::new(backend).unwrap(); - /// # let mut frame = terminal.get_frame(); - /// use ratatui::{layout::Rect, widgets::Block}; - /// - /// let block = Block::new(); - /// let area = Rect::new(0, 0, 5, 5); - /// frame.render_widget_ref(block, area); - /// # } - /// ``` #[allow(clippy::needless_pass_by_value)] pub fn render_widget_ref(&mut self, widget: W, area: Rect) { widget.render_ref(area, self.buffer); @@ -122,24 +92,6 @@ impl Frame<'_> { /// The last argument should be an instance of the [`StatefulWidget::State`] associated to the /// given [`StatefulWidget`]. /// - /// # Example - /// - /// ```rust - /// # use ratatui::{backend::TestBackend, Terminal}; - /// # let backend = TestBackend::new(5, 5); - /// # let mut terminal = Terminal::new(backend).unwrap(); - /// # let mut frame = terminal.get_frame(); - /// use ratatui::{ - /// layout::Rect, - /// widgets::{List, ListItem, ListState}, - /// }; - /// - /// let mut state = ListState::default().with_selected(Some(1)); - /// let list = List::new(vec![ListItem::new("Item 1"), ListItem::new("Item 2")]); - /// let area = Rect::new(0, 0, 5, 5); - /// frame.render_stateful_widget(list, area, &mut state); - /// ``` - /// /// [`Layout`]: crate::layout::Layout pub fn render_stateful_widget(&mut self, widget: W, area: Rect, state: &mut W::State) where @@ -156,26 +108,6 @@ impl Frame<'_> { /// /// The last argument should be an instance of the [`StatefulWidgetRef::State`] associated to /// the given [`StatefulWidgetRef`]. - /// - /// # Example - /// - /// ```rust - /// # #[cfg(feature = "unstable-widget-ref")] { - /// # use ratatui::{backend::TestBackend, Terminal}; - /// # let backend = TestBackend::new(5, 5); - /// # let mut terminal = Terminal::new(backend).unwrap(); - /// # let mut frame = terminal.get_frame(); - /// use ratatui::{ - /// layout::Rect, - /// widgets::{List, ListItem, ListState}, - /// }; - /// - /// let mut state = ListState::default().with_selected(Some(1)); - /// let list = List::new(vec![ListItem::new("Item 1"), ListItem::new("Item 2")]); - /// let area = Rect::new(0, 0, 5, 5); - /// frame.render_stateful_widget_ref(list, area, &mut state); - /// # } - /// ``` #[allow(clippy::needless_pass_by_value)] pub fn render_stateful_widget_ref(&mut self, widget: W, area: Rect, state: &mut W::State) where @@ -216,17 +148,6 @@ impl Frame<'_> { /// This count is particularly useful when dealing with dynamic content or animations where the /// state of the display changes over time. By tracking the frame count, developers can /// synchronize updates or changes to the content with the rendering process. - /// - /// # Examples - /// - /// ```rust - /// # use ratatui::{backend::TestBackend, Terminal}; - /// # let backend = TestBackend::new(5, 5); - /// # let mut terminal = Terminal::new(backend).unwrap(); - /// # let mut frame = terminal.get_frame(); - /// let current_count = frame.count(); - /// println!("Current frame count: {}", current_count); - /// ``` pub const fn count(&self) -> usize { self.count } @@ -277,19 +198,6 @@ where B: Backend, { /// Creates a new [`Terminal`] with the given [`Backend`] and [`TerminalOptions`]. - /// - /// # Example - /// - /// ```rust - /// use std::io::stdout; - /// - /// use ratatui::{backend::CrosstermBackend, layout::Rect, Terminal, TerminalOptions, Viewport}; - /// - /// let backend = CrosstermBackend::new(stdout()); - /// let viewport = Viewport::Fixed(Rect::new(0, 0, 10, 10)); - /// let terminal = Terminal::with_options(backend, TerminalOptions { viewport })?; - /// # std::io::Result::Ok(()) - /// ``` pub fn with_options(mut backend: B) -> io::Result { let screen_size = backend.size()?; let cursor_pos = backend.get_cursor_position()?; @@ -394,29 +302,6 @@ where /// previous frame to determine what has changed, and only the changes are written to the /// terminal. If the render callback does not fully render the frame, the terminal will not be /// in a consistent state. - /// - /// # Examples - /// - /// ``` - /// # let backend = ratatui::backend::TestBackend::new(10, 10); - /// # let mut terminal = ratatui::Terminal::new(backend)?; - /// use ratatui::{layout::Position, widgets::Paragraph}; - /// - /// // with a closure - /// terminal.draw(|frame| { - /// let area = frame.area(); - /// frame.render_widget(Paragraph::new("Hello World!"), area); - /// frame.set_cursor_position(Position { x: 0, y: 0 }); - /// })?; - /// - /// // or with a function - /// terminal.draw(render)?; - /// - /// fn render(frame: &mut ratatui::Frame) { - /// frame.render_widget(Paragraph::new("Hello World!"), frame.area()); - /// } - /// # std::io::Result::Ok(()) - /// ``` pub fn draw(&mut self, render_callback: F) -> io::Result<()> where F: FnOnce(&mut Frame), @@ -462,36 +347,6 @@ where /// previous frame to determine what has changed, and only the changes are written to the /// terminal. If the render function does not fully render the frame, the terminal will not be /// in a consistent state. - /// - /// # Examples - /// - /// ```should_panic - /// # use ratatui::layout::Position;; - /// # let backend = ratatui::backend::TestBackend::new(10, 10); - /// # let mut terminal = ratatui::Terminal::new(backend)?; - /// use std::io; - /// - /// use ratatui::widgets::Paragraph; - /// - /// // with a closure - /// terminal.try_draw(|frame| { - /// let value: u8 = "not a number".parse().map_err(io::Error::other)?; - /// let area = frame.area(); - /// frame.render_widget(Paragraph::new("Hello World!"), area); - /// frame.set_cursor_position(Position { x: 0, y: 0 }); - /// io::Result::Ok(()) - /// })?; - /// - /// // or with a function - /// terminal.try_draw(render)?; - /// - /// fn render(frame: &mut ratatui::Frame) -> io::Result<()> { - /// let value: u8 = "not a number".parse().map_err(io::Error::other)?; - /// frame.render_widget(Paragraph::new("Hello World!"), frame.area()); - /// Ok(()) - /// } - /// # io::Result::Ok(()) - /// ``` pub fn try_draw(&mut self, render_callback: F) -> io::Result<()> where F: FnOnce(&mut Frame) -> Result<(), E>, diff --git a/codex-rs/tui/src/status_indicator_widget.rs b/codex-rs/tui/src/status_indicator_widget.rs index df4910fe..dbdd230d 100644 --- a/codex-rs/tui/src/status_indicator_widget.rs +++ b/codex-rs/tui/src/status_indicator_widget.rs @@ -146,6 +146,15 @@ impl StatusIndicatorWidget { let since_start = self.start_time.elapsed(); (since_start.as_millis() / 100) as usize } + + /// Test-only helper to fast-forward the internal clock so animations + /// advance without sleeping. + #[cfg(test)] + pub(crate) fn test_fast_forward_frames(&mut self, frames: usize) { + let advance_ms = (frames as u64).saturating_mul(100); + // Move the start time into the past so `current_frame()` advances. + self.start_time = std::time::Instant::now() - std::time::Duration::from_millis(advance_ms); + } } impl WidgetRef for StatusIndicatorWidget { @@ -236,8 +245,8 @@ mod tests { w.restart_with_text("Hello".to_string()); let area = ratatui::layout::Rect::new(0, 0, 30, 1); - // Allow a short delay so the typewriter reveals the first character. - std::thread::sleep(std::time::Duration::from_millis(120)); + // Advance animation without sleeping. + w.test_fast_forward_frames(2); let mut buf = ratatui::buffer::Buffer::empty(area); w.render_ref(area, &mut buf); @@ -252,8 +261,8 @@ mod tests { let tx = AppEventSender::new(tx_raw); let mut w = StatusIndicatorWidget::new(tx, crate::tui::FrameRequester::test_dummy()); w.restart_with_text("Hi".to_string()); - // Ensure some frames elapse so we get a stable state. - std::thread::sleep(std::time::Duration::from_millis(120)); + // Advance animation without sleeping. + w.test_fast_forward_frames(2); let area = ratatui::layout::Rect::new(0, 0, 30, 1); let mut buf = ratatui::buffer::Buffer::empty(area); @@ -273,7 +282,7 @@ mod tests { let tx = AppEventSender::new(tx_raw); let mut w = StatusIndicatorWidget::new(tx, crate::tui::FrameRequester::test_dummy()); w.restart_with_text("Hello".to_string()); - std::thread::sleep(std::time::Duration::from_millis(120)); + w.test_fast_forward_frames(2); let area = ratatui::layout::Rect::new(0, 0, 30, 1); let mut buf = ratatui::buffer::Buffer::empty(area);