-
Notifications
You must be signed in to change notification settings - Fork 347
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.
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
Add test root readonly #2976
base: main
Are you sure you want to change the base?
Add test root readonly #2976
Conversation
Signed-off-by: sat0ken <[email protected]>
Signed-off-by: sat0ken <[email protected]>
Signed-off-by: sat0ken <[email protected]>
Signed-off-by: sat0ken <[email protected]>
Signed-off-by: sat0ken <[email protected]>
Signed-off-by: sat0ken <[email protected]>
Signed-off-by: sat0ken <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, I think we need to break this in two tests - one with readonly true and other with false. We will have a corresponding test in the runtime
test, which will check that read operation is successful (maybe create a file and validate the contents) and the the write access will be tested according to config's value.
This is small deviation from the go test, but I think this would be a better test.
Signed-off-by: sat0ken <[email protected]>
Signed-off-by: sat0ken <[email protected]>
Signed-off-by: sat0ken <[email protected]>
Signed-off-by: sat0ken <[email protected]>
Signed-off-by: sat0ken <[email protected]>
Signed-off-by: sat0ken <[email protected]>
@YJDoc2 HI, I've fixed the code according to the comments. Could you please check the code? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, overall ok, but some changes needed.
tests/contest/contest/src/tests/root_readonly_true/root_readonly_tests.rs
Outdated
Show resolved
Hide resolved
Signed-off-by: sat0ken <[email protected]>
Signed-off-by: sat0ken <[email protected]>
Signed-off-by: sat0ken <[email protected]>
Signed-off-by: sat0ken <[email protected]>
Signed-off-by: sat0ken <[email protected]>
…o add-test-root-readonly
@YJDoc2 I fixed code, Could you please check the code? |
Signed-off-by: sat0ken <[email protected]>
@YJDoc2 Ping! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some minor changes needed.
if let Err(e) = test_dir_read_access("/") { | ||
let errno = Errno::from_raw(e.raw_os_error().unwrap()); | ||
if errno == Errno::EROFS { | ||
/* This is expected */ | ||
} else { | ||
eprintln!( | ||
"readonly root filesystem, error in testing read access for path /, error: {}", | ||
errno | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, for readonly=true, read_access should not give any error at all. So any error should be considered test fail. I think you might have incorrectly copy-pasted from the write access case. Please update.
} else if let Err(e) = test_dir_write_access("/") { | ||
if e.raw_os_error().is_some() { | ||
let errno = Errno::from_raw(e.raw_os_error().unwrap()); | ||
eprintln!( | ||
"readt only root filesystem is false but write access for path / is err, error: {}", | ||
errno | ||
); | ||
} else { | ||
/* This is expected */ | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we are checking raw_os_error().is_some()
? If this was error, any error while writing, it should be considered as a test fail, right? Also can we have a read_access check in this branch as well? Basically :
if readonly=true{
check write_access == error;
check read_access == ok;
}else{
check write_access == ok;
check read_access == ok;
}
This implements the root_readonly_true validation in #361