💥 --{newer|older}-{c|m}time-{than} options now use strict >/< check

Author: ChanTsune

cli/src/command/core/time_filter.rs

@@ -4,9 +4,9 @@ use std::{fs, time::SystemTime};
 
 /// Represents a filter based on time, allowing for inclusion of files newer or older than a specific `SystemTime`.
 pub(crate) struct TimeFilter {
-    /// If `Some`, only includes files newer than the specified `SystemTime`.
+    /// If `Some`, only includes files strictly newer than the specified `SystemTime`.
     pub(crate) newer_than: Option<SystemTime>,
-    /// If `Some`, only includes files older than the specified `SystemTime`.
+    /// If `Some`, only includes files strictly older than the specified `SystemTime`.
     pub(crate) older_than: Option<SystemTime>,
 }
 
@@ -33,14 +33,14 @@ impl TimeFilter {
     fn is_retain(&self, time: Option<SystemTime>) -> bool {
         if let Some(newer) = self.newer_than {
             if let Some(t) = time {
-                if t < newer {
+                if t <= newer {
                     return false;
                 }
             }
         }
         if let Some(older) = self.older_than {
             if let Some(t) = time {
-                if t > older {
+                if t >= older {
                     return false;
                 }
             }

cli/tests/cli/append/option_newer_ctime_than.rs

@@ -1,11 +1,15 @@
 use crate::utils::{archive, setup};
 use clap::Parser;
 use portable_network_archive::cli;
-use std::{collections::HashSet, fs, thread, time};
+use std::{
+    collections::HashSet,
+    fs, thread,
+    time::{Duration, SystemTime},
+};
 
 /// Precondition: Create an archive with an older file, then prepare reference and newer files.
 /// Action: Run `pna append` with `--newer-ctime-than reference.txt`, specifying older, reference, and newer files.
-/// Expectation: Files with ctime >= reference are appended (reference and newer); older is not re-added.
+/// Expectation: Files with ctime > reference are appended (newer only); older and reference are not re-added.
 /// Note: This test requires filesystem support for creation time (birth time).
 #[test]
 fn append_with_newer_ctime_than() {
@@ -40,16 +44,21 @@ fn append_with_newer_ctime_than() {
     .unwrap();
 
     // Wait to ensure distinct ctime
-    thread::sleep(time::Duration::from_millis(10));
+    thread::sleep(Duration::from_millis(10));
 
     // Create the reference file
     fs::write(reference_file, "reference time marker").unwrap();
 
     // Wait to ensure the next file has a newer ctime
-    thread::sleep(time::Duration::from_millis(10));
+    thread::sleep(Duration::from_millis(10));
 
     // Create the newer file
     fs::write(newer_file, "newer file content").unwrap();
+    let reference_ctime = fs::metadata(reference_file).unwrap().created().unwrap();
+    if !ensure_ctime_order(older_file, newer_file, reference_ctime) {
+        eprintln!("Skipping test: unable to produce strict ctime ordering on this filesystem");
+        return;
+    }
 
     // Append to the archive with the `--newer-ctime-than` option
     cli::Cli::try_parse_from([
@@ -81,23 +90,55 @@ fn append_with_newer_ctime_than() {
         "older file should be in archive from initial creation: {older_file}"
     );
 
-    // reference_file should be included (appended because ctime >= reference)
+    // reference_file should NOT be included (ctime == reference)
     assert!(
-        seen.contains(reference_file),
-        "reference file should be appended: {reference_file}"
+        !seen.contains(reference_file),
+        "reference file should NOT be appended: {reference_file}"
     );
 
-    // newer_file should be included (appended because ctime >= reference)
+    // newer_file should be included (appended because ctime > reference)
     assert!(
         seen.contains(newer_file),
         "newer file should be appended: {newer_file}"
     );
 
-    // Verify that exactly three entries exist
+    // Verify that exactly two entries exist
     assert_eq!(
         seen.len(),
-        3,
-        "Expected exactly 3 entries, but found {}: {seen:?}",
+        2,
+        "Expected exactly 2 entries, but found {}: {seen:?}",
         seen.len()
     );
 }
+
+fn ensure_ctime_order(older: &str, newer: &str, reference: SystemTime) -> bool {
+    if !confirm_ctime_older_than(older, reference) {
+        return false;
+    }
+    wait_until_ctime_newer_than(newer, reference)
+}
+
+fn wait_until_ctime_newer_than(path: &str, baseline: SystemTime) -> bool {
+    const MAX_ATTEMPTS: usize = 500;
+    const SLEEP_MS: u64 = 10;
+    for _ in 0..MAX_ATTEMPTS {
+        if fs::metadata(path)
+            .ok()
+            .and_then(|meta| meta.created().ok())
+            .map(|ctime| ctime > baseline)
+            .unwrap_or(false)
+        {
+            return true;
+        }
+        thread::sleep(Duration::from_millis(SLEEP_MS));
+    }
+    false
+}
+
+fn confirm_ctime_older_than(path: &str, baseline: SystemTime) -> bool {
+    fs::metadata(path)
+        .ok()
+        .and_then(|meta| meta.created().ok())
+        .map(|ctime| ctime < baseline)
+        .unwrap_or(false)
+}

cli/tests/cli/append/option_newer_mtime_than.rs

@@ -1,11 +1,15 @@
 use crate::utils::{archive, setup};
 use clap::Parser;
 use portable_network_archive::cli;
-use std::{collections::HashSet, fs, thread, time};
+use std::{
+    collections::HashSet,
+    fs, thread,
+    time::{Duration, SystemTime},
+};
 
 /// Precondition: Create an archive with an older file, then prepare reference and newer files.
 /// Action: Run `pna append` with `--newer-mtime-than reference.txt`, specifying older, reference, and newer files.
-/// Expectation: Files with mtime >= reference are appended (reference and newer); older is not re-added.
+/// Expectation: Files with mtime > reference are appended (newer only); older and reference are not re-added.
 #[test]
 fn append_with_newer_mtime_than() {
     setup();
@@ -33,16 +37,21 @@ fn append_with_newer_mtime_than() {
     .unwrap();
 
     // Wait to ensure distinct mtime
-    thread::sleep(time::Duration::from_millis(10));
+    thread::sleep(Duration::from_millis(10));
 
     // Create the reference file
     fs::write(reference_file, "reference time marker").unwrap();
 
     // Wait to ensure the next file has a newer mtime
-    thread::sleep(time::Duration::from_millis(10));
+    thread::sleep(Duration::from_millis(10));
 
     // Create the newer file
     fs::write(newer_file, "newer file content").unwrap();
+    let reference_mtime = fs::metadata(reference_file).unwrap().modified().unwrap();
+    if !ensure_mtime_order(older_file, newer_file, reference_mtime) {
+        eprintln!("Skipping test: unable to produce strict mtime ordering on this filesystem");
+        return;
+    }
 
     // Append to the archive with the `--newer-mtime-than` option
     cli::Cli::try_parse_from([
@@ -74,23 +83,55 @@ fn append_with_newer_mtime_than() {
         "older file should be in archive from initial creation: {older_file}"
     );
 
-    // reference_file should be included (appended because mtime >= reference)
+    // reference_file should NOT be included (mtime == reference)
     assert!(
-        seen.contains(reference_file),
-        "reference file should be appended: {reference_file}"
+        !seen.contains(reference_file),
+        "reference file should NOT be appended: {reference_file}"
     );
 
-    // newer_file should be included (appended because mtime >= reference)
+    // newer_file should be included (appended because mtime > reference)
     assert!(
         seen.contains(newer_file),
         "newer file should be appended: {newer_file}"
     );
 
-    // Verify that exactly three entries exist
+    // Verify that exactly two entries exist
     assert_eq!(
         seen.len(),
-        3,
-        "Expected exactly 3 entries, but found {}: {seen:?}",
+        2,
+        "Expected exactly 2 entries, but found {}: {seen:?}",
         seen.len()
     );
 }
+
+fn ensure_mtime_order(older: &str, newer: &str, reference: SystemTime) -> bool {
+    if !confirm_mtime_older_than(older, reference) {
+        return false;
+    }
+    wait_until_mtime_newer_than(newer, reference)
+}
+
+fn wait_until_mtime_newer_than(path: &str, baseline: SystemTime) -> bool {
+    const MAX_ATTEMPTS: usize = 500;
+    const SLEEP_MS: u64 = 10;
+    for _ in 0..MAX_ATTEMPTS {
+        if fs::metadata(path)
+            .ok()
+            .and_then(|meta| meta.modified().ok())
+            .map(|mtime| mtime > baseline)
+            .unwrap_or(false)
+        {
+            return true;
+        }
+        thread::sleep(Duration::from_millis(SLEEP_MS));
+    }
+    false
+}
+
+fn confirm_mtime_older_than(path: &str, baseline: SystemTime) -> bool {
+    fs::metadata(path)
+        .ok()
+        .and_then(|meta| meta.modified().ok())
+        .map(|mtime| mtime < baseline)
+        .unwrap_or(false)
+}

cli/tests/cli/append/option_older_ctime_than.rs

@@ -10,7 +10,7 @@ use std::{
 /// Precondition: Archive already contains `older.txt`. Prepare `reference.txt` and `newer.txt` with
 /// strictly ordered creation times (older < reference < newer).
 /// Action: Run `pna append` with `--older-ctime-than reference.txt`, appending both candidate files.
-/// Expectation: Only files whose ctime <= reference (i.e., reference itself) are appended; `newer` is skipped.
+/// Expectation: Only files whose ctime < reference are appended; `reference` and `newer` are skipped.
 /// Note: Requires filesystem support for birth time.
 #[test]
 fn append_with_older_ctime_than() {
@@ -49,7 +49,7 @@ fn append_with_older_ctime_than() {
     thread::sleep(Duration::from_millis(10));
     fs::write(&newer_file, "newer content").unwrap();
 
-    if !confirm_ctime_not_newer_than(&older_file, reference_ctime)
+    if !confirm_ctime_older_than(&older_file, reference_ctime)
         || !wait_until_ctime_newer_than(&newer_file, reference_ctime)
     {
         eprintln!(
@@ -84,17 +84,17 @@ fn append_with_older_ctime_than() {
         "older file should remain from initial archive: {older_file}"
     );
     assert!(
-        seen.contains(&reference_file),
-        "reference file should be appended: {reference_file}"
+        !seen.contains(&reference_file),
+        "reference file should NOT be appended: {reference_file}"
     );
     assert!(
         !seen.contains(&newer_file),
         "newer file should NOT be appended: {newer_file}"
     );
     assert_eq!(
         seen.len(),
-        2,
-        "Expected exactly 2 entries (older + reference) but found {}: {seen:?}",
+        1,
+        "Expected exactly 1 entry (older only) but found {}: {seen:?}",
         seen.len()
     );
 }
@@ -116,10 +116,10 @@ fn wait_until_ctime_newer_than(path: &str, baseline: SystemTime) -> bool {
     false
 }
 
-fn confirm_ctime_not_newer_than(path: &str, baseline: SystemTime) -> bool {
+fn confirm_ctime_older_than(path: &str, baseline: SystemTime) -> bool {
     fs::metadata(path)
         .ok()
         .and_then(|meta| meta.created().ok())
-        .map(|ctime| ctime <= baseline)
+        .map(|ctime| ctime < baseline)
         .unwrap_or(false)
 }

cli/tests/cli/append/option_older_mtime_than.rs

@@ -10,7 +10,7 @@ use std::{
 /// Precondition: Archive contains `older.txt`. Prepare `reference.txt` and `newer.txt` with strictly
 /// increasing modification times.
 /// Action: Run `pna append` with `--older-mtime-than reference.txt`, appending both candidates.
-/// Expectation: Only files whose mtime <= reference (reference itself) are appended; `newer` is skipped.
+/// Expectation: Only files whose mtime < reference are appended; `reference` and `newer` are skipped.
 #[test]
 fn append_with_older_mtime_than() {
     setup();
@@ -42,7 +42,7 @@ fn append_with_older_mtime_than() {
     thread::sleep(Duration::from_millis(10));
     fs::write(&newer_file, "newer mtime content").unwrap();
 
-    if !confirm_mtime_not_newer_than(&older_file, reference_mtime)
+    if !confirm_mtime_older_than(&older_file, reference_mtime)
         || !wait_until_mtime_newer_than(&newer_file, reference_mtime)
     {
         eprintln!(
@@ -77,17 +77,17 @@ fn append_with_older_mtime_than() {
         "older file should remain from initial archive: {older_file}"
     );
     assert!(
-        seen.contains(&reference_file),
-        "reference file should be appended: {reference_file}"
+        !seen.contains(&reference_file),
+        "reference file should NOT be appended: {reference_file}"
     );
     assert!(
         !seen.contains(&newer_file),
         "newer file should NOT be appended: {newer_file}"
     );
     assert_eq!(
         seen.len(),
-        2,
-        "Expected exactly 2 entries (older + reference) but found {}: {seen:?}",
+        1,
+        "Expected exactly 1 entry (older only) but found {}: {seen:?}",
         seen.len()
     );
 }
@@ -109,10 +109,10 @@ fn wait_until_mtime_newer_than(path: &str, baseline: SystemTime) -> bool {
     false
 }
 
-fn confirm_mtime_not_newer_than(path: &str, baseline: SystemTime) -> bool {
+fn confirm_mtime_older_than(path: &str, baseline: SystemTime) -> bool {
     fs::metadata(path)
         .ok()
         .and_then(|meta| meta.modified().ok())
-        .map(|mtime| mtime <= baseline)
+        .map(|mtime| mtime < baseline)
         .unwrap_or(false)
 }