New testcase added "verify_essential_kernel_modules" #3864
Conversation
A testcase to verify the presence of essential kernel modules.
| def _get_kernel_modules_configuration(self, node: Node) -> Dict[str, str]: | ||
| """ | ||
| Returns a dictionary of kernel modules and their configuration names. | ||
| """ | ||
| return { | ||
| "wdt": "CONFIG_WATCHDOG", | ||
| "cifs": "CONFIG_CIFS", | ||
| } |
There was a problem hiding this comment.
This can be a class variable. Any reason to return the constant from a function?
There was a problem hiding this comment.
Some times these values depend on OS type. We need to use logic in such cases. That's why method is used instead of class variables
There was a problem hiding this comment.
Right now, it statically returns the dict without an OS check.
How about check if OS is Linux, and Skip the case otherwise?
There was a problem hiding this comment.
Addressed comment
| not_enabled_modules = self._get_not_enabled_modules(node) | ||
|
|
||
| assert_that(not_enabled_modules).described_as( | ||
| "Not enabled essential kernel modules for Hyper-V / Azure platform found." |
There was a problem hiding this comment.
At the test case level, there are no platform requirements specified, but platforms are mentioned here. We should make them consistent.
There was a problem hiding this comment.
Since the test class name is 'AzureImageStandard' should we specify platform requirements at class level?
There was a problem hiding this comment.
Since the test class name is 'AzureImageStandard' should we specify platform requirements at class level?
No, the class-level requirement will be replaced by the method-level requirement if it is defined at the method level. Unless the entire class shares the same requirement, it can be defined at the class level.
| essential_modules_configuration = { | ||
| "wdt": "CONFIG_WATCHDOG", | ||
| "cifs": "CONFIG_CIFS", | ||
| } |
There was a problem hiding this comment.
I would still recommend having this as a class variable like "linux_essential_modules". Someone looking at the test case will immediately understand the modules being verified.
If it's defined within a function, they will need to navigate through the functions
There was a problem hiding this comment.
Modified as suggested.
|
@LiliDeng LGTM |
A testcase to verify the presence of essential kernel modules.