Merge pull request #4498 from jfinkels/dd-seconds-precision-3

dd: fix precision for display of total time spent

src/uu/dd/Cargo.toml

@@ -18,7 +18,7 @@ path = "src/dd.rs"
 clap = { workspace=true }
 gcd = { workspace=true }
 libc = { workspace=true }
-uucore = { workspace=true }
+uucore = { workspace=true, features=["memo"] }
 
 [target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies]
 signal-hook = { workspace=true }

src/uu/dd/src/progress.rs

@@ -13,6 +13,9 @@ use std::io::Write;
 use std::sync::mpsc;
 use std::time::Duration;
 
+use uucore::error::UResult;
+use uucore::memo::sprintf;
+
 use crate::numbers::{to_magnitude_and_suffix, SuffixType};
 
 // On Linux, we register a signal handler that prints progress updates.
@@ -131,7 +134,7 @@ impl ProgUpdate {
     /// prog_update.write_prog_line(&mut cursor, rewrite).unwrap();
     /// assert_eq!(cursor.get_ref(), b"0 bytes copied, 1.0 s, 0.0 B/s\n");
     /// ```
-    fn write_prog_line(&self, w: &mut impl Write, rewrite: bool) -> std::io::Result<()> {
+    fn write_prog_line(&self, w: &mut impl Write, rewrite: bool) -> UResult<()> {
         // The total number of bytes written as a string, in SI and IEC format.
         let btotal = self.write_stat.bytes_total;
         let btotal_metric = to_magnitude_and_suffix(btotal, SuffixType::Si);
@@ -148,27 +151,31 @@ impl ProgUpdate {
         // (`\n`) at the end.
         let (carriage_return, newline) = if rewrite { ("\r", "") } else { ("", "\n") };
 
+        // The duration should be formatted as in `printf %g`.
+        let duration_str = sprintf("%g", &[duration.to_string()])?;
+
         // If the number of bytes written is sufficiently large, then
         // print a more concise representation of the number, like
         // "1.2 kB" and "1.0 KiB".
         match btotal {
             1 => write!(
                 w,
-                "{carriage_return}{btotal} byte copied, {duration:.1} s, {transfer_rate}/s{newline}",
-            ),
+                "{carriage_return}{btotal} byte copied, {duration_str} s, {transfer_rate}/s{newline}",
+            )?,
             0..=999 => write!(
                 w,
-                "{carriage_return}{btotal} bytes copied, {duration:.1} s, {transfer_rate}/s{newline}",
-            ),
+                "{carriage_return}{btotal} bytes copied, {duration_str} s, {transfer_rate}/s{newline}",
+            )?,
             1000..=1023 => write!(
                 w,
-                "{carriage_return}{btotal} bytes ({btotal_metric}) copied, {duration:.1} s, {transfer_rate}/s{newline}",
-            ),
+                "{carriage_return}{btotal} bytes ({btotal_metric}) copied, {duration_str} s, {transfer_rate}/s{newline}",
+            )?,
             _ => write!(
                 w,
-                "{carriage_return}{btotal} bytes ({btotal_metric}, {btotal_bin}) copied, {duration:.1} s, {transfer_rate}/s{newline}",
-            ),
-        }
+                "{carriage_return}{btotal} bytes ({btotal_metric}, {btotal_bin}) copied, {duration_str} s, {transfer_rate}/s{newline}",
+            )?,
+        };
+        Ok(())
     }
 
     /// Write all summary statistics.
