Skip to content

Commit

Permalink
progress: add test of progress bar's periodic output
Browse files Browse the repository at this point in the history
The progress bar is supposed to update only 30 times per second, but
as was reported in #5057, that doesn't seem to be what's
happening. This patch adds tests showing how we update the progress
bar for every event.
  • Loading branch information
martinvonz committed Dec 10, 2024
1 parent 0017747 commit 4fc539b
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 15 deletions.
43 changes: 40 additions & 3 deletions cli/src/progress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ impl Progress {
}
}

pub fn update(
pub fn update<W: std::io::Write>(
&mut self,
now: Instant,
progress: &git::Progress,
output: &mut ProgressOutput,
output: &mut ProgressOutput<W>,
) -> io::Result<()> {
use std::fmt::Write as _;

Expand Down Expand Up @@ -166,7 +166,7 @@ impl RateEstimateState {
pub fn snapshot_progress(ui: &Ui) -> Option<impl Fn(&RepoPath) + '_> {
struct State {
guard: Option<OutputGuard>,
output: ProgressOutput,
output: ProgressOutput<std::io::Stderr>,
next_display_time: Instant,
}

Expand Down Expand Up @@ -215,6 +215,8 @@ pub fn snapshot_progress(ui: &Ui) -> Option<impl Fn(&RepoPath) + '_> {

#[cfg(test)]
mod tests {
use insta::assert_snapshot;

use super::*;

#[test]
Expand All @@ -233,4 +235,39 @@ mod tests {
assert_eq!(buf, "█████▍ ");
buf.clear();
}

#[test]
fn test_update() {
let start = Instant::now();
let mut progress = Progress::new(start);
let mut current_time = start;
let mut update = |duration, overall| -> String {
current_time += duration;
let mut buf = vec![];
let mut output = ProgressOutput::for_test(&mut buf, 25);
progress
.update(
current_time,
&jj_lib::git::Progress {
bytes_downloaded: None,
overall,
},
&mut output,
)
.unwrap();
String::from_utf8(buf).unwrap()
};
// First output is after the initial delay
assert_snapshot!(update(INITIAL_DELAY - Duration::from_millis(1), 0.1), @"");
assert_snapshot!(update(Duration::from_millis(1), 0.10), @"[?25l\r 10% [█▊ ]");
// TODO: No updates for the next 30 milliseconds
assert_snapshot!(update(Duration::from_millis(10), 0.11), @" 11% [██ ]");
assert_snapshot!(update(Duration::from_millis(10), 0.12), @" 12% [██▏ ]");
assert_snapshot!(update(Duration::from_millis(10), 0.13), @" 13% [██▍ ]");
// We get an update now that we go over the threshold
assert_snapshot!(update(Duration::from_millis(100), 0.30), @" 30% [█████▍ ]");
// TODO: Even though we went over by quite a bit, the new threshold is relative
// to the previous output, so we don't get an update here
assert_snapshot!(update(Duration::from_millis(30), 0.40), @" 40% [███████▎ ]");
}
}
42 changes: 30 additions & 12 deletions cli/src/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,10 +454,9 @@ impl Ui {
}
}

pub fn progress_output(&self) -> Option<ProgressOutput> {
self.use_progress_indicator().then(|| ProgressOutput {
output: io::stderr(),
})
pub fn progress_output(&self) -> Option<ProgressOutput<std::io::Stderr>> {
self.use_progress_indicator()
.then(ProgressOutput::for_stderr)
}

/// Writer to print an update that's not part of the command's main output.
Expand Down Expand Up @@ -633,22 +632,31 @@ impl Ui {
}

#[derive(Debug)]
pub struct ProgressOutput {
output: Stderr,
pub struct ProgressOutput<W> {
output: W,
term_width: Option<u16>,
}

impl ProgressOutput {
pub fn write_fmt(&mut self, fmt: fmt::Arguments<'_>) -> io::Result<()> {
self.output.write_fmt(fmt)
impl ProgressOutput<io::Stderr> {
pub fn for_stderr() -> ProgressOutput<io::Stderr> {
ProgressOutput {
output: io::stderr(),
term_width: None,
}
}
}

pub fn flush(&mut self) -> io::Result<()> {
self.output.flush()
impl<W> ProgressOutput<W> {
pub fn for_test(output: W, term_width: u16) -> Self {
Self {
output,
term_width: Some(term_width),
}
}

pub fn term_width(&self) -> Option<u16> {
// Terminal can be resized while progress is displayed, so don't cache it.
term_width()
self.term_width.or_else(term_width)
}

/// Construct a guard object which writes `text` when dropped. Useful for
Expand All @@ -661,6 +669,16 @@ impl ProgressOutput {
}
}

impl<W: Write> ProgressOutput<W> {
pub fn write_fmt(&mut self, fmt: fmt::Arguments<'_>) -> io::Result<()> {
self.output.write_fmt(fmt)
}

pub fn flush(&mut self) -> io::Result<()> {
self.output.flush()
}
}

pub struct OutputGuard {
text: String,
output: Stderr,
Expand Down

0 comments on commit 4fc539b

Please sign in to comment.