Route apt-repo tests through explicit named assertions

rtibblesbot · claude · rtibblesbot · commit 09cf05e8349f · 2026-07-22T22:55:13.000-07:00
Add tests/lib.sh with assert_file/assert_contains/assert_equals/
assert_files_equal/assert_output_contains helpers and a require_tools
guard, and refactor the five apt-repo tests to use them so each check
is an explicit, named assertion. require_tools honours
APT_REPO_TESTS_STRICT, which the CI runner now sets so a missing-tool
SKIP becomes a hard failure instead of a vacuous pass -- guaranteeing
the assertions actually execute in CI.

Co-Authored-By: Claude Opus 4.8 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/.github/workflows/platform-apt-repo-test.yml b/.github/workflows/platform-apt-repo-test.yml
@@ -30,9 +30,16 @@ jobs:
           sudo apt-get update
           sudo apt-get install -y --no-install-recommends reprepro gnupg dpkg-dev rsync
       - name: Run the apt-repo tests
+        # APT_REPO_TESTS_STRICT=1 turns a missing-tool SKIP into a hard failure,
+        # so every test must actually run its assertions here rather than
+        # passing vacuously — the tooling above (plus preinstalled docker) is
+        # exactly what they need.
+        env:
+          APT_REPO_TESTS_STRICT: "1"
         run: |
           set -e
           for t in platforms/apt-repo/tests/*.sh; do
+            [ "$(basename "$t")" = "lib.sh" ] && continue  # shared helpers, not a test
             echo "::group::$t"
             sh "$t"
             echo "::endgroup::"
diff --git a/platforms/apt-repo/tests/e2e_cutover.sh b/platforms/apt-repo/tests/e2e_cutover.sh
@@ -14,9 +14,11 @@
 # resolves to the NEW repo at the docroot root — exactly the production cutover.
 set -eu
 
-command -v docker >/dev/null 2>&1 || { echo "SKIP: needs docker"; exit 0; }
-
 HERE=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)
+. "$HERE/lib.sh"
+
+require_tools docker
+
 REPO_ROOT=$(CDPATH= cd -- "$HERE/../../.." && pwd)
 
 # The container builds both repos, serves them, installs the cutover release, and
@@ -147,10 +149,10 @@ INSTALLED_NEW=$(dpkg-query -W -f='${Version}' kolibri-server)
 
 echo "PASS: old-source client auto-rewritten to apt.learningequality.org and upgraded 1.0 -> 1.0.1 with no manual action"
 EOF
-) || { printf '%s\n' "$RESULT" >&2; echo "FAIL: e2e cutover container run exited non-zero"; exit 1; }
+) || { printf '%s\n' "$RESULT" >&2; fail "e2e cutover container run exited non-zero"; }
 printf '%s\n' "$RESULT"
 
 # Top-level assertion: the container must have reached its PASS line. Without
 # this, a silent early return inside the heredoc would go unnoticed.
-printf '%s\n' "$RESULT" | grep -q '^PASS:' \
-  || { echo "FAIL: e2e cutover did not reach its PASS assertion"; exit 1; }
+assert_output_contains "$RESULT" '^PASS:' \
+  "e2e cutover did not reach its PASS assertion"
diff --git a/platforms/apt-repo/tests/lib.sh b/platforms/apt-repo/tests/lib.sh
@@ -0,0 +1,60 @@
+# Shared assertion + tooling helpers for the apt-repo shell tests.
+#
+# Sourced by every test. Assertions print a "FAIL:" line and exit non-zero
+# when they do not hold, so a regression aborts the test instead of passing
+# silently.
+#
+# require_tools normally SKIPs (exit 0) when a tool is absent, so the suite is
+# still usable on a bare dev box. In CI the runner exports
+# APT_REPO_TESTS_STRICT=1, which turns a missing tool into a hard failure —
+# guaranteeing every test actually reaches its assertions there rather than
+# passing vacuously.
+
+fail() { echo "FAIL: $1"; exit 1; }
+
+# assert_file <path> [message]
+assert_file() {
+  [ -f "$1" ] || fail "${2:-expected file to exist: $1}"
+}
+
+# assert_contains <file> <extended-regex> [message]
+assert_contains() {
+  grep -Eq "$2" "$1" || fail "${3:-expected /$2/ in $1}"
+}
+
+# assert_absent <file-or-dir> <extended-regex> [message] — recursive
+assert_absent() {
+  if grep -rEq "$2" "$1"; then fail "${3:-unexpected match for /$2/ under $1}"; fi
+}
+
+# assert_equals <actual> <expected> [message]
+assert_equals() {
+  [ "$1" = "$2" ] || fail "${3:-expected '$2', got '$1'}"
+}
+
+# assert_output_contains <string> <extended-regex> [message]
+assert_output_contains() {
+  printf '%s\n' "$1" | grep -Eq "$2" || fail "${3:-expected /$2/ in captured output}"
+}
+
+# assert_output_absent <string> <extended-regex> [message]
+assert_output_absent() {
+  if printf '%s\n' "$1" | grep -Eq "$2"; then fail "${3:-unexpected /$2/ in captured output}"; fi
+}
+
+# assert_files_equal <a> <b> [message]
+assert_files_equal() {
+  cmp -s "$1" "$2" || fail "${3:-files differ: $1 vs $2}"
+}
+
+# require_tools <tool>... — SKIP (or FAIL under APT_REPO_TESTS_STRICT=1) if any absent
+require_tools() {
+  for _t in "$@"; do
+    command -v "$_t" >/dev/null 2>&1 && continue
+    if [ "${APT_REPO_TESTS_STRICT:-0}" = "1" ]; then
+      fail "required tool missing under APT_REPO_TESTS_STRICT: $_t (needs: $*)"
+    fi
+    echo "SKIP: needs $* (missing: $_t)"
+    exit 0
+  done
+}
diff --git a/platforms/apt-repo/tests/test_keyring_install.sh b/platforms/apt-repo/tests/test_keyring_install.sh
@@ -9,17 +9,19 @@
 set -eu
 
 HERE=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)
+. "$HERE/lib.sh"
+
 APT_REPO="$HERE/.."
 REPO_ROOT=$(CDPATH= cd -- "$APT_REPO/../.." && pwd)
 
 # --- key-drift guard (runs without docker) ----------------------------------
 PI_KEY="$REPO_ROOT/platforms/raspberry-pi/files/learningequality.asc"
 KEYRING_KEY="$APT_REPO/keyring/kolibri-archive-keyring.asc"
-[ -f "$KEYRING_KEY" ] || { echo "FAIL: $KEYRING_KEY missing"; exit 1; }
-cmp -s "$PI_KEY" "$KEYRING_KEY" \
-  || { echo "FAIL: kolibri-archive-keyring.asc differs from the Pi trust key"; exit 1; }
+assert_file "$KEYRING_KEY" "$KEYRING_KEY missing"
+assert_files_equal "$PI_KEY" "$KEYRING_KEY" \
+  "kolibri-archive-keyring.asc differs from the Pi trust key"
 
-command -v docker >/dev/null 2>&1 || { echo "SKIP: needs docker"; exit 0; }
+require_tools docker
 
 # --- build + install in a clean Debian 13 container -------------------------
 # The container builds the native package, installs it, and asserts the two
@@ -64,10 +66,10 @@ if echo "$OUT" | grep -Eiq 'Malformed|NO_PUBKEY|not signed|invalid|bad'; then
 fi
 echo "PASS: keyring builds, installs source + key, apt-get update parses it"
 EOF
-) || { printf '%s\n' "$RESULT" >&2; echo "FAIL: keyring container run exited non-zero"; exit 1; }
+) || { printf '%s\n' "$RESULT" >&2; fail "keyring container run exited non-zero"; }
 printf '%s\n' "$RESULT"
 
 # Top-level assertion: the container must have reached its PASS line. Without
 # this, a silent early return inside the heredoc would go unnoticed.
-printf '%s\n' "$RESULT" | grep -q '^PASS:' \
-  || { echo "FAIL: keyring test did not reach its PASS assertion"; exit 1; }
+assert_output_contains "$RESULT" '^PASS:' \
+  "keyring test did not reach its PASS assertion"
diff --git a/platforms/apt-repo/tests/test_migrate_apt_source.sh b/platforms/apt-repo/tests/test_migrate_apt_source.sh
@@ -9,14 +9,14 @@
 set -eu
 
 HERE=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)
+. "$HERE/lib.sh"
+
 SNIPPET="$HERE/../migrate-apt-source.sh"
-[ -f "$SNIPPET" ] || { echo "FAIL: $SNIPPET missing"; exit 1; }
+assert_file "$SNIPPET" "$SNIPPET missing"
 
 WORK=$(mktemp -d)
 trap 'rm -rf "$WORK"' EXIT
 
-fail() { echo "FAIL: $1"; exit 1; }
-
 # --- Case A: rewrite (AC#6) --------------------------------------------------
 # A deb822 .sources naming the old kolibri-server host, plus a legacy one-line
 # .list naming the old kolibri-installer-debian host. Both must move to the
@@ -36,13 +36,12 @@ EOF
 
 KOLIBRI_APT_SOURCES_DIR="$A" sh "$SNIPPET"
 
-grep -q 'apt.learningequality.org' "$A/learningequality-kolibri-server.sources" \
-  || fail "Case A: .sources not rewritten to apt.learningequality.org"
-grep -q 'apt.learningequality.org' "$A/learningequality-debian.list" \
-  || fail "Case A: .list not rewritten to apt.learningequality.org"
-if grep -rq 'learningequality\.github\.io' "$A"; then
-  fail "Case A: a github.io host survived the rewrite"
-fi
+assert_contains "$A/learningequality-kolibri-server.sources" 'apt\.learningequality\.org' \
+  "Case A: .sources not rewritten to apt.learningequality.org"
+assert_contains "$A/learningequality-debian.list" 'apt\.learningequality\.org' \
+  "Case A: .list not rewritten to apt.learningequality.org"
+assert_absent "$A" 'learningequality\.github\.io' \
+  "Case A: a github.io host survived the rewrite"
 
 # --- Case B: no-op (AC#7) ----------------------------------------------------
 # A Launchpad-PPA source has no github.io host — the file must be byte-unchanged.
@@ -55,24 +54,24 @@ cp "$B/learningequality-ubuntu-kolibri.list" "$WORK/case_b_orig.list"
 
 KOLIBRI_APT_SOURCES_DIR="$B" sh "$SNIPPET"
 
-cmp -s "$B/learningequality-ubuntu-kolibri.list" "$WORK/case_b_orig.list" \
-  || fail "Case B: Launchpad PPA source was modified (should be no-op)"
+assert_files_equal "$B/learningequality-ubuntu-kolibri.list" "$WORK/case_b_orig.list" \
+  "Case B: Launchpad PPA source was modified (should be no-op)"
 
 # --- Case C: idempotent ------------------------------------------------------
 # A second run over Case A's already-rewritten fixture changes nothing.
 cp -a "$A" "$WORK/case_c"
 C="$WORK/case_c"
 snapshot=$(cat "$C"/* )
 KOLIBRI_APT_SOURCES_DIR="$C" sh "$SNIPPET"
-[ "$snapshot" = "$(cat "$C"/*)" ] \
-  || fail "Case C: second run mutated an already-rewritten fixture"
+assert_equals "$(cat "$C"/*)" "$snapshot" \
+  "Case C: second run mutated an already-rewritten fixture"
 
 # --- Case D: sourced under `set -e`, no match, must not abort (AC#7 path) -----
 # The postinst sources the snippet and calls the function under `set -e`; on a
 # no-match install grep exits 1 — that must not abort. Mimic the integration.
 KOLIBRI_APT_SOURCES_DIR="$B" sh -e -c ". '$SNIPPET'; migrate_kolibri_apt_source; echo OK" \
   > "$WORK/case_d.out" 2>&1 || fail "Case D: aborted under set -e on no-match"
-grep -qx 'OK' "$WORK/case_d.out" \
-  || fail "Case D: did not reach OK (sourced call aborted under set -e)"
+assert_contains "$WORK/case_d.out" '^OK$' \
+  "Case D: did not reach OK (sourced call aborted under set -e)"
 
 echo "PASS: rewrite, no-op, idempotent, and set -e no-match all green"
diff --git a/platforms/apt-repo/tests/test_publish_roundtrip.sh b/platforms/apt-repo/tests/test_publish_roundtrip.sh
@@ -4,14 +4,11 @@
 # only local tooling (no GCS).
 set -eu
 
-for tool in reprepro gpg dpkg-deb rsync; do
-  command -v "$tool" >/dev/null 2>&1 || {
-    echo "SKIP: needs reprepro gnupg dpkg-dev rsync (missing: $tool)"
-    exit 0
-  }
-done
-
 HERE=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)
+. "$HERE/lib.sh"
+
+require_tools reprepro gpg dpkg-deb rsync
+
 PUBLISH="$HERE/../publish.sh"
 
 WORK=$(mktemp -d)
@@ -36,7 +33,7 @@ Expire-Date: 0
 EOF
 gpg --batch --gen-key "$WORK/key.spec" >/dev/null 2>&1
 FPR=$(gpg --list-secret-keys --with-colons | awk -F: '/^fpr:/ { print $10; exit }')
-[ -n "$FPR" ] || { echo "FAIL: could not create signing key"; exit 1; }
+[ -n "$FPR" ] || fail "could not create signing key"
 export REPREPRO_SIGN_KEY="$FPR"
 
 # --- build two minimal arch-all .debs with distinct names -------------------
@@ -69,19 +66,19 @@ bash "$PUBLISH" "$WORK/bbb-test_1.0_all.deb"
 
 # --- AC#3: both present after the second publish ----------------------------
 PKGS="$BUCKET/dists/stable/main/binary-amd64/Packages"
-[ -f "$PKGS" ] || { echo "FAIL: $PKGS missing"; exit 1; }
-grep -q "^Package: aaa-test$" "$PKGS" || { echo "FAIL: aaa-test dropped by second publish"; exit 1; }
-grep -q "^Package: bbb-test$" "$PKGS" || { echo "FAIL: bbb-test missing after second publish"; exit 1; }
+assert_file "$PKGS" "$PKGS missing"
+assert_contains "$PKGS" '^Package: aaa-test$' "aaa-test dropped by second publish"
+assert_contains "$PKGS" '^Package: bbb-test$' "bbb-test missing after second publish"
 
 # --- AC#2: Release signed + pubkey.asc served -------------------------------
 { [ -f "$BUCKET/dists/stable/Release.gpg" ] || [ -f "$BUCKET/dists/stable/InRelease" ]; } \
-  || { echo "FAIL: no signature (Release.gpg / InRelease) on the published Release"; exit 1; }
-[ -f "$BUCKET/pubkey.asc" ] || { echo "FAIL: pubkey.asc not served at bucket root"; exit 1; }
+  || fail "no signature (Release.gpg / InRelease) on the published Release"
+assert_file "$BUCKET/pubkey.asc" "pubkey.asc not served at bucket root"
 
 # --- bootstrap-root path: kolibri-archive-keyring.deb at bucket root ---------
 KOLIBRI_KEYRING_DEB="$WORK/aaa-test_1.0_all.deb" bash "$PUBLISH" "$WORK/aaa-test_1.0_all.deb"
-[ -f "$BUCKET/kolibri-archive-keyring.deb" ] \
-  || { echo "FAIL: kolibri-archive-keyring.deb not served at bucket root"; exit 1; }
+assert_file "$BUCKET/kolibri-archive-keyring.deb" \
+  "kolibri-archive-keyring.deb not served at bucket root"
 
 # --- keyring re-publish across releases (fixed version, rebuilt bytes) --------
 # The keyring version stays fixed across releases but CI rebuilds the .deb every
@@ -107,6 +104,6 @@ build_keyring_variant a
 build_keyring_variant b
 KOLIBRI_KEYRING_DEB="$WORK/kr-keyring-a.deb" bash "$PUBLISH" "$WORK/aaa-test_1.0_all.deb"
 KOLIBRI_KEYRING_DEB="$WORK/kr-keyring-b.deb" bash "$PUBLISH" "$WORK/aaa-test_1.0_all.deb" \
-  || { echo "FAIL: keyring re-publish aborted on a rebuilt same-version .deb"; exit 1; }
+  || fail "keyring re-publish aborted on a rebuilt same-version .deb"
 
 echo "PASS: round-trip preserves prior packages; Release signed; pubkey.asc + bootstrap deb served"
diff --git a/platforms/apt-repo/tests/test_seed_old_pages.sh b/platforms/apt-repo/tests/test_seed_old_pages.sh
@@ -7,16 +7,13 @@
 # KOLIBRI_SEED_REMOTE_BASE points the script at it (no GitHub access).
 set -eu
 
-for tool in reprepro gpg dpkg-deb git; do
-  command -v "$tool" >/dev/null 2>&1 || {
-    echo "SKIP: needs reprepro gnupg dpkg-dev git (missing: $tool)"
-    exit 0
-  }
-done
-
 HERE=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)
+. "$HERE/lib.sh"
+
+require_tools reprepro gpg dpkg-deb git
+
 SEED="$HERE/../seed_old_pages.sh"
-[ -f "$SEED" ] || { echo "FAIL: $SEED missing"; exit 1; }
+assert_file "$SEED" "$SEED missing"
 
 WORK=$(mktemp -d)
 cleanup() {
@@ -40,7 +37,7 @@ Expire-Date: 0
 EOF
 gpg --batch --gen-key "$WORK/key.spec" >/dev/null 2>&1
 FPR=$(gpg --list-secret-keys --with-colons | awk -F: '/^fpr:/ { print $10; exit }')
-[ -n "$FPR" ] || { echo "FAIL: could not create signing key"; exit 1; }
+[ -n "$FPR" ] || fail "could not create signing key"
 
 # --- build two minimal arch-all .debs ---------------------------------------
 build_deb() {
@@ -93,13 +90,13 @@ assert_branch_has_both() {
   chk="$WORK/check-$1"
   git clone --quiet --branch gh-pages --single-branch "$BARE" "$chk"
   P="$chk/dists/stable/main/binary-amd64/Packages"
-  [ -f "$P" ] || { echo "FAIL ($1): $P missing on pushed branch"; exit 1; }
-  grep -q "^Package: kolibri-server$" "$P" \
-    || { echo "FAIL ($1): seeded kolibri-server not on pushed branch"; exit 1; }
-  grep -q "^Package: preexisting-pkg$" "$P" \
-    || { echo "FAIL ($1): pre-existing package dropped (rebuilt fresh?)"; exit 1; }
+  assert_file "$P" "($1): $P missing on pushed branch"
+  assert_contains "$P" '^Package: kolibri-server$' \
+    "($1): seeded kolibri-server not on pushed branch"
+  assert_contains "$P" '^Package: preexisting-pkg$' \
+    "($1): pre-existing package dropped (rebuilt fresh?)"
   ls "$chk"/pool/main/*/*/kolibri-server_1.0_all.deb >/dev/null 2>&1 \
-    || { echo "FAIL ($1): kolibri-server .deb missing from pool/"; exit 1; }
+    || fail "($1): kolibri-server .deb missing from pool/"
 }
 
 # --- first run: pushes the cutover .deb -------------------------------------
@@ -110,7 +107,7 @@ assert_branch_has_both run1
 
 # --- second run: safe to run again (duplicate version is a non-fatal no-op) --
 bash "$SEED" --deb "$WORK/kolibri-server_1.0_all.deb" --repo kolibri-server \
-  || { echo "FAIL: second run exited non-zero (not safe to run again)"; exit 1; }
+  || fail "second run exited non-zero (not safe to run again)"
 assert_branch_has_both run2
 
 echo "PASS: seed pushes cutover .deb, preserves prior package, safe to run again"