@@ -200,7 +207,7 @@ impl ProgUpdate {
     /// assert_eq!(iter.next().unwrap(), b"");
     /// assert!(iter.next().is_none());
     /// ```
-    fn write_transfer_stats(&self, w: &mut impl Write, new_line: bool) -> std::io::Result<()> {
+    fn write_transfer_stats(&self, w: &mut impl Write, new_line: bool) -> UResult<()> {
         if new_line {
             writeln!(w)?;
         }
@@ -496,6 +503,15 @@ mod tests {
         }
     }
 
+    fn prog_update_duration(duration: Duration) -> ProgUpdate {
+        ProgUpdate {
+            read_stat: Default::default(),
+            write_stat: Default::default(),
+            duration: duration,
+            complete: false,
+        }
+    }
+
     #[test]
     fn test_read_stat_report() {
         let read_stat = ReadStat::new(1, 2, 3);
@@ -552,40 +568,40 @@ mod tests {
         //     0 bytes copied, 7.9151e-05 s, 0.0 kB/s
         //
         // The throughput still does not match GNU dd.
-        assert_eq!(cursor.get_ref(), b"0 bytes copied, 1.0 s, 0.0 B/s\n");
+        assert_eq!(cursor.get_ref(), b"0 bytes copied, 1 s, 0.0 B/s\n");
 
         let prog_update = prog_update_write(1);
         let mut cursor = Cursor::new(vec![]);
         prog_update.write_prog_line(&mut cursor, rewrite).unwrap();
-        assert_eq!(cursor.get_ref(), b"1 byte copied, 1.0 s, 0.0 B/s\n");
+        assert_eq!(cursor.get_ref(), b"1 byte copied, 1 s, 0.0 B/s\n");
 
         let prog_update = prog_update_write(999);
         let mut cursor = Cursor::new(vec![]);
         prog_update.write_prog_line(&mut cursor, rewrite).unwrap();
-        assert_eq!(cursor.get_ref(), b"999 bytes copied, 1.0 s, 0.0 B/s\n");
+        assert_eq!(cursor.get_ref(), b"999 bytes copied, 1 s, 0.0 B/s\n");
 
         let prog_update = prog_update_write(1000);
         let mut cursor = Cursor::new(vec![]);
         prog_update.write_prog_line(&mut cursor, rewrite).unwrap();
         assert_eq!(
             cursor.get_ref(),
-            b"1000 bytes (1.0 kB) copied, 1.0 s, 1.0 kB/s\n"
+            b"1000 bytes (1.0 kB) copied, 1 s, 1.0 kB/s\n"
         );
 
         let prog_update = prog_update_write(1023);
         let mut cursor = Cursor::new(vec![]);
         prog_update.write_prog_line(&mut cursor, rewrite).unwrap();
         assert_eq!(
             cursor.get_ref(),
-            b"1023 bytes (1.0 kB) copied, 1.0 s, 1.0 kB/s\n"
+            b"1023 bytes (1.0 kB) copied, 1 s, 1.0 kB/s\n"
         );
 
         let prog_update = prog_update_write(1024);
         let mut cursor = Cursor::new(vec![]);
         prog_update.write_prog_line(&mut cursor, rewrite).unwrap();
         assert_eq!(
             cursor.get_ref(),
-            b"1024 bytes (1.0 kB, 1.0 KiB) copied, 1.0 s, 1.0 kB/s\n"
+            b"1024 bytes (1.0 kB, 1.0 KiB) copied, 1 s, 1.0 kB/s\n"
         );
     }
 
@@ -604,7 +620,7 @@ mod tests {
         let mut iter = cursor.get_ref().split(|v| *v == b'\n');
         assert_eq!(iter.next().unwrap(), b"0+0 records in");
         assert_eq!(iter.next().unwrap(), b"0+0 records out");
-        assert_eq!(iter.next().unwrap(), b"0 bytes copied, 1.0 s, 0.0 B/s");
+        assert_eq!(iter.next().unwrap(), b"0 bytes copied, 1 s, 0.0 B/s");
         assert_eq!(iter.next().unwrap(), b"");
         assert!(iter.next().is_none());
     }
@@ -623,11 +639,20 @@ mod tests {
         prog_update.write_prog_line(&mut cursor, rewrite).unwrap();
         prog_update.write_transfer_stats(&mut cursor, true).unwrap();
         let mut iter = cursor.get_ref().split(|v| *v == b'\n');
-        assert_eq!(iter.next().unwrap(), b"\r0 bytes copied, 1.0 s, 0.0 B/s");
+        assert_eq!(iter.next().unwrap(), b"\r0 bytes copied, 1 s, 0.0 B/s");
         assert_eq!(iter.next().unwrap(), b"0+0 records in");
         assert_eq!(iter.next().unwrap(), b"0+0 records out");
-        assert_eq!(iter.next().unwrap(), b"0 bytes copied, 1.0 s, 0.0 B/s");
+        assert_eq!(iter.next().unwrap(), b"0 bytes copied, 1 s, 0.0 B/s");
         assert_eq!(iter.next().unwrap(), b"");
         assert!(iter.next().is_none());
     }
+
+    #[test]
+    fn test_duration_precision() {
+        let prog_update = prog_update_duration(Duration::from_nanos(123));
+        let mut cursor = Cursor::new(vec![]);
+        let rewrite = false;
+        prog_update.write_prog_line(&mut cursor, rewrite).unwrap();
+        assert_eq!(cursor.get_ref(), b"0 bytes copied, 1.23e-07 s, 0.0 B/s\n");
+    }
 }

tests/by-util/test_dd.rs

@@ -2,6 +2,8 @@
 
 use crate::common::util::*;
 
+use regex::Regex;
+
 use std::fs::{File, OpenOptions};
 use std::io::{BufReader, Read, Write};
 use std::path::PathBuf;
@@ -261,7 +263,9 @@ fn test_final_stats_noxfer() {
 fn test_final_stats_unspec() {
     new_ucmd!()
         .run()
-        .stderr_only("0+0 records in\n0+0 records out\n0 bytes copied, 0.0 s, 0.0 B/s\n")
+        .stderr_contains("0+0 records in\n0+0 records out\n0 bytes copied, ")
+        .stderr_matches(&Regex::new(r"\d\.\d+(e-\d\d)? s, ").unwrap())
+        .stderr_contains("0.0 B/s")
         .success();
 }
 
@@ -375,9 +379,11 @@ fn test_existing_file_truncated() {
 #[test]
 fn test_null_stats() {
     new_ucmd!()
-        .args(&["if=null.txt"])
+        .arg("if=null.txt")
         .run()
-        .stderr_only("0+0 records in\n0+0 records out\n0 bytes copied, 0.0 s, 0.0 B/s\n")
+        .stderr_contains("0+0 records in\n0+0 records out\n0 bytes copied, ")
+        .stderr_matches(&Regex::new(r"\d\.\d+(e-\d\d)? s, ").unwrap())
+        .stderr_contains("0.0 B/s")
         .success();
 }
 

tests/common/util.rs

@@ -692,6 +692,16 @@ impl CmdResult {
         self
     }
 
+    #[track_caller]
+    pub fn stderr_matches(&self, regex: &regex::Regex) -> &Self {
+        assert!(
+            regex.is_match(self.stderr_str()),
+            "Stderr does not match regex:\n{}",
+            self.stderr_str()
+        );
+        self
+    }
+
     #[track_caller]
     pub fn stdout_does_not_match(&self, regex: &regex::Regex) -> &Self {
         assert!(