Repository: uutils/procps
Pull request #212 (2024-09-11 15:23:29)

free: fail if zero is provided for `-c`/`--count` or `-s`/`--seconds`
This PR does three things:

- refactors `test_count` to get rid of `-s 0`, and `test_no_args` to extract code used by both test functions
- fails if `0` is provided for `-c`/`--count`
- fails if `0` is provided for `-s`/`--seconds`

The error messages are different from the original `free`, hopefully better.


Patch:
diff --git a/src/uu/free/src/free.rs b/src/uu/free/src/free.rs
index 0bed27e..a253577 100644
--- a/src/uu/free/src/free.rs
+++ b/src/uu/free/src/free.rs
@@ -15,7 +15,10 @@ use std::ops::Mul;
 use std::process;
 use std::thread::sleep;
 use std::time::Duration;
-use uucore::{error::UResult, format_usage, help_about, help_usage};
+use uucore::{
+    error::{UResult, USimpleError},
+    format_usage, help_about, help_usage,
+};
 
 const ABOUT: &str = help_about!("free.md");
 const USAGE: &str = help_usage!("free.md");
@@ -148,9 +151,27 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let matches = uu_app().try_get_matches_from(args)?;
 
     let count_flag = matches.get_one("count");
-    let mut count: u64 = count_flag.unwrap_or(&1_u64).to_owned();
+    let mut count: u64 = match count_flag {
+        Some(0) => {
+            return Err(USimpleError::new(
+                1,
+                "count argument must be greater than 0",
+            ))
+        }
+        Some(c) => *c,
+        None => 1,
+    };
     let seconds_flag = matches.get_one("seconds");
-    let seconds: f64 = seconds_flag.unwrap_or(&1.0_f64).to_owned();
+    let seconds: f64 = match seconds_flag {
+        Some(0.0) => {
+            return Err(USimpleError::new(
+                1,
+                "seconds argument must be greater than 0",
+            ))
+        }
+        Some(s) => *s,
+        None => 1.0,
+    };
 
     let dur = Duration::from_nanos(seconds.mul(1_000_000_000.0).round() as u64);
 


Test patch:
diff --git a/tests/by-util/test_free.rs b/tests/by-util/test_free.rs
index af3cf5a..33e13e5 100644
--- a/tests/by-util/test_free.rs
+++ b/tests/by-util/test_free.rs
@@ -17,25 +17,10 @@ fn test_invalid_arg() {
 
 #[test]
 fn test_no_args() {
-    let header_pattern = r"^ {15}total {8}used {8}free {6}shared {2}buff/cache {3}available$";
-    let mem_pattern = r"^Mem:( +\d+){6}$";
-    let swap_pattern = r"^Swap: ( +\d+){3}$";
+    let output = new_ucmd!().succeeds().stdout_move_str();
 
-    let patterns = vec![
-        Regex::new(header_pattern).unwrap(),
-        Regex::new(mem_pattern).unwrap(),
-        Regex::new(swap_pattern).unwrap(),
-    ];
-
-    let binding = new_ucmd!().succeeds();
-    let output = binding.stdout_str();
     assert_eq!(output.len(), 207);
-
-    // Check the format for each line output
-    let mut lines = output.lines();
-    for pattern in patterns {
-        assert!(pattern.is_match(lines.next().unwrap()));
-    }
+    assert_default_format(&output);
 }
 
 #[test]
@@ -100,8 +85,29 @@ fn test_total() {
 
 #[test]
 fn test_count() {
-    let result = new_ucmd!().args(&["-c", "2", "-s", "0"]).succeeds();
-    assert_eq!(result.stdout_str().lines().count(), 7);
+    for arg in ["-c", "--count"] {
+        let output = new_ucmd!()
+            // without -s, there would be a delay of 1s between the output of the
+            // two blocks
+            .args(&[arg, "2", "-s", "0.00001"])
+            .succeeds()
+            .stdout_move_str();
+
+        let lines: Vec<&str> = output.lines().collect();
+
+        assert_default_format(&lines[..3].join("\n"));
+        assert!(lines[3].is_empty());
+        assert_default_format(&lines[4..].join("\n"));
+    }
+}
+
+#[test]
+fn test_count_zero() {
+    new_ucmd!()
+        .arg("--count=0")
+        .fails()
+        .code_is(1)
+        .stderr_only("free: count argument must be greater than 0\n");
 }
 
 #[test]
@@ -128,3 +134,33 @@ fn test_committed() {
             .starts_with("Comm:"));
     }
 }
+
+#[test]
+fn test_seconds_zero() {
+    for arg in ["-s", "--seconds"] {
+        new_ucmd!()
+            .arg(arg)
+            .arg("0")
+            .fails()
+            .code_is(1)
+            .stderr_only("free: seconds argument must be greater than 0\n");
+    }
+}
+
+fn assert_default_format(s: &str) {
+    let header_pattern = r"^ {15}total {8}used {8}free {6}shared {2}buff/cache {3}available$";
+    let mem_pattern = r"^Mem:( +\d+){6}$";
+    let swap_pattern = r"^Swap: ( +\d+){3}$";
+
+    let patterns = vec![
+        Regex::new(header_pattern).unwrap(),
+        Regex::new(mem_pattern).unwrap(),
+        Regex::new(swap_pattern).unwrap(),
+    ];
+
+    // Check the format for each line output
+    let mut lines = s.lines();
+    for pattern in patterns {
+        assert!(pattern.is_match(lines.next().unwrap()));
+    }
+}