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

find: Implement -amin -cmin -mmin age ranges. #355

Merged
merged 14 commits into from
Jun 1, 2024
18 changes: 16 additions & 2 deletions src/find/matchers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ use self::regex::RegexMatcher;
use self::size::SizeMatcher;
use self::stat::{InodeMatcher, LinksMatcher};
use self::time::{
FileTimeMatcher, FileTimeType, NewerMatcher, NewerOptionMatcher, NewerOptionType,
NewerTimeMatcher,
FileAgeRangeMatcher, FileTimeMatcher, FileTimeType, NewerMatcher, NewerOptionMatcher,
NewerOptionType, NewerTimeMatcher,
};
use self::type_matcher::TypeMatcher;

Expand Down Expand Up @@ -419,6 +419,20 @@ fn build_matcher_tree(
i += 1;
Some(FileTimeMatcher::new(file_time_type, days).into_box())
}
"-amin" | "-cmin" | "-mmin" => {
if i >= args.len() - 1 {
return Err(From::from(format!("missing argument to {}", args[i])));
}
let file_time_type = match args[i] {
"-amin" => FileTimeType::Accessed,
"-cmin" => FileTimeType::Created,
"-mmin" => FileTimeType::Modified,
_ => unreachable!("Encountered unexpected value {}", args[i]),
};
let minutes = convert_arg_to_comparable_value(args[i], args[i + 1])?;
i += 1;
Some(FileAgeRangeMatcher::new(file_time_type, minutes).into_box())
}
"-size" => {
if i >= args.len() - 1 {
return Err(From::from(format!("missing argument to {}", args[i])));
Expand Down
123 changes: 123 additions & 0 deletions src/find/matchers/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,54 @@
}
}

pub struct FileAgeRangeMatcher {
minutes: ComparableValue,
file_time_type: FileTimeType,
}

impl Matcher for FileAgeRangeMatcher {
fn matches(&self, file_info: &DirEntry, matcher_io: &mut MatcherIO) -> bool {
match self.matches_impl(file_info, matcher_io.now()) {
Err(e) => {
writeln!(
&mut stderr(),
"Error getting {:?} time for {}: {}",
self.file_time_type,
file_info.path().to_string_lossy(),
e
)
.unwrap();
false
}
Ok(t) => t,
}
}
}

impl FileAgeRangeMatcher {
fn matches_impl(&self, file_info: &DirEntry, now: SystemTime) -> Result<bool, Box<dyn Error>> {
let this_time = self.file_time_type.get_file_time(file_info.metadata()?)?;
let mut is_negative = false;
let age = match now.duration_since(this_time) {
Ok(duration) => duration,
Err(e) => {
is_negative = true;
e.duration()

Check warning on line 312 in src/find/matchers/time.rs

View check run for this annotation

Codecov / codecov/patch

src/find/matchers/time.rs#L310-L312

Added lines #L310 - L312 were not covered by tests
}
};
let age_in_seconds: i64 = age.as_secs() as i64 * if is_negative { -1 } else { 1 };
let age_in_minutes = age_in_seconds / 60 + if is_negative { -1 } else { 0 };
Ok(self.minutes.imatches(age_in_minutes))
}

pub fn new(file_time_type: FileTimeType, minutes: ComparableValue) -> Self {
Self {
minutes,
file_time_type,
}
}
}

#[cfg(test)]
mod tests {
use std::fs;
Expand Down Expand Up @@ -647,4 +695,79 @@
);
}
}

