Merged
Conversation
nayuta723
reviewed
Feb 1, 2026
Contributor
nayuta723
left a comment
There was a problem hiding this comment.
Just some minor nitpicks, but I've left a couple of comments.
Comment on lines
+105
to
+111
| // Helper for mounts_recursive tests. | ||
| // - `mount_options`: OCI mount options (e.g. ["rbind", "rro"]). | ||
| // - `process_args`: If `None`, uses the default ["runtimetest", "mounts_recursive"]. | ||
| // - `setup_extra(bundle, dir, subdir)`: Hook to perform additional host-side setup before starting the container. | ||
| // - `bundle_path`: Bundle root (e.g. used to copy files from bundle/bin). | ||
| // - `dir_path`: Host directory that will be mounted at /mnt. | ||
| // - `subdir_path`: Host directory that will be mounted /mnt/mount_subdir. |
Contributor
There was a problem hiding this comment.
Suggested change
| // Helper for mounts_recursive tests. | |
| // - `mount_options`: OCI mount options (e.g. ["rbind", "rro"]). | |
| // - `process_args`: If `None`, uses the default ["runtimetest", "mounts_recursive"]. | |
| // - `setup_extra(bundle, dir, subdir)`: Hook to perform additional host-side setup before starting the container. | |
| // - `bundle_path`: Bundle root (e.g. used to copy files from bundle/bin). | |
| // - `dir_path`: Host directory that will be mounted at /mnt. | |
| // - `subdir_path`: Host directory that will be mounted /mnt/mount_subdir. | |
| // Helper for mounts_recursive tests. | |
| // - `mount_options`: OCI mount options (e.g. ["rbind", "rro"]). | |
| // - `process_args`: If `None`, uses the default ["runtimetest", "mounts_recursive"]. | |
| // - `setup_extra`: Hook to perform additional host-side setup before starting the container. | |
| // It takes three parameters: | |
| // - `bundle_path`: Bundle root (e.g. used to copy files from bundle/bin). | |
| // - `dir_path`: Host directory that will be mounted at /mnt. | |
| // - `subdir_path`: Host subdirectory that will be mounted at /mnt/mount_subdir. |
Comment on lines
+156
to
+168
| // Mount Recursive test | ||
| // Host: | ||
| // - <tmp>/<test>_dir is a mount point (tmpfs) | ||
| // - <tmp>/<test>_dir/<test>_subdir is a submount (tmpfs) | ||
| // | ||
| // rbind: | ||
| // - Bind-mount <tmp>/<test>_dir into the container at /mnt (rbind) | ||
| // | ||
| // In the container, we should observe: | ||
| // - mount at /mnt | ||
| // - submount at /mnt/<test>_subdir | ||
| // | ||
| // Key check: the mount options/attributes must be applied to SUBMOUNTS. |
Contributor
There was a problem hiding this comment.
Suggested change
| // Mount Recursive test | |
| // Host: | |
| // - <tmp>/<test>_dir is a mount point (tmpfs) | |
| // - <tmp>/<test>_dir/<test>_subdir is a submount (tmpfs) | |
| // | |
| // rbind: | |
| // - Bind-mount <tmp>/<test>_dir into the container at /mnt (rbind) | |
| // | |
| // In the container, we should observe: | |
| // - mount at /mnt | |
| // - submount at /mnt/<test>_subdir | |
| // | |
| // Key check: the mount options/attributes must be applied to SUBMOUNTS. | |
| // Mount Recursive test | |
| // Host: | |
| // - <tmp>/mount_dir is a mount point (tmpfs) | |
| // - <tmp>/mount_dir/mount_subdir is a submount (tmpfs) | |
| // | |
| // rbind: | |
| // - Bind-mount <tmp>/mount_dir into the container at /mnt (rbind) | |
| // | |
| // In the container, we should observe: | |
| // - mount at /mnt | |
| // - submount at /mnt/mount_subdir | |
| // | |
| // Key check: the mount options/attributes must be applied to SUBMOUNTS. |
| let rnosuid_dir_path = rnosuid_test_base_dir.path().join("rnosuid_dir"); | ||
| let rnosuid_subdir_path = rnosuid_dir_path.join("rnosuid_subdir"); | ||
| let mount_options = vec!["rbind".to_string(), "rnosuid".to_string()]; | ||
| let mount_dest_path = PathBuf::from_str("/mnt").unwrap(); |
Contributor
There was a problem hiding this comment.
I'd suggest defining /mnt as a constant, as it's currently redefined in both check_recursive and each test. This would help keep the code clean and consistent.
| } | ||
| "rdev" => { | ||
| let device_path = mount.destination().join("rdev_subdir/null"); | ||
| let device_path = mount.destination().join("mount_subdir/null"); |
Contributor
There was a problem hiding this comment.
Suggested change
| let device_path = mount.destination().join("mount_subdir/null"); | |
| let device_path = subdir_path.join("null"); |
d6bba94 to
37c8d79
Compare
Signed-off-by: Yusuke Sakurai <yusuke.sakurai@3-shake.com>
37c8d79 to
06b2d6f
Compare
Member
Author
|
@nayuta723 I’ve made the changes you suggested. |
Member
Author
|
Since this is only a test refactor, I plan to merge it later this week unless anyone has further comments. @youki-dev/youki |
Merged
sat0ken
pushed a commit
to sat0ken/youki
that referenced
this pull request
Mar 17, 2026
Signed-off-by: Yusuke Sakurai <yusuke.sakurai@3-shake.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
I refactored the integration tests for mounts_recursive.
I consolidated the common setup, execution, and teardown into a single helper function—creating the base directory and subdirectory, setting up the mounts, running test_inside_container, and cleaning everything up—so the tests can reuse the same flow.
Type of Change
Testing
Related Issues
Fixes #1698
Additional Context