spec: test the integrity of distro/adaption files #492

j-chiam · 2025-03-07T03:47:27Z

Created spec test to check and remove duplicated lines between the default OS mapping and each OS-specific mappings
Fix duplicated package in OS mapping file

rli9 · 2025-03-07T04:38:33Z

spec/os_mapping_spec.rb

+
+  it 'does not have any package mapping repeated in all groups' do
+    common_packages = find_common_packages(grouped_files)
+    expect(common_packages).to be_empty, "Common package mappings found in all groups: #{common_packages.join(', ')}"


thanks, do you mean that one package appears in all distros?

The initial thinking is we can check for each distro, whether there's a common mapping across all its specific mappings.

Hi @rli9 , currently there are 2 function that handle below requirement:
find_duplicated_lines -> no duplicated line between the default OS mapping and each OS-specific mapping, e.g. amazon_linux against "amazon_linux -*".
find_common_packages -> no line is duplicated across all OS-specific mappings. e.g. "a: b" exists in all os specific mappings is not expected.

find_duplicated_lines will check if certain package installed across specific distro, for example when centos contain arping: iputils, centos-7 also contain arping: iputils, then the test will fail.
find_common_packages will check if certain package installed across all the different distro, for example when apache2: httpd contain in all the distro available for example ubuntu (ubuntu, ubuntu-20.04, ubuntu-), centos (centos, centos-), and all the other, then the test will fail.

Would like to check if this is the required checks? Else can we have more details on what is required for this test?

find_duplicated_lines -> no duplicated line between the default OS mapping and each OS-specific mapping

Thanks, this part is right.

find_common_packages will check if certain package installed across all the different distro

For this, the requirement is to check each distro separately, for example, if ubuntu-* contains same line, then it fails. But ubuntu has no relationship with centos.

rli9 · 2025-03-07T04:38:39Z

spec/os_mapping_spec.rb

+    File.file?(file_path) && !File.symlink?(file_path) && f != 'README.md'
+  end
+
+  mappings = {}


consider to use mappings = Hash.new { |h, k| h[k] = [] }, so later "mappings[base_name] ||= []" is not needed.

Thanks for review, I updated mappings and removed the line

rli9 · 2025-03-07T04:40:16Z

spec/os_mapping_spec.rb

+LKP_SRC_PATH = "#{LKP_SRC}/distro/adaptation".freeze
+
+def find_duplicated_lines(default_mapping_path, os_specific_mapping_path)
+  default_mapping_file = File.readlines(default_mapping_path).map(&:strip)


can here use read_mappings method? and help rename default_mapping_file to default_mapping_lines

Thanks for review, I updated File.readlines to read_mappings and renaled to default_mapping_lines

seowwj · 2025-03-08T03:39:44Z

Rebased and tested locally with rake spec

rli9 · 2025-03-08T09:20:08Z

Consider there's PR that reorganize the dir, you don't need to fix the failure, just submit the spec code is fine.

For this, the requirement is to check each distro separately, for example, if ubuntu-* contains same line, then it fails. But ubuntu has no relationship with centos.

BTW: how about this comment?

yeetengangIntel · 2025-03-11T02:25:24Z

Consider there's PR that reorganize the dir, you don't need to fix the failure, just submit the spec code is fine.

For this, the requirement is to check each distro separately, for example, if ubuntu-* contains same line, then it fails. But ubuntu has no relationship with centos.

BTW: how about this comment?

Thanks, we will be working on updating test based on the comment part today

rli9 · 2025-03-12T08:28:33Z

spec/os_mapping_spec.rb

+      end
+    end
+
+    os_specific_files.combination(2).each do |os_specific_file_1, os_specific_file_2|


thanks, need do one more change that the comparison is not between 2 files, because it's ok that there's same mapping in debian/10 and debian/11 (but not in 12). It is not ok if there's mapping existing in all debian os-specific files (10, 11, 12), because in this case, the map need be moved to default file.

rli9 · 2025-03-12T08:28:47Z

spec/os_mapping_spec.rb

+      end
+    end
+  end
+end


consider to add a new line here

