find: Implement `-amin` `-cmin` `-mmin` age ranges. by hanbings · Pull Request #355 · uutils/findutils

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

+        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

+        "\"-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

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


See my previous comment.

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

+    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

+    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

+    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

+        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

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

		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")

Conversation

hanbings commented Apr 19, 2024

Uh oh!

codecov Bot commented Apr 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sylvestre commented Apr 20, 2024

Uh oh!

hanbings commented Apr 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cakebaker May 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hanbings commented May 23, 2024

Uh oh!

sylvestre commented Jun 1, 2024

Uh oh!

hanbings commented Jun 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Apr 19, 2024 •

edited

Loading

hanbings commented Apr 20, 2024 •

edited

Loading

cakebaker May 13, 2024 •

edited

Loading