test_tail: clean up tests for --follow=name

src/uu/tail/src/tail.rs

@@ -63,6 +63,7 @@ pub mod options {
     pub static SLEEP_INT: &str = "sleep-interval";
     pub static ZERO_TERM: &str = "zero-terminated";
     pub static DISABLE_INOTIFY_TERM: &str = "disable-inotify";
+    pub static USE_POLLING: &str = "use-polling";
     pub static MAX_UNCHANGED_STATS: &str = "max-unchanged-stats";
     pub static ARG_FILES: &str = "files";
 }
@@ -84,7 +85,7 @@ struct Settings {
     max_unchanged_stats: usize,
     beginning: bool,
     follow: Option<FollowMode>,
-    force_polling: bool,
+    use_polling: bool,
     verbose: bool,
     pid: platform::Pid,
 }
@@ -97,7 +98,7 @@ impl Default for Settings {
             max_unchanged_stats: 5,
             beginning: false,
             follow: None,
-            force_polling: false,
+            use_polling: false,
             verbose: false,
             pid: 0,
         }
@@ -170,7 +171,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     settings.mode = mode_and_beginning.0;
     settings.beginning = mode_and_beginning.1;
 
-    settings.force_polling = matches.is_present(options::DISABLE_INOTIFY_TERM);
+    settings.use_polling = matches.is_present(options::USE_POLLING);
 
     if matches.is_present(options::ZERO_TERM) {
         if let FilterMode::Lines(count, _) = settings.mode {
@@ -393,9 +394,9 @@ pub fn uu_app() -> App<'static, 'static> {
                 .help("Line delimiter is NUL, not newline"),
         )
         .arg(
-            Arg::with_name(options::DISABLE_INOTIFY_TERM)
+            Arg::with_name(options::USE_POLLING)
-                .visible_alias("use-polling")
+                .visible_alias(options::DISABLE_INOTIFY_TERM)
-                .long(options::DISABLE_INOTIFY_TERM)
+                .long(options::USE_POLLING)
                 .help(text::BACKEND),
         )
         .arg(
@@ -413,7 +414,7 @@ fn follow(files: &mut FileHandling, settings: &Settings) {
     let (tx, rx) = channel();
 
     let mut watcher: Box<dyn Watcher>;
-    if settings.force_polling {
+    if settings.use_polling {
         // Polling based Watcher implementation
         watcher = Box::new(
             // TODO: [2021-09; jhscheer] remove arc/mutex if upstream merges:
@@ -550,7 +551,7 @@ fn handle_event(
                     // Rename: mv log.bak log.dat
 
                     if settings.follow == Some(FollowMode::Name) {
-                        let msg = if settings.force_polling {
+                        let msg = if settings.use_polling {
                             format!("{} has been replaced", display_name.quote())
                         } else {
                             format!("{} has appeared", display_name.quote())
@@ -623,7 +624,7 @@ fn handle_event(
 }
 
 fn get_path(path: &Path, settings: &Settings) -> PathBuf {
-    if cfg!(target_os = "linux") || settings.force_polling {
+    if cfg!(target_os = "linux") || settings.use_polling {
         // NOTE: Using the parent directory here instead of the file is a workaround.
         // On Linux the watcher can crash for rename/delete/move operations if a file is watched directly.
         // This workaround follows the recommendation of the notify crate authors:

tests/by-util/test_tail.rs

@@ -553,17 +553,10 @@ fn test_tail_follow_descriptor_vs_rename() {
         p.kill().unwrap();
         sleep(Duration::from_millis(delay));
 
-        let mut buf_stderr = String::new();
+        let (buf_stdout, _) = take_stdout_stderr(&mut p);
-        let mut p_stderr = p.stderr.take().unwrap();
-        p_stderr.read_to_string(&mut buf_stderr).unwrap();
-        println!("stderr:\n{}", buf_stderr);
-
-        let mut buf_stdout = String::new();
-        let mut p_stdout = p.stdout.take().unwrap();
-        p_stdout.read_to_string(&mut buf_stdout).unwrap();
         assert_eq!(buf_stdout, "x\ny\n");
 
-        let _ = args.pop();
+        args.pop();
     }
 }
 
@@ -605,15 +598,13 @@ fn test_tail_follow_descriptor_vs_rename_verbose() {
         p.kill().unwrap();
         sleep(Duration::from_millis(delay));
 
-        let mut buf_stdout = String::new();
+        let (buf_stdout, _) = take_stdout_stderr(&mut p);
-        let mut p_stdout = p.stdout.take().unwrap();
-        p_stdout.read_to_string(&mut buf_stdout).unwrap();
         assert_eq!(
             buf_stdout,
             "==> FILE_A <==\n\n==> FILE_B <==\n\n==> FILE_A <==\nx\n"
         );
 
-        let _ = args.pop();
+        args.pop();
     }
 }
 
@@ -627,34 +618,34 @@ fn test_follow_name_remove() {
     let at = &ts.fixtures;
 
     let source = FOLLOW_NAME_TXT;
-    let source_canonical = &at.plus(source);
+    let source_copy = "source_copy";
+    at.copy(source, source_copy);
 
     let expected_stdout = at.read(FOLLOW_NAME_SHORT_EXP);
     let expected_stderr = format!(
         "{}: {}: No such file or directory\n{0}: no files remaining\n",
-        ts.util_name, source
+        ts.util_name, source_copy
     );
 
-    let args = ["--follow=name", source];
-    let mut p = ts.ucmd().args(&args).run_no_wait();
-
     let delay = 1000;
+    let mut args = vec!["--follow=name", source_copy, "--use-polling"];
 
-    sleep(Duration::from_millis(delay));
+    for _ in 0..2 {
-    std::fs::remove_file(source_canonical).unwrap();
+        at.copy(source, source_copy);
-    sleep(Duration::from_millis(delay));
+        let mut p = ts.ucmd().args(&args).run_no_wait();
 
-    p.kill().unwrap();
+        sleep(Duration::from_millis(delay));
+        at.remove(source_copy);
+        sleep(Duration::from_millis(delay));
 
-    let mut buf_stdout = String::new();
+        p.kill().unwrap();
-    let mut p_stdout = p.stdout.take().unwrap();
-    p_stdout.read_to_string(&mut buf_stdout).unwrap();
-    assert_eq!(buf_stdout, expected_stdout);
 
-    let mut buf_stderr = String::new();
+        let (buf_stdout, buf_stderr) = take_stdout_stderr(&mut p);
-    let mut p_stderr = p.stderr.take().unwrap();
+        assert_eq!(buf_stdout, expected_stdout);
-    p_stderr.read_to_string(&mut buf_stderr).unwrap();
+        assert_eq!(buf_stderr, expected_stderr);
-    assert_eq!(buf_stderr, expected_stderr);
+
+        args.pop();
+    }
 }
 
 #[test]
@@ -667,8 +658,7 @@ fn test_follow_name_truncate() {
     let at = &ts.fixtures;
 
     let source = FOLLOW_NAME_TXT;
-    let source_canonical = &at.plus(source);
+    let backup = "backup";
-    let backup = at.plus_as_string("backup");
 
     let expected_stdout = at.read(FOLLOW_NAME_EXP);
     let expected_stderr = format!("{}: {}: file truncated\n", ts.util_name, source);
@@ -678,63 +668,17 @@ fn test_follow_name_truncate() {
 
     let delay = 1000;
 
-    std::fs::copy(&source_canonical, &backup).unwrap();
+    at.copy(source, backup);
     sleep(Duration::from_millis(delay));
-    let _ = std::fs::File::create(source_canonical).unwrap(); // trigger truncate
+    at.touch(source); // trigger truncate
     sleep(Duration::from_millis(delay));
-    std::fs::copy(&backup, &source_canonical).unwrap();
+    at.copy(backup, source);
     sleep(Duration::from_millis(delay));
 
     p.kill().unwrap();
 
-    let mut buf_stdout = String::new();
+    let (buf_stdout, buf_stderr) = take_stdout_stderr(&mut p);
-    let mut p_stdout = p.stdout.take().unwrap();
-    p_stdout.read_to_string(&mut buf_stdout).unwrap();
     assert_eq!(buf_stdout, expected_stdout);
-
-    let mut buf_stderr = String::new();
-    let mut p_stderr = p.stderr.take().unwrap();
-    p_stderr.read_to_string(&mut buf_stderr).unwrap();
-    assert_eq!(buf_stderr, expected_stderr);
-}
-
-#[test]
-#[cfg(not(windows))]
-fn test_follow_name_remove_polling() {
-    // This test triggers a remove event while `tail --follow=name ---disable-inotify logfile` is running.
-    // ((sleep 1 && rm logfile &)>/dev/null 2>&1 &) ; tail --follow=name ---disable-inotify logfile
-
-    let ts = TestScenario::new(util_name!());
-    let at = &ts.fixtures;
-
-    let source = FOLLOW_NAME_TXT;
-    let source_canonical = &at.plus(source);
-
-    let expected_stdout = at.read(FOLLOW_NAME_SHORT_EXP);
-    let expected_stderr = format!(
-        "{}: {}: No such file or directory\n{0}: no files remaining\n",
-        ts.util_name, source
-    );
-
-    let args = ["--follow=name", "--disable-inotify", source];
-    let mut p = ts.ucmd().args(&args).run_no_wait();
-
-    let delay = 1000;
-
-    sleep(Duration::from_millis(delay));
-    std::fs::remove_file(source_canonical).unwrap();
-    sleep(Duration::from_millis(delay));
-
-    p.kill().unwrap();
-
-    let mut buf_stdout = String::new();
-    let mut p_stdout = p.stdout.take().unwrap();
-    p_stdout.read_to_string(&mut buf_stdout).unwrap();
-    assert_eq!(buf_stdout, expected_stdout);
-
-    let mut buf_stderr = String::new();
-    let mut p_stderr = p.stderr.take().unwrap();
-    p_stderr.read_to_string(&mut buf_stderr).unwrap();
     assert_eq!(buf_stderr, expected_stderr);
 }
 
@@ -748,8 +692,7 @@ fn test_follow_name_move_create() {
     let at = &ts.fixtures;
 
     let source = FOLLOW_NAME_TXT;
-    let source_canonical = &at.plus(source);
+    let backup = "backup";
-    let backup = at.plus_as_string("backup");
 
     #[cfg(target_os = "linux")]
     let expected_stdout = at.read(FOLLOW_NAME_EXP);
@@ -771,62 +714,68 @@ fn test_follow_name_move_create() {
     let delay = 1000;
 
     sleep(Duration::from_millis(delay));
-    std::fs::rename(&source_canonical, &backup).unwrap();
+    at.rename(source, backup);
     sleep(Duration::from_millis(delay));
-    std::fs::copy(&backup, &source_canonical).unwrap();
+    at.copy(backup, source);
     sleep(Duration::from_millis(delay));
 
     p.kill().unwrap();
 
-    let mut buf_stdout = String::new();
+    let (buf_stdout, buf_stderr) = take_stdout_stderr(&mut p);
-    let mut p_stdout = p.stdout.take().unwrap();
-    p_stdout.read_to_string(&mut buf_stdout).unwrap();
     assert_eq!(buf_stdout, expected_stdout);
-
-    let mut buf_stderr = String::new();
-    let mut p_stderr = p.stderr.take().unwrap();
-    p_stderr.read_to_string(&mut buf_stderr).unwrap();
     assert_eq!(buf_stderr, expected_stderr);
 }
 
 #[test]
 #[cfg(not(windows))]
-fn test_follow_name_move_polling() {
+fn test_follow_name_move() {
-    // This test triggers a move event while `tail --follow=name --disable-inotify logfile` is running.
+    // This test triggers a move event while `tail --follow=name logfile` is running.
-    // ((sleep 1 && mv logfile backup && sleep 1 && cp backup logfile &)>/dev/null 2>&1 &) ; tail --follow=name ---disable-inotify logfile
+    // ((sleep 1 && mv logfile backup && sleep 1 && cp backup logfile &)>/dev/null 2>&1 &) ; tail --follow=name logfile
-    // NOTE: GNU's tail does not recognize this move event for `---disable-inotify`
+    // NOTE: GNU's tail does not seem to recognize this move event with `---disable-inotify`
 
     let ts = TestScenario::new(util_name!());
     let at = &ts.fixtures;
 
     let source = FOLLOW_NAME_TXT;
-    let source_canonical = &at.plus(source);
+    let backup = "backup";
-    let backup = at.plus_as_string("backup");
 
     let expected_stdout = at.read(FOLLOW_NAME_SHORT_EXP);
-    let expected_stderr = format!(
+    let expected_stderr = [
-        "{}: {}: No such file or directory\n{0}: no files remaining\n",
+        format!(
-        ts.util_name, source
+            "{}: {}: No such file or directory\n{0}: no files remaining\n",
-    );
+            ts.util_name, source
+        ),
+        format!("{}: {}: No such file or directory\n", ts.util_name, source),
+    ];
 
-    let args = ["--follow=name", "--disable-inotify", source];
+    let mut args = vec!["--follow=name", source, "--use-polling"];
-    let mut p = ts.ucmd().args(&args).run_no_wait();
 
-    let delay = 1000;
+    #[allow(clippy::needless_range_loop)]
+    for i in 0..2 {
+        let mut p = ts.ucmd().args(&args).run_no_wait();
+        let delay = 1000;
 
-    sleep(Duration::from_millis(delay));
+        sleep(Duration::from_millis(delay));
-    std::fs::rename(&source_canonical, &backup).unwrap();
+        at.rename(source, backup);
-    sleep(Duration::from_millis(delay));
+        sleep(Duration::from_millis(delay));
 
-    p.kill().unwrap();
+        p.kill().unwrap();
 
+        let (buf_stdout, buf_stderr) = take_stdout_stderr(&mut p);
+        assert_eq!(buf_stdout, expected_stdout);
+        assert_eq!(buf_stderr, expected_stderr[i]);
+
+        at.rename(backup, source);
+        args.pop();
+    }
+}
+
+fn take_stdout_stderr(p: &mut std::process::Child) -> (String, String) {
     let mut buf_stdout = String::new();
     let mut p_stdout = p.stdout.take().unwrap();
     p_stdout.read_to_string(&mut buf_stdout).unwrap();
-    assert_eq!(buf_stdout, expected_stdout);
-
     let mut buf_stderr = String::new();
     let mut p_stderr = p.stderr.take().unwrap();
     p_stderr.read_to_string(&mut buf_stderr).unwrap();
-    assert_eq!(buf_stderr, expected_stderr);
+    (buf_stdout, buf_stderr)
 }

tests/common/util.rs

@@ -555,6 +555,21 @@ impl AtPath {
             .unwrap_or_else(|e| panic!("Couldn't rename {:?} -> {:?}: {}", source, target, e));
     }
 
+    pub fn remove(&self, source: &str) {
+        let source = self.plus(source);
+        log_info("remove", format!("{:?}", source));
+        std::fs::remove_file(&source)
+            .unwrap_or_else(|e| panic!("Couldn't remove {:?}: {}", source, e));
+    }
+
+    pub fn copy(&self, source: &str, target: &str) {
+        let source = self.plus(source);
+        let target = self.plus(target);
+        log_info("copy", format!("{:?} {:?}", source, target));
+        std::fs::copy(&source, &target)
+            .unwrap_or_else(|e| panic!("Couldn't copy {:?} -> {:?}: {}", source, target, e));
+    }
+
     pub fn mkdir(&self, dir: &str) {
         log_info("mkdir", self.plus_as_string(dir));
         fs::create_dir(&self.plus(dir)).unwrap();
@@ -1044,6 +1059,8 @@ impl UCommand {
     }
 }
 
+/// Wrapper for `child.stdout.read_exact()`.
+/// Careful, this blocks indefinitely if `size` bytes is never reached.
 pub fn read_size(child: &mut Child, size: usize) -> String {
     let mut output = Vec::new();
     output.resize(size, 0);