From dad2d9332c548ad2b7049cee7d322e29dc7e03e2 Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Fri, 18 Sep 2026 11:05:05 -0300 Subject: [PATCH] test(blackboxtest): fail a wire case that skips what it tests A case passed when its fixture could not run it, and a fixture passed a stage it left out. provfixture skipped the HTTP stage when the web server answered 404 for the xCAT aliases, which is the regression P-32 to P-39 exist to catch. It ran the discovery and petitboot scenarios only when the file under test already existed. CI prints a passing case's output nowhere, so none of this was visible. A failed check, a skipped stage and a missing discovery or petitboot file now fail the case. The DHCP phase fails when only one backend is installed, since both are shipped. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> --- github_action_xcat_test.pl | 7 ++- .../autotest/testcase/blackboxtest/dhcp_cases | 63 +++++++------------ .../testcase/blackboxtest/dhcpfixture.sh | 7 +-- .../autotest/testcase/blackboxtest/prov_cases | 51 +++++---------- .../testcase/blackboxtest/provfixture.sh | 44 +++++-------- 5 files changed, 61 insertions(+), 111 deletions(-) diff --git a/github_action_xcat_test.pl b/github_action_xcat_test.pl index a7f3348ee..26da5a029 100644 --- a/github_action_xcat_test.pl +++ b/github_action_xcat_test.pl @@ -766,8 +766,11 @@ sub run_dhcp_wire_test{ } my $backend_str = join(" and ", @backends); + # Both backends are shipped, so a pass that served with one of them says + # nothing about the other. if(@backends < 2){ - print "[run_dhcp_wire_test] WARNING: only $backend_str is installed here, so the backends were not compared\n"; + print RED "[run_dhcp_wire_test] only $backend_str is installed here, so the backends were not compared\n"; + push @{$counts{failed}}, "only-$backend_str-installed"; } # Case runs rather than cases: each case is run once per backend. Counted @@ -779,7 +782,7 @@ sub run_dhcp_wire_test{ print RED "[run_dhcp_wire_test] $missing case run(s) produced no verdict\n"; push @{$counts{failed}}, "$missing-without-a-verdict"; } - if($counts{fail} or $missing){ + if($counts{fail} or $missing or @backends < 2){ my $log_str = join(",", @{$counts{failed}}); $check_result_str .= "> **DHCP WIRE TEST Failed** ($backend_str): Totalrun $casenum Passed $counts{pass} Failed $counts{fail} FailedCases: $log_str. Please click ``Details`` label in ``Merge pull request`` box for detailed information\n"; print $check_result_str; diff --git a/xCAT-test/autotest/testcase/blackboxtest/dhcp_cases b/xCAT-test/autotest/testcase/blackboxtest/dhcp_cases index 0ee565848..bfee46364 100644 --- a/xCAT-test/autotest/testcase/blackboxtest/dhcp_cases +++ b/xCAT-test/autotest/testcase/blackboxtest/dhcp_cases @@ -9,8 +9,7 @@ # Each wire case is one script, so its teardown runs from a trap and the machine # is left serving its own config even when an assertion fails. # -# A case that cannot run says so and passes. Read a pass as coverage only when -# the log shows the ok lines. +# A case that cannot run fails, and says what is missing. start:blackbox_dhcp_provision_vs_discovery description:A defined node gets its reservation and an unknown one gets a pool address @@ -32,10 +31,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/dhcpfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 "$FIX" run || exit 1 @@ -53,10 +50,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/dhcpfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 "$FIX" run-arch || exit 1 @@ -77,10 +72,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/dhcpfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 "$FIX" run-netboot || exit 1 @@ -100,10 +93,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/dhcpfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 rc=0 @@ -128,10 +119,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/dhcpfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 rc=0 @@ -153,10 +142,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/dhcpfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 rc=0 @@ -177,10 +164,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/dhcpfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 "$FIX" run-chainload || exit 1 @@ -194,10 +179,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/dhcpfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 "$FIX" run-lease || exit 1 @@ -215,10 +198,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/dhcpfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 "$FIX" run-adoption || exit 1 @@ -232,10 +213,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/dhcpfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 "$FIX" run || exit 1 diff --git a/xCAT-test/autotest/testcase/blackboxtest/dhcpfixture.sh b/xCAT-test/autotest/testcase/blackboxtest/dhcpfixture.sh index d3d49eae9..a2ee59253 100755 --- a/xCAT-test/autotest/testcase/blackboxtest/dhcpfixture.sh +++ b/xCAT-test/autotest/testcase/blackboxtest/dhcpfixture.sh @@ -133,11 +133,8 @@ TOOL=/opt/xcat/share/xcat/tools/autotest/blackboxtest say() { echo "dhcpfixture: $*"; } die() { echo "dhcpfixture: $*" >&2; exit 1; } -# Two different reasons not to run, and they must not be confused. A missing -# tool is a machine that cannot run the cases, and the case passes; leftovers -# from a killed run, or a cluster with nodes of its own, are refusals -- cases0 -# fails on those, because skipping them would make every later run on the same -# machine green without running anything. +# Both fail the case. skip: this machine lacks a tool the cases need. refuse: +# the machine holds this fixture's leftovers, or a cluster of its own. skip() { echo "dhcpfixture skipped: $*"; exit 1; } refuse() { echo "dhcpfixture refused: $*" >&2; exit 2; } diff --git a/xCAT-test/autotest/testcase/blackboxtest/prov_cases b/xCAT-test/autotest/testcase/blackboxtest/prov_cases index eb3ac3d67..267675894 100644 --- a/xCAT-test/autotest/testcase/blackboxtest/prov_cases +++ b/xCAT-test/autotest/testcase/blackboxtest/prov_cases @@ -10,8 +10,7 @@ # Each wire case is one script, so its teardown runs from a trap and the machine # is left serving its own configuration even when an assertion fails. # -# A case that cannot run says so and passes. Read a pass as coverage only when -# the log shows the ok lines. +# A case that cannot run fails, and says what is missing. start:blackbox_prov_name_resolution description:Every name a node resolves during provisioning answers, in both directions @@ -29,10 +28,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/provfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 "$FIX" run-dns || exit 1 @@ -53,10 +50,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/provfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 "$FIX" run-tftp || exit 1 @@ -75,10 +70,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/provfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 "$FIX" run-http || exit 1 @@ -94,10 +87,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/provfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 "$FIX" run-genesis || exit 1 @@ -116,10 +107,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/provfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 "$FIX" run-discovery || exit 1 @@ -138,10 +127,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/provfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 "$FIX" run-xcatd || exit 1 @@ -158,10 +145,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/provfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 "$FIX" run-monitor || exit 1 @@ -181,10 +166,8 @@ os:Linux cmd:#!/bin/bash set -u FIX=/opt/xcat/share/xcat/tools/autotest/testcase/blackboxtest/provfixture.sh - # 0 run, 1 this machine cannot run the wire cases and the case passes, 2 the - # machine is dirty or is a live cluster -- which must fail, or one unfinished - # teardown would make every later run here green without running anything. - "$FIX" check || { [ $? -eq 1 ] && exit 0; exit 1; } + # A machine that cannot run the case fails it: a pass must mean it ran. + "$FIX" check || exit 1 trap '"$FIX" teardown' EXIT "$FIX" setup || exit 1 rc=0 diff --git a/xCAT-test/autotest/testcase/blackboxtest/provfixture.sh b/xCAT-test/autotest/testcase/blackboxtest/provfixture.sh index f9504a3c3..7908e7125 100755 --- a/xCAT-test/autotest/testcase/blackboxtest/provfixture.sh +++ b/xCAT-test/autotest/testcase/blackboxtest/provfixture.sh @@ -130,16 +130,12 @@ TOOL=/opt/xcat/share/xcat/tools/autotest/blackboxtest say() { echo "provfixture: $*"; } die() { echo "provfixture: $*" >&2; exit 1; } -# Three answers, not two. A machine missing a tool cannot run the cases and the -# case passes; a machine holding this fixture's leftovers would have every later -# run skip green forever, and a cluster with nodes of its own would be damaged -# by the setup. Those two are refusals, and cases0 fails the case on them. +# Both fail the case. skip: this machine lacks a tool the cases need. refuse: +# the machine holds this fixture's leftovers, or a cluster of its own. skip() { echo "provfixture skipped: $*"; exit 1; } refuse() { echo "provfixture refused: $*" >&2; exit 2; } -# A stage this machine cannot run: says what is missing and passes. Counted and -# named on one greppable line, because a build that is green because nothing ran -# is the failure this suite exists to catch. +# A stage left out: says what is missing, and fails the run at exit. SKIPPED=0 stage_skip() { SKIPPED=$((SKIPPED + 1)) @@ -927,15 +923,12 @@ do_run_tftp() { --set node="$XNBA_NODE" --set httpport="$port" \ conf/prov/tftp-xnba.conf || rc=1 - if [ -f "$tftp/petitboot/$PTB_NODE" ]; then - prov_run \ - "${COMMON[@]}" \ - --set node="$PTB_NODE" --set hexip="$(hex_ip "$PTB_IP")" \ - --set master="$SRV_IP" --set xcatport="$XCATPORT" --set destiny="$DESTINY" \ - conf/prov/tftp-petitboot.conf || rc=1 - else - stage_skip "no petitboot config was generated for $PTB_NODE on this management node" - fi + # A mixed cluster boots ppc64 nodes from an x86 management node. + prov_run \ + "${COMMON[@]}" \ + --set node="$PTB_NODE" --set hexip="$(hex_ip "$PTB_IP")" \ + --set master="$SRV_IP" --set xcatport="$XCATPORT" --set destiny="$DESTINY" \ + conf/prov/tftp-petitboot.conf || rc=1 return $rc } @@ -991,17 +984,11 @@ do_run_genesis() { have_genesis || { stage_skip "genesis has not been built here ($(genesis_kernel) is missing); run mknb $NODE_ARCH"; return 0; } tftp=$(tftpdir); hexnet=$(hex_net "$NET" "$PREFIX") - # One scenario per loader family, and only the families this architecture is - # given: selecting by name means a family that is not written is absent from - # the report rather than passing vacuously. - [ -e "$tftp/pxelinux.cfg/$hexnet" ] && - select="$select -s pxelinux-discovery-config -s genesis-images-pxelinux" - [ -e "$tftp/xcat/xnba/nets/$NET_FILE" ] && - select="$select -s xnba-discovery-config" + # mknb for x86 writes the pxelinux and xnba files. It writes the grub2 file + # only for riscv64 (mknb.pm), so only grub2 is optional here. + select="-s pxelinux-discovery-config -s genesis-images-pxelinux -s xnba-discovery-config" [ -e "$tftp/boot/grub2/grub.cfg-$hexnet" ] && select="$select -s grub2-discovery-config -s genesis-images-grub2" - - [ -n "$select" ] || { stage_skip "mknb has written no configuration for $NET/$PREFIX, so there is nothing a machine without a definition would fetch"; return 0; } assert_serving udp 69 "the TFTP server" # Unquoted on purpose: the selection is a list this script built itself. @@ -1311,7 +1298,8 @@ dispatch() { dispatch "$@" rc=$? -# Said once at the end, so a reader of a passing run sees that something was -# left out without having to notice a line in the middle of the output. -[ "$SKIPPED" != 0 ] && echo "provfixture SKIPPED-TOTAL: $STAGE: $SKIPPED" +if [ "$SKIPPED" != 0 ]; then + echo "provfixture SKIPPED-TOTAL: $STAGE: $SKIPPED" >&2 + rc=1 +fi exit $rc