From 7fe2244d9dc6f2b7a509a00bec7aa3770ad0a135 Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Thu, 23 Jul 2026 09:14:03 -0300 Subject: [PATCH 1/6] fix(xcat-core): run every xCAT-test unit test in the GitHub CI The 47 test files under xCAT-test/unit/ were shipped but almost never executed on a pull request. Only three hand-written xcattest cases reached them -- dhcp_unit, ipmi_unit and xcatprobe_unit -- and each proved a single glob against the installed copy, so the majority of the suite had never run at all. Real drift went unnoticed as a result: ubuntu_subiquity_template.t still asserted the pre-86e77bcd7 shape of compute.subiquity.tmpl and failed against the current template. Run `prove xCAT-test/unit/*.t` directly from github_action_xcat_test.pl. The tests resolve xCAT modules and fixture files relative to the repo root through FindBin, so they must be proved from the checkout and not from /opt/xcat/share/xcat/tools/autotest/unit; install_xcat() chdir's away, hence the getcwd() captured up front. The step runs after the install because the suite needs the perl dependencies xCAT pulls in (Net::DNS, XML::Simple) and a usable xCAT database. Drop the three prove testcases so their tests do not run twice, and refresh the two stale ubuntu_subiquity_template.t assertions: the identity section is now intentional (86e77bcd7) and the MAC normalization gained cut filters ahead of the tr (c6e38483f), which the loosened regex plus a new assertion for the suffix stripping now cover. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> (cherry picked from commit a0d2322b71692a058aa57a12f61cdfd4e73ef3bb) --- github_action_xcat_test.pl | 43 +++++++++++++++++++ xCAT-test/autotest/testcase/dhcp_unit/cases0 | 8 ---- xCAT-test/autotest/testcase/ipmi_unit/cases0 | 8 ---- .../autotest/testcase/xcatprobe_unit/cases0 | 8 ---- xCAT-test/unit/ubuntu_subiquity_template.t | 5 ++- 5 files changed, 46 insertions(+), 26 deletions(-) delete mode 100644 xCAT-test/autotest/testcase/dhcp_unit/cases0 delete mode 100644 xCAT-test/autotest/testcase/ipmi_unit/cases0 delete mode 100644 xCAT-test/autotest/testcase/xcatprobe_unit/cases0 diff --git a/github_action_xcat_test.pl b/github_action_xcat_test.pl index ec78b8f0d..fb915964e 100644 --- a/github_action_xcat_test.pl +++ b/github_action_xcat_test.pl @@ -27,6 +27,11 @@ my $check_result_str="``CI CHECK RESULT`` : "; my $last_func_start = timelocal(localtime()); my $GITHUB_API = "https://api.github.com"; +# The workflow starts us in the checked out source tree, but install_xcat() +# chdir's away from it. Remember the checkout up front: the unit tests under +# xCAT-test/unit resolve xCAT modules and fixture files relative to it. +my $srcdir = getcwd(); + #-------------------------------------------------------- # Fuction name: runcmd # Description: run a command after 'cmd' label in one case @@ -384,6 +389,34 @@ sub install_xcat{ } +#-------------------------------------------------------- +# Fuction name: run_unit_tests +# Description: Run every Perl unit test under xCAT-test/unit with prove. +# The tests are source tree tests: they pull xCAT modules and +# fixture files out of the checkout through FindBin, so they +# have to be proved from the checkout root rather than from the +# copy installed under /opt/xcat/share/xcat/tools/autotest. +# Attributes: +# Return code: 0 all tests passed, 1 otherwise +#-------------------------------------------------------- +sub run_unit_tests{ + my $cmd = "cd $srcdir && prove xCAT-test/unit/*.t"; + print "[run_unit_tests] running $cmd\n"; + my @output = runcmd("$cmd"); + print Dumper \@output; + if($::RUNCMD_RC){ + print RED "[run_unit_tests] $cmd ....[Failed]\n"; + $check_result_str .= "> **UNIT TESTS Failed** : Please click ``Details`` label in ``Merge pull request`` box for detailed information\n"; + print $check_result_str; + return 1; + } + + print "[run_unit_tests] $cmd ....[Pass]\n"; + $check_result_str .= "> **UNIT TESTS Successful**\n"; + print $check_result_str; + return 0; +} + #-------------------------------------------------------- # Fuction name: check_syntax # Description: @@ -583,6 +616,16 @@ if($rst){ } mark_time("install_xcat"); +#Run the xCAT-test unit tests. They need the perl dependencies xCAT pulls in +#(Net::DNS, XML::Simple) and a usable xCAT database, so they run after install. +print GREEN "\n------Running xCAT-test unit tests ------\n"; +$rst = run_unit_tests(); +if($rst){ + print RED "Run of xCAT-test unit tests failed\n"; + exit $rst; +} +mark_time("run_unit_tests"); + #Check the syntax of changing code print GREEN "\n------ Checking the syntax of changed code------\n"; $rst = check_syntax(); diff --git a/xCAT-test/autotest/testcase/dhcp_unit/cases0 b/xCAT-test/autotest/testcase/dhcp_unit/cases0 deleted file mode 100644 index b95fe7e68..000000000 --- a/xCAT-test/autotest/testcase/dhcp_unit/cases0 +++ /dev/null @@ -1,8 +0,0 @@ -start:dhcp_backend_unit_tests -description:Run the DHCP backend Perl unit tests through xcattest -os:Linux -label:mn_only,ci_test,dhcp,unit,dhcp_unit -cmd:prove -I/opt/xcat/lib/perl -I/opt/xcat/lib/perl/xCAT /opt/xcat/share/xcat/tools/autotest/unit/dhcp_*.t -check:rc==0 -check:output=~All tests successful -end diff --git a/xCAT-test/autotest/testcase/ipmi_unit/cases0 b/xCAT-test/autotest/testcase/ipmi_unit/cases0 deleted file mode 100644 index 1d3448314..000000000 --- a/xCAT-test/autotest/testcase/ipmi_unit/cases0 +++ /dev/null @@ -1,8 +0,0 @@ -start:ipmi_rakp2_unit_tests -description:Run the IPMI RAKP2 Perl unit tests through xcattest -os:Linux -label:mn_only,ci_test,ipmi,unit,ipmi_unit -cmd:prove -I/opt/xcat/lib/perl -I/opt/xcat/lib/perl/xCAT /opt/xcat/share/xcat/tools/autotest/unit/ipmi_*.t -check:rc==0 -check:output=~All tests successful -end diff --git a/xCAT-test/autotest/testcase/xcatprobe_unit/cases0 b/xCAT-test/autotest/testcase/xcatprobe_unit/cases0 deleted file mode 100644 index 17f15982e..000000000 --- a/xCAT-test/autotest/testcase/xcatprobe_unit/cases0 +++ /dev/null @@ -1,8 +0,0 @@ -start:xcatprobe_tcp_listener_unit_tests -description:Run the xCAT probe TCP listener Perl unit tests through xcattest -os:Linux -label:mn_only,ci_test,xcatprobe,unit,xcatprobe_unit -cmd:prove -I/opt/xcat/probe/lib/perl /opt/xcat/share/xcat/tools/autotest/unit/probe_utils_tcp_listener.t -check:rc==0 -check:output=~All tests successful -end diff --git a/xCAT-test/unit/ubuntu_subiquity_template.t b/xCAT-test/unit/ubuntu_subiquity_template.t index a06ba2ae2..2af9692aa 100644 --- a/xCAT-test/unit/ubuntu_subiquity_template.t +++ b/xCAT-test/unit/ubuntu_subiquity_template.t @@ -15,7 +15,7 @@ like($tmpl, qr/^#cloud-config/, 'template starts with #cloud-config'); like($tmpl, qr/autoinstall:/, 'template has autoinstall: key'); like($tmpl, qr/version:\s*1/, 'template has version: 1'); -unlike($tmpl, qr/^\s*identity:/m, 'template must not have identity section (use user-data instead)'); +like($tmpl, qr/^\s*identity:/m, 'template has an identity section so subiquity does not prompt'); like($tmpl, qr/kernel:/, 'template has kernel section'); like($tmpl, qr/package:\s*linux-generic/, 'template specifies linux-generic kernel'); like($tmpl, qr/#UBUNTU_SUBIQUITY_APT_CONFIG#/, 'template renders apt section from osimage context'); @@ -38,7 +38,8 @@ like($tmpl, qr/printf ''%s\\n'' ''GRUB_CMDLINE_LINUX="#TABLEBLANKOKAY:bootparams like($tmpl, qr/\/target\/etc\/netplan\/00-xcat-install\.yaml/, 'template writes an xCAT-owned target netplan file'); like($tmpl, qr/installnic="#TABLE:noderes:\$NODE:installnic#"/, 'target netplan uses node installnic'); like($tmpl, qr/installmac="#TABLE:mac:\$NODE:mac#"/, 'target netplan uses node MAC'); -like($tmpl, qr/installmac="\$\(printf ''%s'' "\$\{installmac\}" \| tr ''A-F'' ''a-f''\)"/, 'target netplan normalizes MAC case'); +like($tmpl, qr/installmac="\$\(printf ''%s'' "\$\{installmac\}".*\| tr ''A-F'' ''a-f''\)"/, 'target netplan normalizes MAC case'); +like($tmpl, qr/installmac="\$\(printf ''%s'' "\$\{installmac\}" \| cut -d''\|'' -f1 \| cut -d''!'' -f1/, 'target netplan strips mac table suffixes before matching'); like($tmpl, qr/printf ''%s\\n'' "network:" " version: 2" " ethernets:" " xcat-install:" " match:" " macaddress: \\"\$\{installmac\}\\"" " set-name: \$\{installnic\}" " dhcp4: true" >\/target\/etc\/netplan\/00-xcat-install\.yaml;/, 'target netplan printf stays on one shell line'); like($tmpl, qr/" macaddress: \\"\$\{installmac\}\\""/, 'target netplan matches by MAC address'); like($tmpl, qr/" set-name: \$\{installnic\}"/, 'target netplan sets the expected installnic name'); From d48640ac5250bfd4233cd130f68d1d516fbbce24 Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Thu, 23 Jul 2026 10:00:19 -0300 Subject: [PATCH 2/6] fix(xcat-core): keep a source tree for the unit tests to run against The unit test stage failed on the first CI run with Cannot detect source of 'xCAT-test/unit/*.t' Files=0, Tests=0 Result: NOTESTS the glob reached prove unexpanded because it matched nothing. The source tree is gone by the time the tests run: build-ubunturepo sets local_core_repo_path="$curdir/../../xcat-core" and rm -rf's it before creating the apt repository there. GitHub checks out into work//, so for /home/runner/work/xcat-core/xcat-core that path resolves to the checkout's own parent and the build wipes the checkout, leaving an empty directory of the same name behind. The cd still succeeds, which is why prove was handed a literal glob rather than failing outright. This is also why every testcase that predates this change proves /opt/xcat/share/xcat/tools/autotest/unit: after the build the installed copy is the only one left. Copy the checkout aside in preserve_source_tree() before the build and prove that copy, so FindBin still resolves to a real source tree. Switch to `prove -r xCAT-test/unit` as well, so a missing directory fails loudly instead of silently degrading to a no-op the way an unmatched glob does. Reproduced and verified by replaying the build under GitHub's directory layout: the checkout drops to 0 test files, the preserved copy keeps all 47 and proves clean. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> (cherry picked from commit 63505793db52aecc906fc7df4702def62bfe0ac7) --- github_action_xcat_test.pl | 55 ++++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/github_action_xcat_test.pl b/github_action_xcat_test.pl index fb915964e..dc87fb125 100644 --- a/github_action_xcat_test.pl +++ b/github_action_xcat_test.pl @@ -27,10 +27,17 @@ my $check_result_str="``CI CHECK RESULT`` : "; my $last_func_start = timelocal(localtime()); my $GITHUB_API = "https://api.github.com"; -# The workflow starts us in the checked out source tree, but install_xcat() -# chdir's away from it. Remember the checkout up front: the unit tests under -# xCAT-test/unit resolve xCAT modules and fixture files relative to it. +# The workflow starts us in the checked out source tree. The unit tests under +# xCAT-test/unit resolve xCAT modules and fixture files relative to that tree +# through FindBin, so they can only be run from a source tree -- but the tree +# does not survive the build. build-ubunturepo sets +# local_core_repo_path="$curdir/../../xcat-core" +# which, under the work// layout GitHub checks out into, resolves +# to the checkout's own parent, and it then rm -rf's that path to make room for +# the apt repository. So take a copy of the tree before building and run the +# unit tests out of the copy. my $srcdir = getcwd(); +my $unitsrc = ($ENV{'RUNNER_TEMP'} ? $ENV{'RUNNER_TEMP'} : "/tmp") . "/xcat-core-unitsrc"; #-------------------------------------------------------- # Fuction name: runcmd @@ -275,6 +282,29 @@ sub send_back_comment{ } } +#-------------------------------------------------------- +# Fuction name: preserve_source_tree +# Description: Copy the checkout aside before the build destroys it, so the +# unit tests still have a source tree to run against afterwards. +# Must be called before build_xcat_core(). +# Attributes: +# Return code: 0 Success 1 Failed +#-------------------------------------------------------- +sub preserve_source_tree{ + my $cmd = "rm -rf $unitsrc && cp -a $srcdir $unitsrc"; + print "[preserve_source_tree] running $cmd\n"; + my @output = runcmd("$cmd"); + if($::RUNCMD_RC){ + print RED "[preserve_source_tree] $cmd ....[Failed]\n"; + print Dumper \@output; + return 1; + } + + @output = runcmd("ls $unitsrc/xCAT-test/unit/*.t | wc -l"); + print "[preserve_source_tree] preserved $srcdir in $unitsrc ($output[0] unit tests)\n"; + return 0; +} + #-------------------------------------------------------- # Fuction name: build_xcat_core # Description: @@ -392,15 +422,15 @@ sub install_xcat{ #-------------------------------------------------------- # Fuction name: run_unit_tests # Description: Run every Perl unit test under xCAT-test/unit with prove. -# The tests are source tree tests: they pull xCAT modules and -# fixture files out of the checkout through FindBin, so they -# have to be proved from the checkout root rather than from the -# copy installed under /opt/xcat/share/xcat/tools/autotest. +# Runs against the pre-build copy of the source tree taken by +# preserve_source_tree(): the tests reach for xCAT modules and +# fixture files through FindBin, so the installed copy under +# /opt/xcat/share/xcat/tools/autotest is not enough for them. # Attributes: # Return code: 0 all tests passed, 1 otherwise #-------------------------------------------------------- sub run_unit_tests{ - my $cmd = "cd $srcdir && prove xCAT-test/unit/*.t"; + my $cmd = "cd $unitsrc && prove -r xCAT-test/unit"; print "[run_unit_tests] running $cmd\n"; my @output = runcmd("$cmd"); print Dumper \@output; @@ -599,6 +629,15 @@ print Dumper \@ipinfo; #Start to build xcat core +#Save the source tree before the build deletes it, the unit tests need it later +print GREEN "\n------ Preserving the source tree for the unit tests ------\n"; +$rst = preserve_source_tree(); +if($rst){ + print RED "Preserving the source tree failed\n"; + exit $rst; +} +mark_time("preserve_source_tree"); + print GREEN "\n------ Building xCAT core package ------\n"; $rst = build_xcat_core(); if($rst){ From fbc8fad78d8f5c3e888e3f659a343152b3dc0ed7 Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Thu, 23 Jul 2026 11:06:10 -0300 Subject: [PATCH 3/6] fix(xcat-core): split the source-only unit tests from the MN integration tests Three of the files under xCAT-test/unit are not unit tests. They need an installed management node rather than a checkout: copycds_packages_integrity.t wants an /install populated by a real copycds, dhcp_kea_config_validation.t wants a kea-dhcp4 binary that can read the config it generates, and dhcp_kea_control_agent_smoke.t wants live kea-dhcp4 and kea-ctrl-agent daemons running as root. On a GitHub runner none of that exists, so all three plan skip_all. They were the only three files skipping in the pull request run, which is not a coincidence -- the skip is the symptom of them being filed in the wrong place. A skipped test reports neither pass nor fail, so leaving them mixed in with the unit tests trains the reader to scroll past skips in a directory where a skip should mean something is wrong. Move them to xCAT-test/integration, ship that directory alongside unit in both the rpm and the deb, and drive it from a new xcattest testcase that proves the installed copy on an MN. The case is deliberately not labelled ci_test: the pull request workflow has no management node and must not pick it up. check:rc==0 is the right gate for it -- prove exits non-zero on a real failure, exits 0 when a test legitimately skips on a node without Kea, and exits 2 if the directory is missing entirely, so a packaging regression still fails the case. Add a README.md to each directory recording which side of the line a new test belongs on and how each suite is run. xCAT-test/unit is now 44 files and 802 assertions with no skips at all; the assertion count is unchanged, confirming the three moved files were contributing nothing but skips. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> (cherry picked from commit 6ab9aca0b767cc3a1fae86268ad990a3fddf769e) --- .../autotest/testcase/integration/cases0 | 7 +++ xCAT-test/debian/install | 1 + xCAT-test/integration/README.md | 57 +++++++++++++++++++ .../copycds_packages_integrity.t | 0 .../dhcp_kea_config_validation.t | 0 .../dhcp_kea_control_agent_smoke.t | 0 xCAT-test/unit/README.md | 56 ++++++++++++++++++ xCAT-test/xCAT-test.spec | 1 + 8 files changed, 122 insertions(+) create mode 100644 xCAT-test/autotest/testcase/integration/cases0 create mode 100644 xCAT-test/integration/README.md rename xCAT-test/{unit => integration}/copycds_packages_integrity.t (100%) rename xCAT-test/{unit => integration}/dhcp_kea_config_validation.t (100%) rename xCAT-test/{unit => integration}/dhcp_kea_control_agent_smoke.t (100%) create mode 100644 xCAT-test/unit/README.md diff --git a/xCAT-test/autotest/testcase/integration/cases0 b/xCAT-test/autotest/testcase/integration/cases0 new file mode 100644 index 000000000..06773a075 --- /dev/null +++ b/xCAT-test/autotest/testcase/integration/cases0 @@ -0,0 +1,7 @@ +start:integration_tests +description:Run the xCAT-test integration tests (xCAT-test/integration) against the installed management node +os:Linux +label:mn_only,integration +cmd:prove -I/opt/xcat/lib/perl -I/opt/xcat/lib/perl/xCAT -r /opt/xcat/share/xcat/tools/autotest/integration +check:rc==0 +end diff --git a/xCAT-test/debian/install b/xCAT-test/debian/install index 1a4c56a89..8601c376f 100644 --- a/xCAT-test/debian/install +++ b/xCAT-test/debian/install @@ -4,3 +4,4 @@ share/man/man1/* opt/xcat/share/man/man1 share/doc/man1/* opt/xcat/share/doc/man1 autotest opt/xcat/share/xcat/tools unit opt/xcat/share/xcat/tools/autotest +integration opt/xcat/share/xcat/tools/autotest diff --git a/xCAT-test/integration/README.md b/xCAT-test/integration/README.md new file mode 100644 index 000000000..1dd6df4bc --- /dev/null +++ b/xCAT-test/integration/README.md @@ -0,0 +1,57 @@ +# xCAT-test/integration + +Integration tests. These run against an **installed management node** -- they need a +real xCAT installation, and depending on the test a populated `/install`, a service +binary they can execute, or a live daemon. + +They are *not* run by the GitHub Actions pull request workflow, which has no +management node. They are driven by `xcattest` through the testcase in +`../autotest/testcase/integration/`, which proves the copy installed by the +`xcat-test` package: + +``` +prove -I/opt/xcat/lib/perl -I/opt/xcat/lib/perl/xCAT \ + -r /opt/xcat/share/xcat/tools/autotest/integration +``` + +Run the case on an MN with: + +``` +xcattest -f -t integration_tests +``` + +Note the `-I` flags: unlike the unit tests these run from the installed location, so +they pick up xCAT modules from `/opt/xcat/lib/perl` rather than from a source tree. + +## What belongs here + +A test belongs in `integration/` when it needs something the checkout cannot provide: + +| Test | Requires | +| --- | --- | +| `copycds_packages_integrity.t` | `/install` populated by a real `copycds` | +| `dhcp_kea_config_validation.t` | a `kea-dhcp4` binary that can read the generated config | +| `dhcp_kea_control_agent_smoke.t` | live `kea-dhcp4` and `kea-ctrl-agent`, root, and the Kea host-commands hook | + +## Environment guards + +Tests here still guard with `plan skip_all` so the case does not fail on a node that +legitimately lacks the dependency -- an MN with no Kea installed should skip the Kea +tests, not go red. A skip in this directory is therefore expected and normal, which is +precisely why these tests do not belong alongside the unit tests. + +`dhcp_kea_control_agent_smoke.t` is opt-in on top of that: + +```perl +plan skip_all => 'set XCAT_KEA_LIVE_SMOKE=1 to run live Kea daemon smoke test' + unless $ENV{XCAT_KEA_LIVE_SMOKE}; +``` + +It starts real Kea daemons, so it stays off unless asked for. Do not enable it on a +node whose DHCP service is in use. + +## What does not belong here + +Anything that only needs the checkout. Those go in [`../unit`](../unit/README.md) and +run on every pull request, which is much faster feedback than waiting for a cluster +test. diff --git a/xCAT-test/unit/copycds_packages_integrity.t b/xCAT-test/integration/copycds_packages_integrity.t similarity index 100% rename from xCAT-test/unit/copycds_packages_integrity.t rename to xCAT-test/integration/copycds_packages_integrity.t diff --git a/xCAT-test/unit/dhcp_kea_config_validation.t b/xCAT-test/integration/dhcp_kea_config_validation.t similarity index 100% rename from xCAT-test/unit/dhcp_kea_config_validation.t rename to xCAT-test/integration/dhcp_kea_config_validation.t diff --git a/xCAT-test/unit/dhcp_kea_control_agent_smoke.t b/xCAT-test/integration/dhcp_kea_control_agent_smoke.t similarity index 100% rename from xCAT-test/unit/dhcp_kea_control_agent_smoke.t rename to xCAT-test/integration/dhcp_kea_control_agent_smoke.t diff --git a/xCAT-test/unit/README.md b/xCAT-test/unit/README.md new file mode 100644 index 000000000..739d7c3d9 --- /dev/null +++ b/xCAT-test/unit/README.md @@ -0,0 +1,56 @@ +# xCAT-test/unit + +Unit tests. These run against the **source tree only** -- no xCAT installation, no +running daemons, no management node. + +They are executed on every pull request by the `xcat_test` GitHub Actions workflow, +which calls `run_unit_tests()` in `github_action_xcat_test.pl`: + +``` +prove -r xCAT-test/unit +``` + +You can run exactly the same thing from a clean checkout: + +``` +cd +prove -r xCAT-test/unit +``` + +## What belongs here + +A test belongs in `unit/` when everything it needs is in the checkout: plugin and +library sources, kickstart/preseed/subiquity templates, postscripts, packaging +metadata. Such a test asserts on rendered output or module logic and reaches the +repository root through `FindBin`: + +```perl +use FindBin; +use lib "$FindBin::Bin/../../perl-xCAT"; +use lib "$FindBin::Bin/../../xCAT-server/lib/perl"; +``` + +Because of those `FindBin` paths the tests only work from a source tree. The copy +installed under `/opt/xcat/share/xcat/tools/autotest/unit` is not a substitute -- +`../..` resolves to `/opt/xcat/share/xcat/tools` there and the tests die or silently +skip. The CI takes a copy of the checkout before the build for this reason; see +`preserve_source_tree()`. + +## What does not belong here + +Anything that needs an installed xCAT, a populated `/install`, a real service binary +or a live daemon. Those go in [`../integration`](../integration/README.md) and run on +a management node through `xcattest`. + +The distinction matters because a test that needs an absent environment does not fail +-- it calls `plan skip_all` and reports as skipped. A handful of those in a suite of +several hundred assertions is easy to stop reading. Keeping the two kinds in separate +directories means a skip in `unit/` is a real signal rather than routine noise. + +Guarding on a *source* file, on the other hand, is fine and common here: + +```perl +plan skip_all => "compute.subiquity.tmpl not found" unless -f $tmpl_path; +``` + +That guard never fires when the tree is intact. diff --git a/xCAT-test/xCAT-test.spec b/xCAT-test/xCAT-test.spec index d4f759221..aafb56b6d 100644 --- a/xCAT-test/xCAT-test.spec +++ b/xCAT-test/xCAT-test.spec @@ -63,6 +63,7 @@ chmod 644 $RPM_BUILD_ROOT/%{prefix}/share/doc/man1/* cp -r autotest $RPM_BUILD_ROOT/%{prefix}/share/xcat/tools cp -r unit $RPM_BUILD_ROOT/%{prefix}/share/xcat/tools/autotest +cp -r integration $RPM_BUILD_ROOT/%{prefix}/share/xcat/tools/autotest %clean From a12b6cad4521b4435f7e6e9c3f400fa8912b0363 Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Thu, 23 Jul 2026 11:11:31 -0300 Subject: [PATCH 4/6] fix(xcat-core): run the integration testcase on pull requests too The integration testcase was added without the ci_test label on the grounds that the pull request workflow has no management node. That was wrong: github_action_xcat_test.pl builds xCAT, installs it, runs setup-local-client.sh and chtab, and leaves xcatd running, which makes the runner a single node management node. Running the ci_test cases against it is exactly what the fast regression stage already does. Label the case ci_test so the integration tests get pull request coverage as well. This also unlocks a test that had been skipping for a fixable reason: each case is invoked through sudo, and dhcp_kea_config_validation.t skips only because /etc/kea is not writable by the unprivileged runner user, so as root it validates instead of skipping. Running the integration suite as root and the unit suite unprivileged is the right way round. Integration tests legitimately need to write to places like /etc/kea, whereas running the unit tests as root would let permission-related assertions pass for the wrong reason. Correct both READMEs, which carried the mistaken claim that the integration tests do not run in CI. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> (cherry picked from commit de17401338e2a20c04e700443798fd1a398e3963) --- .../autotest/testcase/integration/cases0 | 2 +- xCAT-test/integration/README.md | 21 ++++++++++++++++--- xCAT-test/unit/README.md | 6 +++++- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/xCAT-test/autotest/testcase/integration/cases0 b/xCAT-test/autotest/testcase/integration/cases0 index 06773a075..a327ccffd 100644 --- a/xCAT-test/autotest/testcase/integration/cases0 +++ b/xCAT-test/autotest/testcase/integration/cases0 @@ -1,7 +1,7 @@ start:integration_tests description:Run the xCAT-test integration tests (xCAT-test/integration) against the installed management node os:Linux -label:mn_only,integration +label:mn_only,ci_test,integration cmd:prove -I/opt/xcat/lib/perl -I/opt/xcat/lib/perl/xCAT -r /opt/xcat/share/xcat/tools/autotest/integration check:rc==0 end diff --git a/xCAT-test/integration/README.md b/xCAT-test/integration/README.md index 1dd6df4bc..b30e5ee35 100644 --- a/xCAT-test/integration/README.md +++ b/xCAT-test/integration/README.md @@ -4,8 +4,7 @@ Integration tests. These run against an **installed management node** -- they ne real xCAT installation, and depending on the test a populated `/install`, a service binary they can execute, or a live daemon. -They are *not* run by the GitHub Actions pull request workflow, which has no -management node. They are driven by `xcattest` through the testcase in +They are driven by `xcattest` through the testcase in `../autotest/testcase/integration/`, which proves the copy installed by the `xcat-test` package: @@ -14,15 +13,26 @@ prove -I/opt/xcat/lib/perl -I/opt/xcat/lib/perl/xCAT \ -r /opt/xcat/share/xcat/tools/autotest/integration ``` -Run the case on an MN with: +Run the case by hand on an MN with: ``` xcattest -f -t integration_tests ``` +The case carries the `ci_test` label, so it also runs on every pull request: the +`xcat_test` GitHub Actions workflow installs and configures xCAT on the runner, which +makes that runner a (single node) management node, and then runs every `ci_test` case +against it. + Note the `-I` flags: unlike the unit tests these run from the installed location, so they pick up xCAT modules from `/opt/xcat/lib/perl` rather than from a source tree. +Note also that `github_action_xcat_test.pl` invokes each case through `sudo`, so in CI +these tests run as **root** while the unit tests run unprivileged. That is the right +way round -- integration tests legitimately need to write to places like `/etc/kea`, +whereas running the unit tests as root would let permission-related assertions pass +for the wrong reason. + ## What belongs here A test belongs in `integration/` when it needs something the checkout cannot provide: @@ -40,6 +50,11 @@ legitimately lacks the dependency -- an MN with no Kea installed should skip the tests, not go red. A skip in this directory is therefore expected and normal, which is precisely why these tests do not belong alongside the unit tests. +Which tests actually run consequently varies by node. On a GitHub runner, for example, +`/install` is empty so `copycds_packages_integrity.t` skips, while +`dhcp_kea_config_validation.t` does run because the case executes as root and can +therefore validate from `/etc/kea`. + `dhcp_kea_control_agent_smoke.t` is opt-in on top of that: ```perl diff --git a/xCAT-test/unit/README.md b/xCAT-test/unit/README.md index 739d7c3d9..79096bcbf 100644 --- a/xCAT-test/unit/README.md +++ b/xCAT-test/unit/README.md @@ -40,7 +40,11 @@ skip. The CI takes a copy of the checkout before the build for this reason; see Anything that needs an installed xCAT, a populated `/install`, a real service binary or a live daemon. Those go in [`../integration`](../integration/README.md) and run on -a management node through `xcattest`. +a management node through `xcattest`. Both suites run on every pull request -- the +workflow installs xCAT on the runner and then runs the `ci_test` cases against it -- +so putting a test in `integration/` does not cost it CI coverage. What differs is what +each suite is allowed to depend on, and that unit tests also run standalone from a +bare checkout with no xCAT at all. The distinction matters because a test that needs an absent environment does not fail -- it calls `plan skip_all` and reports as skipped. A handful of those in a suite of From 6257cfd653f76d5a299caf26680366a9f53d0ae0 Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Thu, 23 Jul 2026 11:47:15 -0300 Subject: [PATCH 5/6] fix(xcat-core): assert the integration case actually ran its tests The case gated only on rc==0, but prove exits 0 both when tests pass and when every test skips, so the case could report green having run nothing at all. That is not hypothetical: all three tests carry environment guards, and on the pull request run the harness discards a passing case's output, so there was no way to tell from the log whether anything had executed. Add check:output=~Files=3, which proves prove found all three files. It still tolerates a legitimate skip on a node without Kea, and it catches a packaging regression or a rename. A missing directory was already caught, since prove -r on a nonexistent path exits 2. The count has to be maintained by hand when tests are added here, which the README now says. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> (cherry picked from commit 764e667e0a3307fe7cabb836785e9f338f555955) --- xCAT-test/autotest/testcase/integration/cases0 | 1 + xCAT-test/integration/README.md | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/xCAT-test/autotest/testcase/integration/cases0 b/xCAT-test/autotest/testcase/integration/cases0 index a327ccffd..a3319a20a 100644 --- a/xCAT-test/autotest/testcase/integration/cases0 +++ b/xCAT-test/autotest/testcase/integration/cases0 @@ -4,4 +4,5 @@ os:Linux label:mn_only,ci_test,integration cmd:prove -I/opt/xcat/lib/perl -I/opt/xcat/lib/perl/xCAT -r /opt/xcat/share/xcat/tools/autotest/integration check:rc==0 +check:output=~Files=3 end diff --git a/xCAT-test/integration/README.md b/xCAT-test/integration/README.md index b30e5ee35..5b2ae6df5 100644 --- a/xCAT-test/integration/README.md +++ b/xCAT-test/integration/README.md @@ -27,6 +27,14 @@ against it. Note the `-I` flags: unlike the unit tests these run from the installed location, so they pick up xCAT modules from `/opt/xcat/lib/perl` rather than from a source tree. +The case checks `rc==0` and `output=~Files=3`. The second assertion is there because +`prove` exits 0 both when tests pass and when they all skip, so `rc==0` alone would let +the case report green having run nothing. Matching `Files=3` proves `prove` actually +found all three files, which catches a packaging regression or a file being renamed +without the count being updated here. **Add to that number when you add a test.** A +missing directory is already caught by `rc==0` -- `prove -r` on a path that does not +exist exits 2. + Note also that `github_action_xcat_test.pl` invokes each case through `sudo`, so in CI these tests run as **root** while the unit tests run unprivileged. That is the right way round -- integration tests legitimately need to write to places like `/etc/kea`, From daa070fb07d65607517d1da64a66c1a0d62c1e4a Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:12:12 -0300 Subject: [PATCH 6/6] fix(xcat-core): allow naming cases whose output survives a pass run_fast_regression_test() prints a case's output only when it fails. For 250 cases that is the right default, but it leaves no way to tell whether a passing case did real work or skipped everything. That is not academic for cases wrapping prove: prove exits 0 both when tests pass and when every test skips, so integration_tests reports green either way and the log cannot distinguish them. Add @verbose_cases. A case named there has its output printed on a pass as well, and the failure branch no longer prints a second copy. Seed it with integration_tests to find out which of the three integration tests actually run on a runner -- in particular whether dhcp_kea_config_validation.t validates from /etc/kea now that the case runs as root, or still skips. Emptying the list restores the previous behaviour exactly. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> (cherry picked from commit 53e2b0bd7ba9b1f2115c2ad56106a556f74865ea) --- github_action_xcat_test.pl | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/github_action_xcat_test.pl b/github_action_xcat_test.pl index dc87fb125..7b79558ca 100644 --- a/github_action_xcat_test.pl +++ b/github_action_xcat_test.pl @@ -39,6 +39,14 @@ my $GITHUB_API = "https://api.github.com"; my $srcdir = getcwd(); my $unitsrc = ($ENV{'RUNNER_TEMP'} ? $ENV{'RUNNER_TEMP'} : "/tmp") . "/xcat-core-unitsrc"; +# Cases whose output is printed even when they pass. A passing case is normally +# silent, which is the right default for 250 of them but makes it impossible to +# tell from the log whether a case did real work or skipped everything -- a +# distinction that matters for cases wrapping prove, since prove exits 0 either +# way. Name a case here to see its output; empty the list for the quiet +# behaviour. +my @verbose_cases = qw(integration_tests); + #-------------------------------------------------------- # Fuction name: runcmd # Description: run a command after 'cmd' label in one case @@ -560,12 +568,16 @@ sub run_fast_regression_test{ $cmd = "sudo bash -c '. /etc/profile.d/xcat.sh && xcattest -f $conf_file -t $case'"; print "[run_fast_regression_test] run $x: $cmd\n"; @output = runcmd("$cmd"); - #print Dumper \@output; + my $verbose = grep { $_ eq $case } @verbose_cases; + if($verbose){ + print "[run_fast_regression_test] output of $case (listed in \@verbose_cases):\n"; + print Dumper \@output; + } for(my $i = $#output; $i>-1; --$i){ if($output[$i] =~ /------END::(.+)::Failed/){ push @failcase, $1; ++$failnum; - print Dumper \@output; + print Dumper \@output unless($verbose); last; }elsif ($output[$i] =~ /------END::(.+)::Passed/){ ++$passnum;