Skip to content

--show-progress=running enhancements #2729

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
+52 −70
Closed

--show-progress=running enhancements #2729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2e15432
Reduce flicker by not calling suspend() if not needed
glehmann Nov 1, 2025
7a6fe7b
Avoid not redrawn parts by using set_move_cursor(false)
glehmann Nov 1, 2025
47697c1
Don't wait for ShowProgress event to show the new tests
glehmann Nov 1, 2025
7d88cf1
Reducing tick interval reduces flicker
glehmann Nov 1, 2025
5944924
Only create the free bar when writing a message
glehmann Nov 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions nextest-runner/src/reporter/displayer/imp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ use std::{
};

/// The duration after which ticks are generated.
pub const TICK_INTERVAL: Duration = Duration::from_millis(200);
pub const TICK_INTERVAL: Duration = Duration::from_millis(100);

/// The refresh rate for the progress bar, set to half the tick interval in hz.
pub const PROGRESS_REFRESH_RATE_HZ: u8 = 10;
pub const PROGRESS_REFRESH_RATE_HZ: u8 = 20;
Copy link
Member

@sunshowers sunshowers Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I feel like this might be too fast especially over a slower connection like sshing into a remote.

I think setting both to be the same might cause weird issues where both are slightly out of sync with each other. In any case the comment here needs to be updated.

Copy link
Contributor Author

@glehmann glehmann Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reduced the refresh rate to 10 Hz. I'm still experimenting with the tick interval.


