Skip to content

Added recursive mount attr test - #1428

Merged
YJDoc2 merged 6 commits into
youki-dev:mainfrom
higuruchi:Add/Recursive_mount_attrs_test
Jan 30, 2023
Merged

Added recursive mount attr test#1428
YJDoc2 merged 6 commits into
youki-dev:mainfrom
higuruchi:Add/Recursive_mount_attrs_test

Conversation

@higuruchi

@higuruchi higuruchi commented Dec 19, 2022

Copy link
Copy Markdown
Contributor

Added test of rro mount option that implemented in #1398.
/tmp/ is used as the bind mount path used for testing.

Now, this PR implement rro option test only.
And if this test is correct, add other option test(rrw, rnosuid...).

  • rro
  • rrw
  • rnosuid
  • rsuid
  • rnodev
  • rdev
  • rnoexec
  • rexec
  • rnodiratime
  • rdiratime
  • rrelatime
  • rnorelatime
  • rnoatime
  • ratime
  • rstrictatime
  • rnostrictatime
  • rnosymfollow
  • rsymfollow

Signed-off-by: higuruchi fumiya2324@gmail.com

@higuruchi
higuruchi marked this pull request as draft December 19, 2022 08:52
@higuruchi higuruchi changed the title Added recursive mount attr test. Added recursive mount attr test Dec 19, 2022
@higuruchi
higuruchi force-pushed the Add/Recursive_mount_attrs_test branch from ed04ac3 to f897dc2 Compare December 19, 2022 12:00
Signed-off-by: higuruchi <fumiya2324@gmail.com>
@higuruchi
higuruchi force-pushed the Add/Recursive_mount_attrs_test branch from f897dc2 to 88452c5 Compare December 19, 2022 12:19
@utam0k
utam0k requested a review from YJDoc2 December 19, 2022 12:30
@higuruchi
higuruchi marked this pull request as ready for review December 19, 2022 12:34

@YJDoc2 YJDoc2 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hey, overall seems fine, there are some changes suggested and a couple of questions. Please take a look, and add tests for remaining mount types as well.

Comment thread tests/rust-integration-tests/runtimetest/src/tests.rs Outdated
Comment thread tests/rust-integration-tests/runtimetest/src/tests.rs Outdated
Comment thread tests/rust-integration-tests/runtimetest/src/tests.rs Outdated
Comment thread tests/rust-integration-tests/runtimetest/src/tests.rs Outdated
eprintln!("mounts_recursive rro error: {}", e);
}
}
"rrw" => { /* TODO... */ }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Otherwise test looks fine, so please add other tests in this PR as well 👍

Comment thread tests/rust-integration-tests/runtimetest/src/main.rs Outdated
@higuruchi
higuruchi force-pushed the Add/Recursive_mount_attrs_test branch from 59a7afa to 0356169 Compare December 20, 2022 05:11
@codecov-commenter

codecov-commenter commented Dec 20, 2022

Copy link
Copy Markdown

Codecov Report

Merging #1428 (ccbd26b) into main (764cfe8) will decrease coverage by 0.28%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1428      +/-   ##
==========================================
- Coverage   69.00%   68.73%   -0.28%     
==========================================
  Files         119      120       +1     
  Lines       13041    13094      +53     
==========================================
+ Hits         8999     9000       +1     
- Misses       4042     4094      +52     

@higuruchi
higuruchi force-pushed the Add/Recursive_mount_attrs_test branch from 0356169 to f645736 Compare December 21, 2022 15:46
@higuruchi
higuruchi marked this pull request as draft December 21, 2022 15:48
@higuruchi
higuruchi force-pushed the Add/Recursive_mount_attrs_test branch from f645736 to 882032c Compare December 22, 2022 07:06
Signed-off-by: higuruchi <fumiya2324@gmail.com>
@higuruchi
higuruchi force-pushed the Add/Recursive_mount_attrs_test branch 2 times, most recently from 2f0adda to fe498b6 Compare December 23, 2022 05:21
Signed-off-by: higuruchi <fumiya2324@gmail.com>
@higuruchi
higuruchi force-pushed the Add/Recursive_mount_attrs_test branch from fe498b6 to 40e113a Compare December 23, 2022 11:41
Signed-off-by: higuruchi <fumiya2324@gmail.com>
@YJDoc2

YJDoc2 commented Jan 24, 2023

Copy link
Copy Markdown
Collaborator

Hey @higuruchi sorry I haven't looked at this in a while. Is there any issue you are facing? If you are busy and cannot work on this, it is ok too. We can add the partial tests and add another task for remaining.

If there is any issue, feel free to ping or mention!

@higuruchi

Copy link
Copy Markdown
Contributor Author

@YJDoc2
Also, I am unlikely to be able to work on it for a few weeks.
After fixing the clap problem, can I issue the rest?

Signed-off-by: higuruchi <fumiya2324@gmail.com>
@YJDoc2
YJDoc2 marked this pull request as ready for review January 24, 2023 07:29
Comment thread tests/rust-integration-tests/runtimetest/src/main.rs Outdated
@YJDoc2

YJDoc2 commented Jan 24, 2023

Copy link
Copy Markdown
Collaborator

Hey @higuruchi No worries. If you are fine with it , we can merge this now, and when you get time you can come back and work on the rest if you want. In the meantime, if anyone else picks up the missing cases, we can progress that way too.
Thanks for setting up the basic test and doing all this work ❤️

Signed-off-by: higuruchi <fumiya2324@gmail.com>
@higuruchi
higuruchi requested a review from YJDoc2 January 24, 2023 07:58
@higuruchi

higuruchi commented Jan 27, 2023

Copy link
Copy Markdown
Contributor Author

@YJDoc2

If you are fine with it , we can merge this now

Please merge if there is no problem.
And i will add more tests as time permits.

@YJDoc2

YJDoc2 commented Jan 27, 2023

Copy link
Copy Markdown
Collaborator

Hey @higuruchi , sure, the PR seems mostly fine , its just that I didn't have time to take a final look. I'll try to merge this today.

Please merge if there is no problem.

@YJDoc2 YJDoc2 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm 👍

@YJDoc2

YJDoc2 commented Jan 30, 2023

Copy link
Copy Markdown
Collaborator

Hey @higuruchi , I'm merging this. Thank you for the contribution !

@YJDoc2
YJDoc2 merged commit 3675a51 into youki-dev:main Jan 30, 2023
@higuruchi

Copy link
Copy Markdown
Contributor Author

@YJDoc2 May I raise an issue showing the rest of the tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants