From 80ec0361302118d8198a2f82fbae272fde357240 Mon Sep 17 00:00:00 2001 From: Ramil Valitov Date: Thu, 17 Sep 2026 15:33:23 +0300 Subject: [PATCH 1/3] ci: add PR test pipeline across distros and init systems Adds the project's first automated tests. Until now the 13 scripts in tests/ were wired into no runner and no workflow triggered on pull requests. - lint: bash -n plus shellcheck at error severity - unit-tests: the existing suite on Debian 12, Ubuntu 22.04/24.04, Alpine 3.20 and Fedora 41 - systemd-integration / openrc-integration: exercise setup_autostart and main_service_remove against real init systems Alpine installs only bash; adding coreutils would shadow busybox and hide the differences that row exists to catch. --- .github/workflows/ci.yml | 161 +++++++++++++++++++ tests/integration/autostart_openrc.sh | 206 +++++++++++++++++++++++++ tests/integration/autostart_systemd.sh | 193 +++++++++++++++++++++++ tests/run-all.sh | 159 +++++++++++++++++++ 4 files changed, 719 insertions(+) create mode 100644 .github/workflows/ci.yml create mode 100644 tests/integration/autostart_openrc.sh create mode 100644 tests/integration/autostart_systemd.sh create mode 100644 tests/run-all.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..9871106 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,161 @@ +name: CI + +# Runs on every pull request that touches shell code. The workflow deliberately requests +# no secrets and a read-only token: it executes code from pull requests, including forks, +# so it uses `pull_request` (never `pull_request_target`) and grants nothing beyond +# reading the repository. +on: + push: + branches: [main] + paths: + - '**.sh' + - 'tests/**' + - '.github/workflows/ci.yml' + pull_request: + paths: + - '**.sh' + - 'tests/**' + - '.github/workflows/ci.yml' + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: ci-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + lint: + name: Syntax and lint + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + + # Pure parser check — reports the file and line of any syntax error without + # executing anything. This cannot false-positive, so it is a hard gate. + - name: Parse every shell script + run: | + set -euo pipefail + for f in mtproxymax.sh install.sh tests/*.sh tests/integration/*.sh; do + bash -n "$f" + echo "ok $f" + done + + # Errors only. A 19k-line script carries a large backlog of style warnings, and + # gating on those on day one would make this job permanently red and therefore + # ignored. Warnings are still worth having locally: run `shellcheck mtproxymax.sh` + # without -S to see them. Tighten this to `warning` once the backlog is worked down. + - name: shellcheck (errors only) + run: | + shellcheck -S error mtproxymax.sh install.sh tests/*.sh tests/integration/*.sh + + unit-tests: + name: Unit tests (${{ matrix.image }}) + runs-on: ubuntu-latest + timeout-minutes: 15 + strategy: + # Report every distro rather than stopping at the first one that fails — the + # interesting signal is which platforms differ. + fail-fast: false + matrix: + # The install command is per-image and explicit rather than auto-detected: some + # images ship bash but still lack tools the suite needs (fedora:41 has bash but no + # `diff`, which silently failed one assertion until it was added here). + # + # Alpine deliberately installs ONLY bash. Adding coreutils/diffutils would shadow + # busybox and hide exactly the differences this row exists to catch. + # + # `quarantine` lists tests already known to fail on that image. They still run and + # their failures are printed, but they do not fail the build. Every entry here is a + # real, unfixed defect — delete the entry when the underlying bug is fixed. + include: + - image: debian:12 + install: 'apt-get update -qq && apt-get install -y -qq bash diffutils' + quarantine: 'test_client_mss.sh' + - image: ubuntu:22.04 + install: 'apt-get update -qq && apt-get install -y -qq bash diffutils' + quarantine: 'test_client_mss.sh' + - image: ubuntu:24.04 + install: 'apt-get update -qq && apt-get install -y -qq bash diffutils' + quarantine: 'test_client_mss.sh' + # Alpine carries three known failures, all busybox divergences on a platform the + # README lists as supported: + # test_client_mss.sh — broken on every distro (asserts on the stdout of a + # function that writes to a file); see below. + # test_traffic_reset.sh— busybox `flock` has no -w, so `flock -w 5 9` fails; + # `command -v flock` succeeds, so the guard never fires. + # Two call sites fail OPEN and silently write nothing. + # test_guest.sh — `date -d "+24 hours"` is invalid on busybox and the + # `date -r ` fallback also fails (busybox -r means + # reference file), so expiring guest links get no expiry. + # test_client_mss.sh is quarantined everywhere because it is a broken test, not a + # platform difference — it asserts on the stdout of generate_telemt_config, which + # takes a destination path and writes ${CONFIG_DIR}/config.toml instead. + - image: alpine:3.20 + install: 'apk add --no-cache bash' + quarantine: 'test_client_mss.sh,test_traffic_reset.sh,test_guest.sh' + - image: fedora:41 + install: 'dnf install -y -q bash diffutils' + quarantine: 'test_client_mss.sh' + steps: + - uses: actions/checkout@v4 + + # Each distro runs the same suite in a container. Alpine is the row that earns its + # keep: its busybox userland (sed, grep, mktemp, date) differs from GNU, and Alpine + # is a documented supported platform that has never been exercised by CI. + # + # Note this uses `docker run` rather than the job-level `container:` key: JavaScript + # actions such as actions/checkout run with a `node` binary *inside* the job + # container, and the runner does not inject one, so a `container:` job on these + # images fails before it can install anything. + - name: Run suite in ${{ matrix.image }} + run: | + docker run --rm -v "$PWD:/src" -w /src \ + -e MTPROXYMAX_QUARANTINE="${{ matrix.quarantine }}" \ + "${{ matrix.image }}" sh -c ' + set -e + ${{ matrix.install }} + bash tests/run-all.sh + ' + + systemd-integration: + name: Init integration (systemd) + # ubuntu-24.04 is a full VM with systemd as PID 1 and Docker already installed, so the + # generated unit's `Requires=docker.service` resolves against the real unit. Running + # this in a container instead would need --privileged --cgroupns=host, and a systemd + # container is not a faithful enough substitute for the real thing. + runs-on: ubuntu-24.04 + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + + - name: Confirm the runner really is systemd + # If this ever prints something other than "systemd", the job below would be + # testing nothing, so make that visible up front. + run: | + ps -p 1 -o comm= + systemctl is-system-running || true + sudo systemctl start docker || true + + - name: Run systemd integration test + run: sudo bash tests/integration/autostart_systemd.sh + + openrc-integration: + name: Init integration (OpenRC) + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + + # OpenRC in a container needs the softlevel marker (its verify_boot() otherwise + # refuses to run any service) and a bash since the image ships only busybox ash; + # the test script sets both up itself. + - name: Run OpenRC integration test (Alpine) + run: | + docker run --rm -v "$PWD:/src" -w /src alpine:3.20 sh -c ' + set -e + apk add --no-cache bash openrc + bash tests/integration/autostart_openrc.sh + ' diff --git a/tests/integration/autostart_openrc.sh b/tests/integration/autostart_openrc.sh new file mode 100644 index 0000000..e95bdbf --- /dev/null +++ b/tests/integration/autostart_openrc.sh @@ -0,0 +1,206 @@ +#!/bin/bash +# Integration test: setup_autostart() / main_service_remove() against a REAL OpenRC. +# +# Covers the non-systemd path added for Alpine (#130). tests/test_telegram_service_openrc.sh +# verifies that the right rc-update/rc-service commands *would* be invoked, using stubs. +# This runs the real thing: the init script is written to the real /etc/init.d, rc-update +# really registers it in a runlevel, and rc-service really executes it. +# +# Intended to run in a throwaway Alpine container: +# docker run --rm -v "$PWD:/src" -w /src alpine:3.20 sh -c ' +# apk add --no-cache bash openrc && bash tests/integration/autostart_openrc.sh' +# +# Prints SKIP and exits 0 when OpenRC is not present, so it is harmless to run anywhere. + +set -o pipefail + +if [ "${BASH_VERSINFO[0]:-0}" -lt 4 ]; then + echo "SKIP: bash 4+ required (got ${BASH_VERSION:-unknown})" >&2 + exit 0 +fi + +if [ ! -x /sbin/openrc-run ] || ! command -v rc-service >/dev/null 2>&1; then + echo "SKIP: OpenRC not present on this host" >&2 + exit 0 +fi + +if [ "$(id -u)" -ne 0 ]; then + echo "ERROR: must run as root — this writes to /etc/init.d." >&2 + echo " sudo bash $0" >&2 + exit 1 +fi + +# OpenRC's verify_boot() refuses to run any service unless it believes the system was +# booted by OpenRC. In a container nothing booted it, so the marker has to be created. +SOFTLEVEL_CREATED=0 +if [ ! -e /run/openrc/softlevel ]; then + mkdir -p /run/openrc + : >/run/openrc/softlevel + SOFTLEVEL_CREATED=1 +fi + +REPO_ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd) +INITD="/etc/init.d/mtproxymax" +RUNLEVEL_LINK="/etc/runlevels/default/mtproxymax" +DOCKER_STUB="/etc/init.d/docker" +DOCKER_STUB_CREATED=0 +FAKE="/usr/local/bin/mtproxymax" +FAKE_LOG="/tmp/mtproxymax-fake.log" +FAKE_BACKUP="/tmp/mtproxymax-fake.backup" + +FAKE_WAS_PRESENT=0 +if [ -e "$FAKE" ]; then + cp -a "$FAKE" "$FAKE_BACKUP" 2>/dev/null && FAKE_WAS_PRESENT=1 +fi + +# The generated init script declares `depend() { need docker; }`. OpenRC refuses to start +# a service whose hard dependencies cannot be resolved (`ERROR: mtproxymax needs service(s) +# docker`), so a stub is required for the start assertion below to exercise OUR script +# rather than failing on the host's dependency graph. +if [ ! -e "$DOCKER_STUB" ]; then + cat >"$DOCKER_STUB" <<'DOCKER_EOF' +#!/sbin/openrc-run +description="Docker stub (MTProxyMax integration test)" + +start() { + ebegin "Starting docker (stub)" + eend 0 +} + +stop() { + ebegin "Stopping docker (stub)" + eend 0 +} +DOCKER_EOF + chmod +x "$DOCKER_STUB" + DOCKER_STUB_CREATED=1 +fi + +cleanup() { + rc-service mtproxymax stop >/dev/null 2>&1 + rc-update del mtproxymax default >/dev/null 2>&1 + rm -f "$INITD" "$RUNLEVEL_LINK" + [ "$DOCKER_STUB_CREATED" -eq 1 ] && rm -f "$DOCKER_STUB" + [ "$SOFTLEVEL_CREATED" -eq 1 ] && rm -f /run/openrc/softlevel + if [ "$FAKE_WAS_PRESENT" -eq 1 ]; then + cp -a "$FAKE_BACKUP" "$FAKE" 2>/dev/null + else + rm -f "$FAKE" + fi + rm -f "$FAKE_BACKUP" +} +trap cleanup EXIT + +cat >"$FAKE" <<'FAKE_EOF' +#!/bin/bash +# Test double installed by tests/integration/autostart_openrc.sh, so that the generated +# init script's start()/stop() have something real to invoke. +printf '%s\n' "$*" >> /tmp/mtproxymax-fake.log +exit 0 +FAKE_EOF +chmod +x "$FAKE" +: >"$FAKE_LOG" + +MTPROXYMAX_SOURCE_ONLY=true source "$REPO_ROOT/mtproxymax.sh" +set +e + +TESTS_RUN=0 +TESTS_FAILED=0 + +assert_eq() { + local name="$1" want="$2" got="$3" + TESTS_RUN=$((TESTS_RUN + 1)) + if [ "$got" = "$want" ]; then + printf ' PASS %s\n' "$name" + else + printf ' FAIL %s (got=%q want=%q)\n' "$name" "$got" "$want" + TESTS_FAILED=$((TESTS_FAILED + 1)) + fi +} + +assert_ne() { + local name="$1" unwanted="$2" got="$3" + TESTS_RUN=$((TESTS_RUN + 1)) + if [ "$got" != "$unwanted" ]; then + printf ' PASS %s\n' "$name" + else + printf ' FAIL %s (unexpectedly %q)\n' "$name" "$got" + TESTS_FAILED=$((TESTS_FAILED + 1)) + fi +} + +assert_cmd_ok() { + local name="$1" + shift + local out rc + out=$("$@" 2>&1) + rc=$? + # OpenRC tries to enrol each service in cgroup v1 hierarchies that are read-only in a + # container, emitting one "can't create /sys/fs/cgroup/.../tasks" line per controller. + # That is environmental noise rather than test signal, and it buries the real error. + out=$(printf '%s\n' "$out" | grep -v '/sys/fs/cgroup/.*/tasks') + TESTS_RUN=$((TESTS_RUN + 1)) + if [ "$rc" -eq 0 ]; then + printf ' PASS %s\n' "$name" + else + printf ' FAIL %s (exit %d)\n' "$name" "$rc" + printf '%s\n' "$out" | sed 's/^/ | /' + TESTS_FAILED=$((TESTS_FAILED + 1)) + fi +} + +assert_file_contains() { + local name="$1" needle="$2" file="$3" + TESTS_RUN=$((TESTS_RUN + 1)) + if grep -qF -- "$needle" "$file" 2>/dev/null; then + printf ' PASS %s\n' "$name" + else + printf ' FAIL %s (missing %q in %s)\n' "$name" "$needle" "$file" + TESTS_FAILED=$((TESTS_FAILED + 1)) + fi +} + +file_state() { [ -e "$1" ] && echo present || echo absent; } + +# --- the host must actually be what we think it is ---------------------------- +assert_eq "host detects openrc" "openrc" "$(detect_init_system)" + +# --- install ------------------------------------------------------------------ +setup_autostart >/dev/null 2>&1 +assert_eq "setup_autostart succeeds" "0" "$?" +assert_eq "init script written" "present" "$(file_state "$INITD")" +assert_eq "init script is executable" "yes" "$([ -x "$INITD" ] && echo yes || echo no)" +assert_eq "init script has openrc-run shebang" "#!/sbin/openrc-run" "$(head -n1 "$INITD" 2>/dev/null)" + +# The dependency declarations matter independently of whether they can be resolved here; +# rc-service refuses to start a service with unmet hard deps, so this is load-bearing. +assert_file_contains "declares need docker" "need docker" "$INITD" +assert_file_contains "declares need net" "need net" "$INITD" + +# The invariant openrc_enable_service() checks: rc-update's exit status is not trusted, +# the runlevel symlink actually landing is. Guards the #130 bug class. +assert_eq "runlevel symlink created" "present" "$(file_state "$RUNLEVEL_LINK")" + +# --- start / stop ------------------------------------------------------------- +# OpenRC keeps a dependency cache; without it a service's state is indeterminate and +# rc-service refuses every start with "already starting". On a real host the cache is +# built during boot; in a container nothing boots OpenRC, so it has to be built by hand. +# Note this must come *after* setup_autostart, so the new service is in the graph. +rc-update -u >/dev/null 2>&1 + +assert_cmd_ok "rc-service start succeeds" rc-service mtproxymax start +assert_eq "start() reached the mtproxymax binary" "start" "$(head -n1 "$FAKE_LOG" 2>/dev/null)" + +assert_cmd_ok "rc-service stop succeeds" rc-service mtproxymax stop +assert_eq "stop() reached the mtproxymax binary" "stop" "$(tail -n1 "$FAKE_LOG" 2>/dev/null)" + +# --- remove ------------------------------------------------------------------- +main_service_remove >/dev/null 2>&1 +assert_eq "main_service_remove succeeds" "0" "$?" +assert_eq "init script removed" "absent" "$(file_state "$INITD")" +assert_eq "runlevel symlink removed" "absent" "$(file_state "$RUNLEVEL_LINK")" +assert_ne "service no longer registered with rc-update" "mtproxymax" \ + "$(rc-update show default 2>/dev/null | awk '{print $1}' | grep -x mtproxymax)" + +printf '\n%d tests, %d failures\n' "$TESTS_RUN" "$TESTS_FAILED" +[ "$TESTS_FAILED" -eq 0 ] diff --git a/tests/integration/autostart_systemd.sh b/tests/integration/autostart_systemd.sh new file mode 100644 index 0000000..bafa3ef --- /dev/null +++ b/tests/integration/autostart_systemd.sh @@ -0,0 +1,193 @@ +#!/bin/bash +# Integration test: setup_autostart() / main_service_remove() against a REAL systemd. +# +# tests/test_telegram_service_openrc.sh stubs systemctl and asserts that the right +# command *would* be run. This does the real thing instead: the unit file is written to +# the real /etc/systemd/system, systemd is really asked to enable it, and `systemctl +# start` really has to reach the ExecStart binary. A stubbed test cannot catch a unit +# file that systemd refuses to parse, or an ExecStart path that does not exist — which +# is exactly the class of bug this covers. +# +# Requires root (writes /etc/systemd/system) on a host actually running systemd: +# sudo bash tests/integration/autostart_systemd.sh +# +# Prints SKIP and exits 0 when the host is not running systemd, so it is harmless to +# run anywhere. + +set -o pipefail + +if [ "${BASH_VERSINFO[0]:-0}" -lt 4 ]; then + echo "SKIP: bash 4+ required (got ${BASH_VERSION:-unknown})" >&2 + exit 0 +fi + +if [ "$(id -u)" -ne 0 ]; then + echo "ERROR: must run as root — this writes to /etc/systemd/system." >&2 + echo " sudo bash $0" >&2 + exit 1 +fi + +# `systemctl` merely existing is not proof that systemd is PID 1 — that false positive +# is precisely the bug this suite exists to surface, so gate on liveness, not presence. +SYSTEM_STATE=$(systemctl is-system-running 2>/dev/null || true) +case "$SYSTEM_STATE" in +"" | offline) + echo "SKIP: systemd is not running as PID 1 (is-system-running: ${SYSTEM_STATE:-no response})" >&2 + exit 0 + ;; +esac + +REPO_ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd) +UNIT="/etc/systemd/system/mtproxymax.service" +WANTS="/etc/systemd/system/multi-user.target.wants/mtproxymax.service" +FAKE="/usr/local/bin/mtproxymax" +FAKE_LOG="/tmp/mtproxymax-fake.log" +FAKE_BACKUP="/tmp/mtproxymax-fake.backup" + +# The unit hardcodes ExecStart=/usr/local/bin/mtproxymax. If something is really +# installed there, put it back afterwards rather than clobbering it. +FAKE_WAS_PRESENT=0 +if [ -e "$FAKE" ]; then + cp -a "$FAKE" "$FAKE_BACKUP" 2>/dev/null && FAKE_WAS_PRESENT=1 +fi + +# The generated unit declares `Requires=docker.service`, so `systemctl start` fails on the +# unmet dependency rather than on the unit under test when that service is absent. GitHub's +# ubuntu-24.04 runner has a real docker.service; a bare systemd container does not. Stub it +# only when genuinely missing, so a real Docker installation is never shadowed. +DOCKER_STUB="/etc/systemd/system/docker.service" +DOCKER_STUB_CREATED=0 +if ! systemctl cat docker.service >/dev/null 2>&1; then + cat >"$DOCKER_STUB" <<'DOCKER_EOF' +[Unit] +Description=Docker stub (MTProxyMax integration test) +After=network.target + +[Service] +Type=oneshot +RemainAfterExit=yes +ExecStart=/bin/true +ExecStop=/bin/true + +[Install] +WantedBy=multi-user.target +DOCKER_EOF + DOCKER_STUB_CREATED=1 + systemctl daemon-reload >/dev/null 2>&1 +fi + +cleanup() { + systemctl stop mtproxymax.service >/dev/null 2>&1 + systemctl disable mtproxymax.service >/dev/null 2>&1 + rm -f "$UNIT" "$WANTS" + [ "$DOCKER_STUB_CREATED" -eq 1 ] && rm -f "$DOCKER_STUB" + systemctl daemon-reload >/dev/null 2>&1 + if [ "$FAKE_WAS_PRESENT" -eq 1 ]; then + cp -a "$FAKE_BACKUP" "$FAKE" 2>/dev/null + else + rm -f "$FAKE" + fi + rm -f "$FAKE_BACKUP" +} +trap cleanup EXIT + +cat >"$FAKE" <<'FAKE_EOF' +#!/bin/bash +# Test double installed by tests/integration/autostart_systemd.sh, so that ExecStart and +# ExecStop have something real to invoke. Records the arguments it was called with. +printf '%s\n' "$*" >> /tmp/mtproxymax-fake.log +exit 0 +FAKE_EOF +chmod +x "$FAKE" +: >"$FAKE_LOG" + +MTPROXYMAX_SOURCE_ONLY=true source "$REPO_ROOT/mtproxymax.sh" +set +e + +TESTS_RUN=0 +TESTS_FAILED=0 + +assert_eq() { + local name="$1" want="$2" got="$3" + TESTS_RUN=$((TESTS_RUN + 1)) + if [ "$got" = "$want" ]; then + printf ' PASS %s\n' "$name" + else + printf ' FAIL %s (got=%q want=%q)\n' "$name" "$got" "$want" + TESTS_FAILED=$((TESTS_FAILED + 1)) + fi +} + +assert_ne() { + local name="$1" unwanted="$2" got="$3" + TESTS_RUN=$((TESTS_RUN + 1)) + if [ "$got" != "$unwanted" ]; then + printf ' PASS %s\n' "$name" + else + printf ' FAIL %s (unexpectedly %q)\n' "$name" "$got" + TESTS_FAILED=$((TESTS_FAILED + 1)) + fi +} + +# Runs a command and reports its exit status, echoing captured output on failure so a +# rejected unit file shows its diagnostics instead of just "exit 1". +assert_cmd_ok() { + local name="$1" + shift + local out rc + out=$("$@" 2>&1) + rc=$? + TESTS_RUN=$((TESTS_RUN + 1)) + if [ "$rc" -eq 0 ]; then + printf ' PASS %s\n' "$name" + else + printf ' FAIL %s (exit %d)\n' "$name" "$rc" + printf '%s\n' "$out" | sed 's/^/ | /' + TESTS_FAILED=$((TESTS_FAILED + 1)) + fi +} + +file_state() { [ -e "$1" ] && echo present || echo absent; } + +printf 'systemd state: %s\n' "$SYSTEM_STATE" + +# --- the host must actually be what we think it is ---------------------------- +assert_eq "host detects systemd" "systemd" "$(detect_init_system)" + +# --- install ------------------------------------------------------------------ +setup_autostart >/dev/null 2>&1 +assert_eq "setup_autostart succeeds" "0" "$?" +assert_eq "unit file written" "present" "$(file_state "$UNIT")" + +if command -v systemd-analyze >/dev/null 2>&1; then + # --recursive-errors=yes is required for a non-zero exit; without it verify prints + # warnings and still returns 0, which would make this assertion meaningless. + assert_cmd_ok "systemd-analyze accepts the unit" \ + systemd-analyze verify --recursive-errors=yes "$UNIT" +fi + +# is-enabled exits non-zero for "disabled", so compare stdout and keep set -e off. +assert_eq "unit is enabled" "enabled" "$(systemctl is-enabled mtproxymax.service 2>/dev/null)" + +# --- start / stop ------------------------------------------------------------- +systemctl start mtproxymax.service >/dev/null 2>&1 +assert_eq "systemctl start succeeds" "0" "$?" +assert_eq "unit is active after start (RemainAfterExit=yes)" "active" \ + "$(systemctl is-active mtproxymax.service 2>/dev/null)" +assert_eq "ExecStart reached the mtproxymax binary" "start" "$(head -n1 "$FAKE_LOG" 2>/dev/null)" + +systemctl stop mtproxymax.service >/dev/null 2>&1 +assert_eq "unit is inactive after stop" "inactive" \ + "$(systemctl is-active mtproxymax.service 2>/dev/null)" +assert_eq "ExecStop reached the mtproxymax binary" "stop" "$(tail -n1 "$FAKE_LOG" 2>/dev/null)" + +# --- remove ------------------------------------------------------------------- +main_service_remove >/dev/null 2>&1 +assert_eq "main_service_remove succeeds" "0" "$?" +assert_eq "unit file removed" "absent" "$(file_state "$UNIT")" +assert_eq "wants symlink removed" "absent" "$(file_state "$WANTS")" +assert_ne "unit is no longer enabled" "enabled" \ + "$(systemctl is-enabled mtproxymax.service 2>/dev/null)" + +printf '\n%d tests, %d failures\n' "$TESTS_RUN" "$TESTS_FAILED" +[ "$TESTS_FAILED" -eq 0 ] diff --git a/tests/run-all.sh b/tests/run-all.sh new file mode 100644 index 0000000..086312b --- /dev/null +++ b/tests/run-all.sh @@ -0,0 +1,159 @@ +#!/bin/bash +# Aggregate runner for the unit test suite (tests/test_*.sh). +# +# Each test is a self-contained script that sources mtproxymax.sh with +# MTPROXYMAX_SOURCE_ONLY=true, stubs the side-effecting helpers, and exits non-zero +# on any failed assertion. A test that cannot run on this host (e.g. bash too old) +# prints a "SKIP:" line and exits 0 — that is reported as SKIP, never as PASS, so a +# suite that has quietly stopped testing anything stays visible. +# +# Usage: +# tests/run-all.sh # run every tests/test_*.sh +# tests/run-all.sh guest secret # only tests whose name contains a substring +# +# Exits non-zero if any test failed or if the selection matched nothing. +set -uo pipefail + +cd "$(dirname "${BASH_SOURCE[0]}")/.." || exit 1 + +# The tests themselves skip on bash < 4.2. Failing loudly here instead is deliberate: +# if the runner exits 0 on an unsupported shell, every test SKIPs and the suite reports +# a misleading green. +if [ "${BASH_VERSINFO[0]:-0}" -lt 4 ] || + { [ "${BASH_VERSINFO[0]}" -eq 4 ] && [ "${BASH_VERSINFO[1]:-0}" -lt 2 ]; }; then + echo "ERROR: bash 4.2+ required to run the suite (got ${BASH_VERSION:-unknown})" >&2 + exit 1 +fi + +shopt -s nullglob +ALL_TESTS=(tests/test_*.sh) +shopt -u nullglob + +if [ "${#ALL_TESTS[@]}" -eq 0 ]; then + echo "ERROR: no tests/test_*.sh found — run from the repository root" >&2 + exit 1 +fi + +if [ "$#" -eq 0 ]; then + SELECTED=("${ALL_TESTS[@]}") +else + SELECTED=() + for t in "${ALL_TESTS[@]}"; do + for f in "$@"; do + case "$t" in + *"$f"*) + SELECTED+=("$t") + break + ;; + esac + done + done + if [ "${#SELECTED[@]}" -eq 0 ]; then + printf 'ERROR: no test matched: %s\n' "$*" >&2 + exit 1 + fi +fi + +# Tests are invoked as `bash tests/.sh` from the repo root. That matters: +# some tests resolve the script under test via $(dirname "$0") and others via +# ${BASH_SOURCE[0]}, and the two only agree for a repo-root-relative path. + +# Comma- or space-separated basenames known to fail on this platform. A quarantined test +# still RUNS; its failure is reported as QUARANTINE and does not fail the build. The entry +# is deliberately loud and is expected to be deleted once the underlying bug is fixed — +# this is a way to land CI that is honest about what it cannot yet assert, not a way to +# make red things look green. Callers set it (the CI matrix scopes it per distro). +QUARANTINE="${MTPROXYMAX_QUARANTINE:-}" + +is_quarantined() { + local base name + base=$(basename "$1") + for name in ${QUARANTINE//,/ }; do + [ "$name" = "$base" ] && return 0 + done + return 1 +} + +PASSED=0 +FAILED=0 +SKIPPED=0 +QUARANTINED=0 +FAILED_TESTS=() +SUMMARY_ROWS="" + +printf 'Running %d test script(s) in %s\n\n' "${#SELECTED[@]}" "$PWD" + +for t in "${SELECTED[@]}"; do + SECONDS=0 + output=$(bash "$t" 2>&1) + rc=$? + elapsed=$SECONDS + + n_pass=$(printf '%s\n' "$output" | grep -c '^ PASS' || true) + n_fail=$(printf '%s\n' "$output" | grep -c '^ FAIL' || true) + + if [ "$rc" -ne 0 ]; then + if is_quarantined "$t"; then + status="QUARANTINE" + QUARANTINED=$((QUARANTINED + 1)) + printf 'QUARANTINE %-36s exit=%d %ds (known failure — not gating)\n' \ + "$t" "$rc" "$elapsed" + printf ' %s\n' \ + "$(printf '%s\n' "$output" | grep -m1 '^ FAIL' || echo 'failed')" + else + status="FAIL" + FAILED=$((FAILED + 1)) + FAILED_TESTS+=("$t") + printf 'FAIL %-40s exit=%d %ds\n' "$t" "$rc" "$elapsed" + # Indent the captured output so a failing test is readable in the log. + printf '%s\n' "$output" | sed 's/^/ | /' + echo + fi + elif printf '%s\n' "$output" | grep -q '^SKIP:'; then + status="SKIP" + SKIPPED=$((SKIPPED + 1)) + printf 'SKIP %-40s %s\n' "$t" "$(printf '%s\n' "$output" | grep -m1 '^SKIP:')" + else + status="PASS" + PASSED=$((PASSED + 1)) + printf 'PASS %-40s %d/%d assertions %ds\n' "$t" "$n_pass" "$((n_pass + n_fail))" "$elapsed" + # A green test that reported no assertions is worth flagging: it usually means + # the harness never ran, not that everything is fine. + if [ "$n_pass" -eq 0 ]; then + printf ' ! no PASS lines reported — did the test actually run?\n' + fi + fi + + SUMMARY_ROWS="${SUMMARY_ROWS}| \`${t}\` | ${status} | ${n_pass} | ${n_fail} | ${elapsed}s | +" +done + +printf '\n%s\n' "----------------------------------------" +printf 'total=%d passed=%d failed=%d skipped=%d quarantined=%d\n' \ + "${#SELECTED[@]}" "$PASSED" "$FAILED" "$SKIPPED" "$QUARANTINED" + +if [ "${#FAILED_TESTS[@]}" -gt 0 ]; then + printf '\nfailed:\n' + printf ' - %s\n' "${FAILED_TESTS[@]}" +fi + +if [ "$QUARANTINED" -gt 0 ]; then + printf '\nquarantined (known failures, NOT counted as passing — fix and remove from the list):\n' + printf ' - %s\n' "${QUARANTINE//,/ }" +fi + +# Surface the same summary in the GitHub Actions job page when running under CI. +if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then + { + echo "## Unit tests" + echo + echo "| test | result | pass | fail | time |" + echo "|---|---|---|---|---|" + printf '%s' "$SUMMARY_ROWS" + echo + printf '**total=%d passed=%d failed=%d skipped=%d quarantined=%d**\n' \ + "${#SELECTED[@]}" "$PASSED" "$FAILED" "$SKIPPED" "$QUARANTINED" + } >>"$GITHUB_STEP_SUMMARY" +fi + +[ "$FAILED" -eq 0 ] From 379f802ba26389981b6b4d1fc5b6aeb8f5cf6f07 Mon Sep 17 00:00:00 2001 From: Ramil Valitov Date: Thu, 17 Sep 2026 21:53:49 +0300 Subject: [PATCH 2/3] ci: flag quarantined tests that have started passing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A quarantine list rots in a specific way: the upstream fix lands, the entry stays, and the suite keeps advertising a failure it no longer has. Nothing in the output said so, because a quarantined test that passes was reported as an ordinary PASS. run-all.sh now counts those and prints "! still listed as quarantined, but passing — the entry can be removed", with a matching line in the summary. Leaving an entry in place stays harmless, so the list is correct in any merge order; it is simply visible now instead of silent. Also names, in the workflow, which PR retires each Alpine entry (#145, #146, #148), so the maintainer can remove them as those merge rather than having to rediscover the mapping. --- .github/workflows/ci.yml | 31 +++++++++++++++++++------------ tests/run-all.sh | 22 ++++++++++++++++++---- 2 files changed, 37 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9871106..98c3668 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -81,18 +81,25 @@ jobs: install: 'apt-get update -qq && apt-get install -y -qq bash diffutils' quarantine: 'test_client_mss.sh' # Alpine carries three known failures, all busybox divergences on a platform the - # README lists as supported: - # test_client_mss.sh — broken on every distro (asserts on the stdout of a - # function that writes to a file); see below. - # test_traffic_reset.sh— busybox `flock` has no -w, so `flock -w 5 9` fails; - # `command -v flock` succeeds, so the guard never fires. - # Two call sites fail OPEN and silently write nothing. - # test_guest.sh — `date -d "+24 hours"` is invalid on busybox and the - # `date -r ` fallback also fails (busybox -r means - # reference file), so expiring guest links get no expiry. - # test_client_mss.sh is quarantined everywhere because it is a broken test, not a - # platform difference — it asserts on the stdout of generate_telemt_config, which - # takes a destination path and writes ${CONFIG_DIR}/config.toml instead. + # README lists as supported. Each is fixed by an open PR — remove the entry once + # that PR has merged: + # test_client_mss.sh — broken on every distro, not a platform difference: it + # asserts on the stdout of generate_telemt_config, which + # takes a destination path and writes + # ${CONFIG_DIR}/config.toml instead. Fixed by #145. + # test_traffic_reset.sh — busybox `flock` has no -w, so `flock -w 5 9` fails + # while `command -v flock` succeeds, so the guard never + # fires. Two call sites failed OPEN and silently wrote + # nothing. Fixed by #146. + # test_guest.sh — `date -d "+24 hours"` is invalid on busybox and the + # `date -r ` fallback fails as well (busybox -r + # means reference file), so expiring guest links got no + # expiry. Fixed by #148. + # + # Leaving an entry in place after its fix has landed is harmless — a quarantined + # test that passes is reported as PASS — so this list is correct in any merge + # order. run-all.sh flags such entries as "! still listed as quarantined" so a + # stale one is visible rather than silently ignored. - image: alpine:3.20 install: 'apk add --no-cache bash' quarantine: 'test_client_mss.sh,test_traffic_reset.sh,test_guest.sh' diff --git a/tests/run-all.sh b/tests/run-all.sh index 086312b..ff208ad 100644 --- a/tests/run-all.sh +++ b/tests/run-all.sh @@ -78,6 +78,7 @@ PASSED=0 FAILED=0 SKIPPED=0 QUARANTINED=0 +STALE_QUARANTINE=0 FAILED_TESTS=() SUMMARY_ROWS="" @@ -122,6 +123,13 @@ for t in "${SELECTED[@]}"; do if [ "$n_pass" -eq 0 ]; then printf ' ! no PASS lines reported — did the test actually run?\n' fi + # A quarantined test that now passes means the entry is stale. That is the way a + # quarantine list rots: the upstream fix lands, the entry stays, and the suite + # keeps advertising a failure it no longer has. Report it so it gets removed. + if is_quarantined "$t"; then + STALE_QUARANTINE=$((STALE_QUARANTINE + 1)) + printf ' ! still listed as quarantined, but passing — the entry can be removed\n' + fi fi SUMMARY_ROWS="${SUMMARY_ROWS}| \`${t}\` | ${status} | ${n_pass} | ${n_fail} | ${elapsed}s | @@ -129,8 +137,8 @@ for t in "${SELECTED[@]}"; do done printf '\n%s\n' "----------------------------------------" -printf 'total=%d passed=%d failed=%d skipped=%d quarantined=%d\n' \ - "${#SELECTED[@]}" "$PASSED" "$FAILED" "$SKIPPED" "$QUARANTINED" +printf 'total=%d passed=%d failed=%d skipped=%d quarantined=%d stale-quarantine=%d\n' \ + "${#SELECTED[@]}" "$PASSED" "$FAILED" "$SKIPPED" "$QUARANTINED" "$STALE_QUARANTINE" if [ "${#FAILED_TESTS[@]}" -gt 0 ]; then printf '\nfailed:\n' @@ -142,6 +150,12 @@ if [ "$QUARANTINED" -gt 0 ]; then printf ' - %s\n' "${QUARANTINE//,/ }" fi +if [ "$STALE_QUARANTINE" -gt 0 ]; then + printf '\n%d quarantined test(s) now pass — remove those entries from MTPROXYMAX_QUARANTINE:\n' \ + "$STALE_QUARANTINE" + printf ' see the "! still listed as quarantined" markers above\n' +fi + # Surface the same summary in the GitHub Actions job page when running under CI. if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then { @@ -151,8 +165,8 @@ if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then echo "|---|---|---|---|---|" printf '%s' "$SUMMARY_ROWS" echo - printf '**total=%d passed=%d failed=%d skipped=%d quarantined=%d**\n' \ - "${#SELECTED[@]}" "$PASSED" "$FAILED" "$SKIPPED" "$QUARANTINED" + printf '**total=%d passed=%d failed=%d skipped=%d quarantined=%d stale-quarantine=%d**\n' \ + "${#SELECTED[@]}" "$PASSED" "$FAILED" "$SKIPPED" "$QUARANTINED" "$STALE_QUARANTINE" } >>"$GITHUB_STEP_SUMMARY" fi From c2ab72ea510e932ca192278c6ba4138b59224df1 Mon Sep 17 00:00:00 2001 From: Ramil Valitov Date: Thu, 17 Sep 2026 22:06:34 +0300 Subject: [PATCH 3/3] ci: bump actions/checkout to v6 for the Node.js 24 runtime v4 targets Node.js 20, which was removed from GitHub-hosted runners on 2026-09-16. Jobs using it are now forced onto Node.js 24 and emit a deprecation annotation on every job: Node.js 20 is deprecated. The following actions target Node.js 20 but are being forced to run on Node.js 24: actions/checkout@v4 v6 is the current major and targets Node.js 24. It is a drop-in for plain checkout usage; the one behavioural change (persist-credentials moving to $RUNNER_TEMP) only affects Docker container actions and needs runner >= 2.329.0, which hosted runners satisfy. build-engine.yml needs nothing: it uses only docker/* actions and has no checkout step. ci: drop the inline comments on the actions/checkout bump Repeating the rationale above each of the four uses: lines is noise; the version is self-explanatory and the reasoning belongs in the commit that made the change. --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 98c3668..18682fe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,7 +31,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 10 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 # Pure parser check — reports the file and line of any syntax error without # executing anything. This cannot false-positive, so it is a hard gate. @@ -107,7 +107,7 @@ jobs: install: 'dnf install -y -q bash diffutils' quarantine: 'test_client_mss.sh' steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 # Each distro runs the same suite in a container. Alpine is the row that earns its # keep: its busybox userland (sed, grep, mktemp, date) differs from GNU, and Alpine @@ -136,7 +136,7 @@ jobs: runs-on: ubuntu-24.04 timeout-minutes: 10 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Confirm the runner really is systemd # If this ever prints something other than "systemd", the job below would be @@ -154,7 +154,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 10 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 # OpenRC in a container needs the softlevel marker (its verify_boot() otherwise # refuses to run any service) and a bash since the image ships only busybox ash;