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.

Sign up for GitHub

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

Jump to bottom

add rrelatime mount test #1642

Merged
merged 1 commit into from
Mar 11, 2023
Merged

add rrelatime mount test #1642

merged 1 commit into from
Mar 11, 2023

Conversation

lengrongfu
Copy link
Collaborator

issue: #1534

  • rrelatime
  • rnorelatime
  • rnoatime
  • rstrictatime

@yihuaf
Copy link
Collaborator

yihuaf commented Mar 8, 2023

Can you run make lint to make sure clippy passes? Or we can wait to merge this after we fix the CI.

@lengrongfu
Copy link
Collaborator Author

Can you run make lint to make sure clippy passes? Or we can wait to merge this after we fix the CI.

Let me see.

@lengrongfu lengrongfu force-pushed the feat/rrelatime_recursive_mount_test branch from be4e431 to b69209b Compare March 9, 2023 06:15
@yihuaf
Copy link
Collaborator

yihuaf commented Mar 9, 2023

Can you rebase to main so we can test this PR with CI now that #1643 is merged :)

@lengrongfu
Copy link
Collaborator Author

Can you rebase to main so we can test this PR with CI now that #1643 is merged :)

Ok, already rebase main.

@yihuaf
Copy link
Collaborator

yihuaf commented Mar 9, 2023

Apologize, we may need to merge #1638 first so the CI can be fixed.

@lengrongfu
Copy link
Collaborator Author

Ok, waiting #1638 to be merged.

@yihuaf yihuaf self-assigned this Mar 9, 2023
@yihuaf yihuaf self-requested a review March 9, 2023 06:25
@yihuaf
Copy link
Collaborator

yihuaf commented Mar 9, 2023

#1638 merged. Please rebase and I can review.

@lengrongfu lengrongfu force-pushed the feat/rrelatime_recursive_mount_test branch 4 times, most recently from 9e7f149 to 8ca9bbd Compare March 9, 2023 09:05
@lengrongfu
Copy link
Collaborator Author

@yihuaf Hi, can you help review this pr.

yihuaf
yihuaf previously approved these changes Mar 10, 2023
View reviewed changes
.gitignore Outdated Show resolved Hide resolved
tests/rust-integration-tests/integration_test/src/tests/mounts_recursive/mod.rs Outdated Show resolved Hide resolved
@yihuaf yihuaf self-requested a review March 10, 2023 16:21
@yihuaf yihuaf dismissed their stale review March 10, 2023 16:22

pending .gitignore changes

@yihuaf
Copy link
Collaborator

yihuaf commented Mar 10, 2023 •
edited
Loading

The tests LGTM. Pending explanation on the need to change .gitignore.

@yihuaf
Copy link
Collaborator

yihuaf commented Mar 10, 2023 •
edited
Loading

Also, you don't have to do it in the PR, but can you follow up to add more comments explain how these mount test work? I had to trace the code myself to understand where the condition is checked. A simple explanation at top explaining that runtimetest will check if the actual mount matches the spec will be good. Otherwise, the reader's first reaction is: I see the spec contains the mount options, but where is the options actually checked.

@yihuaf yihuaf removed their assignment Mar 10, 2023
@lengrongfu
Copy link
Collaborator Author

Sure, I can add some clarification.

@lengrongfu
add rrelatime mount test
e3b4e9d 
Signed-off-by: lengrongfu <1275177125@qq.com>
@lengrongfu lengrongfu force-pushed the feat/rrelatime_recursive_mount_test branch from 8ca9bbd to e3b4e9d Compare March 11, 2023 02:02
@yihuaf yihuaf merged commit 29babe7 into youki-dev:main Mar 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yihuaf yihuaf yihuaf approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lengrongfu @yihuaf