pub(crate) struct DisplayReporterBuilder {
pub(crate) default_filter: CompiledDefaultFilter,
Expand Down
118 changes: 50 additions & 68 deletions nextest-runner/src/reporter/displayer/progress.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ use crate::{
use iddqd::{IdHashItem, IdHashMap, id_upcast};
use indexmap::IndexSet;
use indicatif::{MultiProgress, ProgressBar, ProgressDrawTarget, ProgressStyle};
use itertools::Itertools as _;
use nextest_metadata::RustBinaryId;
use owo_colors::OwoColorize;
use std::{
collections::VecDeque,
cmp::max,
env, fmt,
io::{self, IsTerminal, Write},
num::NonZero,
Expand Down Expand Up @@ -152,19 +153,16 @@ impl SummaryBar {

#[derive(Debug)]
pub(super) struct TestProgressBars {
// A list of currently unused progress bars.
//
// Once a test bar is removed, we add an empty bar, then use it to avoid the
// overall progress bar shifting rapidly up and down.
free_bars: VecDeque<ProgressBar>,
used_bars: IdHashMap<TestProgressBar>,

// Overflow tests in FIFO order (the IndexSet preserves insertion order).
//
// Tests are displayed if they're in used_bars but not in overflow_order.
overflow_queue: IndexSet<(RustBinaryId, String)>,
// Maximum number of tests to display.
max_displayed: MaxProgressRunning,
max_progress_running: MaxProgressRunning,
// Actual maximum number of test displayed
max_displayed: usize,

// Summary bar showing the overflow count. This is Some if the summary bar
// is currently displayed.
Expand All @@ -174,12 +172,12 @@ pub(super) struct TestProgressBars {
}

impl TestProgressBars {
fn new(spinner_chars: &'static str, max_displayed: MaxProgressRunning) -> Self {
fn new(spinner_chars: &'static str, max_progress_running: MaxProgressRunning) -> Self {
Self {
free_bars: VecDeque::new(),
used_bars: IdHashMap::new(),
overflow_queue: IndexSet::new(),
max_displayed,
max_progress_running,
max_displayed: 0,
summary_bar: None,
spinner_chars,
}
Expand Down Expand Up @@ -231,22 +229,7 @@ impl TestProgressBars {
// multi-progress.
multi.remove(&data.bar);

if self.promote_next_overflow_test(multi) {
// A test was promoted from the overflow queue. The promoted
// test takes this bar's place, so we don't need to add an
// empty bar for spacing.
} else {
// No test was promoted: add an empty bar for spacing.
let free_bar = ProgressBar::hidden();
free_bar.set_style(
ProgressStyle::default_spinner()
.template(" ")
.expect("this template is valid"),
);
let free_bar = multi.add(free_bar);
free_bar.tick();
self.free_bars.push_back(free_bar);
}
self.promote_next_overflow_test(multi);
}

// Update summary bar to reflect the new overflow count.
Expand Down Expand Up @@ -295,17 +278,12 @@ impl TestProgressBars {
.len()
.saturating_sub(self.overflow_queue.len());

let should_display = match self.max_displayed {
let should_display = match self.max_progress_running {
MaxProgressRunning::Infinite => true,
MaxProgressRunning::Count(max) => displayed_count < max.get(),
};

let bar = if should_display {
// Remove a free bar if available.
if let Some(free_bar) = self.free_bars.pop_front() {
multi.remove(&free_bar);
}

// Insert the bar at the correct position.
//
// displayed_count doesn't include this test yet, so insert at
Expand All @@ -332,14 +310,16 @@ impl TestProgressBars {

// Update the summary bar to reflect the overflow count.
self.update_summary_bar(multi, running_count, styles);

self.max_displayed = max(self.max_displayed, self.len());
}

/// Updates or creates/removes the summary bar showing the overflow count.
fn update_summary_bar(&mut self, multi: &MultiProgress, running_count: usize, styles: &Styles) {
// Use the running count rather than the length of the overflow queue.
// Since tests are added to the queue with a bit of a delay, using the
// running count reduces flickering.
let overflow_count = match self.max_displayed {
let overflow_count = match self.max_progress_running {
MaxProgressRunning::Count(count) => running_count.saturating_sub(count.get()),
MaxProgressRunning::Infinite => {
// No summary bar is displayed in this case.
Expand All @@ -352,13 +332,6 @@ impl TestProgressBars {
bar.set_overflow_count(overflow_count, styles);
} else {
// Add a summary bar.
//
// Remove a free bar if available (the summary bar takes its
// place).
if let Some(free_bar) = self.free_bars.pop_front() {
multi.remove(&free_bar);
}

let displayed_count = self
.used_bars
.len()
Expand All @@ -375,16 +348,6 @@ impl TestProgressBars {
// The above Option::take removes the summary bar from
// self.summary_bar when the overflow count reaches 0.
multi.remove(&summary_bar.bar);
// Add a free bar for spacing.
let free_bar = ProgressBar::hidden();
free_bar.set_style(
ProgressStyle::default_spinner()
.template(" ")
.expect("this template is valid"),
);
let free_bar = multi.add(free_bar);
free_bar.tick();
self.free_bars.push_back(free_bar);
}
}

Expand All @@ -403,12 +366,6 @@ impl TestProgressBars {
.find(|tb| tb.binary_id == test_key.0 && tb.test_name == test_key.1);

if let Some(tb) = tb {
// Remove a free bar if available (we're taking up a display
// slot).
if let Some(free_bar) = self.free_bars.pop_front() {
multi.remove(&free_bar);
}

// Make the bar visible by adding it to the multi-progress.
//
// We've already removed this test from overflow_order, so it's
Expand Down Expand Up @@ -443,13 +400,28 @@ impl TestProgressBars {
for tb in &self.used_bars {
tb.bar.finish_and_clear();
}
for bar in &self.free_bars {
bar.finish_and_clear();
}
if let Some(summary_bar) = &self.summary_bar {
summary_bar.bar.finish_and_clear();
}
}

fn len(&self) -> usize {
self.used_bars.len() + self.summary_bar.is_some() as usize
}

fn new_free_bar(&self, multi_progress: &MultiProgress) -> ProgressBar {
// push an bar with as much line as needed to match the max total lines displayed
let missing_lines = self.max_displayed - self.len();
let bar = ProgressBar::hidden();
bar.set_style(
ProgressStyle::default_spinner()
.template(&std::iter::repeat_n(" ", missing_lines).join("\n"))
.expect("this template is valid"),
);
let bar = multi_progress.add(bar);
bar.tick();
bar
}
}

#[derive(Debug)]
Expand Down Expand Up @@ -502,7 +474,7 @@ impl ProgressBarState {
) -> Self {
let multi_progress = MultiProgress::new();
multi_progress.set_draw_target(Self::stderr_target());
multi_progress.set_move_cursor(true);
// multi_progress.set_move_cursor(true);
Copy link
Member

@sunshowers sunshowers Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my testing I found that set_move_cursor(true) made things a bit better -- did it make things worse for you?

Copy link
Contributor Author

@glehmann glehmann Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enabling set_move_cursor(true) causes a visual bug where the end of a longer, previously drawn line isn't cleared when a shorter line overwrites it.
It was not visible before because of the total redraw when calling suspend()


let bar = multi_progress.add(ProgressBar::new(test_count as u64));
let test_count_width = format!("{test_count}").len();
Expand Down Expand Up @@ -574,6 +546,7 @@ impl ProgressBarState {
TestEventKind::TestStarted {
current_stats,
running,
test_instance,
..
} => {
self.hidden_between_sub_runs = false;
Expand All @@ -593,12 +566,6 @@ impl ProgressBarState {
if let Some(test_bars) = &mut self.test_bars {
test_bars.update_summary_bar(&self.multi_progress, *running, styles);
}
}
TestEventKind::TestShowProgress {
test_instance,
running,
..
} => {
Comment on lines -597 to -601
Copy link
Member

@sunshowers sunshowers Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, again worried about this overwhelming the display with fast test suites like clap over a slower connection.

if let Some(test_bars) = &mut self.test_bars {
test_bars.add_test(
&self.multi_progress,
Expand Down Expand Up @@ -743,8 +710,23 @@ impl ProgressBarState {
pub(super) fn write_buf(&self, buf: &[u8]) -> io::Result<()> {
// ProgressBar::println doesn't print status lines if the bar is
// hidden. The suspend method prints it in all cases.
self.multi_progress
.suspend(|| std::io::stderr().write_all(buf))
// suspend forces a full redraw, so we call it only if there is
// something in the buffer
if !buf.is_empty() {
let mut free_bar = None;
if let Some(test_bars) = &self.test_bars {
free_bar = Some(test_bars.new_free_bar(&self.multi_progress));
}
let res = self
.multi_progress
.suspend(|| std::io::stderr().write_all(buf));
if let Some(bar) = &free_bar {
self.multi_progress.remove(bar);
}
res
} else {
Ok(())
}
}

#[inline]
Expand Down
Loading