Skip to content

Commit

Permalink
Partially fix flaky tests (#308)
Browse files Browse the repository at this point in the history
* Minor code style changes

* Disable rendering of timestamps in tests

* Update test snapshots

* Test everything with insta macros (no more `assert(_eq)?`)

- This has the benefit of creating snapshots for everything, allowing for later diffing

* Don't use `assert_debug_snapshot` for large string outputs

- This makes snapshots more human-inspectable

* Code style improvement on two tests

- `pause_by_space`
- `rearranged_by_tab`

* Minor code style improvements
  • Loading branch information
cyqsimon authored Oct 17, 2023
1 parent 5d2ee96 commit d9cc84b
Show file tree
Hide file tree
Showing 91 changed files with 2,680 additions and 2,278 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ insta = "1.34.0"
pnet_base = "0.34.0"
packet-builder = { version = "0.7.0", git = "https://github.com/cyqsimon/packet_builder.git", branch = "patch-update" }
rstest = "0.18.2"
itertools = "0.11.0"

[target.'cfg(target_os="windows")'.build-dependencies]
anyhow = "1.0.75"
Expand Down
68 changes: 31 additions & 37 deletions src/display/components/header_details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,10 @@ use ratatui::{
text::Span,
widgets::Paragraph,
};
use unicode_width::UnicodeWidthStr;

use crate::display::{DisplayBandwidth, UIState};

const SECONDS_IN_DAY: u64 = 86400;

pub struct HeaderDetails<'a> {
pub state: &'a UIState,
pub elapsed_time: std::time::Duration,
pub paused: bool,
}

pub fn elapsed_time(last_start_time: Instant, cumulative_time: Duration, paused: bool) -> Duration {
if paused {
cumulative_time
Expand All @@ -27,27 +20,46 @@ pub fn elapsed_time(last_start_time: Instant, cumulative_time: Duration, paused:
}
}

fn format_duration(d: Duration) -> String {
let s = d.as_secs();
let days = match s / 86400 {
0 => "".to_string(),
1 => "1 day, ".to_string(),
n => format!("{n} days, "),
};
format!(
"{days}{:02}:{:02}:{:02}",
(s / 3600) % 24,
(s / 60) % 60,
s % 60,
)
}

pub struct HeaderDetails<'a> {
pub state: &'a UIState,
pub elapsed_time: Duration,
pub paused: bool,
}

impl<'a> HeaderDetails<'a> {
#[allow(clippy::int_plus_one)]
pub fn render(&self, frame: &mut Frame<impl Backend>, rect: Rect) {
let bandwidth = self.bandwidth_string();
let mut elapsed_time = None;
let print_elapsed_time = if self.state.cumulative_mode {
elapsed_time = Some(self.elapsed_time_string());
bandwidth.len() + elapsed_time.as_ref().unwrap().len() + 1 <= rect.width as usize
} else {
false
};

let color = if self.paused {
Color::Yellow
} else {
Color::Green
};

if print_elapsed_time {
self.render_elapsed_time(frame, rect, elapsed_time.as_ref().unwrap(), color);
// do not render time in tests, otherwise the output becomes non-deterministic
// see: https://github.com/imsnif/bandwhich/issues/303
if cfg!(not(test)) && self.state.cumulative_mode {
let elapsed_time = format_duration(self.elapsed_time);
// only render if there is enough width
if bandwidth.width() + 1 + elapsed_time.width() <= rect.width as usize {
self.render_elapsed_time(frame, rect, &elapsed_time, color);
}
}

self.render_bandwidth(frame, rect, &bandwidth, color);
}

Expand Down Expand Up @@ -97,22 +109,4 @@ impl<'a> HeaderDetails<'a> {
let paragraph = Paragraph::new(elapsed_time_text).alignment(Alignment::Right);
frame.render_widget(paragraph, rect);
}

fn days_string(&self) -> String {
match self.elapsed_time.as_secs() / SECONDS_IN_DAY {
0 => "".to_string(),
1 => "1 day, ".to_string(),
n => format!("{n} days, "),
}
}

fn elapsed_time_string(&self) -> String {
format!(
"{}{:02}:{:02}:{:02} ",
self.days_string(),
(self.elapsed_time.as_secs() % SECONDS_IN_DAY) / 3600,
(self.elapsed_time.as_secs() % 3600) / 60,
self.elapsed_time.as_secs() % 60
)
}
}
27 changes: 10 additions & 17 deletions src/display/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,24 +123,17 @@ where
}

pub fn draw(&mut self, paused: bool, show_dns: bool, elapsed_time: Duration, ui_offset: usize) {
let state = &self.state;
let children = self.get_tables_to_display();
let layout = Layout {
header: HeaderDetails {
state: &self.state,
elapsed_time,
paused,
},
children: self.get_tables_to_display(),
footer: HelpText { paused, show_dns },
};
self.terminal
.draw(|frame| {
let size = frame.size();
let total_bandwidth = HeaderDetails {
state,
elapsed_time,
paused,
};
let help_text = HelpText { paused, show_dns };
let layout = Layout {
header: total_bandwidth,
children,
footer: help_text,
};
layout.render(frame, size, ui_offset);
})
.draw(|frame| layout.render(frame, frame.size(), ui_offset))
.unwrap();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
source: src/tests/cases/ui.rs
expression: "&terminal_draw_events_mirror[0]"
expression: terminal_draw_events.lock().unwrap().join(SNAPSHOT_SECTION_SEPARATOR)
---
Total Up / Down: 0Bps / 0Bps
Utilization by remote address───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
source: src/tests/cases/ui.rs
expression: "&terminal_draw_events_mirror[0]"
expression: terminal_draw_events.lock().unwrap().join(SNAPSHOT_SECTION_SEPARATOR)
---
Total Up / Down: 0Bps / 0Bps
Utilization by connection───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
source: src/tests/cases/ui.rs
expression: "&terminal_draw_events_mirror[0]"
expression: terminal_draw_events.lock().unwrap().join(SNAPSHOT_SECTION_SEPARATOR)
---
Total Up / Down: 0Bps / 0Bps
Utilization by process name─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
source: src/tests/cases/ui.rs
expression: "&terminal_draw_events_mirror[0]"
expression: terminal_draw_events.lock().unwrap().join(SNAPSHOT_SECTION_SEPARATOR)
---
Total Up / Down: 0Bps / 0Bps
Utilization by process name─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
source: src/tests/cases/ui.rs
expression: "&terminal_draw_events_mirror[0]"
expression: terminal_draw_events.lock().unwrap().join(SNAPSHOT_SECTION_SEPARATOR)
---
Total Up / Down: 0Bps / 0Bps
Utilization by process name──────────────────────────────────────────────────────────────────┐┌Utilization by remote address────────────────────────────────────────────────────────────────┐
Expand Down
Original file line number Diff line number Diff line change
@@ -1,55 +1,15 @@
---
source: src/tests/cases/ui.rs
expression: "&terminal_draw_events_mirror[1]"
expression: terminal_events.lock().unwrap().as_slice()
---
24Bps / 25Bps


1 1 24Bps / 25Bps 1.1.1.1 1 24Bps / 25Bps























<interface_name>:443 => 1.1.1.1:12345 (tcp) 1 24Bps / 25Bps























[
Clear,
HideCursor,
Draw,
HideCursor,
Flush,
Draw,
HideCursor,
Flush,
ShowCursor,
]

This file was deleted.

Loading

0 comments on commit d9cc84b

Please sign in to comment.