Restore upgrade testing in GitHub Actions

Required some changes to pgxn/docker-pgxn-tools to allow for an unprivileged user to use `sudo`, but with that in places the changes to `test/test_MVU.sh` are minor: * Use the versions for path args if none passed. * Some indentation tweaks * Use `$sudo` consistently * Remove run_make in favor of explict calls to `make` with `$sudo` used as appropriate. * Fall back on `whoami` if `$USER` is not set * Run tests without `sudo` Changes to `.github/workflows/test.yml` are mostly aesthetic, but the new "Upgrade" step makes sure that pgTAP is installed and installs the packages for the version to upgrade to before handing execution off to `test/test_MVU.sh`. With these changes, we should be ready to release v1.2.0.
theory · Dec 5, 2021 · c72a59e · c72a59e
1 parent 2ccda7f
commit c72a59e
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 83 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -1,4 +1,4 @@
-name: 🐘 Test
+name: ✅ Test
 on:
   push:
   pull_request:
@@ -21,9 +21,9 @@ jobs:
           - { version: 9.2, upgrade_to: 9.3, update_from: "" }     # updatecheck is not supported prior to 9.3
           - { version: 9.1, upgrade_to: 9.2, update_from: "" }
           # Also test pg_upgrade across many versions
-          # - { version: 9.1,  upgrade_to: 14, update_from: 0.99.0 }
-          # - { version: 9.4,  upgrade_to: 14, update_from: 0.99.0 }
-    name: 🐘 PostgreSQL ${{ matrix.version }}
+          - { version: 9.1,  upgrade_to: 14, update_from: "", suffix: –14 }
+          - { version: 9.4,  upgrade_to: 14, update_from: "", suffix: –14 }
+    name: 🐘 PostgreSQL ${{ matrix.version }}${{ matrix.suffix }}
     runs-on: ubuntu-latest
     container: pgxn/pgxn-tools
     env:
@@ -39,16 +39,13 @@ jobs:
       # Test update.
       - run: 'if [ -d "$UPDATE_FROM" ]; then make uninstall clean updatecheck; fi'
 
-      # Test upgrade.
-      # - run: ./test/test_MVU.sh -s 55667 55778 "{{ matrix.version }}" "{{ matrix.upgrade_to }}" "/usr/lib/postgresql/{{ matrix.version }}/bin/" "/usr/lib/postgresql/{{ matrix.upgrade_to }}/bin/"
-
       # Test all, install, test, test-serial, and test-parallel, both from clean
       # repo and repeated with existing build, with and without PARALLEL_CONN=1.
       - run: make uninstall clean all
       - run: make all
       - run: make uninstall clean install
       - run: make install
-      - run: "psql -Ec 'CREATE EXTENSION pgtap'"
+      - run: psql -Ec 'CREATE EXTENSION pgtap'
       - run: make uninstall clean test
       - run: make test
       - run: make uninstall clean test PARALLEL_CONN=1
@@ -61,3 +58,13 @@ jobs:
       - run: make test-parallel
       - run: make uninstall clean test-parallel PARALLEL_CONN=1
       - run: make test-parallel PARALLEL_CONN=1
+
+      # Test upgrade last, since the new version's client will be preferred.
+      - if: ${{ matrix.upgrade_to != '' }}
+        name: Upgrade to ${{ matrix.upgrade_to }}
+        # Based on https://gist.github.com/petere/6023944
+        # See also https://askubuntu.com/a/104912 for --force options
+        run: |
+          make install
+          apt-get install -qq -o Dpkg::Options::="--force-confdef" -o Dpkg::Options::="--force-confold" postgresql-${{ matrix.upgrade_to }} postgresql-server-dev-${{ matrix.upgrade_to }}
+          sudo -u postgres test/test_MVU.sh -s 55432 55433 "${{ matrix.version }}" "${{ matrix.upgrade_to }}"
diff --git a/test/test_MVU.sh b/test/test_MVU.sh
@@ -10,21 +10,20 @@ set -E -e -u -o pipefail
 
 BASEDIR=`dirname $0`
 if ! . $BASEDIR/../tools/util.sh; then
-  echo "FATAL: error sourcing $BASEDIR/../tools/util.sh" 1>&2
-  exit 99
+    echo "FATAL: error sourcing $BASEDIR/../tools/util.sh" 1>&2
+    exit 99
 fi
 trap err_report ERR
 
 debug 19 "Arguments: $@"
 
 rc=0
 
