From f66237fbdfd5aff6004740ec2d6dc31723f4dcae Mon Sep 17 00:00:00 2001 From: Danila Katalshov <56929384+ADanMan@users.noreply.github.com> Date: Tue, 20 Oct 2026 07:47:43 +0300 Subject: [PATCH] fix(scripts): preserve check failures in exit cleanup (#9032) --- .github/ci-paths.json | 1 + .github/workflows/ci.yml | 3 + scripts/check.sh | 5 ++ scripts/check.test.sh | 113 ++++++++++++++++++++++++++++++++++++++ scripts/ci-scope.test.mjs | 1 + 5 files changed, 123 insertions(+) create mode 100644 scripts/check.test.sh diff --git a/.github/ci-paths.json b/.github/ci-paths.json index 4e7a6abda..fc33ec11c 100644 --- a/.github/ci-paths.json +++ b/.github/ci-paths.json @@ -87,6 +87,7 @@ "scripts/install.sh", "scripts/install.ps1", "scripts/check.sh", + "scripts/check.test.sh", "scripts/dev.sh", "scripts/local-env.sh", "scripts/init-worktree-env.sh", diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 360669941..dde8402f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -889,6 +889,9 @@ jobs: - name: Test self-host env derivation run: bash scripts/selfhost-config.test.sh + - name: Test check script + run: bash scripts/check.test.sh + - name: Test worktree database cleanup run: bash scripts/worktree-db.test.sh diff --git a/scripts/check.sh b/scripts/check.sh index d7b9170b6..176e68bef 100755 --- a/scripts/check.sh +++ b/scripts/check.sh @@ -31,6 +31,11 @@ EXIT_CODE=0 # Cleanup: kill only services this script started # -------------------------------------------------------------------------- cleanup() { + local status=$? + # Preserve failures from unguarded commands that trigger errexit. + if [ "$status" -ne 0 ]; then + EXIT_CODE=$status + fi echo "" if [ "$STARTED_BACKEND" = true ] && [ -n "$BACKEND_PID" ]; then kill "$BACKEND_PID" 2>/dev/null && wait "$BACKEND_PID" 2>/dev/null || true diff --git a/scripts/check.test.sh b/scripts/check.test.sh new file mode 100644 index 000000000..70f123129 --- /dev/null +++ b/scripts/check.test.sh @@ -0,0 +1,113 @@ +#!/usr/bin/env bash +set -euo pipefail + +SCRIPT_DIR=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd) +TEST_DIR=$(mktemp -d "${TMPDIR:-/tmp}/multica-check.XXXXXX") +trap 'rm -rf "$TEST_DIR"' EXIT +mkdir -p "$TEST_DIR/scripts" "$TEST_DIR/server" "$TEST_DIR/bin" +# Keep the production script's fixed service logs inside the fixture too. +sed "s|/tmp/multica-check-|$TEST_DIR/multica-check-|g" \ + "$SCRIPT_DIR/check.sh" >"$TEST_DIR/scripts/check.sh" +export CHECK_TEST_DIR="$TEST_DIR" + +cat >"$TEST_DIR/.env" <<'ENV' +PORT=18080 +FRONTEND_PORT=13000 +ENV + +# Override process operations as well as external tools: no real service is +# started, contacted, or killed, even in the ownership/cleanup cases. +cat >"$TEST_DIR/scripts/local-env.sh" <<'FAKE' +kill() { + if [ "$1" = "$BACKEND_PID" ]; then + echo backend >>"$CHECK_TEST_DIR/killed" + elif [ "$1" = "$FRONTEND_PID" ]; then + echo frontend >>"$CHECK_TEST_DIR/killed" + else + echo "unexpected PID: $1" >>"$CHECK_TEST_DIR/killed" + fi + return "${CLEANUP_STATUS:-0}" +} +wait() { return 0; } +FAKE +cat >"$TEST_DIR/scripts/ensure-postgres.sh" <<'FAKE' +exit "${PREREQUISITE_STATUS:-0}" +FAKE +printf 'exit 0\n' >"$TEST_DIR/scripts/test-go.test.sh" +printf 'exit 0\n' >"$TEST_DIR/scripts/test-go.sh" +cat >"$TEST_DIR/bin/pnpm" <<'FAKE' +#!/usr/bin/env bash +printf '%s\n' "$*" >>"$CHECK_TEST_DIR/pnpm-calls" +if [ "$*" = "${FAIL_PHASE:-}" ]; then + exit 23 +fi +FAKE +cat >"$TEST_DIR/bin/go" <<'FAKE' +#!/usr/bin/env bash +exit 0 +FAKE +cat >"$TEST_DIR/bin/curl" <<'FAKE' +#!/usr/bin/env bash +case "$*" in + *18080*) service=backend ;; + *13000*) service=frontend ;; + *) exit 2 ;; +esac +# First probe sees the configured existing service; subsequent probes see the +# newly started stub as ready, so tests never sleep or access the network. +if [ ! -f "$CHECK_TEST_DIR/$service-probed" ]; then + touch "$CHECK_TEST_DIR/$service-probed" + case ":${START_SERVICES:-}:" in + *":$service:"*) exit 1 ;; + esac +fi +FAKE +chmod 755 "$TEST_DIR/bin/"* + +fail() { + echo "$label: $*" >&2 + cat "$TEST_DIR/output" >&2 + exit 1 +} + +# $1: label; $2: status; $3: services expected to be stopped; rest: env overrides. +run_case() { + label=$1 expected_status=$2 expected_killed=$3 + shift 3 + rm -f "$TEST_DIR/"*-probed + : >"$TEST_DIR/killed" + : >"$TEST_DIR/pnpm-calls" + set +e + ( + cd "$TEST_DIR" + env -u BASH_ENV ENV_FILE=.env PATH="$TEST_DIR/bin:$PATH" \ + PREREQUISITE_STATUS=0 FAIL_PHASE= START_SERVICES= CLEANUP_STATUS=0 \ + "$@" bash scripts/check.sh + ) >"$TEST_DIR/output" 2>&1 + status=$? + set -e + [ "$status" -eq "$expected_status" ] || fail "status $status, want $expected_status" + if [ "$expected_status" -eq 0 ]; then + grep -q '^✓ All checks passed\.$' "$TEST_DIR/output" || fail "missing success footer" + ! grep -q 'Checks FAILED' "$TEST_DIR/output" || fail "unexpected failure footer" + else + grep -q '^✗ Checks FAILED\.$' "$TEST_DIR/output" || fail "missing failure footer" + ! grep -q 'All checks passed' "$TEST_DIR/output" || fail "misleading success footer" + fi + [ "$(cat "$TEST_DIR/killed")" = "$expected_killed" ] || fail "wrong cleanup ownership" + echo "$label: PASS" +} + +run_case 'prerequisite failure' 37 '' PREREQUISITE_STATUS=37 +[ ! -s "$TEST_DIR/pnpm-calls" ] || fail 'continued after prerequisite failure' +run_case 'guarded typecheck failure' 1 '' FAIL_PHASE=typecheck +run_case 'success with existing services' 0 '' +grep -q '^exec playwright test$' "$TEST_DIR/pnpm-calls" || fail 'did not reach E2E' +run_case 'success with owned services' 0 'backend +frontend' START_SERVICES=backend:frontend +run_case 'E2E failure with owned backend' 1 backend START_SERVICES=backend FAIL_PHASE='exec playwright test' +run_case 'E2E failure with owned frontend' 1 frontend START_SERVICES=frontend FAIL_PHASE='exec playwright test' +run_case 'cleanup failure preserves failed checks' 1 'backend +frontend' START_SERVICES=backend:frontend FAIL_PHASE='exec playwright test' CLEANUP_STATUS=1 + +echo 'check.test.sh: PASS' diff --git a/scripts/ci-scope.test.mjs b/scripts/ci-scope.test.mjs index 047e87e04..90963d0ea 100644 --- a/scripts/ci-scope.test.mjs +++ b/scripts/ci-scope.test.mjs @@ -32,6 +32,7 @@ for (const [name, files, selected] of [ ["shell installer", ["scripts/install.sh"], ["scripts", "installer"]], ["PowerShell installer", ["scripts/install.ps1.test.ps1"], ["installer"]], ["cleanup script", ["scripts/drop-database.sh"], ["scripts"]], + ["check script regression tests", ["scripts/check.test.sh"], ["scripts"]], ["performance harness", ["scripts/perf-compare.test.sh"], ["scripts"]], ["reserved slug source", ["server/internal/handler/reserved_slugs.json"], ["backend", "runtime", "scripts"]], ["reserved slug output", ["packages/core/paths/reserved-slugs.ts"], ["frontend", "quality", "scripts"]],