Skip to content
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.

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

fix(cmd_duration): Make render_time format more consistent #5825

Merged
merged 3 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 src/modules/aws.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ credential_process = /opt/bin/awscreds-retriever
.collect();

let possible_values = [
"30m2s", "30m1s", "30m", "29m59s", "29m58s", "29m57s", "29m56s", "29m55s",
"30m2s", "30m1s", "30m0s", "29m59s", "29m58s", "29m57s", "29m56s", "29m55s",
];
let possible_values = possible_values.map(|duration| {
let segment_colored = format!("☁️ astronauts (ap-northeast-2) [{duration}] ");
Expand Down Expand Up @@ -761,7 +761,7 @@ aws_secret_access_key=dummy
// In principle, "30m" should be correct. However, bad luck in scheduling
// on shared runners may delay it.
let possible_values = [
"30m2s", "30m1s", "30m", "29m59s", "29m58s", "29m57s", "29m56s", "29m55s",
"30m2s", "30m1s", "30m0s", "29m59s", "29m58s", "29m57s", "29m56s", "29m55s",
];
let possible_values = possible_values.map(|duration| {
let segment_colored = format!("☁️ astronauts (ap-northeast-2) [{duration}] ");
Expand Down
76 changes: 45 additions & 31 deletions src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -611,9 +611,11 @@ pub fn exec_timeout(cmd: &mut Command, time_limit: Duration) -> Option<CommandOu

// Render the time into a nice human-readable string
pub fn render_time(raw_millis: u128, show_millis: bool) -> String {
// Make sure it renders something if the time equals zero instead of an empty string
if raw_millis == 0 {
return "0ms".into();
// Fast returns for zero cases to render something
match (raw_millis, show_millis) {
(0, true) => return "0ms".into(),
(0..=999, false) => return "0s".into(),
_ => (),
}

// Calculate a simple breakdown into days/hours/minutes/seconds/milliseconds
Expand All @@ -622,25 +624,29 @@ pub fn render_time(raw_millis: u128, show_millis: bool) -> String {
let (minutes, raw_hours) = (raw_minutes % 60, raw_minutes / 60);
let (hours, days) = (raw_hours % 24, raw_hours / 24);

let components = [days, hours, minutes, seconds];
let suffixes = ["d", "h", "m", "s"];

let mut rendered_components: Vec<String> = components
.iter()
.zip(&suffixes)
.map(render_time_component)
.collect();
if show_millis || raw_millis < 1000 {
rendered_components.push(render_time_component((&millis, &"ms")));
}
rendered_components.join("")
}
// Calculate how long the string will be to allocate once in most cases
let result_capacity = match raw_millis {
1..=59 => 3,
60..=3599 => 6,
3600..=86399 => 9,
_ => 12,
} + if show_millis { 5 } else { 0 };

let components = [(days, "d"), (hours, "h"), (minutes, "m"), (seconds, "s")];

// Concat components ito result starting from the first non-zero one
let result = components.iter().fold(
String::with_capacity(result_capacity),
|acc, (component, suffix)| match component {
0 if acc.is_empty() => acc,
n => acc + &n.to_string() + suffix,
},
);

/// Render a single component of the time string, giving an empty string if component is zero
fn render_time_component((component, suffix): (&u128, &&str)) -> String {
match component {
0 => String::new(),
n => format!("{n}{suffix}"),
if show_millis {
result + &millis.to_string() + "ms"
} else {
result
}
}

Expand Down Expand Up @@ -702,28 +708,36 @@ mod tests {
use super::*;

#[test]
fn test_0ms() {
fn render_time_test_0ms() {
assert_eq!(render_time(0_u128, true), "0ms")
}
#[test]
fn test_500ms() {
fn render_time_test_0s() {
assert_eq!(render_time(0_u128, false), "0s")
}
#[test]
fn render_time_test_500ms() {
assert_eq!(render_time(500_u128, true), "500ms")
}
#[test]
fn test_10s() {
assert_eq!(render_time(10_000_u128, true), "10s")
fn render_time_test_500ms_no_millis() {
assert_eq!(render_time(500_u128, false), "0s")
}
#[test]
fn render_time_test_10s() {
assert_eq!(render_time(10_000_u128, true), "10s0ms")
}
#[test]
fn test_90s() {
assert_eq!(render_time(90_000_u128, true), "1m30s")
fn render_time_test_90s() {
assert_eq!(render_time(90_000_u128, true), "1m30s0ms")
}
#[test]
fn test_10110s() {
assert_eq!(render_time(10_110_000_u128, true), "2h48m30s")
fn render_time_test_10110s() {
assert_eq!(render_time(10_110_000_u128, true), "2h48m30s0ms")
}
#[test]
fn test_1d() {
assert_eq!(render_time(86_400_000_u128, true), "1d")
fn render_time_test_1d() {
assert_eq!(render_time(86_400_000_u128, false), "1d0h0m0s")
}

#[test]
Expand Down
Loading