Add rspec test to validate config format #365
Conversation
yeetengangIntel
force-pushed
the
open-src/need-kconfig-format-spec-2
branch
5 times, most recently
from
45f0f9f to
a5314bd
Compare
|
For now there are 4 files in /include directory that does not met the requirements due to integers or multiple values passed in:
|
| value.all? { |v| v.match(allowed_chars) || v.empty? } | ||
| else | ||
| value.match(allowed_chars) || value.empty? | ||
| end |
There was a problem hiding this comment.
probably the code can be combined as
Array(value).all? { |v| v.match(allowed_chars) || v.empty? }
But would value.is_a?(Array) become true, since early code has converted the value to String.
| def check_config_value(value) | ||
| # Check if value format is valid: | ||
| # 1. only contain n, m, y or nothing | ||
| # 2. only contain i386, x86_64, or nothing |
There was a problem hiding this comment.
Sorry, I need update the requirement that int is allowed (a valid value). Could you help handle int as well?
spec/need_kconfig_format_spec.rb
Outdated
| def check_config_name(name) | ||
| # Check if config name format is valid: | ||
| # 1. not start with CONFIG_ | ||
| !name.to_s.start_with?('CONFIG_') |
There was a problem hiding this comment.
thanks, prefer the logic to be
expect(name.to_s).to be_start_with('CONFIG_')
So it checks the expected pattern by expect method, and when it fails, the failure output could be more direct to show what the wrong name is.
spec/need_kconfig_format_spec.rb
Outdated
|
|
||
| # If the value is an array, check each element seperately | ||
| if value.is_a?(Array) | ||
| value.all? { |v| v.match(allowed_chars) || v.empty? } |
There was a problem hiding this comment.
similarly, I think here the code could be
value.all? { |v| expect(v).to match(allowed_chars) }
BTW: v.empty? check looks to be not necessary, since the pattern inclues empty string.
spec/need_kconfig_format_spec.rb
Outdated
| files.each do |file| | ||
| begin | ||
| yaml_data = load_yaml_with_conditionals(file) | ||
| next if yaml_data.nil? || !yaml_data.is_a?(Hash) |
There was a problem hiding this comment.
could be
next unless yaml_data.is_a?(Hash)
Sorry, the int value is also valid. I didn't mention it in the initial issue. |
|
Thanks, can you help combine all commits into a single one? |
yeetengangIntel
force-pushed
the
open-src/need-kconfig-format-spec-2
branch
2 times, most recently
from
d20b6ec to
08551aa
Compare
* Check config format for need_kconfig in /include * Format requirements: i. Config name not start with CONFIG_ ii. Config valu only contain n, m, y, i386, x86_64, integers, hexadecimal, or empty
|
thanks for the patch |
Fix for issue #241