-
Notifications
You must be signed in to change notification settings - Fork 173
Create test for kernel_match_version function #451
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
Conversation
lib/kernel_tag.rb
Outdated
@@ -45,7 +45,7 @@ def kernel_match_version?(kernel_version, expected_kernel_versions) | |||
kernel_version = KernelTag.new(kernel_version) | |||
|
|||
expected_kernel_versions.all? do |expected_kernel_version| | |||
match = expected_kernel_version.match(/(?<operator>==|!=|<=|>|>=)?\s*(?<kernel_tag>v[0-9]\.\d+(?:\.\d+)?(?:-rc\d+)?)/) | |||
match = expected_kernel_version.match(/(?<operator>==|!=|<|<=|>|>=)?\s*(?<kernel_tag>v[0-9]\.\d+(?:\.\d+)?(?:-rc\d+)?)/) |
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, looks it's ok not to support <, I can't recall clearly, there's historical reason to avoid confusion so the lkp-tests code doesn't contain any configuration to use <.
spec/kernel_match_ver_spec.rb
Outdated
kernel_version = 'v5.8' | ||
expected_versions = ['>= v5.9'] | ||
expect(kernel_match_version?(kernel_version, expected_versions)).to eq(false) | ||
end |
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 the detail description of each test. Is it ok to write multiple tests like below
[
['v5.10', ['>= v5.9'], true],
['v5.11', ['<= v5.10'], false],
...
].each do |test|
it "handles #{test}" do
expect(kernel_match_version?(test[0], test[1])).to eq(test[2])
end
end
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.
Hi, I have already updated the code based on your review. Thanks for the comment.
Thanks, help resolve the failure. Locally, you can run "bundle exec rake rubocop" to check (and add -a option to fix automatically)
|
Fix intel#242 Co-authored-by: Ong Yen Yee <yen.yee.ong@intel.com> Signed-off-by: yeetengangIntel <yee.teng.ang@intel.com> Signed-off-by: Ong Yen Yee <yen.yee.ong@intel.com>
thanks for the patch |
Fix #242