From 786ea8a7d8faaba82f9284d15f785bac4c6d768b Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Sat, 11 Apr 2026 14:36:58 +0300 Subject: [PATCH] test(hack): assert sudo prefix in single-service HINTs, explicit exit code, and glob regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address three review findings on the bats suite: - The 'standalone containerd service active' and 'standalone docker service active' tests previously asserted 'systemctl disable --now ' but did not require the 'sudo ' prefix, so the two- service test was the only guard against a silent drop of the prefix. Extend both single-service tests to require the prefix exactly as the 'both services active' test does. - The 'both services active' test now captures the exit code explicitly with 'bash ... || status=$?' and asserts '$status -eq 0'. The script contract ('always exits 0') was previously enforced implicitly via 'set -e', which produces a generic test failure on a regression instead of a contract-specific error. The explicit check locks in the contract and makes regressions readable. - New test: 'docker socket paths with glob chars do not expand' — sets COZYSTACK_DOCKER_SOCKET_PATHS to a literal glob against real directories and asserts no docker warning fires. Locks in the array-based parsing of the path list. - Extend the file header with a 'title syntax constraints' section documenting how cozytest.sh's awk parser treats double quotes and non-alphanumeric characters in @test titles, so future contributors do not stumble on the parser's quirks. Assisted-By: Claude Signed-off-by: Aleksei Sviridkin --- hack/check-host-runtime.bats | 64 +++++++++++++++++++++++++++++++++--- 1 file changed, 59 insertions(+), 5 deletions(-) diff --git a/hack/check-host-runtime.bats b/hack/check-host-runtime.bats index 7506bbab..b121b31b 100644 --- a/hack/check-host-runtime.bats +++ b/hack/check-host-runtime.bats @@ -21,6 +21,14 @@ # because cozytest.sh's awk parser only recognizes @test blocks and treats a # bare `}` on its own line as the end of a test function. # +# Title syntax constraints (inherited from cozytest.sh's awk parser): +# - Titles must be delimited by ASCII double quotes; embedded literal +# double quotes are NOT escaped and will silently truncate the title. +# - Only alphanumeric characters from the title survive into the shell +# function name (everything else becomes '_'), so titles that differ +# only in punctuation collapse to the same function name. Keep titles +# distinctive in their alphanumeric run. +# # Run with: hack/cozytest.sh hack/check-host-runtime.bats # (or `bats hack/check-host-runtime.bats` if the bats binary is # installed; cozytest.sh is the CI path.) @@ -88,8 +96,10 @@ STUBEOF exit 1 fi # HINT line must name only the detected service, not advise disabling - # docker.service when only containerd.service is running. - grep -q 'systemctl disable --now containerd.service' "$STDERR_FILE" + # docker.service when only containerd.service is running. The sudo + # prefix is also required — without it the command silently no-ops + # for a non-root operator, so the prefix is part of the contract. + grep -q 'sudo systemctl disable --now containerd.service' "$STDERR_FILE" if grep -q 'systemctl disable --now.*docker' "$STDERR_FILE"; then echo "HINT unexpectedly mentions docker:" >&2 cat "$STDERR_FILE" >&2 @@ -132,8 +142,9 @@ STUBEOF cat "$STDERR_FILE" >&2 exit 1 fi - # HINT line must name only the detected service. - grep -q 'systemctl disable --now docker.service' "$STDERR_FILE" + # HINT line must name only the detected service. As in the + # containerd test, the sudo prefix is part of the contract. + grep -q 'sudo systemctl disable --now docker.service' "$STDERR_FILE" if grep -q 'systemctl disable --now.*containerd' "$STDERR_FILE"; then echo "HINT unexpectedly mentions containerd:" >&2 cat "$STDERR_FILE" >&2 @@ -163,13 +174,21 @@ STUBEOF mkdir -p "$STUB_DIR/var-lib-containerd" "$STUB_DIR/var-lib-docker" STDERR_FILE="$STUB_DIR/stderr" + # Capture exit code explicitly: the script contract says exit 0 + # unconditionally (warning, not blocker). `set -e` in the test + # function body would already fail on a nonzero exit, but an + # explicit status check locks in the contract and makes a + # regression show up as "expected 0, got N" rather than as a + # generic test failure. + status=0 COZYSTACK_CONTAINERD_SOCKET="$STUB_DIR/missing-containerd.sock" \ COZYSTACK_DOCKER_SOCKET_PATHS="$STUB_DIR/missing-docker.sock" \ COZYSTACK_CONTAINERD_DIR="$STUB_DIR/var-lib-containerd" \ COZYSTACK_DOCKER_DIR="$STUB_DIR/var-lib-docker" \ PATH="$STUB_DIR:$PATH" \ - bash hack/check-host-runtime.sh 2>"$STDERR_FILE" + bash hack/check-host-runtime.sh 2>"$STDERR_FILE" || status=$? + [ "$status" -eq 0 ] grep -q 'standalone containerd.service' "$STDERR_FILE" grep -q 'standalone docker.service' "$STDERR_FILE" # HINT block must fire whenever warnings exist; otherwise a future silent @@ -317,6 +336,41 @@ STUBEOF fi } +@test "docker socket paths with glob chars do not expand" { + STUB_DIR=$(mktemp -d) + trap 'rm -rf "$STUB_DIR"' EXIT + + # Create two directories that a naive `for sock in $PATHS` loop + # would glob-expand and treat as existing "sockets". With the + # array-based parsing the literal path "$STUB_DIR/var-lib-*" does + # not exist and no warning must fire. + mkdir -p "$STUB_DIR/var-lib-docker" "$STUB_DIR/var-lib-containerd" + + cat >"$STUB_DIR/systemctl" <<'STUBEOF' +#!/bin/sh +if [ "$1" = "--version" ]; then + echo "systemd stub" + exit 0 +fi +exit 1 +STUBEOF + chmod +x "$STUB_DIR/systemctl" + + STDERR_FILE="$STUB_DIR/stderr" + COZYSTACK_CONTAINERD_SOCKET="$STUB_DIR/missing-containerd.sock" \ + COZYSTACK_DOCKER_SOCKET_PATHS="$STUB_DIR/var-lib-*" \ + COZYSTACK_CONTAINERD_DIR="$STUB_DIR/missing-containerd-dir" \ + COZYSTACK_DOCKER_DIR="$STUB_DIR/missing-docker-dir" \ + PATH="$STUB_DIR:$PATH" \ + bash hack/check-host-runtime.sh 2>"$STDERR_FILE" + + if grep -q 'standalone docker.service' "$STDERR_FILE"; then + echo "glob pattern expanded — docker warning should not fire:" >&2 + cat "$STDERR_FILE" >&2 + exit 1 + fi +} + @test "containerd service plus socket still emits exactly one warning" { STUB_DIR=$(mktemp -d) trap 'rm -rf "$STUB_DIR"' EXIT