This is an automated email from the ASF dual-hosted git repository.
martin-g pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion-ballista.git
The following commit(s) were added to refs/heads/main by this push:
new d49a530b7 feat: TUI make task popup scrollable (#1725)
d49a530b7 is described below
commit d49a530b7df1f2c6da706123490f2c22a10fe5d6
Author: Marko Milenković <[email protected]>
AuthorDate: Tue May 19 09:12:28 2026 +0100
feat: TUI make task popup scrollable (#1725)
* feat: task popup scrollable
* add stages popup scroll bar
* Extract a helper function for counting the stage's tasks
* ScrollbarState::new() sets position to 0
* Select first task only if there is at least one task
* Do not try to scroll up if there are no tasks
* Scroll up/down in tasks popup wraps on overflow
As the other tables do
* Add unit tests for the scroll support in stages and tasks
AI generated
* Add more unit tests for App methods
AI generated
* Make datetime related tests timezone independant
---------
Co-authored-by: Martin Tzvetanov Grigorov <[email protected]>
---
ballista-cli/src/tui/app.rs | 131 +++++++++-
ballista-cli/src/tui/domain/jobs/stages.rs | 264 ++++++++++++++++++++-
.../src/tui/ui/main/jobs/job_stages_popup.rs | 2 +
.../src/tui/ui/main/jobs/stage_tasks_popup.rs | 5 +-
4 files changed, 393 insertions(+), 9 deletions(-)
diff --git a/ballista-cli/src/tui/app.rs b/ballista-cli/src/tui/app.rs
index 1000afe40..7bddf86b5 100644
--- a/ballista-cli/src/tui/app.rs
+++ b/ballista-cli/src/tui/app.rs
@@ -36,6 +36,7 @@ use crate::tui::{
};
use chrono::DateTime;
use crossterm::event::{KeyCode, KeyEvent};
+use std::string::ToString;
use std::sync::Arc;
use tokio::sync::mpsc::Sender;
@@ -45,6 +46,8 @@ use crate::tui::ui::{
load_job_stages_popup, load_jobs_data, load_metrics_data,
};
+const INVALID_DATE: &str = "Invalid date";
+
#[derive(Debug, PartialEq)]
enum Views {
Executors,
@@ -197,10 +200,13 @@ impl App {
}
if let Some(popup) = &mut self.job_stages_popup {
- if popup.is_tasks_view()
- && let KeyCode::Esc = key.code
- {
- popup.set_no_details_view();
+ if popup.is_tasks_view() {
+ match key.code {
+ KeyCode::Esc => popup.set_no_details_view(),
+ KeyCode::Up => popup.scroll_up(),
+ KeyCode::Down => popup.scroll_down(),
+ _ => {}
+ }
} else if popup.is_plan_view() {
match key.code {
KeyCode::Esc => popup.set_no_details_view(),
@@ -604,7 +610,7 @@ impl App {
.format("%Y-%m-%d %H:%M:%S")
.to_string()
})
- .unwrap_or_else(|| "Invalid date".to_string())
+ .unwrap_or_else(|| INVALID_DATE.to_string())
}
// copied from DataFusion Commons to avoid depending on it
@@ -688,7 +694,9 @@ impl App {
mod tests {
use crate::tui::App;
use crate::tui::Settings;
- use crate::tui::app::{ExecutorsSortColumn, JobsSortColumn,
MetricsSortColumn};
+ use crate::tui::app::{
+ ExecutorsSortColumn, INVALID_DATE, JobsSortColumn, MetricsSortColumn,
+ };
use crate::tui::domain::{
SchedulerState, SortOrder,
executors::{Executor, ExecutorDetailsPopup, OsInfo, Specification},
@@ -1112,4 +1120,115 @@ mod tests {
app.jobs_data.table_state.select(Some(0));
assert!(!app.is_selected_job_completed_or_running());
}
+
+ // --- has_selected_job with selection ---
+
+ #[test]
+ fn has_selected_job_true_when_selected() {
+ let mut app = make_app();
+ app.jobs_data.jobs = vec![make_job("j1", "Running")];
+ app.jobs_data.table_state.select(Some(0));
+ assert!(app.has_selected_job());
+ }
+
+ // --- format_datetime tests ---
+
+ #[test]
+ fn format_datetime_zero_timestamp_is_valid_format() {
+ let app = make_app();
+ let result = app.format_datetime(0);
+ assert_ne!(result, INVALID_DATE);
+ }
+
+ #[test]
+ fn format_datetime_known_timestamp_contains_year() {
+ // 1_000_000_000_000 ms = 2001-09-09T01:46:40 UTC
+ let app = make_app();
+ let result = app.format_datetime(1_000_000_000_000);
+ assert!(result.contains("2001"), "{result} must contain year 2001");
+ }
+
+ #[test]
+ fn format_datetime_out_of_range_returns_invalid() {
+ let app = make_app();
+ assert_eq!(app.format_datetime(i64::MAX), "Invalid date");
+ }
+
+ // --- format_duration tests ---
+
+ #[test]
+ fn format_duration_zero_ms_returns_nanoseconds() {
+ let app = make_app();
+ assert_eq!(app.format_duration(0), "0ns");
+ }
+
+ #[test]
+ fn format_duration_one_ms_returns_milliseconds() {
+ let app = make_app();
+ assert_eq!(app.format_duration(1), "1.00ms");
+ }
+
+ #[test]
+ fn format_duration_one_second() {
+ let app = make_app();
+ assert_eq!(app.format_duration(1_000), "1.00s");
+ }
+
+ #[test]
+ fn format_duration_large_value_returns_seconds() {
+ let app = make_app();
+ assert_eq!(app.format_duration(90_000), "90.00s");
+ }
+
+ // --- format_count tests ---
+
+ #[test]
+ fn format_count_zero() {
+ let app = make_app();
+ assert_eq!(app.format_count(0), "0");
+ }
+
+ #[test]
+ fn format_count_below_thousand_returns_raw() {
+ let app = make_app();
+ assert_eq!(app.format_count(999), "999");
+ }
+
+ #[test]
+ fn format_count_thousands_two_decimals() {
+ let app = make_app();
+ assert_eq!(app.format_count(1_000), "1.00K");
+ }
+
+ #[test]
+ fn format_count_thousands_large_one_decimal() {
+ let app = make_app();
+ assert_eq!(app.format_count(100_000), "100.0K");
+ }
+
+ #[test]
+ fn format_count_millions() {
+ let app = make_app();
+ assert_eq!(app.format_count(1_000_000), "1.00M");
+ }
+
+ #[test]
+ fn format_count_billions() {
+ let app = make_app();
+ assert_eq!(app.format_count(1_000_000_000), "1.00B");
+ }
+
+ #[test]
+ fn format_count_trillions() {
+ let app = make_app();
+ assert_eq!(app.format_count(1_000_000_000_000), "1.00T");
+ }
+
+ // --- format_size additional boundary ---
+
+ #[test]
+ fn format_size_terabytes() {
+ let app = make_app();
+ assert_eq!(app.format_size(2 * 1024 * 1024 * 1024 * 1024), "2.0TB");
+ }
}
diff --git a/ballista-cli/src/tui/domain/jobs/stages.rs
b/ballista-cli/src/tui/domain/jobs/stages.rs
index 824217471..6cdacabfe 100644
--- a/ballista-cli/src/tui/domain/jobs/stages.rs
+++ b/ballista-cli/src/tui/domain/jobs/stages.rs
@@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.
-use ratatui::widgets::TableState;
+use ratatui::widgets::{ScrollbarState, TableState};
use serde::Deserialize;
#[derive(Deserialize, Clone, Debug)]
@@ -79,6 +79,9 @@ pub struct JobStagesPopup {
pub job_id: String,
pub stages: JobStagesResponse,
pub table_state: TableState,
+ pub scrollbar_state: ScrollbarState,
+ pub tasks_table_state: TableState,
+ pub tasks_scrollbar_state: ScrollbarState,
details_view: StageDetailsView,
plan_vertical_scroll_position: u16,
plan_horizontal_scroll_position: u16,
@@ -88,8 +91,11 @@ impl JobStagesPopup {
pub fn new(job_id: String, stages: JobStagesResponse) -> Self {
Self {
job_id,
+ scrollbar_state: ScrollbarState::new(stages.stages.len()),
stages,
table_state: TableState::default(),
+ tasks_table_state: TableState::default(),
+ tasks_scrollbar_state: ScrollbarState::new(0),
details_view: StageDetailsView::None,
plan_vertical_scroll_position: 0,
plan_horizontal_scroll_position: 0,
@@ -106,6 +112,8 @@ impl JobStagesPopup {
pub fn set_tasks_view(&mut self) {
self.details_view = StageDetailsView::Tasks;
+ self.tasks_table_state = TableState::default().with_selected(None);
+ self.tasks_scrollbar_state = ScrollbarState::new(self.tasks_count());
}
pub fn set_plan_view(&mut self) {
@@ -140,12 +148,17 @@ impl JobStagesPopup {
if let Some(selected) = self.table_state.selected() {
if selected < len - 1 {
self.table_state.select(Some(selected + 1));
+ self.scrollbar_state =
self.scrollbar_state.position(selected + 1);
} else {
self.table_state.select(None);
+ self.scrollbar_state = self.scrollbar_state.position(0);
}
} else {
self.table_state.select(Some(0));
+ self.scrollbar_state = self.scrollbar_state.position(0);
}
+ } else if self.is_tasks_view() {
+ self.tasks_scroll_down();
} else if self.is_plan_view() {
self.plan_vertical_scroll_position =
self.plan_vertical_scroll_position.saturating_add(1);
@@ -162,18 +175,53 @@ impl JobStagesPopup {
if let Some(selected) = self.table_state.selected() {
if selected == 0 {
self.table_state.select(None);
+ self.scrollbar_state = self.scrollbar_state.position(0);
} else {
self.table_state.select(Some(selected - 1));
+ self.scrollbar_state =
self.scrollbar_state.position(selected - 1);
}
} else {
self.table_state.select(Some(len - 1));
+ self.scrollbar_state = self.scrollbar_state.position(len - 1);
}
+ } else if self.is_tasks_view() {
+ self.tasks_scroll_up();
} else if self.is_plan_view() {
self.plan_vertical_scroll_position =
self.plan_vertical_scroll_position.saturating_sub(1);
}
}
+ fn tasks_scroll_down(&mut self) {
+ let tasks_count = self.tasks_count();
+ if tasks_count == 0 {
+ return;
+ }
+ let next = match self.tasks_table_state.selected() {
+ Some(i) if i < tasks_count - 1 => Some(i + 1),
+ Some(_) => None,
+ None => Some(0),
+ };
+ self.tasks_table_state.select(next);
+ self.tasks_scrollbar_state =
+ self.tasks_scrollbar_state.position(next.unwrap_or(0));
+ }
+
+ fn tasks_scroll_up(&mut self) {
+ let tasks_count = self.tasks_count();
+ if tasks_count == 0 {
+ return;
+ }
+ let prev = match self.tasks_table_state.selected() {
+ Some(i) if i > 0 => Some(i - 1),
+ Some(_) => None,
+ None => Some(tasks_count - 1),
+ };
+ self.tasks_table_state.select(prev);
+ self.tasks_scrollbar_state =
+ self.tasks_scrollbar_state.position(prev.unwrap_or(0));
+ }
+
pub fn scroll_left(&mut self) {
if self.is_plan_view() {
self.plan_horizontal_scroll_position =
@@ -193,6 +241,12 @@ impl JobStagesPopup {
.selected()
.and_then(|i| self.stages.stages.get(i))
}
+
+ fn tasks_count(&self) -> usize {
+ self.selected_stage()
+ .map(|s| s.tasks.iter().flatten().count())
+ .unwrap_or(0)
+ }
}
#[derive(Clone, Debug)]
@@ -228,7 +282,8 @@ impl StagesGraph {
#[cfg(test)]
mod tests {
use super::{
- JobStageResponse, JobStagesPopup, JobStagesResponse, StagesGraph,
TaskPercentiles,
+ JobStageResponse, JobStagesPopup, JobStagesResponse, StageTaskResponse,
+ StagesGraph, TaskPercentiles,
};
fn make_percentiles() -> TaskPercentiles {
@@ -466,4 +521,209 @@ mod tests {
graph.scroll_up();
assert_eq!(graph.scroll_position, 0);
}
+
+ // --- Helpers for task-bearing stages ---
+
+ fn make_task(id: usize) -> StageTaskResponse {
+ StageTaskResponse {
+ id,
+ status: "Completed".to_string(),
+ partition_id: id as u32,
+ input_rows: 0,
+ output_rows: 0,
+ scheduled_time: 0,
+ launch_time: 0,
+ start_exec_time: 0,
+ end_exec_time: 0,
+ finish_time: 0,
+ }
+ }
+
+ fn make_stage_with_tasks(id: &str, task_count: usize) -> JobStageResponse {
+ let mut stage = make_stage(id);
+ stage.tasks = (0..task_count).map(|i| Some(make_task(i))).collect();
+ stage
+ }
+
+ // Creates a popup with one stage that has `task_count` tasks; the stage
is pre-selected.
+ fn make_popup_with_tasks(task_count: usize) -> JobStagesPopup {
+ let stage = make_stage_with_tasks("0", task_count);
+ let mut popup = JobStagesPopup::new(
+ "job1".to_string(),
+ JobStagesResponse {
+ stages: vec![stage],
+ },
+ );
+ popup.table_state.select(Some(0));
+ popup
+ }
+
+ // --- set_tasks_view ---
+
+ #[test]
+ fn set_tasks_view_with_tasks_does_not_preselect() {
+ let mut popup = make_popup_with_tasks(3);
+ popup.set_tasks_view();
+ assert_eq!(popup.tasks_table_state.selected(), None);
+ }
+
+ #[test]
+ fn set_tasks_view_with_no_tasks_selects_none() {
+ let mut popup = make_popup_with_tasks(0);
+ popup.set_tasks_view();
+ assert_eq!(popup.tasks_table_state.selected(), None);
+ }
+
+ // --- tasks_scroll_down (via scroll_down in tasks view) ---
+
+ #[test]
+ fn scroll_down_in_tasks_view_advances_selection() {
+ let mut popup = make_popup_with_tasks(3);
+ popup.set_tasks_view();
+ // set_tasks_view does not pre-select a task; scrolling down should
move to 0
+ popup.scroll_down();
+ assert_eq!(popup.tasks_table_state.selected(), Some(0));
+ }
+
+ #[test]
+ fn scroll_down_in_tasks_view_at_last_deselects() {
+ let mut popup = make_popup_with_tasks(3);
+ popup.set_tasks_view();
+ popup.tasks_table_state.select(Some(2));
+ popup.scroll_down();
+ assert_eq!(popup.tasks_table_state.selected(), None);
+ }
+
+ #[test]
+ fn scroll_down_in_tasks_view_from_none_selects_first() {
+ let mut popup = make_popup_with_tasks(3);
+ popup.set_tasks_view();
+ popup.tasks_table_state.select(None);
+ popup.scroll_down();
+ assert_eq!(popup.tasks_table_state.selected(), Some(0));
+ }
+
+ #[test]
+ fn scroll_down_in_tasks_view_empty_tasks_does_nothing() {
+ let mut popup = make_popup_with_tasks(0);
+ popup.set_tasks_view();
+ popup.scroll_down();
+ assert_eq!(popup.tasks_table_state.selected(), None);
+ }
+
+ // --- tasks_scroll_up (via scroll_up in tasks view) ---
+
+ #[test]
+ fn scroll_up_in_tasks_view_moves_back() {
+ let mut popup = make_popup_with_tasks(3);
+ popup.set_tasks_view();
+ popup.tasks_table_state.select(Some(2));
+ popup.scroll_up();
+ assert_eq!(popup.tasks_table_state.selected(), Some(1));
+ }
+
+ #[test]
+ fn scroll_up_in_tasks_view_at_first_selects_last() {
+ let mut popup = make_popup_with_tasks(3);
+ popup.set_tasks_view();
+ // tasks_table_state is already at None after set_tasks_view
+ popup.scroll_up();
+ assert_eq!(popup.tasks_table_state.selected(), Some(2));
+ }
+
+ #[test]
+ fn scroll_up_in_tasks_view_from_none_selects_last() {
+ let mut popup = make_popup_with_tasks(3);
+ popup.set_tasks_view();
+ popup.tasks_table_state.select(None);
+ popup.scroll_up();
+ assert_eq!(popup.tasks_table_state.selected(), Some(2));
+ }
+
+ #[test]
+ fn scroll_up_in_tasks_view_empty_tasks_does_nothing() {
+ let mut popup = make_popup_with_tasks(0);
+ popup.set_tasks_view();
+ popup.scroll_up();
+ assert_eq!(popup.tasks_table_state.selected(), None);
+ }
+
+ // --- Plan view scrolling ---
+
+ #[test]
+ fn set_plan_view_resets_scroll_positions() {
+ let mut popup = make_popup(2);
+ popup.set_plan_view();
+ popup.scroll_right();
+ popup.scroll_down();
+ popup.set_plan_view();
+ assert_eq!(popup.plan_horizontal_scroll_position(), 0);
+ assert_eq!(popup.plan_vertical_scroll_position(), 0);
+ }
+
+ #[test]
+ fn scroll_down_in_plan_view_increments_vertical() {
+ let mut popup = make_popup(2);
+ popup.set_plan_view();
+ popup.scroll_down();
+ assert_eq!(popup.plan_vertical_scroll_position(), 1);
+ }
+
+ #[test]
+ fn scroll_up_in_plan_view_decrements_vertical() {
+ let mut popup = make_popup(2);
+ popup.set_plan_view();
+ popup.scroll_down();
+ popup.scroll_up();
+ assert_eq!(popup.plan_vertical_scroll_position(), 0);
+ }
+
+ #[test]
+ fn scroll_up_in_plan_view_saturates_at_zero() {
+ let mut popup = make_popup(2);
+ popup.set_plan_view();
+ popup.scroll_up();
+ assert_eq!(popup.plan_vertical_scroll_position(), 0);
+ }
+
+ #[test]
+ fn scroll_right_increments_horizontal() {
+ let mut popup = make_popup(2);
+ popup.set_plan_view();
+ popup.scroll_right();
+ assert_eq!(popup.plan_horizontal_scroll_position(), 1);
+ }
+
+ #[test]
+ fn scroll_left_decrements_horizontal() {
+ let mut popup = make_popup(2);
+ popup.set_plan_view();
+ popup.scroll_right();
+ popup.scroll_left();
+ assert_eq!(popup.plan_horizontal_scroll_position(), 0);
+ }
+
+ #[test]
+ fn scroll_left_saturates_at_zero() {
+ let mut popup = make_popup(2);
+ popup.set_plan_view();
+ popup.scroll_left();
+ assert_eq!(popup.plan_horizontal_scroll_position(), 0);
+ }
+
+ // --- scroll_left / scroll_right no-ops outside plan view ---
+
+ #[test]
+ fn scroll_left_in_no_details_view_does_nothing() {
+ let mut popup = make_popup(2);
+ popup.scroll_left();
+ assert_eq!(popup.plan_horizontal_scroll_position(), 0);
+ }
+
+ #[test]
+ fn scroll_right_in_no_details_view_does_nothing() {
+ let mut popup = make_popup(2);
+ popup.scroll_right();
+ assert_eq!(popup.plan_horizontal_scroll_position(), 0);
+ }
}
diff --git a/ballista-cli/src/tui/ui/main/jobs/job_stages_popup.rs
b/ballista-cli/src/tui/ui/main/jobs/job_stages_popup.rs
index f0b641a13..3e9631a7f 100644
--- a/ballista-cli/src/tui/ui/main/jobs/job_stages_popup.rs
+++ b/ballista-cli/src/tui/ui/main/jobs/job_stages_popup.rs
@@ -80,7 +80,9 @@ pub(crate) fn render_job_stages_popup(f: &mut Frame, app:
&App) {
.highlight_spacing(HighlightSpacing::Always);
let mut table_state = popup.table_state;
+ let mut scroll_state = popup.scrollbar_state;
f.render_stateful_widget(table, area, &mut table_state);
+ crate::tui::ui::vertical_scrollbar::render_scrollbar(f, area, &mut
scroll_state);
}
fn build_stage_row(i: usize, stage: &JobStageResponse, app: &App) ->
Row<'static> {
diff --git a/ballista-cli/src/tui/ui/main/jobs/stage_tasks_popup.rs
b/ballista-cli/src/tui/ui/main/jobs/stage_tasks_popup.rs
index 7b371da91..d012fb0e7 100644
--- a/ballista-cli/src/tui/ui/main/jobs/stage_tasks_popup.rs
+++ b/ballista-cli/src/tui/ui/main/jobs/stage_tasks_popup.rs
@@ -93,7 +93,10 @@ pub(crate) fn render_stage_tasks_popup(f: &mut Frame, app:
&App) {
.row_highlight_style(Style::default().bg(Color::Indexed(29)))
.highlight_spacing(HighlightSpacing::Always);
- f.render_widget(table, area);
+ let mut table_state = popup.tasks_table_state;
+ let mut scroll_state = popup.tasks_scrollbar_state;
+ f.render_stateful_widget(table, area, &mut table_state);
+ crate::tui::ui::vertical_scrollbar::render_scrollbar(f, area, &mut
scroll_state);
}
fn build_stage_task_row(i: usize, task: &StageTaskResponse, app: &App) ->
Row<'static> {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
