From a8e770c42ebd340cf5f35e7ee2497720f45eab38 Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Tue, 8 Sep 2026 15:28:04 -0300 Subject: [PATCH] fix(xcat-core): Prevent BATS checks from missing failures Negative checks could pass when a later command succeeded. The diskless test also read host state and wrote to the host wget log. The SSH fallback test did not prove that the restart waited for the killed process. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> --- xCAT-test/bats/post_xcat_download_policy.bats | 26 +++++++--- xCAT-test/bats/remoteshell_restart.bats | 33 ++++++++---- xCAT-test/bats/sles_pre_script.bats | 8 +-- xCAT/postscripts/xcatdsklspost | 51 ++++++++++--------- 4 files changed, 73 insertions(+), 45 deletions(-) diff --git a/xCAT-test/bats/post_xcat_download_policy.bats b/xCAT-test/bats/post_xcat_download_policy.bats index ec643410c..41810bb97 100644 --- a/xCAT-test/bats/post_xcat_download_policy.bats +++ b/xCAT-test/bats/post_xcat_download_policy.bats @@ -28,25 +28,37 @@ capture_install_scriptlib_wget() capture_xcatdsklspost_wget() { local wget_log="$1" + local host_init_log="${BATS_TEST_TMPDIR}/host-init.log" + local download_log="${BATS_TEST_TMPDIR}/xcatdsklspost-wget-errors.log" + + cat() { printf 'cat %s\n' "$*" >>"$host_init_log"; return 1; } + grep() + { + printf 'grep %s\n' "$*" >>"$host_init_log" + command grep "$@" + } + dirname() { printf 'dirname %s\n' "$*" >>"$host_init_log"; return 1; } + + XCATDSKLSPOST_SOURCE_ONLY=1 + XCAT_WGET_LOG="$download_log" + source "$XCATDSKLSPOST" + [ ! -e "$host_init_log" ] || return 1 + [ "$XCAT_WGET_LOG" = "$download_log" ] || return 1 + unset -f cat grep dirname xcatpost="${BATS_TEST_TMPDIR}/xcatpost" INSTALLDIR=/install - echolog() { :; } sleep() { :; } - grep() - { - [ "${*: -1}" = "/tmp/wget.log" ] && return 1 - command grep "$@" - } wget() { printf '%s\n' "$*" >"$wget_log" + printf '%s\n' 'mock wget stderr' >&2 return 0 } - XCATDSKLSPOST_SOURCE_ONLY=1 source "$XCATDSKLSPOST" download_postscripts 192.0.2.10:80 + [ "$(read_file_or_empty "$download_log")" = "mock wget stderr" ] } assert_download_policy() diff --git a/xCAT-test/bats/remoteshell_restart.bats b/xCAT-test/bats/remoteshell_restart.bats index 1ddce6cfc..b4727d7fc 100644 --- a/xCAT-test/bats/remoteshell_restart.bats +++ b/xCAT-test/bats/remoteshell_restart.bats @@ -1,5 +1,7 @@ #!/usr/bin/env bats +bats_require_minimum_version 1.5.0 + load 'helpers/shell_source' setup() @@ -11,6 +13,8 @@ setup() run_restart_fallback() { + local poll_count=0 + ps() { cat <<'EOF' @@ -19,14 +23,23 @@ EOF } kill() { - printf '%s\n' "$*" >>"$KILL_LOG" - [ "$1" = "-0" ] && return 1 - return 0 + if [ "$1" = "-9" ]; then + printf 'kill %s\n' "$*" >>"$EVENT_LOG" + return 0 + fi + + poll_count=$((poll_count + 1)) + if [ "$poll_count" -eq 1 ]; then + printf 'poll %s alive\n' "$2" >>"$EVENT_LOG" + return 0 + fi + printf 'poll %s gone\n' "$2" >>"$EVENT_LOG" + return 1 } sleep() { :; } sshd() { - printf '%s\n' start >>"$SSHD_LOG" + printf '%s\n' start >>"$EVENT_LOG" } source "$XCATLIB" @@ -44,16 +57,14 @@ run_wait_for_processes() return 0 } -@test "remoteshell restart fallback sends an uncatchable signal before starting sshd" { - KILL_LOG="${BATS_TEST_TMPDIR}/kill.log" - SSHD_LOG="${BATS_TEST_TMPDIR}/sshd.log" - export KILL_LOG SSHD_LOG +@test "remoteshell restart fallback kills, waits, then starts sshd" { + EVENT_LOG="${BATS_TEST_TMPDIR}/events.log" + export EVENT_LOG run run_restart_fallback [ "$status" -eq 0 ] - grep -Fxq -- '-9 4321' "$KILL_LOG" - ! grep -Eq '^9( |$)' "$KILL_LOG" - [ "$(read_file_or_empty "$SSHD_LOG")" = "start" ] + [ "$(read_file_or_empty "$EVENT_LOG")" = $'kill -9 4321\npoll 4321 alive\npoll 4321 gone\nstart' ] + run -1 grep -Eq '^kill 9( |$)' "$EVENT_LOG" } @test "remoteshell wait loop reports a still-running process and gives up" { diff --git a/xCAT-test/bats/sles_pre_script.bats b/xCAT-test/bats/sles_pre_script.bats index f35153200..bd8387ab0 100644 --- a/xCAT-test/bats/sles_pre_script.bats +++ b/xCAT-test/bats/sles_pre_script.bats @@ -1,5 +1,7 @@ #!/usr/bin/env bats +bats_require_minimum_version 1.5.0 + load 'helpers/shell_source' setup() @@ -26,9 +28,9 @@ EOF run set_sles11_uefi_bootloader "$cmdline" "$profile" [ "$status" -eq 0 ] grep -Fxq 'elilo' "$profile" - ! grep -q 'mbr' "$profile" - ! grep -q 'mbr' "$profile" + run -1 grep -q '> /tmp/wget.log + wget -N --waitretry=10 --random-wait -T 60 http://$server$TFTPDIR/mypostscripts/mypostscript.$node -P /$xcatpost 2>> "$XCAT_WGET_LOG" rc=$? # if no error and the file was downloaded if [ $rc -eq 0 ] && [ -f /$xcatpost/mypostscript.$node ]; then @@ -155,16 +158,16 @@ download_postscripts() # These dispatcher scripts are not needed by the legacy netboot post # path. Newer wget parses HTML-looking regex strings inside downloaded # scripts and fails the whole recursive download on bogus URLs. - export LANG=C; wget -l inf -nH -N -r --waitretry=10 --random-wait -e robots=off -T 60 -nH --cut-dirs=2 --reject "index.html*,post.xcat.ng,post.xcat.rhels10" --no-parent http://$server$INSTALLDIR/postscripts/ -P /$xcatpost 2> /tmp/wget.log + export LANG=C; wget -l inf -nH -N -r --waitretry=10 --random-wait -e robots=off -T 60 -nH --cut-dirs=2 --reject "index.html*,post.xcat.ng,post.xcat.rhels10" --no-parent http://$server$INSTALLDIR/postscripts/ -P /$xcatpost 2> "$XCAT_WGET_LOG" rc=$? if [ $rc -eq 0 ]; then # return from wget was 0 but some OS do not return errors, so we # have additional checks for # failed: Connection httpd not running # 404: Not Found - if directory does not exist - grep -i -E "... failed: Connection refused.$" /tmp/wget.log + grep -i -E "... failed: Connection refused.$" "$XCAT_WGET_LOG" rc1=$? - grep -i -E "ERROR 404: Not Found.$" /tmp/wget.log + grep -i -E "ERROR 404: Not Found.$" "$XCAT_WGET_LOG" rc2=$? # check to see no errors at all, grep returns 1 if [ $rc1 -eq 1 ] && [ $rc2 -eq 1 ]; then