diff --git a/docs/testing.md b/docs/testing.md index 2606645..a42c03d 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -167,6 +167,51 @@ rather than per-PR because of its cost, and not because it is optional. A claim in the documentation with only a suite that nobody runs behind it is how coverage becomes stale without notice. +## Extension upgrade + +`PGC_RUN_UPGRADE=1` also runs `test/extension_upgrade.sh`, once, against the first +major in the list. It covers a different upgrade from the one above: not a new +PostgreSQL major, but a new build of pgColumnar on the same one. + +Every function the extension installs records the name of a C symbol. When a build +changes those names, the recorded names stop resolving, and the extension is inert +until `ALTER EXTENSION pgcolumnar UPDATE` runs. A build cannot see that, and neither can a suite that +creates the extension from scratch. That is why it needs its own gate. + +The suite installs the previous release and creates a columnar table with rows in it. +It then installs the tree under test over the top and runs the update. Reads, writes, +table creation and a maintenance call must all still work. + +It also asserts that every C function owned by the extension has a link name inside the +`pgcolumnar` namespace. That catches the next rename as well as the last one. + +It builds the old version, so it needs to be told where to find it. Given a git ref +it builds from a throwaway clone, which needs a checkout with tags: + +```sh +git fetch --tags +test/extension_upgrade.sh /usr/local/pg18/bin/pg_config v1.0-alpha +``` + +The container loop copies the tree without `.git`, so the ref form cannot work there. +Pass a directory holding the old source instead: + +```sh +test/extension_upgrade.sh /usr/local/pg18/bin/pg_config /root/pgcolumnar-1.0-alpha +``` + +Through the runner, give it the same directory by environment: + +```sh +PGC_RUN_UPGRADE=1 PGC_UPGRADE_OLD_SRC=/root/pgcolumnar-1.0-alpha \ + test/run_all_versions.sh +``` + +Without it, in a tree that has no `.git`, the suite reports **SKIP** and the matrix +records `SKIP`. It does not report a pass, because a gate that goes green having run +nothing is the defect this suite exists to catch. It does not report a failure either. Being unable to +obtain an old build is a property of the environment, not of the code under test. + ## make installcheck The conventional entry point for a PostgreSQL extension: diff --git a/test/extension_upgrade.sh b/test/extension_upgrade.sh index efb2c58..28d15e3 100755 --- a/test/extension_upgrade.sh +++ b/test/extension_upgrade.sh @@ -17,15 +17,27 @@ # second gate beside the matrix, run explicitly. # # Usage: -# test/extension_upgrade.sh [PG_CONFIG] [OLD_REF] +# test/extension_upgrade.sh [PG_CONFIG] [OLD_REF_OR_DIR] # -# OLD_REF defaults to the previous release, and may be any ref that still has the old -# link names. The old build is made in a throwaway clone, so the working tree is never -# checked out from under the caller. +# PGC_UPGRADE_OLD_SRC is the same thing by environment, which is how run_all_versions.sh +# supplies it in an environment with no .git. An explicit second argument wins over it. +# +# The second argument is either a git ref or a path to an already-checked-out source +# tree. A ref defaults to the previous release and is built in a throwaway clone, so the +# working tree is never checked out from under the caller. +# +# The directory form exists because the container dev loop copies the tree WITHOUT .git +# (see docs/testing.md), so the ref form cannot work there. Point it at a second copy of +# the old source instead: +# +# test/extension_upgrade.sh /usr/local/pg18a/bin/pg_config /root/pgcolumnar-1.0-alpha +# +# Requires a real checkout with tags when the ref form is used. That is a precondition, +# not a bug, and it is reported as one. set -uo pipefail PG_CONFIG=${1:-pg_config} -OLD_REF=${2:-v1.0-alpha} +OLD_SRC=${2:-${PGC_UPGRADE_OLD_SRC:-v1.0-alpha}} SRCDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) # The cluster binds a port, so it must come from the band portlib.sh carves BELOW the @@ -54,19 +66,64 @@ trap cleanup EXIT runpg () { runuser -u postgres -- "$@"; } q () { runpg "$BINDIR/psql" -h /tmp -p "$PORT" -d extupg -X -Atc "$1" 2>&1; } -echo "== extension_upgrade: PG$PGMAJ, old ref $OLD_REF" +echo "== extension_upgrade: PG$PGMAJ, old $OLD_SRC" # ---- 1. build and install the old extension, from a throwaway clone ------------------ -# A missing ref must fail, not skip. This gate is invoked deliberately, and a skip that -# exits 0 would let it go inert the moment someone clones without tags. That is the same -# shape as the bug it exists to catch: everything green, nothing checked. -if ! git -C "$SRCDIR" rev-parse --verify -q "$OLD_REF^{commit}" >/dev/null 2>&1; then - echo " FAIL $OLD_REF is not present. Fetch tags, or pass an explicit ref:" - echo " git fetch --tags && test/extension_upgrade.sh $PG_CONFIG " - exit 1 +# Skip and fail are different answers, and the difference is who asked for what. +# +# named an old source that cannot be honoured -> FAIL, the caller asked for a thing +# named nothing, and the tree cannot supply one -> SKIP, the environment cannot +# +# A silent skip that exits 0 is the bug this suite exists to catch, so the skip is loud, +# reports SKIP rather than PASS, and exits 2 so the runner can tell the two apart. +EXPLICIT=0 +{ [ "$#" -ge 2 ] || [ -n "${PGC_UPGRADE_OLD_SRC:-}" ]; } && EXPLICIT=1 + +if [ "$EXPLICIT" = 0 ] && [ ! -d "$SRCDIR/.git" ]; then + echo " SKIP $SRCDIR is not a git checkout, so the default ref $OLD_SRC cannot be built." + echo " The container loop copies the tree without .git. Supply the old source:" + echo " PGC_UPGRADE_OLD_SRC=/path/to/old/source, or pass it as the second argument." + echo "== extension_upgrade: SKIP" + exit 2 +fi + +# A value that looks like a path but is not there is a wrong path, not a ref. Falling +# through to the ref branch would answer a question about directories with an error about +# git checkouts, which is the kind of message that costs someone an afternoon. +case "$OLD_SRC" in + */*) + if [ ! -d "$OLD_SRC" ]; then + echo " FAIL $OLD_SRC looks like a path and is not a directory." + echo " Give a checked-out source tree of the previous release." + exit 1 + fi + ;; +esac + +# Directory form: a path to an already-checked-out old tree needs no git at all. +if [ -d "$OLD_SRC" ]; then + [ -f "$OLD_SRC/Makefile" ] || { echo " FAIL $OLD_SRC has no Makefile"; exit 1; } + cp -a "$OLD_SRC" "$TMP/old" || { echo "FATAL: could not copy $OLD_SRC"; exit 1; } + echo " old source: directory $OLD_SRC" +else + # Only reachable when an old source was named explicitly, so this is a failure and + # not a skip: the caller asked for something this tree cannot provide. + if [ ! -d "$SRCDIR/.git" ]; then + echo " FAIL $SRCDIR is not a git checkout, so the ref $OLD_SRC cannot be built." + echo " The container loop copies the tree without .git. Pass a directory:" + echo " test/extension_upgrade.sh $PG_CONFIG /path/to/old/source" + exit 1 + fi + if ! git -C "$SRCDIR" rev-parse --verify -q "$OLD_SRC^{commit}" >/dev/null 2>&1; then + echo " FAIL $OLD_SRC is not present. Fetch tags, or pass an explicit ref or dir:" + echo " git fetch --tags && test/extension_upgrade.sh $PG_CONFIG " + exit 1 + fi + git clone -q --shared "$SRCDIR" "$TMP/old" || { echo "FATAL: clone failed"; exit 1; } + git -C "$TMP/old" checkout -q --detach "$OLD_SRC" \ + || { echo "FATAL: checkout $OLD_SRC failed"; exit 1; } + echo " old source: ref $OLD_SRC" fi -git clone -q --shared "$SRCDIR" "$TMP/old" || { echo "FATAL: clone failed"; exit 1; } -git -C "$TMP/old" checkout -q --detach "$OLD_REF" || { echo "FATAL: checkout $OLD_REF failed"; exit 1; } make -C "$TMP/old" PG_CONFIG="$PG_CONFIG" clean >/dev/null 2>&1 make -C "$TMP/old" PG_CONFIG="$PG_CONFIG" -j"$(nproc)" >"$TMP/build_old.log" 2>&1 \ || { echo "FAIL old build"; tail -20 "$TMP/build_old.log"; exit 1; } diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index a4fea03..5a5365b 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -528,6 +528,52 @@ if [ "${PGC_RUN_UPGRADE:-0}" = 1 ]; then SUMMARY+=("FAIL upgrade (no runnable pair)") overall=1 fi + + # Extension upgrade, on the same opt-in switch and for the same reason. + # test/extension_upgrade.sh had pg_upgrade's exemption from the registration + # check without pg_upgrade's invocation, so nothing ran it (#396). A guard + # nobody runs is the failure mode #257 existed to close, and it matters more + # here: the break it catches is invisible until a user upgrades. + # + # One major is enough, so this runs once against the first config rather than + # per pair. It builds the previous release from a throwaway clone, so it needs + # a checkout with tags. + _ex="${CONFIGS[0]}" + if [ -x "$_ex" ]; then + _exmaj="$("$_ex" --version | sed -E 's/^[^0-9]*([0-9]+).*/\1/')" + _exlog="$(mktemp "/tmp/pgcolumnar-extupgrade-${_exmaj}.XXXXXX.log")" + # The suite builds a previous release, so it has to be told where to find one. + # PGC_UPGRADE_OLD_SRC carries a directory through, which is the only form that + # works where the tree has no .git, and the documented container loop is exactly + # that. Without it the suite falls back to a ref and cannot build one there. + bash "$SRCDIR/test/extension_upgrade.sh" "$_ex" \ + ${PGC_UPGRADE_OLD_SRC:+"$PGC_UPGRADE_OLD_SRC"} >"$_exlog" 2>&1 + _exrc=$? + # Exit 2 is "the environment could not supply an old source", which is not a + # product failure. It is reported as SKIP and not as PASS, because a gate that + # reports green having run nothing is the defect this suite was written for. + if [ "$_exrc" = 0 ]; then + echo " PASS extension_upgrade PG$_exmaj" + SUMMARY+=("PASS extension_upgrade PG$_exmaj") + rm -f "$_exlog" + elif [ "$_exrc" = 2 ]; then + echo " SKIP extension_upgrade PG$_exmaj" + grep -E '^\s*(SKIP| )' "$_exlog" | sed 's/^/ /' + SUMMARY+=("SKIP extension_upgrade PG$_exmaj") + rm -f "$_exlog" + else + echo " FAIL extension_upgrade PG$_exmaj" + grep -E '^\s*FAIL' "$_exlog" | sed 's/^/ >> /' + tail -30 "$_exlog" | sed 's/^/ /' + echo " full log: $_exlog" + SUMMARY+=("FAIL extension_upgrade PG$_exmaj") + overall=1 + fi + else + echo " FAIL PGC_RUN_UPGRADE=1 but no runnable pg_config for extension_upgrade" + SUMMARY+=("FAIL extension_upgrade (no runnable pg_config)") + overall=1 + fi fi echo