From cb7c0c221a96786a9c1dc308312e6b5086af641d Mon Sep 17 00:00:00 2001 From: Alexander Kireev Date: Fri, 3 Jul 2026 09:00:20 +0700 Subject: [PATCH] Fix off-by-one that leaves one stale point in the graph buffer drain(0..idx) skips the element at idx, but idx is the index of the last point that's actually older than the buffer window (it matched the filter right above). So every update() call quietly leaves exactly one out-of-window sample sitting in the data, forever. Switched it to drain(0..=idx) and added a test that seeds a few stale/fresh points and checks nothing older than the window survives after update(). --- gping/src/plot_data.rs | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/gping/src/plot_data.rs b/gping/src/plot_data.rs index 702b53e5..aed960bb 100644 --- a/gping/src/plot_data.rs +++ b/gping/src/plot_data.rs @@ -45,7 +45,10 @@ impl PlotData { .map(|(idx, _)| idx) .next_back(); if let Some(idx) = last_idx { - self.data.drain(0..idx).for_each(drop) + // `idx` itself is still stale (it matched the filter above), so it must be + // included in the drained range too, otherwise one out-of-window point is + // always left behind. + self.data.drain(0..=idx).for_each(drop) } } @@ -114,3 +117,39 @@ impl<'a> From<&'a PlotData> for Dataset<'a> { .data(slice) } } + +#[cfg(test)] +mod tests { + use super::*; + + // Regression test for the buffer trim in `PlotData::update`: every point older than + // `buffer` seconds should be dropped, including the single oldest stale point, which + // used to survive because `drain(0..idx)` excluded the boundary index itself. + #[test] + fn update_drops_all_points_outside_the_buffer_window() { + let buffer_secs = 5u64; + let mut plot = PlotData::new("host".to_string(), buffer_secs, Style::default(), false); + + let now = Local::now().timestamp_millis() as f64 / 1_000f64; + + // Seed with a mix of stale (older than the buffer window) and fresh points, + // pushed in ascending timestamp order like the real update loop would. + plot.data.push((now - 10.0, 100.0)); // stale + plot.data.push((now - 8.0, 200.0)); // stale + plot.data.push((now - 6.0, 300.0)); // stale, closest to the boundary + plot.data.push((now - 2.0, 400.0)); // fresh + plot.data.push((now - 1.0, 500.0)); // fresh + + // Triggers the trim logic; also appends one brand-new point. + plot.update(Some(Duration::from_millis(42))); + + let earliest_allowed = now - buffer_secs as f64; + for &(timestamp, _) in &plot.data { + assert!( + timestamp >= earliest_allowed, + "found a point at {timestamp}, which is older than the buffer window start {earliest_allowed}; data: {:?}", + plot.data + ); + } + } +}