#[test]
fn file_age_range_matcher() {
let temp_dir = Builder::new().prefix("example").tempdir().unwrap();
let temp_dir_path = temp_dir.path().to_string_lossy();
let new_file_name = "newFile";
// this has just been created, so should be newer
File::create(temp_dir.path().join(new_file_name)).expect("create temp file");
let new_file = get_dir_entry_for(&temp_dir_path, new_file_name);

// more test
// mocks:
// - find test_data/simple -amin +1
// - find test_data/simple -cmin +1
// - find test_data/simple -mmin +1
// Means to find files accessed / modified more than 1 minute ago.
[
FileTimeType::Accessed,
FileTimeType::Created,
FileTimeType::Modified,
]
.iter()
.for_each(|time_type| {
let more_matcher = FileAgeRangeMatcher::new(*time_type, ComparableValue::MoreThan(1));
assert!(
!more_matcher.matches(&new_file, &mut FakeDependencies::new().new_matcher_io()),
"{}",
format!(
"more minutes old file should match more than 1 minute old in {} test.",
match *time_type {
FileTimeType::Accessed => "accessed",
FileTimeType::Created => "created",
FileTimeType::Modified => "modified",
}
)
);
});

// less test
// mocks:
// - find test_data/simple -amin -1
// - find test_data/simple -cmin -1
// - find test_data/simple -mmin -1
// Means to find files accessed / modified less than 1 minute ago.
[
FileTimeType::Accessed,
FileTimeType::Created,
FileTimeType::Modified,
]
.iter()
.for_each(|time_type| {
let less_matcher = FileAgeRangeMatcher::new(*time_type, ComparableValue::LessThan(1));
assert!(
less_matcher.matches(&new_file, &mut FakeDependencies::new().new_matcher_io()),
"{}",
format!(
"less minutes old file should not match less than 1 minute old in {} test.",
match *time_type {
FileTimeType::Accessed => "accessed",
FileTimeType::Created => "created",
FileTimeType::Modified => "modified",
}
)
);
});

// catch file error
let _ = fs::remove_file(&*new_file.path().to_string_lossy());
let matcher =
FileAgeRangeMatcher::new(FileTimeType::Modified, ComparableValue::MoreThan(1));
assert!(
!matcher.matches(&new_file, &mut FakeDependencies::new().new_matcher_io()),
"The correct situation is that the file reading here cannot be successful."
);
}
}
23 changes: 23 additions & 0 deletions src/find/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,29 @@ mod tests {
}
}

// Because the time when files exist locally is different
// from the time when Github Actions pulls them,
// it is difficult to write tests that limit a certain time period.
//
// For example, a Github Action may pull files from a new git commit within a few minutes,
// causing the file time to be refreshed to the pull time.
// and The files on the local branch may be several days old.
//
// So this test may not be too accurate and can only ensure that
// the function can be correctly identified.
#[test]
fn find_amin_cmin_mmin() {
let args = ["-amin", "-cmin", "-mmin"];
let times = ["-60", "-120", "-240", "+60", "+120", "+240"];

for (arg, time) in args.iter().zip(times.iter()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you need a nested loop to get the expected result because zip doesn't generate the Cartesian product of args and times.

let deps = FakeDependencies::new();
let rc = find_main(&["find", "./test_data/simple/subdir", arg, time], &deps);

assert_eq!(rc, 0);
}
}

#[test]
fn find_size() {
let deps = FakeDependencies::new();
Expand Down
35 changes: 35 additions & 0 deletions tests/find_cmd_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,3 +518,38 @@ fn expression_empty_parentheses() {
))
.stdout(predicate::str::is_empty());
}

#[test]
fn find_age_range() {
let args = ["-amin", "-cmin", "-mmin"];
let times = ["-60", "-120", "-240", "+60", "+120", "+240"];
let time_strings = [
"\"-60\"", "\"-120\"", "\"-240\"", "\"-60\"", "\"-120\"", "\"-240\"",
];

for arg in args.iter() {
for time in times.iter() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling iter() is not necessary here.

Suggested change
for arg in args.iter() {
for time in times.iter() {
for arg in args {
for time in times {

Command::cargo_bin("find")
.expect("the time should match")
.args(["test_data/simple", arg, time])
.assert()
.success()
.code(0);
}
}

for arg in args.iter() {
for time_string in time_strings.iter() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for arg in args.iter() {
for time_string in time_strings.iter() {
for arg in args {
for time_string in time_strings {

Command::cargo_bin("find")
.expect("the except time should not match")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this one:

Suggested change
.expect("the except time should not match")
.expect("the time should not match")

.args(["test_data/simple", arg, time_string])
.assert()
.failure()
.code(1)
.stderr(predicate::str::contains(
"Error: Expected a decimal integer (with optional + or - prefix) argument to",
))
.stdout(predicate::str::is_empty());
}
}
}
Loading