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

hanbings · 2024-04-19T19:40:05Z

implement: #221

codecov · 2024-04-19T19:43:47Z

Codecov Report

Attention: Patch coverage is 77.96610% with 13 lines in your changes are missing coverage. Please review.

Project coverage is 59.12%. Comparing base (add14db) to head (6579285).

Files	Patch %	Lines
src/find/matchers/time.rs	73.07%	3 Missing and 4 partials ⚠️
src/find/matchers/mod.rs	50.00%	2 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #355      +/-   ##
==========================================
+ Coverage   58.83%   59.12%   +0.28%     
==========================================
  Files          30       30              
  Lines        3855     3914      +59     
  Branches      846      863      +17     
==========================================
+ Hits         2268     2314      +46     
- Misses       1254     1259       +5     
- Partials      333      341       +8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sylvestre · 2024-04-19T23:15:18Z

src/find/matchers/time.rs

+        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);
+
+        // mores test


Suggested change

// mores test

// more test

Thanks. Changed in new commit.

sylvestre · 2024-04-20T08:13:43Z

please add integrations tests in https://github.com/uutils/findutils/blob/main/tests/find_cmd_tests.rs
and please tests with invalid values (like string for numerical values)

hanbings · 2024-04-20T09:14:09Z

please add integrations tests in https://github.com/uutils/findutils/blob/main/tests/find_cmd_tests.rs and please tests with invalid values (like string for numerical values)

I've added tests for string type numbers.
But I found that it is able to recognize error parameters such as 1%2 and +-321, which is different from the GNU find results.

This has to do with the way the convert_arg_to_comparable_value function recognizes and converts numbers, meaning that this problem occurs with both the a/c/mtime and a/c/mmin arguments.

Should I fix them in a new PR?

$ find . -atime 1%2
find: invalid argument `1%2' to `-atime'
$ find . -amin 1%2
find: invalid argument `1%2' to `-amin'
$ ./target/debug/find . -atime 1%2
$ ./target/debug/find . -amin 1%2
./.git
./.git/objects/b1
./.git/objects/7f
./.git/objects/a8
./target/debug/deps
./target/debug/incremental/find_cmd_tests-8yeq2sf4g2ts
...

cakebaker · 2024-05-13T13:33:28Z

tests/find_cmd_tests.rs

+        "\"-60\"", "\"-120\"", "\"-240\"", "\"-60\"", "\"-120\"", "\"-240\"",
+    ];
+
+    for (arg, time) in args.iter().zip(times.iter()) {


I think this piece of code doesn't do what you think it does ;-) It doesn't create the Cartesian product of args and times:

for (arg, time) in args.iter().zip(times.iter()) { println!("{arg}: {time}"); }

outputs:

-amin: -60 -cmin: -120 -mmin: -240

Agree. I replaced it with a nested loop. : )

cakebaker · 2024-05-13T13:34:37Z

tests/find_cmd_tests.rs

+            .code(0);
+    }
+
+    for (arg, time_string) in args.iter().zip(time_strings.iter()) {


See my previous comment.

cakebaker · 2024-05-23T14:29:36Z

tests/find_cmd_tests.rs

+    for arg in args.iter() {
+        for time in times.iter() {


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 {

cakebaker · 2024-05-23T14:30:12Z

tests/find_cmd_tests.rs

+    for arg in args.iter() {
+        for time_string in time_strings.iter() {


Suggested change

for arg in args.iter() {

for time_string in time_strings.iter() {

for arg in args {

for time_string in time_strings {

cakebaker · 2024-05-23T14:39:02Z

tests/find_cmd_tests.rs

+    for arg in args.iter() {
+        for time_string in time_strings.iter() {
+            Command::cargo_bin("find")
+                .expect("the except time should not match")


I'm not sure about this one:

Suggested change

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

.expect("the time should not match")

cakebaker · 2024-05-23T14:43:43Z

src/find/mod.rs

+        let args = ["-amin", "-cmin", "-mmin"];
+        let times = ["-60", "-120", "-240", "+60", "+120", "+240"];
+
+        for (arg, time) in args.iter().zip(times.iter()) {


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

hanbings · 2024-05-23T15:24:25Z

Thanks, the conflict has been resolved and the code modified in new commits.

sylvestre · 2024-06-01T08:33:03Z

it would be nice to work on #360 next
it will be easier to detect improvement / regression wrt the GNU test suite

hanbings · 2024-06-01T09:45:38Z

it would be nice to work on #360 next it will be easier to detect improvement / regression wrt the GNU test suite

Thanks! I will complete it as soon as I can.

hanbings added 5 commits April 19, 2024 16:25

Implement -amin -cmin -mmin args.

c60b060

Rename matcher: FileAgeRangeMatcher.

7180e7d

Add test for FileAgeRangeMatcher matcher.

297c250

Add command line test for age range.

54dea2a

Fix test error caused by FileAgeRangeMatcher test file creation time.

ff60a23

hanbings added 2 commits April 20, 2024 03:48

Improve test coverage.

1b48456

Improve test code readability.

aa8f348

sylvestre reviewed Apr 19, 2024

View reviewed changes

Typo: mores -> more and lesses -> less.

08dc112

cakebaker linked an issue Apr 20, 2024 that may be closed by this pull request

Feature request: support -amin -cmin -mmin age ranges #221

Closed

Add exception parameter test.

1847f3d

hanbings mentioned this pull request Apr 23, 2024

find: Fix convert_arg_to_comparable_value() function parsing unexpected characters. #361

Merged

cakebaker reviewed May 13, 2024

View reviewed changes

Fix nest loops missed.

90d09c9

cakebaker reviewed May 23, 2024

View reviewed changes

hanbings added 3 commits May 23, 2024 23:10

Resolve branch conflicts.

1c48744

Merge branch 'main' into implement-221

5f1fd39

Clean up duplicate code.

981b449

Merge branch 'main' into implement-221

6579285

sylvestre merged commit d7057e0 into uutils:main Jun 1, 2024
18 checks passed

hanbings deleted the implement-221 branch June 1, 2024 09:41

BrewTestBot mentioned this pull request Jun 29, 2024

uutils-findutils 0.6.0 Homebrew/homebrew-core#175943

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

hanbings commented Apr 19, 2024

codecov bot commented Apr 19, 2024 •

edited

Loading

sylvestre Apr 19, 2024

hanbings Apr 20, 2024

sylvestre commented Apr 20, 2024

hanbings commented Apr 20, 2024 •

edited

Loading

cakebaker May 13, 2024 •

edited

Loading

hanbings May 13, 2024

cakebaker May 13, 2024

cakebaker May 23, 2024

cakebaker May 23, 2024

cakebaker May 23, 2024

cakebaker May 23, 2024

hanbings commented May 23, 2024

sylvestre commented Jun 1, 2024

hanbings commented Jun 1, 2024

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

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

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

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

Conversation

hanbings commented Apr 19, 2024

codecov bot commented Apr 19, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sylvestre commented Apr 20, 2024

hanbings commented Apr 20, 2024 • edited Loading

cakebaker May 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanbings commented May 23, 2024

sylvestre commented Jun 1, 2024

hanbings commented Jun 1, 2024

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

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

codecov bot commented Apr 19, 2024 •

edited

Loading

hanbings commented Apr 20, 2024 •

edited

Loading

cakebaker May 13, 2024 •

edited

Loading