-
Notifications
You must be signed in to change notification settings - Fork 181
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
Skip storage testcases if nvme cmd doesn't have 'NameSpace' in output. #3685
base: main
Are you sure you want to change the base?
Conversation
Older versions of nvme-cli do not have the NameSpace key in the output skip the test if NameSpace key is not available instead of failing.
lisa/tools/nvmecli.py
Outdated
# skip the test if NameSpace key is not available | ||
if not nvme_devices or "NameSpace" not in nvme_devices[0]: | ||
raise SkippedException( | ||
"'NameSpace' key is not available in 'nvme -list -o json' output" |
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.
The message is not clear enough. It says what's verified, but not explain why the test case is skipped from the test purpose. Why is the test skipped, when the NameSpace is not available? Even the Namespace doesn't exist, it's still a nvme device.
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.
Older images like CentOS don't have latest nvme cli which is missing support for "Namespace" field in the output.
This is needed only for serial hot add storage testcases (with disk controller == NVMe) but not for actual NVMe testcases.
Parallel storage testcases and other tests are not impacted by this.
The "Namespace" field is used to get the LUN numbers of the remote disks which is used by all serial hot add disk tests. Without this the tests will not work on VMs with NVMe disk controller.
Modified the exception message saying the reason and added a debug message
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.
Thanks for fixing, it's better, and can be improved too.
- It needs to raise skipped exceptions in tool level, because it doesn't know how the tools will be used. Some test cases maybe skipped unexpected. Please raise general exception here, and catch/rethrow it to skipped in test cases.
- The message should tell the details like in the review comments. Some information is important to help understanding why it's skipped like: The "Namespace" field is used to get the LUN numbers of the remote disks which is used by all serial hot add disk tests. Without this the tests will not work on VMs with NVMe disk controller.
Older versions of nvme-cli do not have the 'NameSpace' field in the output.
This change will make the storage testcases to skip if NameSpace field is not available instead of failing.