-
 byte_len() (
-[ $# -eq 1 ] || die 99 "Expected 1 argument, not $# ($@)"
-LANG=C LC_ALL=C
-debug 99 "byte_len($@) = ${#1}"
-echo ${#1}
+    [ $# -eq 1 ] || die 99 "Expected 1 argument, not $# ($@)"
+    LANG=C LC_ALL=C
+    debug 99 "byte_len($@) = ${#1}"
+    echo ${#1}
 )
 
 check_bin() {
@@ -36,22 +35,22 @@ check_bin() {
 # mktemp on OS X results is a super-long path name that can cause problems, ie:
 #   connection to database failed: Unix-domain socket path "/private/var/folders/rp/mv0457r17cg0xqyw5j7701892tlc0h/T/test_pgtap_upgrade.upgrade.7W4BLF/.s.PGSQL.50432" is too long (maximum 103 bytes)
 #
-# This function looks for that condition and replaces the output with something more sane
+# This function looks for that condition and replaces the output with something more legible
 short_tmpdir() (
-[ $# -eq 1 ] || die 99 "Expected 1 argument, not $# ($@)"
-[ "$TMPDIR" != "" ] || die 99 '$TMPDIR not set'
-out=$(mktemp -p '' -d $1.XXXXXX)
-if echo "$out" | egrep -q '^(/private)?/var/folders'; then
-    newout=$(echo "$out" | sed -e "s#.*/$TMPDIR#$TMPDIR#")
-    debug 19 "replacing '$out' with '$newout'"
-fi
+    [ $# -eq 1 ] || die 99 "Expected 1 argument, not $# ($@)"
+    [ "$TMPDIR" != "" ] || die 99 '$TMPDIR not set'
+    out=$(mktemp -p '' -d $1.XXXXXX)
+    if echo "$out" | egrep -q '^(/private)?/var/folders'; then
+        newout=$(echo "$out" | sed -e "s#.*/$TMPDIR#$TMPDIR#")
+        debug 19 "replacing '$out' with '$newout'"
+    fi
 
-debug 9 "$0($@) = $out"
-# Postgres blows up if this is too long. Technically the limit is 103 bytes,
-# but need to account for the socket name, plus the fact that OS X might
-# prepend '/private' to what we return. :(
-[ $(byte_len "$out") -lt 75 ] || die 9 "short_tmpdir($@) returning a value >= 75 bytes ('$out')"
-echo "$out"
+    debug 9 "$0($@) = $out"
+    # Postgres blows up if this is too long. Technically the limit is 103 bytes,
+    # but need to account for the socket name, plus the fact that OS X might
+    # prepend '/private' to what we return. :(
+    [ $(byte_len "$out") -lt 75 ] || die 9 "short_tmpdir($@) returning a value >= 75 bytes ('$out')"
+    echo "$out"
 )
 
 banner() {
@@ -62,47 +61,42 @@ banner() {
     echo
 }
 
-run_make() (
-cd $(dirname $0)/..
-$sudo make $@
-)
-
 modify_config() (
-# See below for definition of ctl_separator
-if [ -z "$ctl_separator" ]; then
-    confDir=$PGDATA
-    conf=$confDir/postgresql.conf
-    debug 6 "$0: conf = $conf"
+    # See below for definition of ctl_separator
+    if [ -z "$ctl_separator" ]; then
+        confDir=$PGDATA
+        conf=$confDir/postgresql.conf
+        debug 6 "$0: conf = $conf"
+
+        debug 0 "Modifying NATIVE $conf"
+
+        echo "port = $PGPORT" >> $conf
+    else
+        confDir="/etc/postgresql/$1/$cluster_name"
+        conf="$confDir/postgresql.conf"
+        debug 6 "$0: confDir = $confDir conf=$conf"
+        debug_ls 9 -la $confDir
+
+        debug 0 "Modifying DEBIAN $confDir and $PGDATA"
+
+        debug 2 ln -s $conf $PGDATA/
+        ln -s $conf $PGDATA/
+        # Some versions also have a conf.d ...
+        if [ -e "$confDir/conf.d" ]; then
+            debug 2 ln -s $confDir/conf.d $PGDATA/
+            ln -s $confDir/conf.d $PGDATA/
+        fi
+        debug_ls 8 -la $PGDATA
 
-    debug 0 "Modifying NATIVE $conf"
+        # Shouldn't need to muck with PGPORT...
 
-    echo "port = $PGPORT" >> $conf
-else
-    confDir="/etc/postgresql/$1/$cluster_name"
-    conf="$confDir/postgresql.conf"
-    debug 6 "$0: confDir = $confDir conf=$conf"
-    debug_ls 9 -la $confDir
-
-    debug 0 "Modifying DEBIAN $confDir and $PGDATA"
-
-    debug 2 ln -s $conf $PGDATA/
-    ln -s $conf $PGDATA/
-    # Some versions also have a conf.d ...
-    if [ -e "$confDir/conf.d" ]; then
-        debug 2 ln -s $confDir/conf.d $PGDATA/
-        ln -s $confDir/conf.d $PGDATA/
+        # GUC changed somewhere between 9.1 and 9.5, so read config to figure out correct value
+        guc=$(grep unix_socket_director $conf | sed -e 's/^# *//' | cut -d ' ' -f 1)
+        debug 4 "$0: guc = $guc"
+        echo "$guc = '/tmp'" >> $conf
     fi
-    debug_ls 8 -la $PGDATA
-
-    # Shouldn't need to muck with PGPORT...
 
-    # GUC changed somewhere between 9.1 and 9.5, so read config to figure out correct value
-    guc=$(grep unix_socket_director $conf | sed -e 's/^# *//' | cut -d ' ' -f 1)
-    debug 4 "$0: guc = $guc"
-    echo "$guc = '/tmp'" >> $conf
-fi
-
-echo "synchronous_commit = off" >> $conf
+    echo "synchronous_commit = off" >> $conf
 )
 
 #############################
@@ -127,8 +121,8 @@ OLD_PORT=$1
 NEW_PORT=$2
 OLD_VERSION=$3
 NEW_VERSION=$4
-OLD_PATH=$5
-NEW_PATH=$6
+OLD_PATH="${5:-/usr/lib/postgresql/$OLD_VERSION/bin}"
+NEW_PATH="${5:-/usr/lib/postgresql/$NEW_VERSION/bin}"
 
 export PGDATABASE=test_pgtap_upgrade
 
@@ -174,15 +168,15 @@ if command -v pg_ctlcluster > /dev/null; then
     export PGHOST=/tmp
 
     # And force current user
-    export PGUSER=$USER
+    export PGUSER=${USER:-$(whoami)}
 
-    old_initdb="sudo pg_createcluster $OLD_VERSION $cluster_name -u $USER -p $OLD_PORT -d $old_dir -- -A trust"
-    old_pg_ctl="sudo pg_ctlcluster $OLD_VERSION test_pg_upgrade"
-    new_initdb="sudo pg_createcluster $NEW_VERSION $cluster_name -u $USER -p $NEW_PORT -d $new_dir -- -A trust"
-    new_pg_ctl="sudo pg_ctlcluster $NEW_VERSION test_pg_upgrade"
+    old_initdb="$sudo pg_createcluster $OLD_VERSION $cluster_name -u $PGUSER -p $OLD_PORT -d $old_dir -- -A trust"
+    old_pg_ctl="$sudo pg_ctlcluster $OLD_VERSION test_pg_upgrade"
+    new_initdb="$sudo pg_createcluster $NEW_VERSION $cluster_name -u $PGUSER -p $NEW_PORT -d $new_dir -- -A trust"
+    new_pg_ctl="$sudo pg_ctlcluster $NEW_VERSION test_pg_upgrade"
 
     # See also ../.github/workflows/test.yml
-    new_pg_upgrade=/usr/lib/postgresql/$NEW_VERSION/bin/pg_upgrade
+    new_pg_upgrade="/usr/lib/postgresql/$NEW_VERSION/bin/pg_upgrade"
 else
     ctl_separator=''
     old_initdb="$(find_at_path "$OLD_PATH" initdb) -D $old_dir -N"
@@ -218,16 +212,14 @@ echo "Installing pgtap"
 # If user requested sudo then we need to use it for the install step. TODO:
 # it'd be nice to move this into the Makefile, if the PGXS make stuff allows
 # it...
-run_make clean install 
+$sudo make clean install
 
 banner "Loading extension"
 psql -c 'CREATE EXTENSION pgtap' # Also uses PGPORT
 
 echo "Stopping OLD postgres via $old_pg_ctl"
 $old_pg_ctl stop $ctl_separator -w # older versions don't support --wait
 
-
-
 ##################################################################################################
 banner "Running pg_upgrade"
 export PGDATA=$new_dir
@@ -258,8 +250,6 @@ modify_config $NEW_VERSION
     fi
 )
 
-
-
 ##################################################################################################
 banner "Testing UPGRADED cluster"
 
@@ -286,7 +276,7 @@ export PG_CONFIG=$(find_at_path "$NEW_PATH" pg_config)
 [ -x "$PG_CONFIG" ] || ( debug_ls 1 "$NEW_PATH"; die 4 "unable to find executable pg_config at $NEW_PATH" )
 
 # When crossing certain upgrade boundaries we need to exclude some tests
-# because they test functions not available in the previous version.
+# because the test functions are not available in the previous version.
 int_ver() {
     local ver
     ver=$(echo $1 | tr -d .)
@@ -316,18 +306,18 @@ add_exclude 9.6 10 test/sql/partitions.sql
 #(cd $(dirname $0)/..; pg_prove -v --pset tuples_only=1 test/sql/throwtap.sql)
 
 export EXCLUDE_TEST_FILES
-run_make clean test
+$sudo make clean
+make test
 
 if [ -n "$EXCLUDE_TEST_FILES" ]; then
     banner "Rerunning test after a reinstall due to version differences"
     echo "Excluded tests: $EXCLUDE_TEST_FILES"
     export EXCLUDED_TEST_FILES=''
 
     # Need to build with the new version, then install
-    run_make install
+    $sudo make install
 
     psql -E -c 'DROP EXTENSION pgtap; CREATE EXTENSION pgtap;'
 
-    run_make test
+    make test
 fi
-