…fault OS mapping and each OS-specific mappings. Fix intel#484 Signed-off-by: Ong, Yen Yee <[email protected]> Signed-off-by: Chiam, Jia Ying <[email protected]> Signed-off-by: yeetengangIntel <[email protected]> Signed-off-by: Wei Shan <[email protected]> Signed-off-by: Seow, Wen Jie <[email protected]>

yenyeeong · 2025-03-12T10:41:07Z

Hi, I have changed the spec test accordingly. I have also manual solved the errors being caught by this spec test and push to another branch. Let me know if you would like me to cherry-pick the commit into this branch.

rli9 · 2025-03-12T12:04:49Z

Let me know if you would like me to cherry-pick the commit into this branch.

Thanks, the code looks good. And it's very helpful if you also have the fix.

rli9 · 2025-03-13T02:47:10Z

distro/adaptation/fedora/default

@@ -364,6 +365,7 @@ nbd-client: nbd
 nbd-server: nbd
 ncurses-bin: ncurses
 netcat-openbsd: nmap-ncat
+netpipe-lam:
 netpipe-lam: NetPIPE


thanks, the above 2 lines looks wrong, both mapping to netpipe-lam. And help check failure of Checks 'Ruby / test-packages (fedora/41) (pull_request)"

Removed the duplicated lines in os_specific files and add the duplicated lines which appear in all os_specific to the default file. Signed-off-by: Ong, Yen Yee <[email protected]> Signed-off-by: Chiam, Jia Ying <[email protected]>

yenyeeong · 2025-03-13T03:08:29Z

The netpipe-lam error solved. Currently working on the github action failure on fedora package.

Signed-off-by: Seow, Wen Jie <[email protected]>

rli9 · 2025-03-13T04:01:03Z

thanks for the patch

yeetengangIntel force-pushed the dev/484-spec-test branch 2 times, most recently from c978bf5 to 0c340d3 Compare March 7, 2025 04:29

rli9 reviewed Mar 7, 2025

View reviewed changes

yeetengangIntel force-pushed the dev/484-spec-test branch from 0c340d3 to 1637dfe Compare March 7, 2025 08:44

seowwj force-pushed the dev/484-spec-test branch 2 times, most recently from c9adc8e to 10b4e6b Compare March 7, 2025 19:01

yenyeeong force-pushed the dev/484-spec-test branch 2 times, most recently from a981840 to 41c4b2c Compare March 12, 2025 08:17

rli9 reviewed Mar 12, 2025

View reviewed changes

spec/os_mapping_spec.rb Outdated

end

end

end

end

Copy link

rli9 Mar 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider to add a new line here

yenyeeong force-pushed the dev/484-spec-test branch from 41c4b2c to 6742974 Compare March 12, 2025 10:37

rli9 reviewed Mar 13, 2025

View reviewed changes

Removed duplicated lines in distro/adaption

02d9751

Removed the duplicated lines in os_specific files and add the duplicated lines which appear in all os_specific to the default file. Signed-off-by: Ong, Yen Yee <[email protected]> Signed-off-by: Chiam, Jia Ying <[email protected]>

yenyeeong force-pushed the dev/484-spec-test branch from 3b00c7e to 02d9751 Compare March 13, 2025 02:55

Remove NetPIPE from fedora

77c6756

Signed-off-by: Seow, Wen Jie <[email protected]>

rli9 merged commit 733a0bf into intel:master Mar 13, 2025
12 checks passed

spec: test the integrity of distro/adaption files #492

spec: test the integrity of distro/adaption files #492

Uh oh!

Conversation

j-chiam commented Mar 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yeetengangIntel Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rli9 Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seowwj commented Mar 8, 2025

Uh oh!

rli9 commented Mar 8, 2025

Uh oh!

yeetengangIntel commented Mar 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yenyeeong commented Mar 12, 2025

Uh oh!

rli9 commented Mar 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yenyeeong commented Mar 13, 2025

Uh oh!

Uh oh!

rli9 commented Mar 13, 2025

Uh oh!

Uh oh!

yeetengangIntel Mar 7, 2025 •

edited

Loading

rli9 Mar 7, 2025 •

edited

Loading